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

It contains throttle filter nodename processing(new nodename,
topnodename, parse and format nodename), throttle filter
attaching/detaching preparation and implementation.

* Updated "qemuDomainDiskGetTopNodename", so if throttlefilter is used
  together with copyOnRead, top node is throttle filter node, e.g.
  device -> throttle -> copyOnRead Layer-> image chain
* In qemuBuildThrottleFiltersAttachPrepareBlockdev, if copy_on_read
  is on, build throttle nodename chain on top of copy_on_read nodename
* In status xml, throttle filter nodename(virDomainDiskDef.nodename) is
  saved at disk/privateData/nodenames/nodename(type='throttle-filter'),
  corresponding parse/format sits in qemuDomainDiskPrivateParse and
  qemuDomainDiskPrivateFormat
* If filter nodename hasn't been set by qemuDomainDiskPrivateParse,
  in qemuDomainPrepareThrottleFilterBlockdev, filter nodename index
  can be generated by reusing qemuDomainStorageIDNew and current
  global sequence number is persistented in virDomainObj-
  >privateData(qemuDomainObjPrivate)->nodenameindex.
  qemuDomainPrepareThrottleFilterBlockdev is called by
  qemuDomainPrepareDiskSourceBlockdev, which in turn used by both
  hotplug and qemuProcessStart to prepare throttle filter node name
* Define method qemuBlockThrottleFilterGetProps, which is used by
  both hotplug and command to build throttle object for QEMU
* Define methods for throttle filter attach/detach/rollback

Signed-off-by: Chun Feng Wu <danielwuwy@163.com>
---
 src/qemu/qemu_block.c   | 136 ++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_block.h   |  49 +++++++++++++++
 src/qemu/qemu_command.c |  81 ++++++++++++++++++++++++
 src/qemu/qemu_command.h |   6 ++
 src/qemu/qemu_domain.c  |  73 +++++++++++++++++++--
 5 files changed, 341 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 692b4d350e..5ff7319ceb 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2755,6 +2755,142 @@ qemuBlockStorageSourceCreateDetectSize(GHashTable *blockNamedNodeData,
 }
 
 
