[PATCH v2] Support IDE/SATA disk 'product' parameter

Since we supported 'product' parameter for SCSI, just expanded existing solution makes IDE/SATA parameter works too. QEMU requires parameter 'model' in case of IDE/SATA (instead of 'product'), so the process of making JSON object is slightly modified. Resolves: https://gitlab.com/libvirt/libvirt/-/issues/697 Signed-off-by: Adam Julis <ajulis(a)redhat.com&gt; --- Changes to v1: - introduce new variables model and product for being consistent in orders of commands - modified test file docs/formatdomain.rst | 7 ++-- src/qemu/qemu_command.c | 9 ++++- src/qemu/qemu_validate.c | 14 ++++++-- ...disk-product-build-error.x86_64-latest.err | 1 + .../disk-scsi-disk-product-build-error.xml | 34 +++++++++++++++++++ ...-disk-vendor-build-error.x86_64-latest.err | 1 + ... => disk-scsi-disk-vendor-build-error.xml} | 0 ...csi-disk-vpd-build-error.x86_64-latest.err | 1 - .../disk-scsi-disk-vpd.x86_64-latest.args | 4 +-- .../disk-scsi-disk-vpd.x86_64-latest.xml | 7 ++-- tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml | 4 +-- tests/qemuxmlconftest.c | 3 +- 12 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err create mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml create mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err rename tests/qemuxmlconfdata/{disk-scsi-disk-vpd-build-error.xml => disk-scsi-disk-vendor-build-error.xml} (100%) delete mode 100644 tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 60bee8bd4f..c93a321401 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3551,12 +3551,13 @@ paravirtualized driver is specified via the ``disk`` element. :since:`Since 0.10.1` ``vendor`` If present, this element specifies the vendor of a virtual hard disk or - CD-ROM device. It must not be longer than 8 printable characters. - :since:`Since 1.0.1` + CD-ROM device. It must not be longer than 8 printable characters. Only for + devices using 'scsi' ``bus``. :since:`Since 1.0.1` ``product`` If present, this element specifies the product of a virtual hard disk or CD-ROM device. It must not be longer than 16 printable characters. - :since:`Since 1.0.1` + Only for devices using 'scsi' (:since:`Since 1.0.1`), 'sata' or 'ide' ``bus``. + :since:`Since 11.0.0` ``address`` If present, the ``address`` element ties the disk to a given slot of a controller (the actual ``<controller>`` device can often be inferred by diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dcb9c4934e..b6ec2c4a79 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1619,6 +1619,8 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, const char *biosCHSTrans = NULL; const char *wpolicy = NULL; const char *rpolicy = NULL; + const char *model = NULL; + const char *product = NULL; switch (disk->bus) { case VIR_DOMAIN_DISK_BUS_IDE: @@ -1628,6 +1630,8 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, else driver = "ide-hd"; + model = disk->product; + break; case VIR_DOMAIN_DISK_BUS_SCSI: @@ -1654,6 +1658,8 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, } } + product = disk->product; + break; case VIR_DOMAIN_DISK_BUS_VIRTIO: { @@ -1803,7 +1809,8 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "A:wwn", &wwn, "p:rotation_rate", disk->rotation_rate, "S:vendor", disk->vendor, - "S:product", disk->product, + "S:product", product, + "S:model", model, "T:removable", removable, "S:write-cache", qemuOnOffAuto(writeCache), "p:cyls", disk->geometry.cylinders, diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index aaa056379e..f0be236533 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2947,10 +2947,20 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } } - if (disk->vendor || disk->product) { + if (disk->vendor) { if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only scsi disk supports vendor and product")); + _("Only scsi disk supports vendor")); + return -1; + } + } + + if (disk->product) { + if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) && + (disk->bus != VIR_DOMAIN_DISK_BUS_SATA) && + (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only ide, sata and scsi disk supports product")); return -1; } } diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err new file mode 100644 index 0000000000..93dfac0d1e --- /dev/null +++ b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.x86_64-latest.err @@ -0,0 +1 @@ +unsupported configuration: Only ide, sata and scsi disk supports product diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml new file mode 100644 index 0000000000..da2fc59da3 --- /dev/null +++ b/tests/qemuxmlconfdata/disk-scsi-disk-product-build-error.xml @@ -0,0 +1,34 @@ +<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='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='sda' bus='virtio'/> + <product>ST3146707LC</product> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='sdb' bus='scsi'/> + <vendor>SEAGATE</vendor> + <product>ST3567807GD</product> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='scsi' index='1' model='lsilogic'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err b/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err new file mode 100644 index 0000000000..88bd9e5468 --- /dev/null +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.x86_64-latest.err @@ -0,0 +1 @@ +unsupported configuration: Only scsi disk supports vendor diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.xml b/tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.xml similarity index 100% rename from tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.xml rename to tests/qemuxmlconfdata/disk-scsi-disk-vendor-build-error.xml diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err b/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err deleted file mode 100644 index f70b7a774f..0000000000 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd-build-error.x86_64-latest.err +++ /dev/null @@ -1 +0,0 @@ -unsupported configuration: Only scsi disk supports vendor and product diff --git a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args index 4234a7e677..049a7998e4 100644 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.args @@ -30,9 +30,9 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -device '{"driver":"virtio-scsi-pci","id":"scsi0","bus":"pci.0","addr":"0x2"}' \ -device '{"driver":"lsi","id":"scsi1","bus":"pci.0","addr":"0x3"}' \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-2-storage","read-only":true}' \ --device '{"driver":"scsi-cd","bus":"scsi0.0","channel":0,"scsi-id":0,"lun":0,"device_id":"drive-scsi0-0-0-0","drive":"libvirt-2-storage","id":"scsi0-0-0-0","vendor":"SEAGATE","product":"ST3146707LC"}' \ +-device '{"driver":"ide-cd","bus":"ide.0","unit":0,"drive":"libvirt-2-storage","id":"ide0-0-0","model":"ST3146707LC"}' \ -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest2","node-name":"libvirt-1-storage","read-only":true}' \ --device '{"driver":"scsi-hd","bus":"scsi1.0","scsi-id":0,"device_id":"drive-scsi1-0-0","drive":"libvirt-1-storage","id":"scsi1-0-0","bootindex":1,"vendor":"SEA GATE","product":"ST67 807GD"}' \ +-device '{"driver":"scsi-hd","bus":"scsi1.0","scsi-id":0,"device_id":"drive-scsi1-0-0","drive":"libvirt-1-storage","id":"scsi1-0-0","bootindex":1,"product":"ST67 807GD"}' \ -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-scsi-disk-vpd.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.xml index 4b23fbfcfe..39148f6ce7 100644 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.x86_64-latest.xml @@ -20,9 +20,8 @@ <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='scsi'/> + <target dev='sda' bus='ide'/> <readonly/> - <vendor>SEAGATE</vendor> <product>ST3146707LC</product> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> @@ -31,7 +30,6 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEA GATE</vendor> <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> @@ -45,6 +43,9 @@ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </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-scsi-disk-vpd.xml b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml index 36dd2a89ba..e3665d3afa 100644 --- a/tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml +++ b/tests/qemuxmlconfdata/disk-scsi-disk-vpd.xml @@ -16,9 +16,8 @@ <emulator>/usr/bin/qemu-system-x86_64</emulator> <disk type='block' device='cdrom'> <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='scsi'/> + <target dev='sda' bus='ide'/> <readonly/> - <vendor>SEAGATE</vendor> <product>ST3146707LC</product> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> @@ -26,7 +25,6 @@ <source dev='/dev/HostVG/QEMUGuest2'/> <target dev='sdb' bus='scsi'/> <readonly/> - <vendor>SEA GATE</vendor> <product>ST67 807GD</product> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 21b56dc94e..083b0ab7f6 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -1620,7 +1620,8 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-scsi-disk-split"); DO_TEST_CAPS_LATEST("disk-scsi-disk-wwn"); DO_TEST_CAPS_LATEST("disk-scsi-disk-vpd"); - DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-scsi-disk-vpd-build-error"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-scsi-disk-vendor-build-error"); + DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-scsi-disk-product-build-error"); DO_TEST_CAPS_LATEST("controller-virtio-scsi"); DO_TEST_CAPS_LATEST("controller-scsi-auto"); DO_TEST_CAPS_LATEST("disk-sata-device"); -- 2.47.1