At the end of a reopen, we already call bdrv_refresh_limits(), which should update bs->request_alignment according to the new file descriptor. However, raw_probe_alignment() relies on s->needs_alignment and just uses 1 if it isn't set. We neglected to update this field, so starting with cache=writeback and then reopening with cache=none means that we get an incorrect bs->request_alignment == 1 and unaligned requests fail instead of being automatically aligned.
Fix this by recalculating s->needs_alignment in raw_refresh_limits() before calling raw_probe_alignment(). Signed-off-by: Kevin Wolf <kw...@redhat.com> Message-Id: <20211104113109.56336-1-kw...@redhat.com> Reviewed-by: Hanna Reitz <hre...@redhat.com> Reviewed-by: Stefano Garzarella <sgarz...@redhat.com> Signed-off-by: Kevin Wolf <kw...@redhat.com> --- block/file-posix.c | 20 ++++++++++++++++---- tests/qemu-iotests/142 | 22 ++++++++++++++++++++++ tests/qemu-iotests/142.out | 15 +++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index 7a27c83060..b283093e5b 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -167,6 +167,7 @@ typedef struct BDRVRawState { int page_cache_inconsistent; /* errno from fdatasync failure */ bool has_fallocate; bool needs_alignment; + bool force_alignment; bool drop_cache; bool check_cache_dropped; struct { @@ -351,6 +352,17 @@ static bool dio_byte_aligned(int fd) return false; } +static bool raw_needs_alignment(BlockDriverState *bs) +{ + BDRVRawState *s = bs->opaque; + + if ((bs->open_flags & BDRV_O_NOCACHE) != 0 && !dio_byte_aligned(s->fd)) { + return true; + } + + return s->force_alignment; +} + /* Check if read is allowed with given memory buffer and length. * * This function is used to check O_DIRECT memory buffer and request alignment. @@ -728,9 +740,6 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, s->has_discard = true; s->has_write_zeroes = true; - if ((bs->open_flags & BDRV_O_NOCACHE) != 0 && !dio_byte_aligned(s->fd)) { - s->needs_alignment = true; - } if (fstat(s->fd, &st) < 0) { ret = -errno; @@ -784,9 +793,10 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, * so QEMU makes sure all IO operations on the device are aligned * to sector size, or else FreeBSD will reject them with EINVAL. */ - s->needs_alignment = true; + s->force_alignment = true; } #endif + s->needs_alignment = raw_needs_alignment(bs); #ifdef CONFIG_XFS if (platform_test_xfs_fd(s->fd)) { @@ -1251,7 +1261,9 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) BDRVRawState *s = bs->opaque; struct stat st; + s->needs_alignment = raw_needs_alignment(bs); raw_probe_alignment(bs, s->fd, errp); + bs->bl.min_mem_alignment = s->buf_align; bs->bl.opt_mem_alignment = MAX(s->buf_align, qemu_real_host_page_size); diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142 index 69fd10ef51..07003c597a 100755 --- a/tests/qemu-iotests/142 +++ b/tests/qemu-iotests/142 @@ -350,6 +350,28 @@ info block backing-file" echo "$hmp_cmds" | run_qemu -drive "$files","$ids" | grep "Cache" +echo +echo "--- Alignment after changing O_DIRECT ---" +echo + +# Directly test the protocol level: Can unaligned requests succeed even if +# O_DIRECT was only enabled through a reopen and vice versa? + +$QEMU_IO --cache=writeback -f file $TEST_IMG <<EOF | _filter_qemu_io +read 42 42 +reopen -o cache.direct=on +read 42 42 +reopen -o cache.direct=off +read 42 42 +EOF +$QEMU_IO --cache=none -f file $TEST_IMG <<EOF | _filter_qemu_io +read 42 42 +reopen -o cache.direct=off +read 42 42 +reopen -o cache.direct=on +read 42 42 +EOF + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out index a92b948edd..f167038230 100644 --- a/tests/qemu-iotests/142.out +++ b/tests/qemu-iotests/142.out @@ -747,4 +747,19 @@ cache.no-flush=on on backing-file Cache mode: writeback Cache mode: writeback, direct Cache mode: writeback, ignore flushes + +--- Alignment after changing O_DIRECT --- + +read 42/42 bytes at offset 42 +42 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 42/42 bytes at offset 42 +42 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 42/42 bytes at offset 42 +42 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 42/42 bytes at offset 42 +42 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 42/42 bytes at offset 42 +42 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 42/42 bytes at offset 42 +42 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) *** done -- 2.31.1