[PATCH] Added support for disk model string for QEMU IDE and SATA devices

Benedek Major posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230718203926.25809-1-benedek@major.onl
src/conf/domain_validate.c | 18 +++++++++++++++---
src/qemu/qemu_command.c    |  8 ++++++--
src/qemu/qemu_validate.c   | 28 +++++++++++++---------------
3 files changed, 34 insertions(+), 20 deletions(-)
[PATCH] Added support for disk model string for QEMU IDE and SATA devices
Posted by Benedek Major 10 months ago
The QEMU drivers ide-hd and ide-cd used for IDE and SATA bus devices support
adding a disk model that is returned on guest query by the IDE or SATA disk to the guest.
The already existing product tag got changed to allow for up to 40 characters,
which is the maximum allowed by the specification, if the controller is set to SATA or IDE.
If set, the product gets passed to QEMU using the -model parameter.
The product represents the model until command creation,
where it gets processed depending on the current type of disk controller.

Dead code checking for QEMU version 1.2.0 got removed, since minimum is 4.2.0.

Signed-off-by: Benedek Major <benedek@major.onl>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_validate.c | 18 +++++++++++++++---
 src/qemu/qemu_command.c    |  8 ++++++--
 src/qemu/qemu_validate.c   | 28 +++++++++++++---------------
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 16bf3b559f..60454f3745 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..a3b0380695 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;
+    const char *modelstr = NULL;
+    const char *product = disk->product;
     virTristateSwitch removable = VIR_TRISTATE_SWITCH_ABSENT;
     virTristateSwitch writeCache = VIR_TRISTATE_SWITCH_ABSENT;
     const char *biosCHSTrans = NULL;
@@ -1775,7 +1777,8 @@ qemuBuildDiskDeviceProps(const virDomainDef *def,
             driver = "ide-cd";
         else
             driver = "ide-hd";
-
+        /* IDE disk product name for IDE is passed via 'model' property */
+        modelstr = g_steal_pointer(&product);
         break;
 
     case VIR_DOMAIN_DISK_BUS_SCSI:
@@ -1944,7 +1947,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..84f9af3f16 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -2870,22 +2870,20 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk,
         }
     }
 
-    if (disk->vendor || disk->product) {
-        if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("Only scsi disk supports vendor and product"));
-            return -1;
-        }
+    if (disk->vendor &&
+        disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                        _("Only scsi disk supports vendor and product"));
+        return -1;
+    }
 
-        /* Properties wwn, vendor and product were introduced in the
-         * same QEMU release (1.2.0).
-         */
-        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_WWN)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("Setting vendor or product for scsi disk is not "
-                             "supported by this QEMU"));
-            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;
     }
 
     if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
-- 
2.41.0
Re: [PATCH] Added support for disk model string for QEMU IDE and SATA devices
Posted by Peter Krempa 10 months ago
On Tue, Jul 18, 2023 at 22:39:26 +0200, Benedek Major wrote:
> The QEMU drivers ide-hd and ide-cd used for IDE and SATA bus devices support
> adding a disk model that is returned on guest query by the IDE or SATA disk to the guest.
> The already existing product tag got changed to allow for up to 40 characters,
> which is the maximum allowed by the specification, if the controller is set to SATA or IDE.
> If set, the product gets passed to QEMU using the -model parameter.
> The product represents the model until command creation,
> where it gets processed depending on the current type of disk controller.
> 
> Dead code checking for QEMU version 1.2.0 got removed, since minimum is 4.2.0.

I've posted proper patches to do this part properly as I've originally
replied in the review.

I'll rebase this patch on top of that series so that it's done properly.

I'll also also adjust the commit message to go along with that, so
there's no need to resend.

> Signed-off-by: Benedek Major <benedek@major.onl>
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/conf/domain_validate.c | 18 +++++++++++++++---
>  src/qemu/qemu_command.c    |  8 ++++++--
>  src/qemu/qemu_validate.c   | 28 +++++++++++++---------------
>  3 files changed, 34 insertions(+), 20 deletions(-)