Re: [PATCH v2 08/13] conf: introduce usb disk models 'usb-storage' and 'usb-bot'

From: Peter Krempa <pkrempa(a)redhat.com&gt; 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(a)redhat.com&gt; --- 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}' \