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@xxxxxxxxxx> > --- > > 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@xxxxxxxxxx>
