From: Chun Feng Wu <danielwuwy@163.com>

For hot attaching/detaching
* Leverage qemuBlockThrottleFiltersData to prepare attaching/detaching
  throttle filter data for qemuMonitorBlockdevAdd and qemuMonitorBlockdevDel
* For hot attaching, within qemuDomainAttachDiskGeneric,prepare throttle
  filters json data, and create corresponding blockdev for QMP request
  ("blockdev-add" with "driver":"throttle")
* Each filter has a nodename, and those filters are chained up,
  create them in sequence, and delete them reversely
* Delete filters by "qemuBlockThrottleFiltersDetach"("blockdev-del")
  when detaching device

For throttle group commandline
* Add qemuBuildThrottleGroupCommandLine in qemuBuildCommandLine to add
  "object" of throttle-group
* Verify throttle group definition when lauching vm
* Check QEMU_CAPS_OBJECT_JSON before "qemuBuildObjectCommandlineFromJSON",
  which is to build "-object" option

For throttle filter commandline
* Add qemuBuildDiskThrottleFiltersCommandLine in qemuBuildDiskCommandLine
  to add "blockdev"

Signed-off-by: Chun Feng Wu <danielwuwy@163.com>

* Apply suggested coding style changes.
* Update of code documentation comments.

Signed-off-by: Harikumar Rajkumar <harirajkumar230@gmail.com>
---
 src/qemu/qemu_command.c | 99 +++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_hotplug.c | 21 +++++++++
 2 files changed, 120 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 750edd4faa..6d9efbe887 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2045,6 +2045,99 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommand *cmd,
 }
 
 
