Re: [PATCH RFC v2 05/12] qemu: hotplug: Support hot attach block disk along with throttle filters
On Thu, Apr 11, 2024 at 19:01:53 -0700, w...@linux.ibm.com wrote:
> From: Chun Feng Wu
>
> When attaching disk along with specified throttle groups, those groups will
> be chained up by parent node name, this change includes service side codes:
> * Each filter references one throttle group by group name
> * Each filter has a nodename, and those filters are chained up in sequence
> * Filter nodename index is persistented in
> virDomainObj->privateData(qemuDomainObjPrivate)
> * During hotplug, filter is created through QMP request("blockdev-add" with
> "driver":"throttle") to QEMU
> * Finally, "device_add"(attach) QMP request is adjusted to set "drive" to be
> top filter nodename in chain.
>
> Signed-off-by: Chun Feng Wu
> ---a
As noted before, the proper approach is to add the XML bits first so
some asumptions may be off.
Before next version please add implementation only after the XML and
docs is added.
> src/qemu/qemu_block.c | 131 ++
> src/qemu/qemu_block.h | 53 +++
> src/qemu/qemu_command.c | 64 +
> src/qemu/qemu_command.h | 6 +
> src/qemu/qemu_domain.c| 71 ++
> src/qemu/qemu_domain.h| 19 ++-
> src/qemu/qemu_hotplug.c | 24
> .../qemustatusxml2xmldata/backup-pull-in.xml | 1 +
> .../blockjob-blockdev-in.xml | 1 +
> .../blockjob-mirror-in.xml| 1 +
> .../migration-in-params-in.xml| 1 +
> .../migration-out-nbd-bitmaps-in.xml | 1 +
> .../migration-out-nbd-out.xml | 1 +
> .../migration-out-nbd-tls-out.xml | 1 +
> .../migration-out-params-in.xml | 1 +
> tests/qemustatusxml2xmldata/modern-in.xml | 1 +
> tests/qemustatusxml2xmldata/upgrade-out.xml | 1 +
> .../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 +
> 18 files changed, 375 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 738b72d7ea..c53de2465e 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -2739,6 +2739,137 @@ 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.
> + * e.g. {"execute":"blockdev-add", "arguments":{"driver":"throttle",
> + * "node-name":"libvirt-2-filter", "throttle-group":"limits0",
> + * "file": "libvirt-1-format"}}
> + */
> +virJSONValue *
> +qemuBlockThrottleFilterGetProps(virDomainThrottleFilterDef *filter,
> +const char *parentNodeName)
> +{
> +g_autoptr(virJSONValue) props = NULL;
> +
> +if (virJSONValueObjectAdd(&props,
> + "s:driver", "throttle",
> + "s:node-name",
> qemuBlockThrottleFilterGetNodename(filter),
> + "s:throttle-group", filter->group_name,
> + "s:file", parentNodeName,
> + NULL) < 0)
> +return 0;
> +
> +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);
> +data->filterAttached = true;
You can't claim that the filter node was attached at this point. The
rollback code won't work properly.
> +
> +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->filterN
[PATCH RFC v2 05/12] qemu: hotplug: Support hot attach block disk along with throttle filters
From: Chun Feng Wu
When attaching disk along with specified throttle groups, those groups will be
chained up by parent node name, this change includes service side codes:
* Each filter references one throttle group by group name
* Each filter has a nodename, and those filters are chained up in sequence
* Filter nodename index is persistented in
virDomainObj->privateData(qemuDomainObjPrivate)
* During hotplug, filter is created through QMP request("blockdev-add" with
"driver":"throttle") to QEMU
* Finally, "device_add"(attach) QMP request is adjusted to set "drive" to be
top filter nodename in chain.
Signed-off-by: Chun Feng Wu
---
src/qemu/qemu_block.c | 131 ++
src/qemu/qemu_block.h | 53 +++
src/qemu/qemu_command.c | 64 +
src/qemu/qemu_command.h | 6 +
src/qemu/qemu_domain.c| 71 ++
src/qemu/qemu_domain.h| 19 ++-
src/qemu/qemu_hotplug.c | 24
.../qemustatusxml2xmldata/backup-pull-in.xml | 1 +
.../blockjob-blockdev-in.xml | 1 +
.../blockjob-mirror-in.xml| 1 +
.../migration-in-params-in.xml| 1 +
.../migration-out-nbd-bitmaps-in.xml | 1 +
.../migration-out-nbd-out.xml | 1 +
.../migration-out-nbd-tls-out.xml | 1 +
.../migration-out-params-in.xml | 1 +
tests/qemustatusxml2xmldata/modern-in.xml | 1 +
tests/qemustatusxml2xmldata/upgrade-out.xml | 1 +
.../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 +
18 files changed, 375 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 738b72d7ea..c53de2465e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -2739,6 +2739,137 @@ 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.
+ * e.g. {"execute":"blockdev-add", "arguments":{"driver":"throttle",
+ * "node-name":"libvirt-2-filter", "throttle-group":"limits0",
+ * "file": "libvirt-1-format"}}
+ */
+virJSONValue *
+qemuBlockThrottleFilterGetProps(virDomainThrottleFilterDef *filter,
+const char *parentNodeName)
+{
+g_autoptr(virJSONValue) props = NULL;
+
+if (virJSONValueObjectAdd(&props,
+ "s:driver", "throttle",
+ "s:node-name",
qemuBlockThrottleFilterGetNodename(filter),
+ "s:throttle-group", filter->group_name,
+ "s:file", parentNodeName,
+ NULL) < 0)
+return 0;
+
+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);
+data->filterAttached = true;
+
+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
+qemuBlockThrottleFilterChainDataFree(qemuBlockThrottleFilterChainData *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);
+}
+
+
+int
+qemuBlockThrottleFilterChainAttach(qemuMonitor *mon,
+ qemuBlockThrottleFilterChainData *data)
+{
+size_t i;
+
+for (i = 0; i < data->nfilterdata; i++) {
+if (qemuMonitorBlockdevAdd(mon, &data->filterdata[