+void
+qemuBlockThrottleFilterSetNodename(virDomainThrottleFilterDef *filter,
+                                   char *nodename)
+{
+    g_free(filter->nodename);
+    filter->nodename = nodename;
+}
+
+
+const char *
+qemuBlockThrottleFilterGetNodename(virDomainThrottleFilterDef *filter)
+{
+    return filter->nodename;
+}
+
+
+/**
+ * qemuBlockThrottleFilterGetProps:
+ * @filter: throttle filter
+ * @parentNodeName: parent nodename of @filter
+ *
+ * Build "arguments" part of "blockdev-add" QMP cmd.
+ */
+static virJSONValue *
+qemuBlockThrottleFilterGetProps(virDomainThrottleFilterDef *filter,
+                                const char *parentNodeName)
+{
+    g_autoptr(virJSONValue) props = NULL;
+    /* prefix group name with "throttle-" in QOM */
+    g_autofree char *prefixed_group_name = g_strdup_printf("throttle-%s", filter->group_name);
+    if (virJSONValueObjectAdd(&props,
+                              "s:driver", "throttle",
+                              "s:node-name", qemuBlockThrottleFilterGetNodename(filter),
+                              "s:throttle-group", prefixed_group_name,
+                              "s:file", parentNodeName,
+                              NULL) < 0)
+        return NULL;
+
+    return g_steal_pointer(&props);
+}
+
+
+void
+qemuBlockThrottleFilterAttachDataFree(qemuBlockThrottleFilterAttachData *data)
+{
+    if (!data)
+        return;
+
+    virJSONValueFree(data->filterProps);
+    g_free(data);
+}
+
+
+qemuBlockThrottleFilterAttachData *
+qemuBlockThrottleFilterAttachPrepareBlockdev(virDomainThrottleFilterDef *filter,
+                                             const char *parentNodeName)
+{
+    g_autoptr(qemuBlockThrottleFilterAttachData) data = NULL;
+
+    data = g_new0(qemuBlockThrottleFilterAttachData, 1);
+
+    if (!(data->filterProps = qemuBlockThrottleFilterGetProps(filter, parentNodeName)))
+        return NULL;
+
+    data->filterNodeName = qemuBlockThrottleFilterGetNodename(filter);
+
+    return g_steal_pointer(&data);
+}
+
+
+void
+qemuBlockThrottleFilterAttachRollback(qemuMonitor *mon,
+                                      qemuBlockThrottleFilterAttachData *data)
+{
+    virErrorPtr orig_err;
+
+    virErrorPreserveLast(&orig_err);
+
+    if (data->filterAttached)
+        ignore_value(qemuMonitorBlockdevDel(mon, data->filterNodeName));
+
+    virErrorRestore(&orig_err);
+}
+
+
+void
+qemuBlockThrottleFiltersDataFree(qemuBlockThrottleFiltersData *data)
+{
+    size_t i;
+
+    if (!data)
+        return;
+
+    for (i = 0; i < data->nfilterdata; i++)
+        qemuBlockThrottleFilterAttachDataFree(data->filterdata[i]);
+
+    g_free(data->filterdata);
+    g_free(data);
+}
+
+
+/**
+ * qemuBlockThrottleFiltersAttach:
+ * @mon: monitor object
+ * @data: filter chain data
+ *
+ * Attach throttle filters.
+ * Caller must enter @mon prior calling this function.
+ */
+int
+qemuBlockThrottleFiltersAttach(qemuMonitor *mon,
+                               qemuBlockThrottleFiltersData *data)
+{
+    size_t i;
+
+    for (i = 0; i < data->nfilterdata; i++) {
+        if (qemuMonitorBlockdevAdd(mon, &data->filterdata[i]->filterProps) < 0)
+            return -1;
+        data->filterdata[i]->filterAttached = true;
+    }
+
+    return 0;
+}
+
+
+void
+qemuBlockThrottleFiltersDetach(qemuMonitor *mon,
+                               qemuBlockThrottleFiltersData *data)
+{
+    size_t i;
+
+    for (i = data->nfilterdata; i > 0; i--)
+        qemuBlockThrottleFilterAttachRollback(mon, data->filterdata[i-1]);
+}
+
+
 int
 qemuBlockRemoveImageMetadata(virQEMUDriver *driver,
                              virDomainObj *vm,
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index f13a4a4a9a..b9e950e494 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -215,6 +215,55 @@ qemuBlockStorageSourceCreateDetectSize(GHashTable *blockNamedNodeData,
                                        virStorageSource *src,
                                        virStorageSource *templ);
 
+void
+qemuBlockThrottleFilterSetNodename(virDomainThrottleFilterDef *filter,
+                                   char *nodename);
+
+const char *
+qemuBlockThrottleFilterGetNodename(virDomainThrottleFilterDef *filter);
+
+typedef struct qemuBlockThrottleFilterAttachData qemuBlockThrottleFilterAttachData;
+struct qemuBlockThrottleFilterAttachData {
+    virJSONValue *filterProps;
+    const char *filterNodeName;
+    bool filterAttached;
+};
+
+qemuBlockThrottleFilterAttachData *
+qemuBlockThrottleFilterAttachPrepareBlockdev(virDomainThrottleFilterDef *throttlefilter,
+                                             const char *parentNodeName);
+
+void
+qemuBlockThrottleFilterAttachDataFree(qemuBlockThrottleFilterAttachData *data);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockThrottleFilterAttachData,
+                              qemuBlockThrottleFilterAttachDataFree);
+
+void
+qemuBlockThrottleFilterAttachRollback(qemuMonitor *mon,
+                                      qemuBlockThrottleFilterAttachData *data);
+
+struct _qemuBlockThrottleFiltersData {
+    qemuBlockThrottleFilterAttachData **filterdata;
+    size_t nfilterdata;
+};
+
+typedef struct _qemuBlockThrottleFiltersData qemuBlockThrottleFiltersData;
+
+void
+qemuBlockThrottleFiltersDataFree(qemuBlockThrottleFiltersData *data);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuBlockThrottleFiltersData,
+                              qemuBlockThrottleFiltersDataFree);
+
+int
+qemuBlockThrottleFiltersAttach(qemuMonitor *mon,
+                               qemuBlockThrottleFiltersData *data);
+
+void
+qemuBlockThrottleFiltersDetach(qemuMonitor *mon,
+                               qemuBlockThrottleFiltersData *data);
+
 int
 qemuBlockRemoveImageMetadata(virQEMUDriver *driver,
                              virDomainObj *vm,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 696f891b47..0c119c8827 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -10996,6 +10996,87 @@ qemuBuildStorageSourceChainAttachPrepareBlockdevOne(qemuBlockStorageSourceChainD
 }
 
 
+/**
+ * qemuBuildThrottleFiltersAttachPrepareBlockdevOne:
+ * @data: filter chain data, which consists of array of filters and size of such array
+ * @throttlefilter: new filter to be added into filter array
+ * @parentNodeName: parent nodename for this new throttlefilter
+ *
+ * Build filter node chain to provide more flexibility for block disk I/O limits
+ */
+static int
+qemuBuildThrottleFiltersAttachPrepareBlockdevOne(qemuBlockThrottleFiltersData *data,
+                                                 virDomainThrottleFilterDef *throttlefilter,
+                                                 const char *parentNodeName)
+{
+    g_autoptr(qemuBlockThrottleFilterAttachData) elem = NULL;
+
+    if (!(elem = qemuBlockThrottleFilterAttachPrepareBlockdev(throttlefilter, parentNodeName)))
+        return -1;
+
+    VIR_APPEND_ELEMENT(data->filterdata, data->nfilterdata, elem);
+    return 0;
+}
+
+
+/**
+ * qemuBuildThrottleFiltersAttachPrepareBlockdev:
+ * @disk: domain disk
+ *
+ * Build filter node chain to provide more flexibility for block disk I/O limits
+ */
+qemuBlockThrottleFiltersData *
+qemuBuildThrottleFiltersAttachPrepareBlockdev(virDomainDiskDef *disk)
+{
+    g_autoptr(qemuBlockThrottleFiltersData) data = NULL;
+    size_t i;
+    const char * parentNodeName = NULL;
+    qemuDomainDiskPrivate *priv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+    data = g_new0(qemuBlockThrottleFiltersData, 1);
+    /* if copy_on_read is enabled, put throttle chain on top of it */
+    if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) {
+       parentNodeName = priv->nodeCopyOnRead;
+    } else {
+        /* get starting parentNodename, e.g. libvirt-1-format */
+        parentNodeName = qemuBlockStorageSourceGetEffectiveNodename(disk->src);
+    }
+    /* build filterdata, which contains all filters info and sequence info through parentNodeName */
+    for (i = 0; i < disk->nthrottlefilters; i++) {
+        if (qemuBuildThrottleFiltersAttachPrepareBlockdevOne(data, disk->throttlefilters[i], parentNodeName) < 0)
+            return NULL;
+        parentNodeName = disk->throttlefilters[i]->nodename;
+    }
+
+    return g_steal_pointer(&data);
+}
+
+
+/**
+ * qemuBuildThrottleFiltersDetachPrepareBlockdev:
+ * @disk: domain disk
+ *
+ * Build filters data for later "blockdev-del"
+ */
+qemuBlockThrottleFiltersData *
+qemuBuildThrottleFiltersDetachPrepareBlockdev(virDomainDiskDef *disk)
+{
+    g_autoptr(qemuBlockThrottleFiltersData) data = g_new0(qemuBlockThrottleFiltersData, 1);
+    size_t i;
+
+    /* build filterdata, which contains filters info and sequence info */
+    for (i = 0; i < disk->nthrottlefilters; i++) {
+        g_autoptr(qemuBlockThrottleFilterAttachData) elem = g_new0(qemuBlockThrottleFilterAttachData, 1);
+        /* ignore other fields since the following info are enough for "blockdev-del" */
+        elem->filterNodeName = qemuBlockThrottleFilterGetNodename(disk->throttlefilters[i]);
+        elem->filterAttached = true;
+
+        VIR_APPEND_ELEMENT(data->filterdata, data->nfilterdata, elem);
+    }
+    return g_steal_pointer(&data);
+}
+
+
 /**
  * qemuBuildStorageSourceChainAttachPrepareBlockdev:
  * @top: storage source chain
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 76c514b5f7..1b0296ea42 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -116,6 +116,12 @@ qemuBlockStorageSourceChainData *
 qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSource *top,
                                                     virStorageSource *backingStore);
 
+qemuBlockThrottleFiltersData *
+qemuBuildThrottleFiltersAttachPrepareBlockdev(virDomainDiskDef *disk);
+
+qemuBlockThrottleFiltersData *
+qemuBuildThrottleFiltersDetachPrepareBlockdev(virDomainDiskDef *disk);
+
 virJSONValue *
 qemuBuildDiskDeviceProps(const virDomainDef *def,
                          virDomainDiskDef *disk,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f3b810a564..b6acf895c3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2235,6 +2235,34 @@ qemuStorageSourcePrivateDataFormat(virStorageSource *src,
 }
 
 
+static int
+virDomainDiskThrottleFilterNodeNamesParse(xmlXPathContextPtr ctxt,
+                                          virDomainDiskDef *def)
+{
+    size_t i;
+    int n = 0;
+    g_autofree xmlNodePtr *nodes = NULL;
+    g_autoptr(GHashTable) throttleFiltersMap = virHashNew(g_free);
+
+    if ((n = virXPathNodeSet("./nodenames/nodename[@type='throttle-filter']", ctxt, &nodes)) < 0)
+        return -1;
+
+    for (i = 0; i < n; i++) {
+        g_hash_table_insert(throttleFiltersMap, virXMLPropString(nodes[i], "group"), virXMLPropString(nodes[i], "name"));
+    }
+
+    for (i = 0; i < def->nthrottlefilters; i++) {
+        char* nodename = g_hash_table_lookup(throttleFiltersMap, def->throttlefilters[i]->group_name);
+        if (nodename) {
+            /* first time to set nodename in filter */
+            def->throttlefilters[i]->nodename = g_strdup(nodename);
+        }
+    }
+
+    return 0;
+}
+
+
 static int
 qemuDomainDiskPrivateParse(xmlXPathContextPtr ctxt,
                            virDomainDiskDef *disk)
