10:05 a.m.
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
10:05 a.m.
New subject: [PATCH 1/3] qemuxmlconftest: Allow testing of the 'writable' flag for passed FDs for disks
Pass also the 'writable' state to the fake passed FDs so that we can
test it.
Signed-off-by: Peter Krempa <pkrempa(a)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
10:05 a.m.
New subject: [PATCH 2/3] qemuxmlconftest: Add testing of FDs with 'writable' flag in 'disk-source-fd'
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(a)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
10:05 a.m.
New subject: [PATCH 3/3] qemu: domain: Initialize FD passthrough for a virStorageSource before using it
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(a)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
5:59 a.m.
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(a)redhat.com>
177
days inactive
178
days old
4 comments
2 participants
participants (2)
-
Jiri Denemark -
Peter Krempa