From: Peter Krempa <pkrempa@redhat.com>
Historically libvirt specified 'usb-storage' as driver for USB disks.
This though combined with '-blockdev' doesn't properly configure the
device to look like CDROM for <disk type='cdrom'>.
'usb-bot' acts like a controler and allows explicitly connecting a
-device to it.
In qemu the devices share implementation so they are effectively
identical and can be used interchangably. There is though a slight
difference in how the storage device itself (the SCSI bit) looks when
CDROM as they were not declared as cdrom before.
As this is effectively a bugfix we'll be fixing the behaviour for the
default configuration. The possibility to explicitly set the model is
added as a possibility for working around possible problems if they'd
appear.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
docs/formatdomain.rst | 23 +++++--
src/conf/domain_conf.c | 2 +
src/conf/domain_conf.h | 3 +
src/conf/schemas/domaincommon.rng | 2 +
src/qemu/qemu_domain_address.c | 2 +
.../disk-usb-device-model.x86_64-latest.args | 46 +++++++++++++
.../disk-usb-device-model.x86_64-latest.xml | 64 +++++++++++++++++++
.../qemuxmlconfdata/disk-usb-device-model.xml | 46 +++++++++++++
tests/qemuxmlconftest.c | 1 +
9 files changed, 185 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
create mode 100644 tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/disk-usb-device-model.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index ae054a52b3..e4ebf061c7 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2862,10 +2862,25 @@ paravirtualized driver is specified via the ``disk`` element.
``model``
Indicates the emulated device model of the disk. Typically this is
- indicated solely by the ``bus`` property but for ``bus`` "virtio" the
- model can be specified further with "virtio", "virtio-transitional" or
- "virtio-non-transitional". See `virtio device models`_
- for more details. :since:`Since 5.2.0`
+ indicated solely by the ``bus`` property.
+
+ For ``bus`` "virtio" the model can be specified further with "virtio",
+ "virtio-transitional" or "virtio-non-transitional". See `virtio device
+ models`_ for more details. :since:`Since 5.2.0`
+
+ For ``bus`` "usb" the model can be specified further with ``usb-storage``
+ or ``usb-bot``. There is no difference between the two models for
+ ``<disk type='disk'``. However with ``usb-bot`` a device configured as
+ ``<disk type='cdrom'>`` is properly exposed as a cdrom device inside the
+ guest OS. Unfortunately this configuration is not ABI compatible and thus
+ it can't be interchanged.
+
+ The QEMU hypervisor driver will pick ``usb-bot`` for cold starts or
+ hotplug for cdrom devices to properly configure the devices. This is
+ not compatible for migration to older versions of libvirt and explicit
+ configuration needs to be used.
+ :since:`Since 11.5.0`; relevant only for ``QEMU`` hypervisor.
+
``rawio``
Indicates whether the disk needs rawio capability. Valid settings are
"yes" or "no" (default is "no"). If any one disk in a domain has
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1a6c8afb1d..2882a7746b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1399,6 +1399,8 @@ VIR_ENUM_IMPL(virDomainDiskModel,
"virtio",
"virtio-transitional",
"virtio-non-transitional",
+ "usb-storage",
+ "usb-bot",
);
VIR_ENUM_IMPL(virDomainDiskMirrorState,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3d380073cf..73e8a2fb99 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -439,6 +439,9 @@ typedef enum {
VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL,
VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL,
+ VIR_DOMAIN_DISK_MODEL_USB_STORAGE,
+ VIR_DOMAIN_DISK_MODEL_USB_BOT,
+
VIR_DOMAIN_DISK_MODEL_LAST
} virDomainDiskModel;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index b1fe51f519..a80005562a 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -1766,6 +1766,8 @@
<value>virtio</value>
<value>virtio-transitional</value>
<value>virtio-non-transitional</value>
+ <value>usb-storage</value>
+ <value>usb-bot</value>
</choice>
</attribute>
</optional>
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index bb86cfa0c3..3a23f70d39 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -729,6 +729,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
case VIR_DOMAIN_DISK_MODEL_DEFAULT:
return virtioFlags;
case VIR_DOMAIN_DISK_MODEL_LAST:
+ case VIR_DOMAIN_DISK_MODEL_USB_STORAGE:
+ case VIR_DOMAIN_DISK_MODEL_USB_BOT:
break;
}
return 0;
diff --git a/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
new file mode 100644
index 0000000000..6d31319a49
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
@@ -0,0 +1,46 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=219136k \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-device '{"driver":"usb-hub","id":"hub0","bus":"usb.0","port":"1"}' \
+-blockdev '{"driver":"file","filename":"/tmp/img1","node-name":"libvirt-6-storage","read-only":false}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"2","drive":"libvirt-6-storage","id":"usb-disk0","bootindex":1,"removable":false}' \
+-blockdev '{"driver":"file","filename":"/tmp/img2","node-name":"libvirt-5-storage","read-only":true}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"1.1","drive":"libvirt-5-storage","id":"usb-disk1","removable":false}' \
+-blockdev '{"driver":"file","filename":"/tmp/img3","node-name":"libvirt-4-storage","read-only":false}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"1.2","drive":"libvirt-4-storage","id":"usb-disk2","removable":false}' \
+-blockdev '{"driver":"file","filename":"/tmp/img4","node-name":"libvirt-3-storage","read-only":true}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"1.3","drive":"libvirt-3-storage","id":"usb-disk3","removable":false}' \
+-blockdev '{"driver":"file","filename":"/tmp/img5","node-name":"libvirt-2-storage","read-only":false}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"1.4","drive":"libvirt-2-storage","id":"usb-disk4","removable":false}' \
+-blockdev '{"driver":"file","filename":"/tmp/img6","node-name":"libvirt-1-storage","read-only":true}' \
+-device '{"driver":"usb-storage","bus":"usb.0","port":"1.5","drive":"libvirt-1-storage","id":"usb-disk5","removable":false}' \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.xml
new file mode 100644
index 0000000000..351257bc4a
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.xml
@@ -0,0 +1,64 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <cpu mode='custom' match='exact' check='none'>
+ <model fallback='forbid'>qemu64</model>
+ </cpu>
+ <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='disk' model='usb-storage'>
+ <driver name='qemu' type='raw'/>
+ <source file='/tmp/img1'/>
+ <target dev='sda' bus='usb'/>
+ </disk>
+ <disk type='file' device='cdrom' model='usb-storage'>
+ <driver name='qemu' type='raw'/>
+ <source file='/tmp/img2'/>
+ <target dev='sdb' bus='usb'/>
+ <readonly/>
+ </disk>
+ <disk type='file' device='disk' model='usb-bot'>
+ <driver name='qemu' type='raw'/>
+ <source file='/tmp/img3'/>
+ <target dev='sdc' bus='usb'/>
+ </disk>
+ <disk type='file' device='cdrom' model='usb-bot'>
+ <driver name='qemu' type='raw'/>
+ <source file='/tmp/img4'/>
+ <target dev='sdd' bus='usb'/>
+ <readonly/>
+ </disk>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source file='/tmp/img5'/>
+ <target dev='sde' bus='usb'/>
+ </disk>
+ <disk type='file' device='cdrom'>
+ <driver name='qemu' type='raw'/>
+ <source file='/tmp/img6'/>
+ <target dev='sdf' bus='usb'/>
+ <readonly/>
+ </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'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <audio id='1' type='none'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconfdata/disk-usb-device-model.xml b/tests/qemuxmlconfdata/disk-usb-device-model.xml
new file mode 100644
index 0000000000..47c8b698e9
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-usb-device-model.xml
@@ -0,0 +1,46 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</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='disk' model='usb-storage'>
+ <source file='/tmp/img1'/>
+ <target dev='sda' bus='usb'/>
+ </disk>
+ <disk type='file' device='cdrom' model='usb-storage'>
+ <source file='/tmp/img2'/>
+ <target dev='sdb' bus='usb'/>
+ <readonly/>
+ </disk>
+ <disk type='file' device='disk' model='usb-bot'>
+ <source file='/tmp/img3'/>
+ <target dev='sdc' bus='usb'/>
+ </disk>
+ <disk type='file' device='cdrom' model='usb-bot'>
+ <source file='/tmp/img4'/>
+ <target dev='sdd' bus='usb'/>
+ <readonly/>
+ </disk>
+ <disk type='file' device='disk'>
+ <source file='/tmp/img5'/>
+ <target dev='sde' bus='usb'/>
+ </disk>
+ <disk type='file' device='cdrom'>
+ <source file='/tmp/img6'/>
+ <target dev='sdf' bus='usb'/>
+ <readonly/>
+ </disk>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index f2fc09d9fd..858dde6ae8 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1621,6 +1621,7 @@ mymain(void)
ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
ARG_QEMU_CAPS_DEL, QEMU_CAPS_DEVICE_USB_BOT, QEMU_CAPS_LAST,
ARG_END);
+ DO_TEST_CAPS_LATEST("disk-usb-device-model");
DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-usb-pci");
DO_TEST_CAPS_LATEST("disk-scsi");
DO_TEST_CAPS_LATEST("disk-scsi-device-auto");
--
2.49.0On Mon, Jun 23, 2025 at 21:59:13 +0200, Peter Krempa wrote:
> From: Peter Krempa <pkrempa@redhat.com>
>
> Historically libvirt specified 'usb-storage' as driver for USB disks.
> This though combined with '-blockdev' doesn't properly configure the
> device to look like CDROM for <disk type='cdrom'>.
>
> 'usb-bot' acts like a controler and allows explicitly connecting a
> -device to it.
>
> In qemu the devices share implementation so they are effectively
> identical and can be used interchangably. There is though a slight
> difference in how the storage device itself (the SCSI bit) looks when
> CDROM as they were not declared as cdrom before.
Looks like some words were dropped from the last sentence above.
> As this is effectively a bugfix we'll be fixing the behaviour for the
> default configuration. The possibility to explicitly set the model is
> added as a possibility for working around possible problems if they'd
> appear.
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> docs/formatdomain.rst | 23 +++++--
> src/conf/domain_conf.c | 2 +
> src/conf/domain_conf.h | 3 +
> src/conf/schemas/domaincommon.rng | 2 +
> src/qemu/qemu_domain_address.c | 2 +
> .../disk-usb-device-model.x86_64-latest.args | 46 +++++++++++++
> .../disk-usb-device-model.x86_64-latest.xml | 64 +++++++++++++++++++
> .../qemuxmlconfdata/disk-usb-device-model.xml | 46 +++++++++++++
> tests/qemuxmlconftest.c | 1 +
> 9 files changed, 185 insertions(+), 4 deletions(-)
> create mode 100644 tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
> create mode 100644 tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.xml
> create mode 100644 tests/qemuxmlconfdata/disk-usb-device-model.xml
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index ae054a52b3..e4ebf061c7 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -2862,10 +2862,25 @@ paravirtualized driver is specified via the ``disk`` element.
>
> ``model``
> Indicates the emulated device model of the disk. Typically this is
> - indicated solely by the ``bus`` property but for ``bus`` "virtio" the
> - model can be specified further with "virtio", "virtio-transitional" or
> - "virtio-non-transitional". See `virtio device models`_
> - for more details. :since:`Since 5.2.0`
> + indicated solely by the ``bus`` property.
> +
> + For ``bus`` "virtio" the model can be specified further with "virtio",
> + "virtio-transitional" or "virtio-non-transitional". See `virtio device
> + models`_ for more details. :since:`Since 5.2.0`
> +
> + For ``bus`` "usb" the model can be specified further with ``usb-storage``
> + or ``usb-bot``. There is no difference between the two models for
> + ``<disk type='disk'``. However with ``usb-bot`` a device configured as
Missing > in the disk element.
> + ``<disk type='cdrom'>`` is properly exposed as a cdrom device inside the
> + guest OS. Unfortunately this configuration is not ABI compatible and thus
> + it can't be interchanged.
This is not ABI compatible with usb-storage model, right? I suggest
being explicit about it.
> +
> + The QEMU hypervisor driver will pick ``usb-bot`` for cold starts or
> + hotplug for cdrom devices to properly configure the devices. This is
> + not compatible for migration to older versions of libvirt and explicit
> + configuration needs to be used.
So are you saying starting an existing domain with usb cdrom after
upgrading libvirt will cause issues during migration to an older
libvirt? I don't think we can do this. We should rather pick usb-storage
(which is the case now as I understood from the description) unless
usb-bot is set explicitly in the XML.
> + :since:`Since 11.5.0`; relevant only for ``QEMU`` hypervisor.
> +
> ``rawio``
> Indicates whether the disk needs rawio capability. Valid settings are
> "yes" or "no" (default is "no"). If any one disk in a domain has
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 1a6c8afb1d..2882a7746b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1399,6 +1399,8 @@ VIR_ENUM_IMPL(virDomainDiskModel,
> "virtio",
> "virtio-transitional",
> "virtio-non-transitional",
> + "usb-storage",
> + "usb-bot",
> );
>
> VIR_ENUM_IMPL(virDomainDiskMirrorState,
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3d380073cf..73e8a2fb99 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -439,6 +439,9 @@ typedef enum {
> VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL,
> VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL,
>
Extra empty line?
> + VIR_DOMAIN_DISK_MODEL_USB_STORAGE,
> + VIR_DOMAIN_DISK_MODEL_USB_BOT,
> +
> VIR_DOMAIN_DISK_MODEL_LAST
> } virDomainDiskModel;
>
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index b1fe51f519..a80005562a 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -1766,6 +1766,8 @@
> <value>virtio</value>
> <value>virtio-transitional</value>
> <value>virtio-non-transitional</value>
> + <value>usb-storage</value>
> + <value>usb-bot</value>
> </choice>
> </attribute>
> </optional>
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index bb86cfa0c3..3a23f70d39 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -729,6 +729,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
> case VIR_DOMAIN_DISK_MODEL_DEFAULT:
> return virtioFlags;
> case VIR_DOMAIN_DISK_MODEL_LAST:
> + case VIR_DOMAIN_DISK_MODEL_USB_STORAGE:
> + case VIR_DOMAIN_DISK_MODEL_USB_BOT:
> break;
> }
> return 0;
> diff --git a/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
> new file mode 100644
> index 0000000000..6d31319a49
> --- /dev/null
> +++ b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
> @@ -0,0 +1,46 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
> +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
> +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
> +/usr/bin/qemu-system-x86_64 \
> +-name guest=QEMUGuest1,debug-threads=on \
> +-S \
> +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
> +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
> +-accel tcg \
> +-cpu qemu64 \
> +-m size=219136k \
> +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
> +-overcommit mem-lock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-boot strict=on \
> +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
> +-device '{"driver":"usb-hub","id":"hub0","bus":"usb.0","port":"1"}' \
> +-blockdev '{"driver":"file","filename":"/tmp/img1","node-name":"libvirt-6-storage","read-only":false}' \
> +-device '{"driver":"usb-storage","bus":"usb.0","port":"2","drive":"libvirt-6-storage","id":"usb-disk0","bootindex":1,"removable":false}' \
> +-blockdev '{"driver":"file","filename":"/tmp/img2","node-name":"libvirt-5-storage","read-only":true}' \
> +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.1","drive":"libvirt-5-storage","id":"usb-disk1","removable":false}' \
> +-blockdev '{"driver":"file","filename":"/tmp/img3","node-name":"libvirt-4-storage","read-only":false}' \
> +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.2","drive":"libvirt-4-storage","id":"usb-disk2","removable":false}' \
> +-blockdev '{"driver":"file","filename":"/tmp/img4","node-name":"libvirt-3-storage","read-only":true}' \
> +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.3","drive":"libvirt-3-storage","id":"usb-disk3","removable":false}' \
> +-blockdev '{"driver":"file","filename":"/tmp/img5","node-name":"libvirt-2-storage","read-only":false}' \
> +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.4","drive":"libvirt-2-storage","id":"usb-disk4","removable":false}' \
> +-blockdev '{"driver":"file","filename":"/tmp/img6","node-name":"libvirt-1-storage","read-only":true}' \
> +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.5","drive":"libvirt-1-storage","id":"usb-disk5","removable":false}' \
Well, this is strange. The XML uses both usb-storage and usb-bot, but
only usb-storage is present in *.args. This kind of makes sense since
the code for handling model is not wired up yet, but having the tests in
a commit that describes the new models and how libvirt handles them is
confusing.
Jirka
On Tue, Jun 24, 2025 at 11:25:03 +0200, Jiri Denemark wrote:
> On Mon, Jun 23, 2025 at 21:59:13 +0200, Peter Krempa wrote:
> > From: Peter Krempa <pkrempa@redhat.com>
> >
> > Historically libvirt specified 'usb-storage' as driver for USB disks.
> > This though combined with '-blockdev' doesn't properly configure the
> > device to look like CDROM for <disk type='cdrom'>.
> >
> > 'usb-bot' acts like a controler and allows explicitly connecting a
> > -device to it.
> >
> > In qemu the devices share implementation so they are effectively
> > identical and can be used interchangably. There is though a slight
> > difference in how the storage device itself (the SCSI bit) looks when
> > CDROM as they were not declared as cdrom before.
>
> Looks like some words were dropped from the last sentence above.
>
> > As this is effectively a bugfix we'll be fixing the behaviour for the
> > default configuration. The possibility to explicitly set the model is
> > added as a possibility for working around possible problems if they'd
> > appear.
> >
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
[...]
> > + ``<disk type='cdrom'>`` is properly exposed as a cdrom device inside the
> > + guest OS. Unfortunately this configuration is not ABI compatible and thus
> > + it can't be interchanged.
>
> This is not ABI compatible with usb-storage model, right? I suggest
> being explicit about it.
>
> > +
> > + The QEMU hypervisor driver will pick ``usb-bot`` for cold starts or
> > + hotplug for cdrom devices to properly configure the devices. This is
> > + not compatible for migration to older versions of libvirt and explicit
> > + configuration needs to be used.
>
> So are you saying starting an existing domain with usb cdrom after
> upgrading libvirt will cause issues during migration to an older
> libvirt? I don't think we can do this. We should rather pick usb-storage
> (which is the case now as I understood from the description) unless
> usb-bot is set explicitly in the XML.
Yes it will not work if you start a VM and attempt to migrate it to an
older version. The reason why I did this is that this very technically
is a ABI bug fix.
Prior to use of '-blockdev', when the disk backend was instantiated via
-drive, we'd format 'media=cdrom' with the backend which would be picked
up with the 'usb-storage' frontend to make it into a cdrom:
Now this went unnoticed at that time.
While I debated just changing it, the fact that it silently corrupts
inside the guest was a dealbreaker for me.
But at the same time it's IMO unacceptable to default to the broken
configuration which actually did work before.
Thus I decided to do this where users can switch to the broken
configuration (or just detach the cdrom) but the config with no extra
bits will result to the proper configuration as it should have before.
Resulting to the broken config is obviously much easier on the logic
here but I really don't think we should do it.7
>
> > + :since:`Since 11.5.0`; relevant only for ``QEMU`` hypervisor.
> > +
> > ``rawio``
> > Indicates whether the disk needs rawio capability. Valid settings are
> > "yes" or "no" (default is "no"). If any one disk in a domain has
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 1a6c8afb1d..2882a7746b 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -1399,6 +1399,8 @@ VIR_ENUM_IMPL(virDomainDiskModel,
> > "virtio",
> > "virtio-transitional",
> > "virtio-non-transitional",
> > + "usb-storage",
> > + "usb-bot",
> > );
> >
> > VIR_ENUM_IMPL(virDomainDiskMirrorState,
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 3d380073cf..73e8a2fb99 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -439,6 +439,9 @@ typedef enum {
> > VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL,
> > VIR_DOMAIN_DISK_MODEL_VIRTIO_NON_TRANSITIONAL,
> >
>
> Extra empty line?
>
> > + VIR_DOMAIN_DISK_MODEL_USB_STORAGE,
> > + VIR_DOMAIN_DISK_MODEL_USB_BOT,
> > +
> > VIR_DOMAIN_DISK_MODEL_LAST
> > } virDomainDiskModel;
> >
> > diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> > index b1fe51f519..a80005562a 100644
> > --- a/src/conf/schemas/domaincommon.rng
> > +++ b/src/conf/schemas/domaincommon.rng
> > @@ -1766,6 +1766,8 @@
> > <value>virtio</value>
> > <value>virtio-transitional</value>
> > <value>virtio-non-transitional</value>
> > + <value>usb-storage</value>
> > + <value>usb-bot</value>
> > </choice>
> > </attribute>
> > </optional>
> > diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> > index bb86cfa0c3..3a23f70d39 100644
> > --- a/src/qemu/qemu_domain_address.c
> > +++ b/src/qemu/qemu_domain_address.c
> > @@ -729,6 +729,8 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
> > case VIR_DOMAIN_DISK_MODEL_DEFAULT:
> > return virtioFlags;
> > case VIR_DOMAIN_DISK_MODEL_LAST:
> > + case VIR_DOMAIN_DISK_MODEL_USB_STORAGE:
> > + case VIR_DOMAIN_DISK_MODEL_USB_BOT:
> > break;
> > }
> > return 0;
> > diff --git a/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
> > new file mode 100644
> > index 0000000000..6d31319a49
> > --- /dev/null
> > +++ b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args
> > @@ -0,0 +1,46 @@
> > +LC_ALL=C \
> > +PATH=/bin \
> > +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
> > +USER=test \
> > +LOGNAME=test \
> > +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
> > +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
> > +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
> > +/usr/bin/qemu-system-x86_64 \
> > +-name guest=QEMUGuest1,debug-threads=on \
> > +-S \
> > +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
> > +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
> > +-accel tcg \
> > +-cpu qemu64 \
> > +-m size=219136k \
> > +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
> > +-overcommit mem-lock=off \
> > +-smp 1,sockets=1,cores=1,threads=1 \
> > +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> > +-display none \
> > +-no-user-config \
> > +-nodefaults \
> > +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
> > +-mon chardev=charmonitor,id=monitor,mode=control \
> > +-rtc base=utc \
> > +-no-shutdown \
> > +-boot strict=on \
> > +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
> > +-device '{"driver":"usb-hub","id":"hub0","bus":"usb.0","port":"1"}' \
> > +-blockdev '{"driver":"file","filename":"/tmp/img1","node-name":"libvirt-6-storage","read-only":false}' \
> > +-device '{"driver":"usb-storage","bus":"usb.0","port":"2","drive":"libvirt-6-storage","id":"usb-disk0","bootindex":1,"removable":false}' \
> > +-blockdev '{"driver":"file","filename":"/tmp/img2","node-name":"libvirt-5-storage","read-only":true}' \
> > +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.1","drive":"libvirt-5-storage","id":"usb-disk1","removable":false}' \
> > +-blockdev '{"driver":"file","filename":"/tmp/img3","node-name":"libvirt-4-storage","read-only":false}' \
> > +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.2","drive":"libvirt-4-storage","id":"usb-disk2","removable":false}' \
> > +-blockdev '{"driver":"file","filename":"/tmp/img4","node-name":"libvirt-3-storage","read-only":true}' \
> > +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.3","drive":"libvirt-3-storage","id":"usb-disk3","removable":false}' \
> > +-blockdev '{"driver":"file","filename":"/tmp/img5","node-name":"libvirt-2-storage","read-only":false}' \
> > +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.4","drive":"libvirt-2-storage","id":"usb-disk4","removable":false}' \
> > +-blockdev '{"driver":"file","filename":"/tmp/img6","node-name":"libvirt-1-storage","read-only":true}' \
> > +-device '{"driver":"usb-storage","bus":"usb.0","port":"1.5","drive":"libvirt-1-storage","id":"usb-disk5","removable":false}' \
>
> Well, this is strange. The XML uses both usb-storage and usb-bot, but
> only usb-storage is present in *.args. This kind of makes sense since
> the code for handling model is not wired up yet, but having the tests in
> a commit that describes the new models and how libvirt handles them is
> confusing.
I thought about activating the capability later, but decided against as
it was showing all updates together.
So while this ordering creates a history point where this doesn't work ,
it's much easier to see what the patches are doing actually.
On Tue, Jun 24, 2025 at 12:51:57 +0200, Peter Krempa wrote: > On Tue, Jun 24, 2025 at 11:25:03 +0200, Jiri Denemark wrote: > > On Mon, Jun 23, 2025 at 21:59:13 +0200, Peter Krempa wrote: > > > From: Peter Krempa <pkrempa@redhat.com> > > > > > > Historically libvirt specified 'usb-storage' as driver for USB disks. > > > This though combined with '-blockdev' doesn't properly configure the > > > device to look like CDROM for <disk type='cdrom'>. > > > > > > 'usb-bot' acts like a controler and allows explicitly connecting a > > > -device to it. > > > > > > In qemu the devices share implementation so they are effectively > > > identical and can be used interchangably. There is though a slight > > > difference in how the storage device itself (the SCSI bit) looks when > > > CDROM as they were not declared as cdrom before. > > > > Looks like some words were dropped from the last sentence above. > > > > > As this is effectively a bugfix we'll be fixing the behaviour for the > > > default configuration. The possibility to explicitly set the model is > > > added as a possibility for working around possible problems if they'd > > > appear. > > > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > > --- > > [...] > > > > + ``<disk type='cdrom'>`` is properly exposed as a cdrom device inside the > > > + guest OS. Unfortunately this configuration is not ABI compatible and thus > > > + it can't be interchanged. > > > > This is not ABI compatible with usb-storage model, right? I suggest > > being explicit about it. > > > > > + > > > + The QEMU hypervisor driver will pick ``usb-bot`` for cold starts or > > > + hotplug for cdrom devices to properly configure the devices. This is > > > + not compatible for migration to older versions of libvirt and explicit > > > + configuration needs to be used. > > > > So are you saying starting an existing domain with usb cdrom after > > upgrading libvirt will cause issues during migration to an older > > libvirt? I don't think we can do this. We should rather pick usb-storage > > (which is the case now as I understood from the description) unless > > usb-bot is set explicitly in the XML. > > Yes it will not work if you start a VM and attempt to migrate it to an > older version. The reason why I did this is that this very technically > is a ABI bug fix. > > Prior to use of '-blockdev', when the disk backend was instantiated via > -drive, we'd format 'media=cdrom' with the backend which would be picked > up with the 'usb-storage' frontend to make it into a cdrom: > > Now this went unnoticed at that time. > > While I debated just changing it, the fact that it silently corrupts > inside the guest was a dealbreaker for me. > > But at the same time it's IMO unacceptable to default to the broken > configuration which actually did work before. > > Thus I decided to do this where users can switch to the broken > configuration (or just detach the cdrom) but the config with no extra > bits will result to the proper configuration as it should have before. > > Resulting to the broken config is obviously much easier on the logic > here but I really don't think we should do it.7 As I said in my comments to the following patch, you change the model only when VIR_DOMAIN_DEF_PARSE_ABI_UPDATE and don't change where the flag is actually used (except for tests). So this series is OK. > > > diff --git a/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args > > > new file mode 100644 > > > index 0000000000..6d31319a49 > > > --- /dev/null > > > +++ b/tests/qemuxmlconfdata/disk-usb-device-model.x86_64-latest.args > > > > Well, this is strange. The XML uses both usb-storage and usb-bot, but > > only usb-storage is present in *.args. This kind of makes sense since > > the code for handling model is not wired up yet, but having the tests in > > a commit that describes the new models and how libvirt handles them is > > confusing. > > I thought about activating the capability later, but decided against as > it was showing all updates together. > > So while this ordering creates a history point where this doesn't work , > it's much easier to see what the patches are doing actually. Yeah, I was just thinking the test addition would deserve a dedicated commit to avoid the discrepancy between the test and the documentation in a single patch :-) Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
© 2016 - 2025 Red Hat, Inc.