@@ -2244,6 +2272,9 @@ qemuDomainDiskPrivateParse(xmlXPathContextPtr ctxt,
     priv->qomName = virXPathString("string(./qom/@name)", ctxt);
     priv->nodeCopyOnRead = virXPathString("string(./nodenames/nodename[@type='copyOnRead']/@name)", ctxt);
 
+    if (virDomainDiskThrottleFilterNodeNamesParse(ctxt, disk) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -2253,14 +2284,22 @@ qemuDomainDiskPrivateFormat(virDomainDiskDef *disk,
                             virBuffer *buf)
 {
     qemuDomainDiskPrivate *priv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+    size_t i;
 
     virBufferEscapeString(buf, "<qom name='%s'/>\n", priv->qomName);
 
-    if (priv->nodeCopyOnRead) {
+    if (priv->nodeCopyOnRead || disk->nthrottlefilters > 0) {
         virBufferAddLit(buf, "<nodenames>\n");
         virBufferAdjustIndent(buf, 2);
-        virBufferEscapeString(buf, "<nodename type='copyOnRead' name='%s'/>\n",
-                              priv->nodeCopyOnRead);
+        if (priv->nodeCopyOnRead)
+            virBufferEscapeString(buf, "<nodename type='copyOnRead' name='%s'/>\n",
+                                  priv->nodeCopyOnRead);
+        if (disk->nthrottlefilters > 0) {
+            for (i = 0; i < disk->nthrottlefilters; i++) {
+                virBufferEscapeString(buf, "<nodename type='throttle-filter' name='%s' ", disk->throttlefilters[i]->nodename);
+                virBufferEscapeString(buf, "group='%s'/>\n", disk->throttlefilters[i]->group_name);
+            }
+        }
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</nodenames>\n");
     }
@@ -6348,7 +6387,8 @@ qemuDomainDetermineDiskChain(virQEMUDriver *driver,
  * @disk: disk definition object
  *
  * Returns the pointer to the node-name of the topmost layer used by @disk as
- * backend. Currently returns the nodename of the copy-on-read filter if enabled
+ * backend. Currently returns the nodename of top throttle filter if enabled
+ * or the nodename of the copy-on-read filter if enabled
  * or the nodename of the top image's format driver. Empty disks return NULL.
  * This must be used only with disks instantiated via -blockdev (thus not
  * for SD cards).
@@ -6361,6 +6401,10 @@ qemuDomainDiskGetTopNodename(virDomainDiskDef *disk)
     if (virStorageSourceIsEmpty(disk->src))
         return NULL;
 
+    /* If disk has throttles, take top throttle node name */
+    if (disk->nthrottlefilters > 0)
+        return disk->throttlefilters[disk->nthrottlefilters - 1]->nodename;
+
     if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON)
         return priv->nodeCopyOnRead;
 
@@ -9724,6 +9768,22 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk,
 }
 
 
+static void
+qemuDomainPrepareThrottleFilterBlockdev(virDomainThrottleFilterDef *filter,
+                                        qemuDomainObjPrivate *priv)
+{
+    g_autofree char *nodenameprefix = NULL;
+
+    /* skip setting throttle filter nodename if it's set by parsing statusxml */
+    if (filter->nodename) {
+        return;
+    }
+    nodenameprefix = g_strdup_printf("libvirt-%u", qemuDomainStorageIDNew(priv));
+    /* generate and set nodename into filter for later QEMU cmd preparation */
+    qemuBlockThrottleFilterSetNodename(filter, g_strdup_printf("%s-filter", nodenameprefix));
+}
+
+
 int
 qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk,
                                        virStorageSource *src,
@@ -9747,6 +9807,7 @@ qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDef *disk,
 {
     qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
     virStorageSource *n;
+    size_t i;
 
     if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
         !diskPriv->nodeCopyOnRead)
@@ -9757,6 +9818,10 @@ qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDef *disk,
             return -1;
     }
 
+    for (i = 0; i < disk->nthrottlefilters; i++) {
+        qemuDomainPrepareThrottleFilterBlockdev(disk->throttlefilters[i], priv);
+    }
+
     return 0;
 }
 
-- 
2.39.5 (Apple Git-154)

On Mon, Nov 18, 2024 at 19:24:18 +0530, Harikumar R wrote:

[...]

> ---
>  src/qemu/qemu_block.c   | 136 ++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_block.h   |  49 +++++++++++++++
>  src/qemu/qemu_command.c |  81 ++++++++++++++++++++++++
>  src/qemu/qemu_command.h |   6 ++
>  src/qemu/qemu_domain.c  |  73 +++++++++++++++++++--
>  5 files changed, 341 insertions(+), 4 deletions(-)

Note this is a preliminary comment; please don't send an updated
version just because of this:

Fails to compile:

../../../libvirt/src/qemu/qemu_domain.c: In function ‘qemuDomainPrepareDiskSourceBlockdev’:
../../../libvirt/src/qemu/qemu_domain.c:9708:12: error: unused variable ‘i’ [-Werror=unused-variable]
 9708 |     size_t i;
      |            ^
../../../libvirt/src/qemu/qemu_domain.c: In function ‘qemuDomainPrepareDiskSource’:
../../../libvirt/src/qemu/qemu_domain.c:9754:10: error: ‘i’ undeclared (first use in this function); did you mean ‘ip’?
 9754 |     for (i = 0; i < disk->nthrottlefilters; i++) {
      |          ^
      |          ip

"If you're going to submit multiple patches, the automated tests must
pass after each patch, not just after the last one."

https://www.libvirt.org/hacking.html#preparing-patches