+static inline bool
+qemuDiskConfigThrottleGroupEnabled(const virDomainThrottleGroupDef *group)
+{
+    return !!group->group_name &&
+           virDomainBlockIoTuneInfoHasAny(group);
+}
+
+
+/**
+ * qemuBuildThrottleGroupCommandLine:
+ * @cmd: the command to modify
+ * @def: domain definition
+ * @qemuCaps: qemu capabilities object
+ *
+ * build throttle group object in json format
+ */
+static int
+qemuBuildThrottleGroupCommandLine(virCommand *cmd,
+                                  const virDomainDef *def,
+                                  virQEMUCaps *qemuCaps)
+{
+    size_t i;
+
+    for (i = 0; i < def->nthrottlegroups; i++) {
+        g_autoptr(virJSONValue) props = NULL;
+        g_autoptr(virJSONValue) limits = virJSONValueNewObject();
+        virDomainThrottleGroupDef *group = def->throttlegroups[i];
+        /* prefix group name with "throttle-" in QOM */
+        g_autofree char *prefixed_group_name = g_strdup_printf("throttle-%s", group->group_name);
+
+        if (!qemuDiskConfigThrottleGroupEnabled(group)) {
+            continue;
+        }
+
+        if (qemuMonitorThrottleGroupLimits(limits, group) < 0)
+            return -1;
+
+        if (qemuMonitorCreateObjectProps(&props, "throttle-group", prefixed_group_name,
+                                         "a:limits", &limits,
+                                         NULL) < 0)
+            return -1;
+
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_JSON)) {
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                           _("This QEMU doesn't support throttle group creation"));
+            return -1;
+        }
+
+        if (qemuBuildObjectCommandlineFromJSON(cmd, props, qemuCaps) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+qemuBuildBlockThrottleFilterCommandline(virCommand *cmd,
+                                        qemuBlockThrottleFilterAttachData *data)
+{
+    if (data->filterProps) {
+        g_autofree char *tmp = NULL;
+        if (!(tmp = virJSONValueToString(data->filterProps, false)))
+            return -1;
+
+        virCommandAddArgList(cmd, "-blockdev", tmp, NULL);
+    }
+
+    return 0;
+}
+
+
+static int
+qemuBuildDiskThrottleFiltersCommandLine(virCommand *cmd,
+                                        virDomainDiskDef *disk)
+{
+    g_autoptr(qemuBlockThrottleFiltersData) data = NULL;
+    size_t i;
+
+    data = qemuBuildThrottleFiltersAttachPrepareBlockdev(disk);
+    if (!data)
+        return -1;
+
+    for (i = 0; i < data->nfilterdata; i++) {
+        if (qemuBuildBlockThrottleFilterCommandline(cmd,
+                                                    data->filterdata[i]) < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 qemuBuildDiskSourceCommandLine(virCommand *cmd,
                                virDomainDiskDef *disk,
@@ -2102,6 +2195,9 @@ qemuBuildDiskCommandLine(virCommand *cmd,
     if (qemuBuildDiskSourceCommandLine(cmd, disk, qemuCaps) < 0)
         return -1;
 
+    if (qemuBuildDiskThrottleFiltersCommandLine(cmd, disk) < 0)
+        return -1;
+
     /* SD cards are currently instantiated via -drive if=sd, so the -device
      * part must be skipped */
     if (qemuDiskBusIsSD(disk->bus))
@@ -10479,6 +10575,9 @@ qemuBuildCommandLine(virDomainObj *vm,
     if (qemuBuildIOThreadCommandLine(cmd, def, qemuCaps) < 0)
         return NULL;
 
+    if (qemuBuildThrottleGroupCommandLine(cmd, def, qemuCaps) < 0)
+        return NULL;
+
     if (virDomainNumaGetNodeCount(def->numa) &&
         qemuBuildNumaCommandLine(cfg, def, cmd, priv) < 0)
         return NULL;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 28ca321c5c..75437164ca 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -708,6 +708,7 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
                             virDomainAsyncJob asyncJob)
 {
     g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
+    g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL;
     qemuDomainObjPrivate *priv = vm->privateData;
     g_autoptr(virJSONValue) devprops = NULL;
     bool extensionDeviceAttached = false;
@@ -746,6 +747,15 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
         if (rc < 0)
             goto rollback;
 
+        if ((filterData = qemuBuildThrottleFiltersAttachPrepareBlockdev(disk))) {
+            if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
+                return -1;
+            rc = qemuBlockThrottleFiltersAttach(priv->mon, filterData);
+            qemuDomainObjExitMonitor(vm);
+            if (rc < 0)
+                goto rollback;
+        }
+
         if (disk->transient) {
             g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL;
             g_autoptr(GHashTable) blockNamedNodeData = NULL;
@@ -817,6 +827,8 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
     if (extensionDeviceAttached)
         ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info));
 
+    qemuBlockThrottleFiltersDetach(priv->mon, filterData);
+
     qemuBlockStorageSourceChainDetach(priv->mon, data);
 
     qemuDomainObjExitMonitor(vm);
@@ -4686,6 +4698,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver,
 {
     qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
     g_autoptr(qemuBlockStorageSourceChainData) diskBackend = NULL;
+    g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL;
     size_t i;
     qemuDomainObjPrivate *priv = vm->privateData;
     int ret = -1;
@@ -4726,6 +4739,14 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver,
 
     qemuDomainObjEnterMonitor(vm);
 
+    if ((filterData = qemuBuildThrottleFiltersDetachPrepareBlockdev(disk))) {
+
+        qemuBlockThrottleFiltersDetach(priv->mon, filterData);
+    }
+    qemuDomainObjExitMonitor(vm);
+
+    qemuDomainObjEnterMonitor(vm);
+
     if (diskBackend)
         qemuBlockStorageSourceChainDetach(priv->mon, diskBackend);
 
-- 
2.39.5 (Apple Git-154)

On Wed, Feb 19, 2025 at 22:27:15 +0530, Harikumar Rajkumar wrote:
> From: Chun Feng Wu <danielwuwy@163.com>
> 
> For hot attaching/detaching
> * Leverage qemuBlockThrottleFiltersData to prepare attaching/detaching
>   throttle filter data for qemuMonitorBlockdevAdd and qemuMonitorBlockdevDel
> * For hot attaching, within qemuDomainAttachDiskGeneric,prepare throttle
>   filters json data, and create corresponding blockdev for QMP request
>   ("blockdev-add" with "driver":"throttle")
> * Each filter has a nodename, and those filters are chained up,
>   create them in sequence, and delete them reversely
> * Delete filters by "qemuBlockThrottleFiltersDetach"("blockdev-del")
>   when detaching device
> 
> For throttle group commandline
> * Add qemuBuildThrottleGroupCommandLine in qemuBuildCommandLine to add
>   "object" of throttle-group
> * Verify throttle group definition when lauching vm
> * Check QEMU_CAPS_OBJECT_JSON before "qemuBuildObjectCommandlineFromJSON",
>   which is to build "-object" option
> 
> For throttle filter commandline
> * Add qemuBuildDiskThrottleFiltersCommandLine in qemuBuildDiskCommandLine
>   to add "blockdev"
> 
> Signed-off-by: Chun Feng Wu <danielwuwy@163.com>
> 
> * Apply suggested coding style changes.
> * Update of code documentation comments.
> 
> Signed-off-by: Harikumar Rajkumar <harirajkumar230@gmail.com>
> ---
>  src/qemu/qemu_command.c | 99 +++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_hotplug.c | 21 +++++++++
>  2 files changed, 120 insertions(+)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 750edd4faa..6d9efbe887 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2045,6 +2045,99 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommand *cmd,
>  }
>  
>  
> +static inline bool
> +qemuDiskConfigThrottleGroupEnabled(const virDomainThrottleGroupDef *group)
> +{
> +    return !!group->group_name &&
> +           virDomainBlockIoTuneInfoHasAny(group);
> +}
> +
> +
> +/**
> + * qemuBuildThrottleGroupCommandLine:
> + * @cmd: the command to modify
> + * @def: domain definition
> + * @qemuCaps: qemu capabilities object
> + *
> + * build throttle group object in json format
> + */
> +static int
> +qemuBuildThrottleGroupCommandLine(virCommand *cmd,
> +                                  const virDomainDef *def,
> +                                  virQEMUCaps *qemuCaps)
> +{
> +    size_t i;
> +
> +    for (i = 0; i < def->nthrottlegroups; i++) {
> +        g_autoptr(virJSONValue) props = NULL;
> +        g_autoptr(virJSONValue) limits = virJSONValueNewObject();
> +        virDomainThrottleGroupDef *group = def->throttlegroups[i];
> +        /* prefix group name with "throttle-" in QOM */
> +        g_autofree char *prefixed_group_name = g_strdup_printf("throttle-%s", group->group_name);
> +
> +        if (!qemuDiskConfigThrottleGroupEnabled(group)) {
> +            continue;
> +        }
> +
> +        if (qemuMonitorThrottleGroupLimits(limits, group) < 0)
> +            return -1;
> +
> +        if (qemuMonitorCreateObjectProps(&props, "throttle-group", prefixed_group_name,
> +                                         "a:limits", &limits,
> +                                         NULL) < 0)
> +            return -1;
> +
> +        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_JSON)) {
> +            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                           _("This QEMU doesn't support throttle group creation"));
> +            return -1;

This would be considered too late for this check; Luckily all of it can
be deledet now.

> +        }
> +
> +        if (qemuBuildObjectCommandlineFromJSON(cmd, props, qemuCaps) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int
> +qemuBuildBlockThrottleFilterCommandline(virCommand *cmd,
> +                                        qemuBlockThrottleFilterAttachData *data)
> +{
> +    if (data->filterProps) {
> +        g_autofree char *tmp = NULL;
> +        if (!(tmp = virJSONValueToString(data->filterProps, false)))
> +            return -1;
> +
> +        virCommandAddArgList(cmd, "-blockdev", tmp, NULL);
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int
> +qemuBuildDiskThrottleFiltersCommandLine(virCommand *cmd,
> +                                        virDomainDiskDef *disk)
> +{
> +    g_autoptr(qemuBlockThrottleFiltersData) data = NULL;
> +    size_t i;
> +
> +    data = qemuBuildThrottleFiltersAttachPrepareBlockdev(disk);
> +    if (!data)
> +        return -1;
> +
> +    for (i = 0; i < data->nfilterdata; i++) {
> +        if (qemuBuildBlockThrottleFilterCommandline(cmd,
> +                                                    data->filterdata[i]) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  static int
>  qemuBuildDiskSourceCommandLine(virCommand *cmd,
>                                 virDomainDiskDef *disk,
> @@ -2102,6 +2195,9 @@ qemuBuildDiskCommandLine(virCommand *cmd,
>      if (qemuBuildDiskSourceCommandLine(cmd, disk, qemuCaps) < 0)
>          return -1;
>  
> +    if (qemuBuildDiskThrottleFiltersCommandLine(cmd, disk) < 0)
> +        return -1;
> +
>      /* SD cards are currently instantiated via -drive if=sd, so the -device
>       * part must be skipped */
>      if (qemuDiskBusIsSD(disk->bus))
> @@ -10479,6 +10575,9 @@ qemuBuildCommandLine(virDomainObj *vm,
>      if (qemuBuildIOThreadCommandLine(cmd, def, qemuCaps) < 0)
>          return NULL;
>  
> +    if (qemuBuildThrottleGroupCommandLine(cmd, def, qemuCaps) < 0)
> +        return NULL;
> +
>      if (virDomainNumaGetNodeCount(def->numa) &&
>          qemuBuildNumaCommandLine(cfg, def, cmd, priv) < 0)
>          return NULL;
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 28ca321c5c..75437164ca 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -708,6 +708,7 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
>                              virDomainAsyncJob asyncJob)
>  {
>      g_autoptr(qemuBlockStorageSourceChainData) data = NULL;
> +    g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL;
>      qemuDomainObjPrivate *priv = vm->privateData;
>      g_autoptr(virJSONValue) devprops = NULL;
>      bool extensionDeviceAttached = false;
> @@ -746,6 +747,15 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
>          if (rc < 0)
>              goto rollback;
>  
> +        if ((filterData = qemuBuildThrottleFiltersAttachPrepareBlockdev(disk))) {
> +            if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0)
> +                return -1;
> +            rc = qemuBlockThrottleFiltersAttach(priv->mon, filterData);
> +            qemuDomainObjExitMonitor(vm);
> +            if (rc < 0)
> +                goto rollback;
> +        }
> +
>          if (disk->transient) {
>              g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL;
>              g_autoptr(GHashTable) blockNamedNodeData = NULL;
> @@ -817,6 +827,8 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
>      if (extensionDeviceAttached)
>          ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info));
>  
> +    qemuBlockThrottleFiltersDetach(priv->mon, filterData);
> +
>      qemuBlockStorageSourceChainDetach(priv->mon, data);
>  
>      qemuDomainObjExitMonitor(vm);
> @@ -4686,6 +4698,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver,
>  {
>      qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>      g_autoptr(qemuBlockStorageSourceChainData) diskBackend = NULL;
> +    g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL;
>      size_t i;
>      qemuDomainObjPrivate *priv = vm->privateData;
>      int ret = -1;
> @@ -4726,6 +4739,14 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver,
>  
>      qemuDomainObjEnterMonitor(vm);
>  
> +    if ((filterData = qemuBuildThrottleFiltersDetachPrepareBlockdev(disk))) {

I'll have to also check the layering with copy on read which is detached
below

> +
> +        qemuBlockThrottleFiltersDetach(priv->mon, filterData);
> +    }
> +    qemuDomainObjExitMonitor(vm);
> +
> +    qemuDomainObjEnterMonitor(vm);

No need to exit the monitor to just enter it back.

> +
>      if (diskBackend)
>          qemuBlockStorageSourceChainDetach(priv->mon, diskBackend);
>  
> -- 
> 2.39.5 (Apple Git-154)
>