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
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@redhat.com>
---
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.1On Mon, Dec 30, 2024 at 21:44:20 +0100, Adam Julis wrote:
> 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@redhat.com>
> ---
>
> 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.
Looking at the ATAPI spec [1] and qemu code the model for IDE/SATA disks
is 40 characters long. Both this documentation and the validation of the
virDomainDiskDef 'product' field (in virDomainDiskDefValidate) are wrong
when applied to ATA disks.
Note that qemu will silently truncate anything longer.
Also don't forget to fix the version to 11.1.0.
[1] https://people.freebsd.org/~imp/asiabsdcon2015/works/d2161r5-ATAATAPI_Command_Set_-_3.pdf
A.11.7.4 MODEL NUMBER field (page 456)
> - :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,
This looks good.
> 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"));
Since you are changing this please enclose vendor in single quotes.
> + 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"));
same here for '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
Semantically testing 'ide'/'sata' disks doesn't belong to a test case
named 'disk-scsi'. Please add another one.
> @@ -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>
Also deleting the vendor isn't right.
> <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'/>
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. Length of the 'product' parameter is
different in SCSI (16 chars) and ATA/SATA (40 chars).
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/697
Signed-off-by: Adam Julis <ajulis@redhat.com>
---
Changes to v2:
- doc (mainly mentioned the different length of SCSI and SATA/ATA)
- modified schemas (for supporting longer string)
- added tests
- ..details in formatting
docs/formatdomain.rst | 9 ++--
src/conf/domain_validate.c | 14 +++++--
src/conf/schemas/domaincommon.rng | 2 +-
src/qemu/qemu_command.c | 9 +++-
src/qemu/qemu_validate.c | 14 ++++++-
.../disk-sata-product.x86_64-latest.args | 36 ++++++++++++++++
.../disk-sata-product.x86_64-latest.xml | 41 +++++++++++++++++++
tests/qemuxmlconfdata/disk-sata-product.xml | 28 +++++++++++++
...csi-disk-vpd-build-error.x86_64-latest.err | 2 +-
...disk-scsi-product-length.x86_64-latest.err | 1 +
.../disk-scsi-product-length.xml | 28 +++++++++++++
tests/qemuxmlconftest.c | 2 +
12 files changed, 173 insertions(+), 13 deletions(-)
create mode 100644 tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.args
create mode 100644 tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/disk-sata-product.xml
create mode 100644 tests/qemuxmlconfdata/disk-scsi-product-length.x86_64-latest.err
create mode 100644 tests/qemuxmlconfdata/disk-scsi-product-length.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 620daae9af..0544b11fb9 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -3562,12 +3562,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
+ '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`
+ CD-ROM device. It must not be longer than 16 printable characters for 'scsi'
+ (:since:`Since 1.0.1`). For 'sata' or 'ide' not longer than 40 printable
+ characters (:since:`Since 11.0.1`). Other ``bus`` is not supported.
``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/conf/domain_validate.c b/src/conf/domain_validate.c
index 6aed74dd8d..01dda3a278 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -603,8 +603,8 @@ virDomainDiskDefValidateSource(const virStorageSource *src)
#define VENDOR_LEN 8
-#define PRODUCT_LEN 16
-
+#define PRODUCT_SCSI_LEN 16
+#define PRODUCT_ATA_SATA_LEN 40
/**
* virDomainDiskDefSourceLUNValidate:
@@ -880,10 +880,16 @@ virDomainDiskDefValidate(const virDomainDef *def,
return -1;
}
- if (strlen(disk->product) > PRODUCT_LEN) {
+ if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI &&
+ strlen(disk->product) > PRODUCT_SCSI_LEN) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk product is more than %1$d characters"),
- PRODUCT_LEN);
+ PRODUCT_SCSI_LEN);
+ return -1;
+ } else if (strlen(disk->product) > PRODUCT_ATA_SATA_LEN) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("disk product is more than %1$d characters"),
+ PRODUCT_ATA_SATA_LEN);
return -1;
}
}
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 7121519ca3..0fb1050f1e 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -1669,7 +1669,7 @@
<element name="product">
<data type="string">
<!-- All printable characters -->
- <param name="pattern">[ -~]{0,16}</param>
+ <param name="pattern">[ -~]{0,40}</param>
</data>
</element>
</optional>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1f28de6194..b636079417 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 086c66b602..1194c4743c 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-sata-product.x86_64-latest.args b/tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.args
new file mode 100644
index 0000000000..a171f5c447
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.args
@@ -0,0 +1,36 @@
+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":"ahci","id":"sata0","bus":"pci.0","addr":"0x2"}' \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUG uest1","node-name":"libvirt-1-storage","read-only":false}' \
+-device '{"driver":"ide-hd","bus":"sata0.0","drive":"libvirt-1-storage","id":"sata0-0-0","bootindex":1,"model":"WD10EZEX-00BN5A0"}' \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x3"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.xml
new file mode 100644
index 0000000000..7a75731146
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.xml
@@ -0,0 +1,41 @@
+<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='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUG uest1'/>
+ <target dev='sda' bus='sata'/>
+ <product>WD10EZEX-00BN5A0</product>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0' model='piix3-uhci'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='sata' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </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='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconfdata/disk-sata-product.xml b/tests/qemuxmlconfdata/disk-sata-product.xml
new file mode 100644
index 0000000000..a5c8ddbd50
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-sata-product.xml
@@ -0,0 +1,28 @@
+<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'>
+ <driver cache='default'/>
+ <source dev='/dev/HostVG/QEMUG uest1'/>
+ <target dev='sda' bus='sata'/>
+ <product>WD10EZEX-00BN5A0</product>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='sata' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
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
index f70b7a774f..b0a5eb6af2 100644
--- 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
@@ -1 +1 @@
-unsupported configuration: Only scsi disk supports vendor and product
+unsupported configuration: Only scsi disk supports 'vendor'
diff --git a/tests/qemuxmlconfdata/disk-scsi-product-length.x86_64-latest.err b/tests/qemuxmlconfdata/disk-scsi-product-length.x86_64-latest.err
new file mode 100644
index 0000000000..70b3a4de73
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-scsi-product-length.x86_64-latest.err
@@ -0,0 +1 @@
+unsupported configuration: disk product is more than 16 characters
diff --git a/tests/qemuxmlconfdata/disk-scsi-product-length.xml b/tests/qemuxmlconfdata/disk-scsi-product-length.xml
new file mode 100644
index 0000000000..f3ecd38cf9
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-scsi-product-length.xml
@@ -0,0 +1,28 @@
+<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'>
+ <driver cache='default'/>
+ <source dev='/dev/HostVG/QEMUG uest1'/>
+ <target dev='sda' bus='scsi'/>
+ <product>WD10EZEX-00BN5A00</product>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='sata' index='0'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 6a46bfc7a3..baf47e550d 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1621,9 +1621,11 @@ mymain(void)
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-product-length");
DO_TEST_CAPS_LATEST("controller-virtio-scsi");
DO_TEST_CAPS_LATEST("controller-scsi-auto");
DO_TEST_CAPS_LATEST("disk-sata-device");
+ DO_TEST_CAPS_LATEST("disk-sata-product");
DO_TEST_CAPS_LATEST_PARSE_ERROR("disk-target-overflow");
DO_TEST_CAPS_LATEST("disk-aio");
DO_TEST_CAPS_LATEST("disk-aio-io_uring");
--
2.47.1On Sat, Jan 18, 2025 at 11:30:20 +0100, Adam Julis wrote:
> 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. Length of the 'product' parameter is
> different in SCSI (16 chars) and ATA/SATA (40 chars).
>
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/697
> Signed-off-by: Adam Julis <ajulis@redhat.com>
> ---
>
> Changes to v2:
> - doc (mainly mentioned the different length of SCSI and SATA/ATA)
> - modified schemas (for supporting longer string)
> - added tests
> - ..details in formatting
>
> docs/formatdomain.rst | 9 ++--
> src/conf/domain_validate.c | 14 +++++--
> src/conf/schemas/domaincommon.rng | 2 +-
> src/qemu/qemu_command.c | 9 +++-
> src/qemu/qemu_validate.c | 14 ++++++-
> .../disk-sata-product.x86_64-latest.args | 36 ++++++++++++++++
> .../disk-sata-product.x86_64-latest.xml | 41 +++++++++++++++++++
> tests/qemuxmlconfdata/disk-sata-product.xml | 28 +++++++++++++
> ...csi-disk-vpd-build-error.x86_64-latest.err | 2 +-
> ...disk-scsi-product-length.x86_64-latest.err | 1 +
> .../disk-scsi-product-length.xml | 28 +++++++++++++
> tests/qemuxmlconftest.c | 2 +
> 12 files changed, 173 insertions(+), 13 deletions(-)
> create mode 100644 tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.args
> create mode 100644 tests/qemuxmlconfdata/disk-sata-product.x86_64-latest.xml
> create mode 100644 tests/qemuxmlconfdata/disk-sata-product.xml
> create mode 100644 tests/qemuxmlconfdata/disk-scsi-product-length.x86_64-latest.err
> create mode 100644 tests/qemuxmlconfdata/disk-scsi-product-length.xml
>
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 620daae9af..0544b11fb9 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -3562,12 +3562,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
> + '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`
> + CD-ROM device. It must not be longer than 16 printable characters for 'scsi'
> + (:since:`Since 1.0.1`). For 'sata' or 'ide' not longer than 40 printable
> + characters (:since:`Since 11.0.1`). Other ``bus`` is not supported.
11.1.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/conf/domain_validate.c b/src/conf/domain_validate.c
> index 6aed74dd8d..01dda3a278 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -603,8 +603,8 @@ virDomainDiskDefValidateSource(const virStorageSource *src)
>
>
> #define VENDOR_LEN 8
> -#define PRODUCT_LEN 16
> -
> +#define PRODUCT_SCSI_LEN 16
> +#define PRODUCT_ATA_SATA_LEN 40
>
> /**
> * virDomainDiskDefSourceLUNValidate:
> @@ -880,10 +880,16 @@ virDomainDiskDefValidate(const virDomainDef *def,
> return -1;
> }
>
> - if (strlen(disk->product) > PRODUCT_LEN) {
> + if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI &&
> + strlen(disk->product) > PRODUCT_SCSI_LEN) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("disk product is more than %1$d characters"),
> - PRODUCT_LEN);
> + PRODUCT_SCSI_LEN);
> + return -1;
> + } else if (strlen(disk->product) > PRODUCT_ATA_SATA_LEN) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("disk product is more than %1$d characters"),
> + PRODUCT_ATA_SATA_LEN);
> return -1;
> }
> }
I think I'll change this to add a temporary variable for the length
rather than duplicate the whole code.
[...]
I'll change the two things and push this.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
© 2016 - 2025 Red Hat, Inc.