[PATCH 0/3] qemu: fix setup of FD passed disks
See 3/3 Peter Krempa (3): qemuxmlconftest: Allow testing of the 'writable' flag for passed FDs for disks qemuxmlconftest: Add testing of FDs with 'writable' flag in 'disk-source-fd' qemu: domain: Initialize FD passthrough for a virStorageSource before using it src/qemu/qemu_domain.c | 6 ++-- .../disk-source-fd.x86_64-latest.args | 36 +++++++++++-------- .../disk-source-fd.x86_64-latest.xml | 21 +++++++++-- tests/qemuxmlconfdata/disk-source-fd.xml | 16 +++++++-- tests/qemuxmlconftest.c | 9 +++-- tests/testutilsqemu.c | 2 ++ tests/testutilsqemu.h | 2 +- 7 files changed, 67 insertions(+), 25 deletions(-) -- 2.48.1
Pass also the 'writable' state to the fake passed FDs so that we can test it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxmlconftest.c | 6 +++--- tests/testutilsqemu.c | 2 ++ tests/testutilsqemu.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 3ddb19a7ed..99c4efc9d6 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1655,9 +1655,9 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-backing-chains-noindex"); DO_TEST_CAPS_LATEST("disk-qcow2-datafile-store"); DO_TEST_CAPS_ARCH_LATEST_FULL("disk-source-fd", "x86_64", - ARG_FD_GROUP, "testgroup2", 2, 200, 205, - ARG_FD_GROUP, "testgroup5", 1, 204, - ARG_FD_GROUP, "testgroup6", 2, 247, 248); + ARG_FD_GROUP, "testgroup2", false, 2, 200, 205, + ARG_FD_GROUP, "testgroup5", false, 1, 204, + ARG_FD_GROUP, "testgroup6", false, 2, 247, 248); DO_TEST_CAPS_LATEST("disk-slices"); DO_TEST_CAPS_LATEST("disk-rotation"); diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index abc425b9b7..44011c2b36 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -704,11 +704,13 @@ testQemuInfoSetArgs(testQemuInfo *info, virStorageSourceFDTuple *new = virStorageSourceFDTupleNew(); const char *fdname = va_arg(argptr, char *); VIR_AUTOCLOSE fakefd = open("/dev/zero", O_RDWR); + bool writable = va_arg(argptr, int); size_t i; new->nfds = va_arg(argptr, unsigned int); new->fds = g_new0(int, new->nfds); new->testfds = g_new0(int, new->nfds); + new->writable = writable; for (i = 0; i < new->nfds; i++) { new->testfds[i] = va_arg(argptr, unsigned int); diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 74e307d653..20135b8390 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -49,7 +49,7 @@ typedef enum { ARG_CAPS_VER, ARG_CAPS_VARIANT, ARG_CAPS_HOST_CPU_MODEL, - ARG_FD_GROUP, /* name, nfds, fd[0], ... fd[n-1] */ + ARG_FD_GROUP, /* name, writable, nfds, fd[0], ... fd[n-1] */ ARG_VDPA_FD, /* vdpadev, fd */ ARG_NBDKIT_CAPS, ARG_END, -- 2.48.1
Add few examples of fd groups with the 'writable' flag set, when passing a single FD. Notably as a top level image of a readonly disk (even when that doesn't make much sense) and also as a base image of a chain. Note that this documents a status quo of a bug fixed in upcoming patch. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../disk-source-fd.x86_64-latest.args | 36 +++++++++++-------- .../disk-source-fd.x86_64-latest.xml | 21 +++++++++-- tests/qemuxmlconfdata/disk-source-fd.xml | 16 +++++++-- tests/qemuxmlconftest.c | 3 ++ 4 files changed, 57 insertions(+), 19 deletions(-) diff --git a/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args b/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args index 1341b7d032..d77b3ca505 100644 --- a/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args @@ -27,21 +27,27 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -no-shutdown \ -boot strict=on \ -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ --add-fd set=2,fd=200,opaque=libvirt-4-storage0 \ --add-fd set=2,fd=205,opaque=libvirt-4-storage1 \ --blockdev '{"driver":"file","filename":"/dev/fdset/2","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"qcow2","file":"libvirt-4-storage"}' \ --device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-4-format","id":"virtio-disk4","bootindex":1}' \ --blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071876","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"node-name":"libvirt-3-format","read-only":true,"driver":"qcow2","file":"libvirt-3-storage","backing":null}' \ --add-fd set=1,fd=247,opaque=libvirt-2-storage0 \ --add-fd set=1,fd=248,opaque=libvirt-2-storage1 \ --blockdev '{"driver":"file","filename":"/dev/fdset/1","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2","file":"libvirt-2-storage","backing":"libvirt-3-format"}' \ --add-fd set=0,fd=204,opaque=libvirt-1-storage0 \ --blockdev '{"driver":"file","filename":"/dev/fdset/0","node-name":"libvirt-1-storage","read-only":false,"discard":"unmap"}' \ --blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage","backing":"libvirt-2-format"}' \ --device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-1-format","id":"virtio-disk5"}' \ +-add-fd set=5,fd=200,opaque=libvirt-6-storage0 \ +-add-fd set=5,fd=205,opaque=libvirt-6-storage1 \ +-blockdev '{"driver":"file","filename":"/dev/fdset/5","node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-6-format","read-only":false,"driver":"qcow2","file":"libvirt-6-storage"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-6-format","id":"virtio-disk4","bootindex":1}' \ +-add-fd set=4,fd=209,opaque=libvirt-5-storage0 \ +-blockdev '{"driver":"file","filename":"/dev/fdset/4","node-name":"libvirt-5-storage","read-only":true}' \ +-add-fd set=3,fd=247,opaque=libvirt-4-storage0 \ +-add-fd set=3,fd=248,opaque=libvirt-4-storage1 \ +-blockdev '{"driver":"file","filename":"/dev/fdset/3","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"qcow2","file":"libvirt-4-storage","backing":"libvirt-5-storage"}' \ +-add-fd set=2,fd=204,opaque=libvirt-3-storage0 \ +-blockdev '{"driver":"file","filename":"/dev/fdset/2","node-name":"libvirt-3-storage","read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"qcow2","file":"libvirt-3-storage","backing":"libvirt-4-format"}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-3-format","id":"virtio-disk5"}' \ +-add-fd set=1,fd=207,opaque=libvirt-2-storage0 \ +-blockdev '{"driver":"file","filename":"/dev/fdset/1","node-name":"libvirt-2-storage","read-only":true}' \ +-device '{"driver":"ide-cd","bus":"ide.0","unit":0,"drive":"libvirt-2-storage","id":"ide0-0-0"}' \ +-add-fd set=0,fd=208,opaque=libvirt-1-storage0 \ +-blockdev '{"driver":"file","filename":"/dev/fdset/0","node-name":"libvirt-1-storage","read-only":true}' \ +-device '{"driver":"ide-cd","bus":"ide.0","unit":1,"drive":"libvirt-1-storage","id":"ide0-0-1"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x4"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ diff --git a/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.xml index 9ab5e9443f..d9917c7d53 100644 --- a/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.xml @@ -30,18 +30,35 @@ <format type='qcow2'/> <source file='/var/lib/libvirt/images/rhel7.1484071877' fdgroup='testgroup6'/> <backingStore type='file'> - <format type='qcow2'/> - <source file='/var/lib/libvirt/images/rhel7.1484071876'/> + <format type='raw'/> + <source file='/var/lib/libvirt/images/rhel7.1484071876' fdgroup='raw-rw-base'/> <backingStore/> </backingStore> </backingStore> <target dev='vdf' bus='virtio'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/path/to/cdimage-ro' fdgroup='cdimage-ro'/> + <target dev='hda' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/path/to/cdimage-rw' fdgroup='cdimage-rw'/> + <target dev='hdb' bus='ide'/> + <readonly/> + <address type='drive' controller='0' bus='0' target='0' unit='1'/> + </disk> <controller type='usb' index='0' model='piix3-uhci'> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> </controller> <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <audio id='1' type='none'/> diff --git a/tests/qemuxmlconfdata/disk-source-fd.xml b/tests/qemuxmlconfdata/disk-source-fd.xml index d8c47fa364..93a3fcfbf7 100644 --- a/tests/qemuxmlconfdata/disk-source-fd.xml +++ b/tests/qemuxmlconfdata/disk-source-fd.xml @@ -19,6 +19,18 @@ <source file='/path/to/blah' fdgroup='testgroup2'/> <target dev='vde' bus='virtio'/> </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/path/to/cdimage-ro' fdgroup='cdimage-ro'/> + <readonly/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/path/to/cdimage-rw' fdgroup='cdimage-rw'/> + <readonly/> + <target dev='hdb' bus='ide'/> + </disk> <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/rhel7.1484071880' fdgroup='testgroup5'/> @@ -26,8 +38,8 @@ <format type='qcow2'/> <source file='/var/lib/libvirt/images/rhel7.1484071877' fdgroup='testgroup6'/> <backingStore type='file'> - <format type='qcow2'/> - <source file='/var/lib/libvirt/images/rhel7.1484071876'/> + <format type='raw'/> + <source file='/var/lib/libvirt/images/rhel7.1484071876' fdgroup='raw-rw-base'/> <backingStore/> </backingStore> </backingStore> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 99c4efc9d6..1f0068864a 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1657,6 +1657,9 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST_FULL("disk-source-fd", "x86_64", ARG_FD_GROUP, "testgroup2", false, 2, 200, 205, ARG_FD_GROUP, "testgroup5", false, 1, 204, + ARG_FD_GROUP, "cdimage-ro", false, 1, 207, + ARG_FD_GROUP, "cdimage-rw", true, 1, 208, + ARG_FD_GROUP, "raw-rw-base", true, 1, 209, ARG_FD_GROUP, "testgroup6", false, 2, 247, 248); DO_TEST_CAPS_LATEST("disk-slices"); -- 2.48.1
The call to 'qemuBlockStorageSourceNeedsFormatLayer()' bases the decision also on the state of the passed FD, so we must initialize the passthrough data via 'qemuDomainPrepareStorageSourceFDs()' before the aforementioned call. In the test change it's visible that we didn't add the necessary 'raw' driver which allows the 'protocol' blockdev to be opened in 'rw' mode so that qemu picks the proper file descriptior while keeping the device read-only. Resolves: https://issues.redhat.com/browse/RHEL-37519 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 6 +++--- .../qemuxmlconfdata/disk-source-fd.x86_64-latest.args | 10 ++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 56f86140cd..cf05dca55a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9661,6 +9661,9 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, /* qemuBlockStorageSourceSetStorageNodename steals 'nodestorage' */ qemuBlockStorageSourceSetStorageNodename(src, nodestorage); + if (qemuDomainPrepareStorageSourceFDs(src, priv) < 0) + return -1; + if (qemuBlockStorageSourceNeedsFormatLayer(src, priv->qemuCaps)) { char *nodeformat = g_strdup_printf("%s-format", nodenameprefix); @@ -9701,9 +9704,6 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, if (qemuDomainPrepareStorageSourceNFS(src) < 0) return -1; - if (qemuDomainPrepareStorageSourceFDs(src, priv) < 0) - return -1; - return 0; } diff --git a/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args b/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args index d77b3ca505..27d852cf32 100644 --- a/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-source-fd.x86_64-latest.args @@ -33,11 +33,12 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -blockdev '{"node-name":"libvirt-6-format","read-only":false,"driver":"qcow2","file":"libvirt-6-storage"}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-6-format","id":"virtio-disk4","bootindex":1}' \ -add-fd set=4,fd=209,opaque=libvirt-5-storage0 \ --blockdev '{"driver":"file","filename":"/dev/fdset/4","node-name":"libvirt-5-storage","read-only":true}' \ +-blockdev '{"driver":"file","filename":"/dev/fdset/4","node-name":"libvirt-5-storage","read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-5-format","read-only":true,"driver":"raw","file":"libvirt-5-storage"}' \ -add-fd set=3,fd=247,opaque=libvirt-4-storage0 \ -add-fd set=3,fd=248,opaque=libvirt-4-storage1 \ -blockdev '{"driver":"file","filename":"/dev/fdset/3","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ --blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"qcow2","file":"libvirt-4-storage","backing":"libvirt-5-storage"}' \ +-blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"qcow2","file":"libvirt-4-storage","backing":"libvirt-5-format"}' \ -add-fd set=2,fd=204,opaque=libvirt-3-storage0 \ -blockdev '{"driver":"file","filename":"/dev/fdset/2","node-name":"libvirt-3-storage","read-only":false,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"qcow2","file":"libvirt-3-storage","backing":"libvirt-4-format"}' \ @@ -46,8 +47,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -blockdev '{"driver":"file","filename":"/dev/fdset/1","node-name":"libvirt-2-storage","read-only":true}' \ -device '{"driver":"ide-cd","bus":"ide.0","unit":0,"drive":"libvirt-2-storage","id":"ide0-0-0"}' \ -add-fd set=0,fd=208,opaque=libvirt-1-storage0 \ --blockdev '{"driver":"file","filename":"/dev/fdset/0","node-name":"libvirt-1-storage","read-only":true}' \ --device '{"driver":"ide-cd","bus":"ide.0","unit":1,"drive":"libvirt-1-storage","id":"ide0-0-1"}' \ +-blockdev '{"driver":"file","filename":"/dev/fdset/0","node-name":"libvirt-1-storage","read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw","file":"libvirt-1-storage"}' \ +-device '{"driver":"ide-cd","bus":"ide.0","unit":1,"drive":"libvirt-1-format","id":"ide0-0-1"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x4"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -- 2.48.1
On Thu, Feb 06, 2025 at 17:05:43 +0100, Peter Krempa wrote:
See 3/3
Peter Krempa (3): qemuxmlconftest: Allow testing of the 'writable' flag for passed FDs for disks qemuxmlconftest: Add testing of FDs with 'writable' flag in 'disk-source-fd' qemu: domain: Initialize FD passthrough for a virStorageSource before using it
src/qemu/qemu_domain.c | 6 ++-- .../disk-source-fd.x86_64-latest.args | 36 +++++++++++-------- .../disk-source-fd.x86_64-latest.xml | 21 +++++++++-- tests/qemuxmlconfdata/disk-source-fd.xml | 16 +++++++-- tests/qemuxmlconftest.c | 9 +++-- tests/testutilsqemu.c | 2 ++ tests/testutilsqemu.h | 2 +- 7 files changed, 67 insertions(+), 25 deletions(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Jiri Denemark -
Peter Krempa