On Mon, Nov 18, 2024 at 19:24:12 +0530, Harikumar R wrote:
From: Chun Feng Wu <danielwuwy@163.com>
Introduce throttle filter along with corresponding operations.
* Define new struct 'virDomainThrottleFilterDef' and corresponding destructor * Update _virDomainDiskDef to include virDomainThrottleFilterDef * Support throttle filter "Parse" and "Format" for operations between DOM XML and structs. Note, this commit just contains parse/format of group name for throttle filter in domain_conf.c, there is other commit to handle throttle filter nodename parse/format between throttlefilter and diskPrivateData for statusxml in qemu_domain.c when processing qemuDomainDiskPrivate and qemuDomainDiskPrivate
Signed-off-by: Chun Feng Wu <danielwuwy@163.com> --- src/conf/domain_conf.c | 105 ++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 18 +++++++ src/conf/virconftypes.h | 2 + 3 files changed, 125 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ecad148783..8841425adb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3816,6 +3816,16 @@ virDomainThrottleGroupDefArrayFree(virDomainThrottleGroupDef **def, }
+void +virDomainThrottleFilterDefFree(virDomainThrottleFilterDef *def) +{ + if (!def) + return; + g_free(def->group_name); + g_free(def->nodename);
This doesn't free 'def' itself. If it's just meant to free the contents please use the 'Clear' naming instead of 'Free'
+} + + void virDomainResourceDefFree(virDomainResourceDef *resource) { @@ -7913,6 +7923,53 @@ virDomainDefThrottleGroupsParse(virDomainDef *def, }
+static virDomainThrottleFilterDef * +virDomainDiskThrottleFilterDefParse(xmlNodePtr node) +{ + g_autoptr(virDomainThrottleFilterDef) filter = g_new0(virDomainThrottleFilterDef, 1); + + filter->group_name = virXMLPropString(node, "group");
This doesn't report error ...
+ + if (!filter->group_name) + return NULL;
And you break out here ..
+ + return g_steal_pointer(&filter); +} + + +static int +virDomainDiskDefThrottleFiltersParse(virDomainDiskDef *def, + xmlXPathContextPtr ctxt) +{ + size_t i; + int n = 0; + g_autofree xmlNodePtr *nodes = NULL; + + if ((n = virXPathNodeSet("./throttlefilters/throttlefilter", ctxt, &nodes)) < 0) + return -1; + + if (n) + def->throttlefilters = g_new0(virDomainThrottleFilterDef *, n); + + for (i = 0; i < n; i++) { + g_autoptr(virDomainThrottleFilterDef) filter = NULL; + + if (!(filter = virDomainDiskThrottleFilterDefParse(nodes[i]))) { + return -1;
... and here too so this function will not report an error. Use virXMLPropStringRequired instead if the function field is required.
+ } + + if (virDomainThrottleFilterFind(def, filter->group_name)) { + virReportError(VIR_ERR_XML_ERROR, + _("duplicate filter name '%1$s' found"), + filter->group_name); + return -1; + } + def->throttlefilters[def->nthrottlefilters++] = g_steal_pointer(&filter); + } + return 0; +} + + static int virDomainDiskDefMirrorParse(virDomainDiskDef *def, xmlNodePtr cur, @@ -8403,6 +8460,9 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, if (virDomainDiskDefIotuneParse(def, ctxt) < 0) return NULL;
+ if (virDomainDiskDefThrottleFiltersParse(def, ctxt) < 0) + return NULL; + def->domain_name = virXPathString("string(./backenddomain/@name)", ctxt); def->serial = virXPathString("string(./serial)", ctxt); def->wwn = virXPathString("string(./wwn)", ctxt); @@ -22621,6 +22681,34 @@ virDomainThrottleGroupDel(virDomainDef *def, }
+/** + * virDomainThrottleFilterFind: + * @def: domain disk definition + * @name: throttle group name + * + * Search domain disk to find throttle filter referencing + * throttle group with name @name. + * + * Return a pointer to throttle filter found + */ +virDomainThrottleFilterDef * +virDomainThrottleFilterFind(const virDomainDiskDef *def, + const char *name) +{ + size_t i; + + if (!def->throttlefilters || def->nthrottlefilters == 0) + return NULL;
virDomainDiskDefFormatThrottleFilters below checks only the 'nthrottlefilters'. In fact all of the above check is not needed as the loop simply won't find anything in a 0 sized array and return NULL at the end.
+ + for (i = 0; i < def->nthrottlefilters; i++) { + if (STREQ(name, def->throttlefilters[i]->group_name)) + return def->throttlefilters[i]; + } + + return NULL; +} + + static int virDomainEventActionDefFormat(virBuffer *buf, int type, @@ -23235,6 +23323,21 @@ virDomainDiskDefFormatIotune(virBuffer *buf, #undef FORMAT_IOTUNE
+static void +virDomainDiskDefFormatThrottleFilters(virBuffer *buf, + virDomainDiskDef *disk) +{ + size_t i; + g_auto(virBuffer) throttleChildBuf = VIR_BUFFER_INIT_CHILD(buf); + for (i = 0; i < disk->nthrottlefilters; i++) { + g_auto(virBuffer) throttleAttrBuf = VIR_BUFFER_INITIALIZER; + virBufferEscapeString(&throttleAttrBuf, " group='%s'", disk->throttlefilters[i]->group_name); + virXMLFormatElement(&throttleChildBuf, "throttlefilter", &throttleAttrBuf, NULL); + } + virXMLFormatElement(buf, "throttlefilters", NULL, &throttleChildBuf); +} + + static void virDomainDiskDefFormatDriver(virBuffer *buf, virDomainDiskDef *disk) @@ -23521,6 +23624,8 @@ virDomainDiskDefFormat(virBuffer *buf,
virDomainDiskDefFormatIotune(&childBuf, def);
+ virDomainDiskDefFormatThrottleFilters(&childBuf, def); + if (def->src->readonly) virBufferAddLit(&childBuf, "<readonly/>\n"); if (def->src->shared) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2f40d304b4..f1e85ab42f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -520,6 +520,13 @@ void virDomainDiskIothreadDefFree(virDomainDiskIothreadDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainDiskIothreadDef, virDomainDiskIothreadDefFree);
+/* Stores information related to a ThrottleFilter resource. */ +struct _virDomainThrottleFilterDef { + char *group_name;
Add a separator comment here here as node name is a qemu driver implementation detail and not a configuration field.
+ char *nodename; /* node name of throttle filter object */
This will also later have to be serialized into the qemu driver's disk private data (I didn't check if it is). Rest looks okay to me.