This is the patch, as suggested by Daniel, using only the product tag for IDE/SATA. Since qemu supports the model tag, if the drive bus is IDE OR SATA the product string gets passed to qemu using the model parameter If the bus is either SATA or IDE, the product string can have a max lenght of 40, as per ATAPI spec in the field "model" of the IDENTIFY PACKET DEVICE. This should be the better solution, since you do not need to rely on space checking in the vendor field. Tell me what you think about this version, and if it is satisfactory, I will write the missing tests.
Thanks for your time and effort, Benedek Signed-off-by: Benedek Major <[email protected]> --- src/conf/domain_validate.c | 18 +++++++++++++++--- src/qemu/qemu_command.c | 10 ++++++++-- src/qemu/qemu_validate.c | 12 +++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 16bf3b559f..b9940c78a5 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -585,7 +585,8 @@ virDomainDiskDefValidateSource(const virStorageSource *src) #define VENDOR_LEN 8 -#define PRODUCT_LEN 16 +#define PRODUCT_LEN_SCSI 16 +#define PRODUCT_LEN_IDE 40 /** @@ -856,10 +857,21 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } - if (strlen(disk->product) > PRODUCT_LEN) { + if ((disk->bus == VIR_DOMAIN_DISK_BUS_SATA || + disk->bus == VIR_DOMAIN_DISK_BUS_IDE) && + strlen(disk->product) > PRODUCT_LEN_IDE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk product is more than %1$d characters"), - PRODUCT_LEN); + PRODUCT_LEN_IDE); + return -1; + } + + if (disk->bus != VIR_DOMAIN_DISK_BUS_SATA && + disk->bus != VIR_DOMAIN_DISK_BUS_IDE && + strlen(disk->product) > PRODUCT_LEN_SCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk product is more than %1$d characters"), + PRODUCT_LEN_SCSI); return -1; } } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ad224571f3..7bc1d8c682 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1762,6 +1762,8 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, unsigned int physical_block_size = disk->blockio.physical_block_size; g_autoptr(virJSONValue) wwn = NULL; g_autofree char *serial = NULL; + g_autofree char *modelstr = NULL; + g_autofree char *product = g_strdup(disk->product); virTristateSwitch removable = VIR_TRISTATE_SWITCH_ABSENT; virTristateSwitch writeCache = VIR_TRISTATE_SWITCH_ABSENT; const char *biosCHSTrans = NULL; @@ -1775,7 +1777,10 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, driver = "ide-cd"; else driver = "ide-hd"; - + /* exchange product to model for QEMU ide disk */ + modelstr = g_strdup(product); + g_free(product); + product = NULL; break; case VIR_DOMAIN_DISK_BUS_SCSI: @@ -1944,7 +1949,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", modelstr, "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 7e09e2c52f..4893b979d3 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2871,12 +2871,22 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, } if (disk->vendor || disk->product) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { + if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI && disk->vendor) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only scsi disk supports vendor and product")); return -1; } + if (disk->product && + 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 scsi, ide, sata disk supports product")); + return -1; + } + + /* Properties wwn, vendor and product were introduced in the * same QEMU release (1.2.0). */ -- 2.41.0
