* [PATCH 0/2] Improve the fallocate() EINVAL in handle_aiocb_write_zeroes()
@ 2021-05-27 17:20 Thomas Huth
2021-05-27 17:20 ` [PATCH 1/2] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS Thomas Huth
2021-05-27 17:20 ` [PATCH 2/2] block/file-posix: Try other fallbacks after invalid FALLOC_FL_ZERO_RANGE Thomas Huth
0 siblings, 2 replies; 4+ messages in thread
From: Thomas Huth @ 2021-05-27 17:20 UTC (permalink / raw
To: qemu-block, Kevin Wolf, Max Reitz
Cc: Andrey Shinkevich, Viktor Mihajlovski, qemu-devel,
Christian Borntraeger
On buggy file systems, fallocate() can return EINVAL for unaligned accesses.
Improve the situation by ignoring this for PUNCH_HOLE, too (but we also
print out an error message in this case now, since PUNCH_HOLE should really
never return EINVAL according to the man page). The second patch reworks
the handling for ZERO_RANGE a little bit so that we now also try the other
fallbacks in this case now.
Thomas Huth (2):
block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS
block/file-posix: Try other fallbacks after invalid
FALLOC_FL_ZERO_RANGE
block/file-posix.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS
2021-05-27 17:20 [PATCH 0/2] Improve the fallocate() EINVAL in handle_aiocb_write_zeroes() Thomas Huth
@ 2021-05-27 17:20 ` Thomas Huth
2021-06-01 11:35 ` Kevin Wolf
2021-05-27 17:20 ` [PATCH 2/2] block/file-posix: Try other fallbacks after invalid FALLOC_FL_ZERO_RANGE Thomas Huth
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Huth @ 2021-05-27 17:20 UTC (permalink / raw
To: qemu-block, Kevin Wolf, Max Reitz
Cc: Andrey Shinkevich, Viktor Mihajlovski, qemu-devel,
Christian Borntraeger
A customer reported that running
qemu-img convert -t none -O qcow2 -f qcow2 input.qcow2 output.qcow2
fails for them with the following error message when the images are
stored on a GPFS file system :
qemu-img: error while writing sector 0: Invalid argument
After analyzing the strace output, it seems like the problem is in
handle_aiocb_write_zeroes(): The call to fallocate(FALLOC_FL_PUNCH_HOLE)
returns EINVAL, which can apparently happen if the file system has
a different idea of the granularity of the operation. It's arguably
a bug in GPFS, since the PUNCH_HOLE mode should not result in EINVAL
according to the man-page of fallocate(), but the file system is out
there in production and so we have to deal with it. In commit 294682cc3a
("block: workaround for unaligned byte range in fallocate()") we also
already applied the a work-around for the same problem to the earlier
fallocate(FALLOC_FL_ZERO_RANGE) call, so do it now similar with the
PUNCH_HOLE call. But instead of silently catching and returning
-ENOTSUP (which causes the caller to fall back to writing zeroes),
let's rather inform the user once about the buggy file system and
try the other fallback instead.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
block/file-posix.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index 10b71d9a13..134ff01d82 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1650,6 +1650,16 @@ static int handle_aiocb_write_zeroes(void *opaque)
return ret;
}
s->has_fallocate = false;
+ } else if (ret == -EINVAL) {
+ /*
+ * Some file systems like older versions of GPFS do not like un-
+ * aligned byte ranges, and return EINVAL in such a case, though
+ * they should not do it according to the man-page of fallocate().
+ * Warn about the bad filesystem and try the final fallback instead.
+ */
+ warn_report_once("Your file system is misbehaving: "
+ "fallocate(FALLOC_FL_PUNCH_HOLE) returned EINVAL. "
+ "Please report this bug to your file sytem vendor.");
} else if (ret != -ENOTSUP) {
return ret;
} else {
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] block/file-posix: Try other fallbacks after invalid FALLOC_FL_ZERO_RANGE
2021-05-27 17:20 [PATCH 0/2] Improve the fallocate() EINVAL in handle_aiocb_write_zeroes() Thomas Huth
2021-05-27 17:20 ` [PATCH 1/2] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS Thomas Huth
@ 2021-05-27 17:20 ` Thomas Huth
1 sibling, 0 replies; 4+ messages in thread
From: Thomas Huth @ 2021-05-27 17:20 UTC (permalink / raw
To: qemu-block, Kevin Wolf, Max Reitz
Cc: Andrey Shinkevich, Viktor Mihajlovski, qemu-devel,
Christian Borntraeger
If fallocate(... FALLOC_FL_ZERO_RANGE ...) returns EINVAL, it's likely
an indication that the file system is buggy and does not implement
unaligned accesses right. We still might be lucky with the other
fallback fallocate() calls later in this function, though, so we should
not return immediately and try the others first.
Since FALLOC_FL_ZERO_RANGE could also return EINVAL if the file descriptor
is not a regular file, we ignore this filesystem bug silently, without
printing an error message for the user.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
block/file-posix.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 134ff01d82..a96b6fa8fb 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1625,17 +1625,17 @@ static int handle_aiocb_write_zeroes(void *opaque)
if (s->has_write_zeroes) {
int ret = do_fallocate(s->fd, FALLOC_FL_ZERO_RANGE,
aiocb->aio_offset, aiocb->aio_nbytes);
- if (ret == -EINVAL) {
- /*
- * Allow falling back to pwrite for file systems that
- * do not support fallocate() for an unaligned byte range.
- */
- return -ENOTSUP;
- }
- if (ret == 0 || ret != -ENOTSUP) {
+ if (ret == -ENOTSUP) {
+ s->has_write_zeroes = false;
+ } else if (ret == 0 || ret != -EINVAL) {
return ret;
}
- s->has_write_zeroes = false;
+ /*
+ * Note: Some file systems do not like unaligned byte ranges, and
+ * return EINVAL in such a case, though they should not do it according
+ * to the man-page of fallocate(). Thus we simply ignore this return
+ * value and try the other fallbacks instead.
+ */
}
#endif
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS
2021-05-27 17:20 ` [PATCH 1/2] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS Thomas Huth
@ 2021-06-01 11:35 ` Kevin Wolf
0 siblings, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2021-06-01 11:35 UTC (permalink / raw
To: Thomas Huth
Cc: qemu-block, qemu-devel, Max Reitz, Christian Borntraeger,
Andrey Shinkevich, Viktor Mihajlovski
Am 27.05.2021 um 19:20 hat Thomas Huth geschrieben:
> A customer reported that running
>
> qemu-img convert -t none -O qcow2 -f qcow2 input.qcow2 output.qcow2
>
> fails for them with the following error message when the images are
> stored on a GPFS file system :
>
> qemu-img: error while writing sector 0: Invalid argument
>
> After analyzing the strace output, it seems like the problem is in
> handle_aiocb_write_zeroes(): The call to fallocate(FALLOC_FL_PUNCH_HOLE)
> returns EINVAL, which can apparently happen if the file system has
> a different idea of the granularity of the operation. It's arguably
> a bug in GPFS, since the PUNCH_HOLE mode should not result in EINVAL
> according to the man-page of fallocate(), but the file system is out
> there in production and so we have to deal with it. In commit 294682cc3a
> ("block: workaround for unaligned byte range in fallocate()") we also
> already applied the a work-around for the same problem to the earlier
> fallocate(FALLOC_FL_ZERO_RANGE) call, so do it now similar with the
> PUNCH_HOLE call. But instead of silently catching and returning
> -ENOTSUP (which causes the caller to fall back to writing zeroes),
> let's rather inform the user once about the buggy file system and
> try the other fallback instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> block/file-posix.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 10b71d9a13..134ff01d82 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1650,6 +1650,16 @@ static int handle_aiocb_write_zeroes(void *opaque)
> return ret;
> }
> s->has_fallocate = false;
> + } else if (ret == -EINVAL) {
> + /*
> + * Some file systems like older versions of GPFS do not like un-
> + * aligned byte ranges, and return EINVAL in such a case, though
> + * they should not do it according to the man-page of fallocate().
> + * Warn about the bad filesystem and try the final fallback instead.
> + */
> + warn_report_once("Your file system is misbehaving: "
> + "fallocate(FALLOC_FL_PUNCH_HOLE) returned EINVAL. "
> + "Please report this bug to your file sytem vendor.");
This line is too long.
I've fixed it up and applied the series, thanks.
Kevin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-01 11:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-27 17:20 [PATCH 0/2] Improve the fallocate() EINVAL in handle_aiocb_write_zeroes() Thomas Huth
2021-05-27 17:20 ` [PATCH 1/2] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS Thomas Huth
2021-06-01 11:35 ` Kevin Wolf
2021-05-27 17:20 ` [PATCH 2/2] block/file-posix: Try other fallbacks after invalid FALLOC_FL_ZERO_RANGE Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).