Re: [PATCH] block/file-posix: Fix return value translation for AIO discards.
Reviewed-by: Akihiko Odaki On Tue, Oct 19, 2021 at 3:08 AM Ari Sundholm wrote: > > AIO discards regressed as a result of the following commit: > 0dfc7af2 block/file-posix: Optimize for macOS > > When trying to run blkdiscard within a Linux guest, the request would > fail, with some errors in dmesg: > > [ snip ] > [4.010070] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK > driverbyte=DRIVER_SENSE > [4.011061] sd 2:0:0:0: [sda] tag#0 Sense Key : Aborted Command > [current] > [4.011061] sd 2:0:0:0: [sda] tag#0 Add. Sense: I/O process > terminated > [4.011061] sd 2:0:0:0: [sda] tag#0 CDB: Unmap/Read sub-channel 42 > 00 00 00 00 00 00 00 18 00 > [4.011061] blk_update_request: I/O error, dev sda, sector 0 > [ snip ] > > This turns out to be a result of a flaw in changes to the error value > translation logic in handle_aiocb_discard(). The default return value > may be left untranslated in some configurations, and the wrong variable > is used in one translation. > > Fix both issues. > > Signed-off-by: Ari Sundholm > Signed-off-by: Emil Karlson > --- > block/file-posix.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block/file-posix.c b/block/file-posix.c > index 53be0bdc1b..6def2a4cba 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -1807,7 +1807,7 @@ static int handle_aiocb_copy_range(void *opaque) > static int handle_aiocb_discard(void *opaque) > { > RawPosixAIOData *aiocb = opaque; > -int ret = -EOPNOTSUPP; > +int ret = -ENOTSUP; > BDRVRawState *s = aiocb->bs->opaque; > > if (!s->has_discard) { > @@ -1829,7 +1829,7 @@ static int handle_aiocb_discard(void *opaque) > #ifdef CONFIG_FALLOCATE_PUNCH_HOLE > ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, > aiocb->aio_offset, aiocb->aio_nbytes); > -ret = translate_err(-errno); > +ret = translate_err(ret); > #elif defined(__APPLE__) && (__MACH__) > fpunchhole_t fpunchhole; > fpunchhole.fp_flags = 0; > -- > 2.31.1 >
Re: [PATCH] block/file-posix: Fix return value translation for AIO discards.
+Stefan On 10/18/21 20:07, Ari Sundholm wrote: > AIO discards regressed as a result of the following commit: > 0dfc7af2 block/file-posix: Optimize for macOS > > When trying to run blkdiscard within a Linux guest, the request would > fail, with some errors in dmesg: > > [ snip ] > [4.010070] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK > driverbyte=DRIVER_SENSE > [4.011061] sd 2:0:0:0: [sda] tag#0 Sense Key : Aborted Command > [current] > [4.011061] sd 2:0:0:0: [sda] tag#0 Add. Sense: I/O process > terminated > [4.011061] sd 2:0:0:0: [sda] tag#0 CDB: Unmap/Read sub-channel 42 > 00 00 00 00 00 00 00 18 00 > [4.011061] blk_update_request: I/O error, dev sda, sector 0 > [ snip ] > > This turns out to be a result of a flaw in changes to the error value > translation logic in handle_aiocb_discard(). The default return value > may be left untranslated in some configurations, and the wrong variable > is used in one translation. > > Fix both issues. Worth including: Cc: qemu-sta...@nongnu.org Fixes: 0dfc7af2b28 ("block/file-posix: Optimize for macOS") > Signed-off-by: Ari Sundholm > Signed-off-by: Emil Karlson > --- > block/file-posix.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block/file-posix.c b/block/file-posix.c > index 53be0bdc1b..6def2a4cba 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -1807,7 +1807,7 @@ static int handle_aiocb_copy_range(void *opaque) > static int handle_aiocb_discard(void *opaque) > { > RawPosixAIOData *aiocb = opaque; > -int ret = -EOPNOTSUPP; > +int ret = -ENOTSUP; > BDRVRawState *s = aiocb->bs->opaque; > > if (!s->has_discard) { > @@ -1829,7 +1829,7 @@ static int handle_aiocb_discard(void *opaque) > #ifdef CONFIG_FALLOCATE_PUNCH_HOLE > ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, > aiocb->aio_offset, aiocb->aio_nbytes); > -ret = translate_err(-errno); > +ret = translate_err(ret); > #elif defined(__APPLE__) && (__MACH__) > fpunchhole_t fpunchhole; > fpunchhole.fp_flags = 0; >
Re: [PATCH] block/file-posix: Fix return value translation for AIO discards.
On Tue, Oct 19, 2021 at 05:40:13AM +0200, Philippe Mathieu-Daudé wrote: > +Stefan > > On 10/18/21 20:07, Ari Sundholm wrote: > > AIO discards regressed as a result of the following commit: > > 0dfc7af2 block/file-posix: Optimize for macOS > > > > When trying to run blkdiscard within a Linux guest, the request would > > fail, with some errors in dmesg: > > > > [ snip ] > > [4.010070] sd 2:0:0:0: [sda] tag#0 FAILED Result: hostbyte=DID_OK > > driverbyte=DRIVER_SENSE > > [4.011061] sd 2:0:0:0: [sda] tag#0 Sense Key : Aborted Command > > [current] > > [4.011061] sd 2:0:0:0: [sda] tag#0 Add. Sense: I/O process > > terminated > > [4.011061] sd 2:0:0:0: [sda] tag#0 CDB: Unmap/Read sub-channel 42 > > 00 00 00 00 00 00 00 18 00 > > [4.011061] blk_update_request: I/O error, dev sda, sector 0 > > [ snip ] > > > > This turns out to be a result of a flaw in changes to the error value > > translation logic in handle_aiocb_discard(). The default return value > > may be left untranslated in some configurations, and the wrong variable > > is used in one translation. > > > > Fix both issues. > > Worth including: > > Cc: qemu-sta...@nongnu.org > Fixes: 0dfc7af2b28 ("block/file-posix: Optimize for macOS") > > > Signed-off-by: Ari Sundholm > > Signed-off-by: Emil Karlson > > --- > > block/file-posix.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/block/file-posix.c b/block/file-posix.c > > index 53be0bdc1b..6def2a4cba 100644 > > --- a/block/file-posix.c > > +++ b/block/file-posix.c > > @@ -1807,7 +1807,7 @@ static int handle_aiocb_copy_range(void *opaque) > > static int handle_aiocb_discard(void *opaque) > > { > > RawPosixAIOData *aiocb = opaque; > > -int ret = -EOPNOTSUPP; > > +int ret = -ENOTSUP; > > BDRVRawState *s = aiocb->bs->opaque; > > > > if (!s->has_discard) { > > @@ -1829,7 +1829,7 @@ static int handle_aiocb_discard(void *opaque) > > #ifdef CONFIG_FALLOCATE_PUNCH_HOLE > > ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | > > FALLOC_FL_KEEP_SIZE, > > aiocb->aio_offset, aiocb->aio_nbytes); > > -ret = translate_err(-errno); > > +ret = translate_err(ret); > > #elif defined(__APPLE__) && (__MACH__) > > fpunchhole_t fpunchhole; > > fpunchhole.fp_flags = 0; > > > Thanks for the fix! Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature