On Fri, Aug 16, 2024 at 04:42:33PM +0200, Peter Krempa wrote:
Attempting to start qemu with or hotplug an empty 'usb-storage' based
disk results in the following error:
qemu-system-x86_64: -device {"driver":"usb-storage","bus":"usb.0","port":"2","id":"usb-disk1","removable":true}: drive property not set
Reject such config at validation step and adjust tests.
It's weird that we even support usb cdrom as qemu cannot even emulate that, at least to my knowledge. Other than that the patch looks fine, but I cannot shake off the feeling of fishiness with this one. Could you bring me up to speed how this even happened to be "supported"? Thanks, Martin
Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
src/qemu/qemu_validate.c | 6 ++++
.../disk-cdrom-bus-other.x86_64-latest.args | 5 ++--
.../disk-cdrom-bus-other.x86_64-latest.xml | 5 ----
.../qemuxmlconfdata/disk-cdrom-bus-other.xml | 5 ----
.../disk-cdrom-usb-empty.x86_64-latest.err | 1 +
.../qemuxmlconfdata/disk-cdrom-usb-empty.xml | 29 +++++++++++++++++++
tests/qemuxmlconftest.c | 1 +
7 files changed, 39 insertions(+), 13 deletions(-)
create mode 100644 tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.err
create mode 100644 tests/qemuxmlconfdata/disk-cdrom-usb-empty.xml
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index b885fe7c77..48abc3f850 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3037,6 +3037,12 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk,
return -1;
}
+ if (virStorageSourceIsEmpty(disk->src)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("'usb' disk must not be empty"));
+ return -1;
+ }
+
if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args
index e0d2452a10..e1406af663 100644
--- a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args
+++ b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.args
@@ -27,9 +27,8 @@ 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"}' \
--blockdev '{"driver":"file","filename":"/root/boot.iso","node-name":"libvirt-2-storage","read-only":true}' \
--device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-2-storage","id":"usb-disk0","removable":false}' \
--device '{"driver":"usb-storage","bus":"usb.0","port":"2","id":"usb-disk1","removable":false}' \
+-blockdev '{"driver":"file","filename":"/root/boot.iso","node-name":"libvirt-1-storage","read-only":true}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"1","drive":"libvirt-1-storage","id":"usb-disk0","removable":false}' \
-audiodev '{"id":"audio1","driver":"none"}' \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
diff --git a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml
index 312e735947..f16fe63057 100644
--- a/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml
+++ b/tests/qemuxmlconfdata/disk-cdrom-bus-other.x86_64-latest.xml
@@ -23,11 +23,6 @@
<target dev='sda' bus='usb'/>
<readonly/>
</disk>
- <disk type='file' device='cdrom'>
- <driver name='qemu' type='raw'/>
- <target dev='sdb' bus='usb'/>
- <readonly/>
- </disk>
<controller type='usb' index='0' model='piix3-uhci'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
</controller>
diff --git a/tests/qemuxmlconfdata/disk-cdrom-bus-other.xml b/tests/qemuxmlconfdata/disk-cdrom-bus-other.xml
index 9564dc11be..119ce297a0 100644
--- a/tests/qemuxmlconfdata/disk-cdrom-bus-other.xml
+++ b/tests/qemuxmlconfdata/disk-cdrom-bus-other.xml
@@ -20,11 +20,6 @@
<target dev='sda' bus='usb'/>
<readonly/>
</disk>
- <disk type='file' device='cdrom'>
- <driver name='qemu' type='raw'/>
- <target dev='sdb' bus='usb'/>
- <readonly/>
- </disk>
<controller type='usb' index='0'/>
<controller type='ide' index='0'/>
<controller type='pci' index='0' model='pci-root'/>
diff --git a/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.err b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.err
new file mode 100644
index 0000000000..a00cb0a4b2
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.x86_64-latest.err
@@ -0,0 +1 @@
+unsupported configuration: 'usb' disk must not be empty
diff --git a/tests/qemuxmlconfdata/disk-cdrom-usb-empty.xml b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.xml
new file mode 100644
index 0000000000..fc1cf45085
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-cdrom-usb-empty.xml
@@ -0,0 +1,29 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <disk type='file' device='cdrom'>
+ <driver name='qemu' type='raw'/>
+ <target dev='sdb' bus='usb'/>
+ <readonly/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 89292af300..436d58b977 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1536,6 +1536,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("disk-cdrom");
DO_TEST_CAPS_LATEST("disk-cdrom-empty-network-invalid");
DO_TEST_CAPS_LATEST("disk-cdrom-bus-other");
+ DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-cdrom-usb-empty");
DO_TEST_CAPS_LATEST("disk-cdrom-network");
DO_TEST_CAPS_LATEST("disk-cdrom-tray");
DO_TEST_CAPS_LATEST("disk-floppy");
--
2.46.0
Attachment:
signature.asc
Description: PGP signature
