On 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>