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:
> 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;
The above call can be inlined, there's a bit too much indirection going
on here.
> +
> + 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;
coding style:
const char *parent...
> + 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 */
pointless comment
> + parentNodeName = qemuBlockStorageSourceGetEffectiveNodename(disk->src);
> + }
Add empty line.
> + /* 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);
Coding style dictates: 'char *nodename';
> + if (nodename) {
> + /* first time to set nodename in filter */
pointless comment
> + def->throttlefilters[i]->nodename = g_strdup(nodename);
use the accessor defined before.
> + }
> + }
> +
> + 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);
virBufferEscapeString skips the formatting of the complete format string
in case when the 3rd argument is NULL. This has the potential of
breaking the XML if the nodename or group_name are missing. Please check
them, as it's better to have one broken feature than lose the whole VM
when restartingf the daemon/.
> + }
> + }
> 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 */
pointless comment
> + 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;
> }
The rest looks reasonable so if you do all the changes:
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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
© 2016 - 2026 Red Hat, Inc.