* [PATCH 0/2] file-posix: Add 'accept-read-only-device' option
@ 2026-10-28 17:10 Kevin Wolf
2026-10-28 17:10 ` [PATCH 1/2] " Kevin Wolf
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Kevin Wolf @ 2026-10-28 17:10 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, stefanha, afaria, qemu-devel
Kevin Wolf (2):
file-posix: Add 'accept-read-only-device' option
iotests: Add loop-accept-ro-dev test
qapi/block-core.json | 19 +++-
block/file-posix.c | 23 ++++-
tests/qemu-iotests/tests/loop-accept-ro-dev | 87 +++++++++++++++++++
.../qemu-iotests/tests/loop-accept-ro-dev.out | 19 ++++
4 files changed, 144 insertions(+), 4 deletions(-)
create mode 100755 tests/qemu-iotests/tests/loop-accept-ro-dev
create mode 100644 tests/qemu-iotests/tests/loop-accept-ro-dev.out
--
2.56.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] file-posix: Add 'accept-read-only-device' option 2026-10-28 17:10 [PATCH 0/2] file-posix: Add 'accept-read-only-device' option Kevin Wolf @ 2026-10-28 17:10 ` Kevin Wolf 2026-10-28 17:54 ` Eric Blake 2026-10-28 17:10 ` [PATCH 2/2] iotests: Add loop-accept-ro-dev test Kevin Wolf 2026-10-28 19:49 ` [PATCH 0/2] file-posix: Add 'accept-read-only-device' option Stefan Hajnoczi 2 siblings, 1 reply; 7+ messages in thread From: Kevin Wolf @ 2026-10-28 17:10 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, stefanha, afaria, qemu-devel The host_device block driver refuses to open a block device read-write if the BLKROGET ioctl tells it that it's actually read-only. For such devices it's possible to open a read-write file descriptor, but any writes to the device would fail. There are valid use cases for which this check gets in the way, as explained in the bug report: However, this behavior prevents a legitimate enterprise use case: running VMs against storage replication target LUNs in a disaster recovery configuration. Enterprise storage arrays (Dell Powermax, Pure Storage) set the SCSI Write Protect bit on replication target LUNs, causing the Linux kernel to mark them read-only via BLKROGET. The VMs on the passive DR site need to start with these LUNs attached - writes are expected to fail (the application middleware knows it's on the passive side), and after a DR role swap the LUN becomes writable and writes should succeed immediately without any VM restart. In order to allow such use cases in which a currently read-only device is deliberately opened read-write, add a new option to the host_device driver that allows the user to skip the check. Buglink: https://redhat.atlassian.net/browse/RHEL-186136 Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- qapi/block-core.json | 19 ++++++++++++++++++- block/file-posix.c | 23 ++++++++++++++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 741c8165c06..80272b878c5 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3390,6 +3390,23 @@ 'features': [ { 'name': 'dynamic-auto-read-only', 'if': 'CONFIG_POSIX' } ] } +## +# @BlockdevOptionsHostDevice: +# Driver specific block device options for the host_device +# backend. +# +# @accept-read-only-device: allow opening a read-only device as +# read-write. If this is false, opening a host device read-write +# requires on Linux that the BLKROGET ioctl considers the device +# writable. If it is true, this check is skipped. (since: 11.2, +# default: false) +# +# Since: 11.2 +## +{ 'struct': 'BlockdevOptionsHostDevice', + 'base': 'BlockdevOptionsFile', + 'data': { '*accept-read-only-device': 'bool' } } + ## # @BlockdevOptionsNull: # Driver specific block device options for the null backend. @@ -4777,7 +4794,7 @@ 'ftps': 'BlockdevOptionsCurlFtps', 'host_cdrom': { 'type': 'BlockdevOptionsFile', 'if': 'HAVE_HOST_BLOCK_DEVICE' }, - 'host_device': { 'type': 'BlockdevOptionsFile', + 'host_device': { 'type': 'BlockdevOptionsHostDevice', 'if': 'HAVE_HOST_BLOCK_DEVICE' }, 'http': 'BlockdevOptionsCurlHttp', 'https': 'BlockdevOptionsCurlHttps', diff --git a/block/file-posix.c b/block/file-posix.c index bad7ccdb733..3318912b5ac 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -177,6 +177,7 @@ typedef struct BDRVRawState { bool has_laio_fdsync:1; bool use_linux_io_uring:1; bool use_mpath:1; + bool accept_read_only_device:1; int page_cache_inconsistent; /* errno from fdatasync failure */ bool has_fallocate; bool needs_alignment; @@ -513,7 +514,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) } } -static int check_hdev_writable(int fd) +static int check_hdev_writable(BlockDriverState *bs, int fd) { #if defined(BLKROGET) /* Linux block devices can be configured "read-only" using blockdev(8). @@ -523,10 +524,19 @@ static int check_hdev_writable(int fd) * bdrv_open() is supposed to fail if the disk is read-only. Explicitly * check for read-only block devices so that Linux block devices behave * properly. + * + * Users can use the accept-read-only-device option to skip the check if + * they deliberately open a read-only block device as writable, e.g. because + * it may become read-write later. */ + BDRVRawState *s = bs->opaque; struct stat st; int readonly = 0; + if (s->accept_read_only_device) { + return 0; + } + if (fstat(fd, &st)) { return -errno; } @@ -620,6 +630,11 @@ static QemuOptsList raw_runtime_opts = { .type = QEMU_OPT_BOOL, .help = "check that page cache was dropped on live migration (default: off)" }, + { + .name = "accept-read-only-device", + .type = QEMU_OPT_BOOL, + .help = "allow opening a read-only device as read-write (default: off)" + }, { /* end of list */ } }, }; @@ -737,8 +752,10 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, s->fd = fd; /* Check s->open_flags rather than bdrv_flags due to auto-read-only */ + s->accept_read_only_device = + qemu_opt_get_bool(opts, "accept-read-only-device", false); if (s->open_flags & O_RDWR) { - ret = check_hdev_writable(s->fd); + ret = check_hdev_writable(bs, s->fd); if (ret < 0) { error_setg_errno(errp, -ret, "The device is not writable"); goto fail; @@ -1116,7 +1133,7 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, int flags, } if (fd != -1 && (*open_flags & O_RDWR)) { - ret = check_hdev_writable(fd); + ret = check_hdev_writable(bs, fd); if (ret < 0) { qemu_close(fd); error_setg_errno(errp, -ret, "The device is not writable"); -- 2.56.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] file-posix: Add 'accept-read-only-device' option 2026-10-28 17:10 ` [PATCH 1/2] " Kevin Wolf @ 2026-10-28 17:54 ` Eric Blake 0 siblings, 0 replies; 7+ messages in thread From: Eric Blake @ 2026-10-28 17:54 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-block, stefanha, afaria, qemu-devel On Wed, Oct 28, 2026 at 06:10:56PM +0100, Kevin Wolf wrote: > The host_device block driver refuses to open a block device read-write > if the BLKROGET ioctl tells it that it's actually read-only. For such > devices it's possible to open a read-write file descriptor, but any > writes to the device would fail. > > There are valid use cases for which this check gets in the way, as > explained in the bug report: > > However, this behavior prevents a legitimate enterprise use case: > running VMs against storage replication target LUNs in a disaster > recovery configuration. Enterprise storage arrays (Dell Powermax, > Pure Storage) set the SCSI Write Protect bit on replication target > LUNs, causing the Linux kernel to mark them read-only via BLKROGET. > The VMs on the passive DR site need to start with these LUNs > attached - writes are expected to fail (the application middleware > knows it's on the passive side), and after a DR role swap the LUN > becomes writable and writes should succeed immediately without any > VM restart. > > In order to allow such use cases in which a currently read-only device > is deliberately opened read-write, add a new option to the host_device > driver that allows the user to skip the check. > > Buglink: https://redhat.atlassian.net/browse/RHEL-186136 > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qapi/block-core.json | 19 ++++++++++++++++++- > block/file-posix.c | 23 ++++++++++++++++++++--- > 2 files changed, 38 insertions(+), 4 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] iotests: Add loop-accept-ro-dev test 2026-10-28 17:10 [PATCH 0/2] file-posix: Add 'accept-read-only-device' option Kevin Wolf 2026-10-28 17:10 ` [PATCH 1/2] " Kevin Wolf @ 2026-10-28 17:10 ` Kevin Wolf 2026-10-28 18:10 ` Eric Blake 2026-10-28 19:49 ` [PATCH 0/2] file-posix: Add 'accept-read-only-device' option Stefan Hajnoczi 2 siblings, 1 reply; 7+ messages in thread From: Kevin Wolf @ 2026-10-28 17:10 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, stefanha, afaria, qemu-devel Add a test for the accept-read-only-device option of host_device. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/tests/loop-accept-ro-dev | 87 +++++++++++++++++++ .../qemu-iotests/tests/loop-accept-ro-dev.out | 19 ++++ 2 files changed, 106 insertions(+) create mode 100755 tests/qemu-iotests/tests/loop-accept-ro-dev create mode 100644 tests/qemu-iotests/tests/loop-accept-ro-dev.out diff --git a/tests/qemu-iotests/tests/loop-accept-ro-dev b/tests/qemu-iotests/tests/loop-accept-ro-dev new file mode 100755 index 00000000000..83ca67a0cf0 --- /dev/null +++ b/tests/qemu-iotests/tests/loop-accept-ro-dev @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# group: quick +# +# SPDX-License-Identifier: GPL-2.0-or-later +# +# Copyright Red Hat, Inc. +# +# Test the accept-read-only-device option on a loop device + +seq="$(basename $0)" +echo "QA output created by $seq" + +status=1 # failure is the default! + +_cleanup() { + if [ -n "$loopdev" ]; then + sudo losetup --detach "$loopdev" + fi + + _cleanup_test_img +} + +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +cd .. +. ./common.rc +. ./common.filter + +_supported_fmt raw +_supported_proto file +_supported_os Linux + +do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp stdio -nodefaults "$@" + echo +} + +run_qemu() +{ + echo -e '{"execute":"qmp_capabilities"}\n{"execute":"quit"}' | + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp | + _filter_generated_node_ids | _filter_imgfmt | + _filter_actual_image_size | _filter_img_info +} + +if ! sudo -n losetup &>/dev/null; then + _notrun "sudo losetup not available" +fi + +if ! command -v blockdev >/dev/null 2>&1; then + _notrun "blockdev tool not available" +fi + +echo +echo "=== Start QEMU on a loop device after blockdev --setro ===" +echo + +_make_test_img -f $IMGFMT 1M + +loopdev=$(sudo losetup --find --show "$TEST_IMG") +if [ -z "$loopdev" ]; then + _fail +fi + +sudo blockdev --setro "$loopdev" +sudo chmod a+rw "$loopdev" + +run_qemu \ + -blockdev host_device,node-name=disk,filename="$loopdev" \ + -device virtio-blk,drive=disk + +run_qemu \ + -blockdev host_device,node-name=disk,filename="$loopdev",accept-read-only-device=off \ + -device virtio-blk,drive=disk + +run_qemu \ + -blockdev host_device,node-name=disk,filename="$loopdev",accept-read-only-device=on \ + -device virtio-blk,drive=disk + +# success, all done +echo +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/tests/loop-accept-ro-dev.out b/tests/qemu-iotests/tests/loop-accept-ro-dev.out new file mode 100644 index 00000000000..dbf3de9fdf6 --- /dev/null +++ b/tests/qemu-iotests/tests/loop-accept-ro-dev.out @@ -0,0 +1,19 @@ +QA output created by loop-accept-ro-dev + +=== Start QEMU on a loop device after blockdev --setro === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 +Testing: -blockdev host_device,node-name=disk,filename=/dev/loop1 -device virtio-blk,drive=disk +QEMU_PROG: -blockdev host_device,node-name=disk,filename=/dev/loop1: The device is not writable: Permission denied + +Testing: -blockdev host_device,node-name=disk,filename=/dev/loop1,accept-read-only-device=off -device virtio-blk,drive=disk +QEMU_PROG: -blockdev host_device,node-name=disk,filename=/dev/loop1,accept-read-only-device=off: The device is not writable: Permission denied + +Testing: -blockdev host_device,node-name=disk,filename=/dev/loop1,accept-read-only-device=on -device virtio-blk,drive=disk +QMP_VERSION +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} +{"return": {}} + + +*** done -- 2.56.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] iotests: Add loop-accept-ro-dev test 2026-10-28 17:10 ` [PATCH 2/2] iotests: Add loop-accept-ro-dev test Kevin Wolf @ 2026-10-28 18:10 ` Eric Blake 2026-10-29 16:42 ` Kevin Wolf 0 siblings, 1 reply; 7+ messages in thread From: Eric Blake @ 2026-10-28 18:10 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-block, stefanha, afaria, qemu-devel On Wed, Oct 28, 2026 at 06:10:57PM +0100, Kevin Wolf wrote: > Add a test for the accept-read-only-device option of host_device. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/tests/loop-accept-ro-dev | 87 +++++++++++++++++++ > .../qemu-iotests/tests/loop-accept-ro-dev.out | 19 ++++ > 2 files changed, 106 insertions(+) > create mode 100755 tests/qemu-iotests/tests/loop-accept-ro-dev > create mode 100644 tests/qemu-iotests/tests/loop-accept-ro-dev.out > > + > +run_qemu() > +{ > + echo -e '{"execute":"qmp_capabilities"}\n{"execute":"quit"}' | 'printf %s' is slightly more portable than 'echo -e'; even though this script uses /bin/bash, you can use 'shopt -s xpg_echo' to break the latter. On the other hand, this is copy-paste from other tests, so not a showstopper. > + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp | > + _filter_generated_node_ids | _filter_imgfmt | > + _filter_actual_image_size | _filter_img_info > +} > + > +if ! sudo -n losetup &>/dev/null; then > + _notrun "sudo losetup not available" > +fi Huh - we seem to have several outstanding patches related to probing whether losetup is usable: https://lists.nongnu.org/archive/html/qemu-devel/2026-10/msg02963.html https://lists.nongnu.org/archive/html/qemu-devel/2026-10/msg01918.html Is this a common enough pattern in our qemu-iotests that we should factor it out? > +++ b/tests/qemu-iotests/tests/loop-accept-ro-dev.out > @@ -0,0 +1,19 @@ > +QA output created by loop-accept-ro-dev > + > +=== Start QEMU on a loop device after blockdev --setro === > + > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 > +Testing: -blockdev host_device,node-name=disk,filename=/dev/loop1 -device virtio-blk,drive=disk > +QEMU_PROG: -blockdev host_device,node-name=disk,filename=/dev/loop1: The device is not writable: Permission denied > + > +Testing: -blockdev host_device,node-name=disk,filename=/dev/loop1,accept-read-only-device=off -device virtio-blk,drive=disk /dev/loop1 might already be busy, at which point losetup will pick /dev/loop2 or similar. This output needs to be filtered. The rest of the test looks okay. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] iotests: Add loop-accept-ro-dev test 2026-10-28 18:10 ` Eric Blake @ 2026-10-29 16:42 ` Kevin Wolf 0 siblings, 0 replies; 7+ messages in thread From: Kevin Wolf @ 2026-10-29 16:42 UTC (permalink / raw) To: Eric Blake; +Cc: qemu-block, stefanha, afaria, qemu-devel Am 28.10.2026 um 19:10 hat Eric Blake geschrieben: > On Wed, Oct 28, 2026 at 06:10:57PM +0100, Kevin Wolf wrote: > > Add a test for the accept-read-only-device option of host_device. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > tests/qemu-iotests/tests/loop-accept-ro-dev | 87 +++++++++++++++++++ > > .../qemu-iotests/tests/loop-accept-ro-dev.out | 19 ++++ > > 2 files changed, 106 insertions(+) > > create mode 100755 tests/qemu-iotests/tests/loop-accept-ro-dev > > create mode 100644 tests/qemu-iotests/tests/loop-accept-ro-dev.out > > > > > + > > +run_qemu() > > +{ > > + echo -e '{"execute":"qmp_capabilities"}\n{"execute":"quit"}' | > > 'printf %s' is slightly more portable than 'echo -e'; even though this > script uses /bin/bash, you can use 'shopt -s xpg_echo' to break the > latter. On the other hand, this is copy-paste from other tests, so not > a showstopper. Given that I have to fix the test anyway, I'll change it. > > + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp | > > + _filter_generated_node_ids | _filter_imgfmt | > > + _filter_actual_image_size | _filter_img_info > > +} > > + > > +if ! sudo -n losetup &>/dev/null; then > > + _notrun "sudo losetup not available" > > +fi > > Huh - we seem to have several outstanding patches related to probing > whether losetup is usable: > > https://lists.nongnu.org/archive/html/qemu-devel/2026-10/msg02963.html > https://lists.nongnu.org/archive/html/qemu-devel/2026-10/msg01918.html > > Is this a common enough pattern in our qemu-iotests that we should > factor it out? Maybe. There seem to be four tests (including this one) that use losetup. This can be done in a separate patch, though. > > +++ b/tests/qemu-iotests/tests/loop-accept-ro-dev.out > > @@ -0,0 +1,19 @@ > > +QA output created by loop-accept-ro-dev > > + > > +=== Start QEMU on a loop device after blockdev --setro === > > + > > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 > > +Testing: -blockdev host_device,node-name=disk,filename=/dev/loop1 -device virtio-blk,drive=disk > > +QEMU_PROG: -blockdev host_device,node-name=disk,filename=/dev/loop1: The device is not writable: Permission denied > > + > > +Testing: -blockdev host_device,node-name=disk,filename=/dev/loop1,accept-read-only-device=off -device virtio-blk,drive=disk > > /dev/loop1 might already be busy, at which point losetup will pick > /dev/loop2 or similar. This output needs to be filtered. Oops, I'm too used to qemu-iotests filtering out all the filenames, but obviously that doesn't work for something outside the scratch directory. Thanks! Kevin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] file-posix: Add 'accept-read-only-device' option 2026-10-28 17:10 [PATCH 0/2] file-posix: Add 'accept-read-only-device' option Kevin Wolf 2026-10-28 17:10 ` [PATCH 1/2] " Kevin Wolf 2026-10-28 17:10 ` [PATCH 2/2] iotests: Add loop-accept-ro-dev test Kevin Wolf @ 2026-10-28 19:49 ` Stefan Hajnoczi 2 siblings, 0 replies; 7+ messages in thread From: Stefan Hajnoczi @ 2026-10-28 19:49 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-block, afaria, qemu-devel On Wed, Oct 28, 2026 at 06:10:55PM +0100, Kevin Wolf wrote: > Kevin Wolf (2): > file-posix: Add 'accept-read-only-device' option > iotests: Add loop-accept-ro-dev test > > qapi/block-core.json | 19 +++- > block/file-posix.c | 23 ++++- > tests/qemu-iotests/tests/loop-accept-ro-dev | 87 +++++++++++++++++++ > .../qemu-iotests/tests/loop-accept-ro-dev.out | 19 ++++ > 4 files changed, 144 insertions(+), 4 deletions(-) > create mode 100755 tests/qemu-iotests/tests/loop-accept-ro-dev > create mode 100644 tests/qemu-iotests/tests/loop-accept-ro-dev.out > > -- > 2.56.0 > Modulo Eric's comment about /dev/loop1 being hardcoded in the test output: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-29 16:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-28 17:10 [PATCH 0/2] file-posix: Add 'accept-read-only-device' option Kevin Wolf 2026-10-28 17:10 ` [PATCH 1/2] " Kevin Wolf 2026-10-28 17:54 ` Eric Blake 2026-10-28 17:10 ` [PATCH 2/2] iotests: Add loop-accept-ro-dev test Kevin Wolf 2026-10-28 18:10 ` Eric Blake 2026-10-29 16:42 ` Kevin Wolf 2026-10-28 19:49 ` [PATCH 0/2] file-posix: Add 'accept-read-only-device' option Stefan Hajnoczi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.