Re: [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; } }

--- 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'/>