New subject: [PATCH v9 01/17] schema: Add new domain elements to support multiple throttle groups

18 Mar 2025

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>

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

Signed-off-by: Harikumar Rajkumar <harirajkumar230@gmail.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.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  |  77 +++++++++++++++++++++--
 5 files changed, 345 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 2468725bf7..e370411e16 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2743,6 +2743,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 inline 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 84ff62cd6c..ce5f45b66b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -11021,6 +11021,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 {
+        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 903ffa62ef..79d4f47690 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -124,6 +124,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 efc1ebddd5..fcdf28f3fc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2236,6 +2236,33 @@ 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) {
+            qemuBlockThrottleFilterSetNodename(def->throttlefilters[i], g_strdup(nodename));
+        }
+    }
+
+    return 0;
+}
+
+
 static int
 qemuDomainDiskPrivateParse(xmlXPathContextPtr ctxt,
                            virDomainDiskDef *disk)
@@ -2245,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;
 }

@@ -2254,14 +2284,27 @@ 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++) {
+
+                if (disk->throttlefilters[i]->nodename)
+                    virBufferEscapeString(buf, "<nodename type='throttle-filter' name='%s' ",
+                                          disk->throttlefilters[i]->nodename);
+
+                if (disk->throttlefilters[i]->group_name)
+                    virBufferEscapeString(buf, "group='%s'/>\n", disk->throttlefilters[i]->group_name);
+            }
+        }
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</nodenames>\n");
     }
@@ -6292,7 +6335,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).
@@ -6305,6 +6349,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;

@@ -9707,6 +9755,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));
+
+    qemuBlockThrottleFilterSetNodename(filter, g_strdup_printf("%s-filter", nodenameprefix));
+}
+
+
 int
 qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk,
                                        virStorageSource *src,
@@ -9730,6 +9794,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)
@@ -9744,6 +9809,10 @@ qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDef *disk,
             return -1;
     }

+    for (i = 0; i < disk->nthrottlefilters; i++) {
+        qemuDomainPrepareThrottleFilterBlockdev(disk->throttlefilters[i], priv);
+    }
+
     return 0;
 }

-- 
2.48.1


[PATCH v9 reviewed 00/17] qemu: block: Support block disk along with throttle filters

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Ján Tomko

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Peter Krempa

Ján Tomko

Peter Krempa

tags

participants (2)