From: Chun Feng Wu <wucf@linux.ibm.com>
* Define new struct 'virDomainThrottleGroupDef' and corresponding destructor
* Add operations(Add, Update, Del, Find, Copy, Free) for 'virDomainThrottleGroupDef'
* Update _virDomainDef to include virDomainThrottleGroupDef
* Support new resource "Parse" and "Format" for operations between struct and DOM XML
* Make sure "group_name" is defined in xml
Signed-off-by: Chun Feng Wu <wucf@linux.ibm.com>
---
src/conf/domain_conf.c | 281 ++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 31 +++++
src/conf/virconftypes.h | 2 +
3 files changed, 314 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fde594f811..05d6f7ad3a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3750,6 +3750,32 @@ virDomainIOThreadIDDefArrayInit(virDomainDef *def,
}
+void
+virDomainThrottleGroupDefFree(virDomainThrottleGroupDef *def)
+{
+ if (!def)
+ return;
+ g_free(def->group_name);
+ g_free(def);
+}
+
+
+static void
+virDomainThrottleGroupDefArrayFree(virDomainThrottleGroupDef **def,
+ int nthrottlegroups)
+{
+ size_t i;
+
+ if (!def)
+ return;
+
+ for (i = 0; i < nthrottlegroups; i++)
+ virDomainThrottleGroupDefFree(def[i]);
+
+ g_free(def);
+}
+
+
void
virDomainResourceDefFree(virDomainResourceDef *resource)
{
@@ -4029,6 +4055,8 @@ void virDomainDefFree(virDomainDef *def)
virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids);
+ virDomainThrottleGroupDefArrayFree(def->throttlegroups, def->nthrottlegroups);
+
g_free(def->defaultIOThread);
virBitmapFree(def->cputune.emulatorpin);
@@ -7697,6 +7725,113 @@ virDomainDiskDefIotuneParse(virDomainDiskDef *def,
#undef PARSE_IOTUNE
+#define PARSE_THROTTLEGROUP(val) \
+ if (virXPathULongLong("string(./" #val ")", \
+ ctxt, &group->val) == -2) { \
+ virReportError(VIR_ERR_XML_ERROR, \
+ _("throttle group field '%1$s' must be an integer"), #val); \
+ return NULL; \
+ }
+
+
+static virDomainThrottleGroupDef *
+virDomainThrottleGroupDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
+{
+ g_autoptr(virDomainThrottleGroupDef) group = g_new0(virDomainThrottleGroupDef, 1);
+
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+ ctxt->node = node;
+
+ PARSE_THROTTLEGROUP(total_bytes_sec);
+ PARSE_THROTTLEGROUP(read_bytes_sec);
+ PARSE_THROTTLEGROUP(write_bytes_sec);
+ PARSE_THROTTLEGROUP(total_iops_sec);
+ PARSE_THROTTLEGROUP(read_iops_sec);
+ PARSE_THROTTLEGROUP(write_iops_sec);
+
+ PARSE_THROTTLEGROUP(total_bytes_sec_max);
+ PARSE_THROTTLEGROUP(read_bytes_sec_max);
+ PARSE_THROTTLEGROUP(write_bytes_sec_max);
+ PARSE_THROTTLEGROUP(total_iops_sec_max);
+ PARSE_THROTTLEGROUP(read_iops_sec_max);
+ PARSE_THROTTLEGROUP(write_iops_sec_max);
+
+ PARSE_THROTTLEGROUP(size_iops_sec);
+
+ PARSE_THROTTLEGROUP(total_bytes_sec_max_length);
+ PARSE_THROTTLEGROUP(read_bytes_sec_max_length);
+ PARSE_THROTTLEGROUP(write_bytes_sec_max_length);
+ PARSE_THROTTLEGROUP(total_iops_sec_max_length);
+ PARSE_THROTTLEGROUP(read_iops_sec_max_length);
+ PARSE_THROTTLEGROUP(write_iops_sec_max_length);
+
+ /* group_name is required */
+ if (!(group->group_name = virXPathString("string(./group_name)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing group name"));
+ return NULL;
+ }
+
+ return g_steal_pointer(&group);
+}
+#undef PARSE_THROTTLEGROUP
+
+
+virDomainThrottleGroupDef *
+virDomainThrottleGroupByName(virDomainDef *def,
+ const char *name)
+{
+ virDomainThrottleGroupDef *tgroup;
+ size_t i;
+ int idx = -1;
+
+ for (i = 0; i < def->nthrottlegroups; i++) {
+ tgroup = def->throttlegroups[i];
+ if (STREQ(tgroup->group_name, name))
+ idx = i;
+ }
+
+ if (idx < 0)
+ return NULL;
+
+ return def->throttlegroups[idx];
+}
+
+
+static int
+virDomainDefThrottleGroupsParse(virDomainDef *def,
+ xmlXPathContextPtr ctxt)
+{
+ size_t i;
+ int n = 0;
+ g_autofree xmlNodePtr *nodes = NULL;
+
+ if ((n = virXPathNodeSet("./throttlegroups/throttlegroup", ctxt, &nodes)) < 0)
+ return -1;
+
+ if (n)
+ def->throttlegroups = g_new0(virDomainThrottleGroupDef *, n);
+
+ for (i = 0; i < n; i++) {
+ g_autoptr(virDomainThrottleGroupDef) group = NULL;
+
+ if (!(group = virDomainThrottleGroupDefParseXML(nodes[i], ctxt))) {
+ return -1;
+ }
+
+ if (virDomainThrottleGroupFind(def, group->group_name)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("duplicate group name '%1$s' found"),
+ group->group_name);
+ return -1;
+ }
+ def->throttlegroups[def->nthrottlegroups++] = g_steal_pointer(&group);
+ }
+ return 0;
+}
+
+
static int
virDomainDiskDefMirrorParse(virDomainDiskDef *def,
xmlNodePtr cur,
@@ -18880,6 +19015,9 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
if (virDomainDefParseBootOptions(def, ctxt, xmlopt, flags) < 0)
return NULL;
+ if (virDomainDefThrottleGroupsParse(def, ctxt) < 0)
+ return NULL;
+
/* analysis of the disk devices */
if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0)
return NULL;
@@ -22090,6 +22228,88 @@ virDomainIOThreadIDDel(virDomainDef *def,
}
+virDomainThrottleGroupDef *
+virDomainThrottleGroupFind(const virDomainDef *def,
+ const char *name)
+{
+ size_t i;
+
+ if (!def->throttlegroups || def->nthrottlegroups == 0)
+ return NULL;
+
+ for (i = 0; i < def->nthrottlegroups; i++) {
+ if (STREQ(name, def->throttlegroups[i]->group_name))
+ return def->throttlegroups[i];
+ }
+
+ return NULL;
+}
+
+
+void
+virDomainThrottleGroupDefCopy(const virDomainThrottleGroupDef *src,
+ virDomainThrottleGroupDef *dst)
+{
+ *dst = *src;
+ dst->group_name = g_strdup(src->group_name);
+}
+
+
+virDomainThrottleGroupDef *
+virDomainThrottleGroupAdd(virDomainDef *def,
+ virDomainThrottleGroupDef *throttle_group)
+{
+ virDomainThrottleGroupDef * new_group = g_new0(virDomainThrottleGroupDef, 1);
+ virDomainThrottleGroupDefCopy(throttle_group, new_group);
+ VIR_APPEND_ELEMENT_COPY(def->throttlegroups, def->nthrottlegroups, new_group);
+ return new_group;
+}
+
+
+/**
+ * virDomainThrottleGroupUpdate:
+ * @def: domain definition
+ * @info: throttle group definition within domain
+ *
+ * search existing throttle group within domain definition
+ * by group_name and then overwrite it with @info,
+ * it's to update existing throttle group
+ */
+void
+virDomainThrottleGroupUpdate(virDomainDef *def,
+ virDomainThrottleGroupDef *info)
+{
+ size_t i;
+
+ if (!info->group_name)
+ return;
+
+ for (i = 0; i < def->nthrottlegroups; i++) {
+ virDomainThrottleGroupDef *t = def->throttlegroups[i];
+
+ if (STREQ_NULLABLE(t->group_name, info->group_name)) {
+ VIR_FREE(t->group_name);
+ virDomainThrottleGroupDefCopy(info, t);
+ }
+ }
+}
+
+
+void
+virDomainThrottleGroupDel(virDomainDef *def,
+ const char *name)
+{
+ size_t i;
+ for (i = 0; i < def->nthrottlegroups; i++) {
+ if (STREQ_NULLABLE(def->throttlegroups[i]->group_name, name)) {
+ virDomainThrottleGroupDefFree(def->throttlegroups[i]);
+ VIR_DELETE_ELEMENT(def->throttlegroups, i, def->nthrottlegroups);
+ return;
+ }
+ }
+}
+
+
static int
virDomainEventActionDefFormat(virBuffer *buf,
int type,
@@ -27174,6 +27394,65 @@ virDomainDefIOThreadsFormat(virBuffer *buf,
}
+#define FORMAT_THROTTLE_GROUP(val) \
+ if (group->val > 0) { \
+ virBufferAsprintf(&childBuf, "<" #val ">%llu</" #val ">\n", \
+ group->val); \
+ }
+
+
+static void
+virDomainThrottleGroupFormat(virBuffer *buf,
+ virDomainThrottleGroupDef *group)
+{
+ g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+
+ FORMAT_THROTTLE_GROUP(total_bytes_sec);
+ FORMAT_THROTTLE_GROUP(read_bytes_sec);
+ FORMAT_THROTTLE_GROUP(write_bytes_sec);
+ FORMAT_THROTTLE_GROUP(total_iops_sec);
+ FORMAT_THROTTLE_GROUP(read_iops_sec);
+ FORMAT_THROTTLE_GROUP(write_iops_sec);
+
+ FORMAT_THROTTLE_GROUP(total_bytes_sec_max);
+ FORMAT_THROTTLE_GROUP(read_bytes_sec_max);
+ FORMAT_THROTTLE_GROUP(write_bytes_sec_max);
+ FORMAT_THROTTLE_GROUP(total_iops_sec_max);
+ FORMAT_THROTTLE_GROUP(read_iops_sec_max);
+ FORMAT_THROTTLE_GROUP(write_iops_sec_max);
+ FORMAT_THROTTLE_GROUP(size_iops_sec);
+
+ virBufferEscapeString(&childBuf, "<group_name>%s</group_name>\n",
+ group->group_name);
+
+ FORMAT_THROTTLE_GROUP(total_bytes_sec_max_length);
+ FORMAT_THROTTLE_GROUP(read_bytes_sec_max_length);
+ FORMAT_THROTTLE_GROUP(write_bytes_sec_max_length);
+ FORMAT_THROTTLE_GROUP(total_iops_sec_max_length);
+ FORMAT_THROTTLE_GROUP(read_iops_sec_max_length);
+ FORMAT_THROTTLE_GROUP(write_iops_sec_max_length);
+
+ virXMLFormatElement(buf, "throttlegroup", NULL, &childBuf);
+}
+
+#undef FORMAT_THROTTLE_GROUP
+
+
+static void
+virDomainDefThrottleGroupsFormat(virBuffer *buf,
+ const virDomainDef *def)
+{
+ g_auto(virBuffer) childrenBuf = VIR_BUFFER_INIT_CHILD(buf);
+ ssize_t n;
+
+ for (n = 0; n < def->nthrottlegroups; n++) {
+ virDomainThrottleGroupFormat(&childrenBuf, def->throttlegroups[n]);
+ }
+
+ virXMLFormatElement(buf, "throttlegroups", NULL, &childrenBuf);
+}
+
+
static void
virDomainIOMMUDefFormat(virBuffer *buf,
const virDomainIOMMUDef *iommu)
@@ -27839,6 +28118,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
virDomainDefIOThreadsFormat(buf, def);
+ virDomainDefThrottleGroupsFormat(buf, def);
+
if (virDomainCputuneDefFormat(buf, def, flags) < 0)
return -1;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a06f015444..c9e3fcd924 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3000,6 +3000,9 @@ struct _virDomainDef {
virDomainDefaultIOThreadDef *defaultIOThread;
+ size_t nthrottlegroups;
+ virDomainThrottleGroupDef **throttlegroups;
+
virDomainCputune cputune;
virDomainResctrlDef **resctrls;
@@ -4512,3 +4515,31 @@ virDomainObjGetMessages(virDomainObj *vm,
bool
virDomainDefHasSpiceGraphics(const virDomainDef *def);
+
+void
+virDomainThrottleGroupDefFree(virDomainThrottleGroupDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainThrottleGroupDef, virDomainThrottleGroupDefFree);
+
+virDomainThrottleGroupDef *
+virDomainThrottleGroupAdd(virDomainDef *def,
+ virDomainThrottleGroupDef *throttle_group);
+
+void
+virDomainThrottleGroupUpdate(virDomainDef *def,
+ virDomainThrottleGroupDef *info);
+
+virDomainThrottleGroupDef *
+virDomainThrottleGroupFind(const virDomainDef *def,
+ const char *name);
+
+void
+virDomainThrottleGroupDel(virDomainDef *def,
+ const char *name);
+
+virDomainThrottleGroupDef *
+virDomainThrottleGroupByName(virDomainDef *def,
+ const char *name);
+
+void
+virDomainThrottleGroupDefCopy(const virDomainThrottleGroupDef *src,
+ virDomainThrottleGroupDef *dst);
diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
index 0779bc224b..d51e8f5f40 100644
--- a/src/conf/virconftypes.h
+++ b/src/conf/virconftypes.h
@@ -80,6 +80,8 @@ typedef struct _virDomainBlkiotune virDomainBlkiotune;
typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo;
+typedef struct _virDomainBlockIoTuneInfo virDomainThrottleGroupDef;
+
typedef struct _virDomainCheckpointDef virDomainCheckpointDef;
typedef struct _virDomainCheckpointObj virDomainCheckpointObj;
--
2.34.1On Wed, Jun 12, 2024 at 03:02:11 -0700, wucf@linux.ibm.com wrote:
> From: Chun Feng Wu <wucf@linux.ibm.com>
>
> * Define new struct 'virDomainThrottleGroupDef' and corresponding destructor
> * Add operations(Add, Update, Del, Find, Copy, Free) for 'virDomainThrottleGroupDef'
> * Update _virDomainDef to include virDomainThrottleGroupDef
> * Support new resource "Parse" and "Format" for operations between struct and DOM XML
> * Make sure "group_name" is defined in xml
>
> Signed-off-by: Chun Feng Wu <wucf@linux.ibm.com>
> ---
> src/conf/domain_conf.c | 281 ++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 31 +++++
> src/conf/virconftypes.h | 2 +
> 3 files changed, 314 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index fde594f811..05d6f7ad3a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3750,6 +3750,32 @@ virDomainIOThreadIDDefArrayInit(virDomainDef *def,
> }
>
>
> +void
> +virDomainThrottleGroupDefFree(virDomainThrottleGroupDef *def)
> +{
> + if (!def)
> + return;
> + g_free(def->group_name);
> + g_free(def);
> +}
> +
> +
> +static void
> +virDomainThrottleGroupDefArrayFree(virDomainThrottleGroupDef **def,
> + int nthrottlegroups)
> +{
> + size_t i;
> +
> + if (!def)
> + return;
> +
> + for (i = 0; i < nthrottlegroups; i++)
> + virDomainThrottleGroupDefFree(def[i]);
> +
> + g_free(def);
> +}
> +
> +
> void
> virDomainResourceDefFree(virDomainResourceDef *resource)
> {
> @@ -4029,6 +4055,8 @@ void virDomainDefFree(virDomainDef *def)
>
> virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids);
>
> + virDomainThrottleGroupDefArrayFree(def->throttlegroups, def->nthrottlegroups);
> +
> g_free(def->defaultIOThread);
>
> virBitmapFree(def->cputune.emulatorpin);
> @@ -7697,6 +7725,113 @@ virDomainDiskDefIotuneParse(virDomainDiskDef *def,
> #undef PARSE_IOTUNE
>
>
> +#define PARSE_THROTTLEGROUP(val) \
> + if (virXPathULongLong("string(./" #val ")", \
> + ctxt, &group->val) == -2) { \
This can also return -1, in which case you also want to at least return
NULL, but for simplicity reporting the error as below is okay.
> + virReportError(VIR_ERR_XML_ERROR, \
> + _("throttle group field '%1$s' must be an integer"), #val); \
> + return NULL; \
> + }
> +
> +
> +static virDomainThrottleGroupDef *
> +virDomainThrottleGroupDefParseXML(xmlNodePtr node,
> + xmlXPathContextPtr ctxt)
> +{
> + g_autoptr(virDomainThrottleGroupDef) group = g_new0(virDomainThrottleGroupDef, 1);
> +
> + VIR_XPATH_NODE_AUTORESTORE(ctxt)
> + ctxt->node = node;
> +
> + PARSE_THROTTLEGROUP(total_bytes_sec);
> + PARSE_THROTTLEGROUP(read_bytes_sec);
> + PARSE_THROTTLEGROUP(write_bytes_sec);
> + PARSE_THROTTLEGROUP(total_iops_sec);
> + PARSE_THROTTLEGROUP(read_iops_sec);
> + PARSE_THROTTLEGROUP(write_iops_sec);
> +
> + PARSE_THROTTLEGROUP(total_bytes_sec_max);
> + PARSE_THROTTLEGROUP(read_bytes_sec_max);
> + PARSE_THROTTLEGROUP(write_bytes_sec_max);
> + PARSE_THROTTLEGROUP(total_iops_sec_max);
> + PARSE_THROTTLEGROUP(read_iops_sec_max);
> + PARSE_THROTTLEGROUP(write_iops_sec_max);
> +
> + PARSE_THROTTLEGROUP(size_iops_sec);
> +
> + PARSE_THROTTLEGROUP(total_bytes_sec_max_length);
> + PARSE_THROTTLEGROUP(read_bytes_sec_max_length);
> + PARSE_THROTTLEGROUP(write_bytes_sec_max_length);
> + PARSE_THROTTLEGROUP(total_iops_sec_max_length);
> + PARSE_THROTTLEGROUP(read_iops_sec_max_length);
> + PARSE_THROTTLEGROUP(write_iops_sec_max_length);
I'd prefer if we had only one copy of the code parsing this and the
equivalent disk throttling data so that the code doesn't need to be
duplicated. The cleanup can be done as a followup though.
> +
> + /* group_name is required */
> + if (!(group->group_name = virXPathString("string(./group_name)", ctxt))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing group name"));
> + return NULL;
> + }
> +
> + return g_steal_pointer(&group);
> +}
> +#undef PARSE_THROTTLEGROUP
> +
> +
> +virDomainThrottleGroupDef *
> +virDomainThrottleGroupByName(virDomainDef *def,
> + const char *name)
> +{
> + virDomainThrottleGroupDef *tgroup;
> + size_t i;
> + int idx = -1;
> +
> + for (i = 0; i < def->nthrottlegroups; i++) {
> + tgroup = def->throttlegroups[i];
> + if (STREQ(tgroup->group_name, name))
> + idx = i;
> + }
> +
> + if (idx < 0)
> + return NULL;
> +
> + return def->throttlegroups[idx];
> +}
> +
> +
> +static int
> +virDomainDefThrottleGroupsParse(virDomainDef *def,
> + xmlXPathContextPtr ctxt)
> +{
> + size_t i;
> + int n = 0;
> + g_autofree xmlNodePtr *nodes = NULL;
> +
> + if ((n = virXPathNodeSet("./throttlegroups/throttlegroup", ctxt, &nodes)) < 0)
> + return -1;
> +
> + if (n)
> + def->throttlegroups = g_new0(virDomainThrottleGroupDef *, n);
> +
> + for (i = 0; i < n; i++) {
> + g_autoptr(virDomainThrottleGroupDef) group = NULL;
> +
> + if (!(group = virDomainThrottleGroupDefParseXML(nodes[i], ctxt))) {
> + return -1;
> + }
> +
> + if (virDomainThrottleGroupFind(def, group->group_name)) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("duplicate group name '%1$s' found"),
> + group->group_name);
> + return -1;
> + }
> + def->throttlegroups[def->nthrottlegroups++] = g_steal_pointer(&group);
> + }
> + return 0;
> +}
> +
> +
> static int
> virDomainDiskDefMirrorParse(virDomainDiskDef *def,
> xmlNodePtr cur,
> @@ -18880,6 +19015,9 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
> if (virDomainDefParseBootOptions(def, ctxt, xmlopt, flags) < 0)
> return NULL;
>
> + if (virDomainDefThrottleGroupsParse(def, ctxt) < 0)
> + return NULL;
> +
> /* analysis of the disk devices */
> if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0)
> return NULL;
> @@ -22090,6 +22228,88 @@ virDomainIOThreadIDDel(virDomainDef *def,
> }
>
>
> +virDomainThrottleGroupDef *
> +virDomainThrottleGroupFind(const virDomainDef *def,
> + const char *name)
> +{
> + size_t i;
> +
> + if (!def->throttlegroups || def->nthrottlegroups == 0)
> + return NULL;
> +
> + for (i = 0; i < def->nthrottlegroups; i++) {
> + if (STREQ(name, def->throttlegroups[i]->group_name))
> + return def->throttlegroups[i];
> + }
> +
> + return NULL;
> +}
> +
> +
> +void
> +virDomainThrottleGroupDefCopy(const virDomainThrottleGroupDef *src,
> + virDomainThrottleGroupDef *dst)
> +{
> + *dst = *src;
> + dst->group_name = g_strdup(src->group_name);
> +}
> +
> +
> +virDomainThrottleGroupDef *
> +virDomainThrottleGroupAdd(virDomainDef *def,
> + virDomainThrottleGroupDef *throttle_group)
> +{
> + virDomainThrottleGroupDef * new_group = g_new0(virDomainThrottleGroupDef, 1);
> + virDomainThrottleGroupDefCopy(throttle_group, new_group);
> + VIR_APPEND_ELEMENT_COPY(def->throttlegroups, def->nthrottlegroups, new_group);
> + return new_group;
> +}
> +
> +
> +/**
> + * virDomainThrottleGroupUpdate:
> + * @def: domain definition
> + * @info: throttle group definition within domain
> + *
> + * search existing throttle group within domain definition
> + * by group_name and then overwrite it with @info,
> + * it's to update existing throttle group
> + */
> +void
> +virDomainThrottleGroupUpdate(virDomainDef *def,
> + virDomainThrottleGroupDef *info)
> +{
> + size_t i;
> +
> + if (!info->group_name)
> + return;
> +
> + for (i = 0; i < def->nthrottlegroups; i++) {
> + virDomainThrottleGroupDef *t = def->throttlegroups[i];
> +
> + if (STREQ_NULLABLE(t->group_name, info->group_name)) {
> + VIR_FREE(t->group_name);
> + virDomainThrottleGroupDefCopy(info, t);
> + }
> + }
> +}
> +
> +
Please add docs for any function that is being exported.
> +void
> +virDomainThrottleGroupDel(virDomainDef *def,
> + const char *name)
> +{
> + size_t i;
> + for (i = 0; i < def->nthrottlegroups; i++) {
> + if (STREQ_NULLABLE(def->throttlegroups[i]->group_name, name)) {
> + virDomainThrottleGroupDefFree(def->throttlegroups[i]);
> + VIR_DELETE_ELEMENT(def->throttlegroups, i, def->nthrottlegroups);
> + return;
> + }
> + }
> +}
> +
> +
> static int
> virDomainEventActionDefFormat(virBuffer *buf,
> int type,
> @@ -27174,6 +27394,65 @@ virDomainDefIOThreadsFormat(virBuffer *buf,
> }
>
>
> +#define FORMAT_THROTTLE_GROUP(val) \
> + if (group->val > 0) { \
> + virBufferAsprintf(&childBuf, "<" #val ">%llu</" #val ">\n", \
> + group->val); \
> + }
> +
> +
> +static void
> +virDomainThrottleGroupFormat(virBuffer *buf,
> + virDomainThrottleGroupDef *group)
> +{
> + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
> +
> + FORMAT_THROTTLE_GROUP(total_bytes_sec);
> + FORMAT_THROTTLE_GROUP(read_bytes_sec);
> + FORMAT_THROTTLE_GROUP(write_bytes_sec);
> + FORMAT_THROTTLE_GROUP(total_iops_sec);
> + FORMAT_THROTTLE_GROUP(read_iops_sec);
> + FORMAT_THROTTLE_GROUP(write_iops_sec);
> +
> + FORMAT_THROTTLE_GROUP(total_bytes_sec_max);
> + FORMAT_THROTTLE_GROUP(read_bytes_sec_max);
> + FORMAT_THROTTLE_GROUP(write_bytes_sec_max);
> + FORMAT_THROTTLE_GROUP(total_iops_sec_max);
> + FORMAT_THROTTLE_GROUP(read_iops_sec_max);
> + FORMAT_THROTTLE_GROUP(write_iops_sec_max);
> + FORMAT_THROTTLE_GROUP(size_iops_sec);
> +
> + virBufferEscapeString(&childBuf, "<group_name>%s</group_name>\n",
> + group->group_name);
> +
> + FORMAT_THROTTLE_GROUP(total_bytes_sec_max_length);
> + FORMAT_THROTTLE_GROUP(read_bytes_sec_max_length);
> + FORMAT_THROTTLE_GROUP(write_bytes_sec_max_length);
> + FORMAT_THROTTLE_GROUP(total_iops_sec_max_length);
> + FORMAT_THROTTLE_GROUP(read_iops_sec_max_length);
> + FORMAT_THROTTLE_GROUP(write_iops_sec_max_length);
> +
> + virXMLFormatElement(buf, "throttlegroup", NULL, &childBuf);
> +}
> +
> +#undef FORMAT_THROTTLE_GROUP
> +
> +
> +static void
> +virDomainDefThrottleGroupsFormat(virBuffer *buf,
> + const virDomainDef *def)
> +{
> + g_auto(virBuffer) childrenBuf = VIR_BUFFER_INIT_CHILD(buf);
> + ssize_t n;
> +
> + for (n = 0; n < def->nthrottlegroups; n++) {
> + virDomainThrottleGroupFormat(&childrenBuf, def->throttlegroups[n]);
> + }
> +
> + virXMLFormatElement(buf, "throttlegroups", NULL, &childrenBuf);
> +}
> +
> +
> static void
> virDomainIOMMUDefFormat(virBuffer *buf,
> const virDomainIOMMUDef *iommu)
> @@ -27839,6 +28118,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
>
> virDomainDefIOThreadsFormat(buf, def);
>
> + virDomainDefThrottleGroupsFormat(buf, def);
> +
> if (virDomainCputuneDefFormat(buf, def, flags) < 0)
> return -1;
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index a06f015444..c9e3fcd924 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3000,6 +3000,9 @@ struct _virDomainDef {
>
> virDomainDefaultIOThreadDef *defaultIOThread;
>
> + size_t nthrottlegroups;
> + virDomainThrottleGroupDef **throttlegroups;
> +
> virDomainCputune cputune;
>
> virDomainResctrlDef **resctrls;
> @@ -4512,3 +4515,31 @@ virDomainObjGetMessages(virDomainObj *vm,
>
> bool
> virDomainDefHasSpiceGraphics(const virDomainDef *def);
> +
> +void
> +virDomainThrottleGroupDefFree(virDomainThrottleGroupDef *def);
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainThrottleGroupDef, virDomainThrottleGroupDefFree);
> +
> +virDomainThrottleGroupDef *
> +virDomainThrottleGroupAdd(virDomainDef *def,
> + virDomainThrottleGroupDef *throttle_group);
> +
> +void
> +virDomainThrottleGroupUpdate(virDomainDef *def,
> + virDomainThrottleGroupDef *info);
> +
> +virDomainThrottleGroupDef *
> +virDomainThrottleGroupFind(const virDomainDef *def,
> + const char *name);
> +
> +void
> +virDomainThrottleGroupDel(virDomainDef *def,
> + const char *name);
> +
> +virDomainThrottleGroupDef *
> +virDomainThrottleGroupByName(virDomainDef *def,
> + const char *name);
> +
> +void
> +virDomainThrottleGroupDefCopy(const virDomainThrottleGroupDef *src,
> + virDomainThrottleGroupDef *dst);
> diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
> index 0779bc224b..d51e8f5f40 100644
> --- a/src/conf/virconftypes.h
> +++ b/src/conf/virconftypes.h
> @@ -80,6 +80,8 @@ typedef struct _virDomainBlkiotune virDomainBlkiotune;
>
> typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo;
>
> +typedef struct _virDomainBlockIoTuneInfo virDomainThrottleGroupDef;
> +
> typedef struct _virDomainCheckpointDef virDomainCheckpointDef;
>
> typedef struct _virDomainCheckpointObj virDomainCheckpointObj;
> --
> 2.34.1
>
On 2024/7/26 21:03, Peter Krempa wrote:
>> +static virDomainThrottleGroupDef *
>> +virDomainThrottleGroupDefParseXML(xmlNodePtr node,
>> + xmlXPathContextPtr ctxt)
>> +{
>> + g_autoptr(virDomainThrottleGroupDef) group = g_new0(virDomainThrottleGroupDef, 1);
>> +
>> + VIR_XPATH_NODE_AUTORESTORE(ctxt)
>> + ctxt->node = node;
>> +
>> + PARSE_THROTTLEGROUP(total_bytes_sec);
>> + PARSE_THROTTLEGROUP(read_bytes_sec);
>> + PARSE_THROTTLEGROUP(write_bytes_sec);
>> + PARSE_THROTTLEGROUP(total_iops_sec);
>> + PARSE_THROTTLEGROUP(read_iops_sec);
>> + PARSE_THROTTLEGROUP(write_iops_sec);
>> +
>> + PARSE_THROTTLEGROUP(total_bytes_sec_max);
>> + PARSE_THROTTLEGROUP(read_bytes_sec_max);
>> + PARSE_THROTTLEGROUP(write_bytes_sec_max);
>> + PARSE_THROTTLEGROUP(total_iops_sec_max);
>> + PARSE_THROTTLEGROUP(read_iops_sec_max);
>> + PARSE_THROTTLEGROUP(write_iops_sec_max);
>> +
>> + PARSE_THROTTLEGROUP(size_iops_sec);
>> +
>> + PARSE_THROTTLEGROUP(total_bytes_sec_max_length);
>> + PARSE_THROTTLEGROUP(read_bytes_sec_max_length);
>> + PARSE_THROTTLEGROUP(write_bytes_sec_max_length);
>> + PARSE_THROTTLEGROUP(total_iops_sec_max_length);
>> + PARSE_THROTTLEGROUP(read_iops_sec_max_length);
>> + PARSE_THROTTLEGROUP(write_iops_sec_max_length);
> I'd prefer if we had only one copy of the code parsing this and the
> equivalent disk throttling data so that the code doesn't need to be
> duplicated. The cleanup can be done as a followup though.
in v4 drafting, I am defining the following common macro to avoid
duplication of iterating all options in all parsing and formatting codes
for both iotune and throttlegroup:
#define FOR_EACH_IOTUNE_ULL_OPTION(process) \
process(total_bytes_sec) \
process(read_bytes_sec) \
process(write_bytes_sec) \
process(total_iops_sec) \
process(read_iops_sec) \
process(write_iops_sec) \
process(total_bytes_sec_max) \
process(read_bytes_sec_max) \
process(write_bytes_sec_max) \
process(total_iops_sec_max) \
process(read_iops_sec_max) \
process(write_iops_sec_max) \
process(size_iops_sec) \
process(total_bytes_sec_max_length) \
process(read_bytes_sec_max_length) \
process(write_bytes_sec_max_length) \
process(total_iops_sec_max_length) \
process(read_iops_sec_max_length) \
process(write_iops_sec_max_length)
and define different "process" macro for parse and format, and this can
be reused by iotune and format by just calling e.g.
FOR_EACH_IOTUNE_ULL_OPTION(PARSE_IOTUNE);
FOR_EACH_IOTUNE_ULL_OPTION(FORMAT_IOTUNE);
--
Thanks and Regards,
Wu
On Fri, Aug 23, 2024 at 14:00:25 +0800, Chun Feng Wu wrote:
>
> On 2024/7/26 21:03, Peter Krempa wrote:
> > > +static virDomainThrottleGroupDef *
> > > +virDomainThrottleGroupDefParseXML(xmlNodePtr node,
> > > + xmlXPathContextPtr ctxt)
> > > +{
> > > + g_autoptr(virDomainThrottleGroupDef) group = g_new0(virDomainThrottleGroupDef, 1);
> > > +
> > > + VIR_XPATH_NODE_AUTORESTORE(ctxt)
> > > + ctxt->node = node;
> > > +
> > > + PARSE_THROTTLEGROUP(total_bytes_sec);
> > > + PARSE_THROTTLEGROUP(read_bytes_sec);
> > > + PARSE_THROTTLEGROUP(write_bytes_sec);
> > > + PARSE_THROTTLEGROUP(total_iops_sec);
> > > + PARSE_THROTTLEGROUP(read_iops_sec);
> > > + PARSE_THROTTLEGROUP(write_iops_sec);
> > > +
> > > + PARSE_THROTTLEGROUP(total_bytes_sec_max);
> > > + PARSE_THROTTLEGROUP(read_bytes_sec_max);
> > > + PARSE_THROTTLEGROUP(write_bytes_sec_max);
> > > + PARSE_THROTTLEGROUP(total_iops_sec_max);
> > > + PARSE_THROTTLEGROUP(read_iops_sec_max);
> > > + PARSE_THROTTLEGROUP(write_iops_sec_max);
> > > +
> > > + PARSE_THROTTLEGROUP(size_iops_sec);
> > > +
> > > + PARSE_THROTTLEGROUP(total_bytes_sec_max_length);
> > > + PARSE_THROTTLEGROUP(read_bytes_sec_max_length);
> > > + PARSE_THROTTLEGROUP(write_bytes_sec_max_length);
> > > + PARSE_THROTTLEGROUP(total_iops_sec_max_length);
> > > + PARSE_THROTTLEGROUP(read_iops_sec_max_length);
> > > + PARSE_THROTTLEGROUP(write_iops_sec_max_length);
> > I'd prefer if we had only one copy of the code parsing this and the
> > equivalent disk throttling data so that the code doesn't need to be
> > duplicated. The cleanup can be done as a followup though.
>
> in v4 drafting, I am defining the following common macro to avoid
> duplication of iterating all options in all parsing and formatting codes for
> both iotune and throttlegroup:
I was referring to the fact that the code that parses the old throttling
XML is not refactored to use this new helper, thus keeping duplications.
>
> #define FOR_EACH_IOTUNE_ULL_OPTION(process) \
> process(total_bytes_sec) \
> process(read_bytes_sec) \
> process(write_bytes_sec) \
> process(total_iops_sec) \
> process(read_iops_sec) \
> process(write_iops_sec) \
> process(total_bytes_sec_max) \
> process(read_bytes_sec_max) \
> process(write_bytes_sec_max) \
> process(total_iops_sec_max) \
> process(read_iops_sec_max) \
> process(write_iops_sec_max) \
> process(size_iops_sec) \
> process(total_bytes_sec_max_length) \
> process(read_bytes_sec_max_length) \
> process(write_bytes_sec_max_length) \
> process(total_iops_sec_max_length) \
> process(read_iops_sec_max_length) \
> process(write_iops_sec_max_length)
>
> and define different "process" macro for parse and format, and this can be
> reused by iotune and format by just calling e.g.
>
> FOR_EACH_IOTUNE_ULL_OPTION(PARSE_IOTUNE);
>
> FOR_EACH_IOTUNE_ULL_OPTION(FORMAT_IOTUNE);
Please do not do this. Do not hide this any deeper into macros. It's
hard to understand and harder to debug.
On 2024/8/23 15:10, Peter Krempa wrote:
> On Fri, Aug 23, 2024 at 14:00:25 +0800, Chun Feng Wu wrote:
>> On 2024/7/26 21:03, Peter Krempa wrote:
>>>> +static virDomainThrottleGroupDef *
>>>> +virDomainThrottleGroupDefParseXML(xmlNodePtr node,
>>>> + xmlXPathContextPtr ctxt)
>>>> +{
>>>> + g_autoptr(virDomainThrottleGroupDef) group = g_new0(virDomainThrottleGroupDef, 1);
>>>> +
>>>> + VIR_XPATH_NODE_AUTORESTORE(ctxt)
>>>> + ctxt->node = node;
>>>> +
>>>> + PARSE_THROTTLEGROUP(total_bytes_sec);
>>>> + PARSE_THROTTLEGROUP(read_bytes_sec);
>>>> + PARSE_THROTTLEGROUP(write_bytes_sec);
>>>> + PARSE_THROTTLEGROUP(total_iops_sec);
>>>> + PARSE_THROTTLEGROUP(read_iops_sec);
>>>> + PARSE_THROTTLEGROUP(write_iops_sec);
>>>> +
>>>> + PARSE_THROTTLEGROUP(total_bytes_sec_max);
>>>> + PARSE_THROTTLEGROUP(read_bytes_sec_max);
>>>> + PARSE_THROTTLEGROUP(write_bytes_sec_max);
>>>> + PARSE_THROTTLEGROUP(total_iops_sec_max);
>>>> + PARSE_THROTTLEGROUP(read_iops_sec_max);
>>>> + PARSE_THROTTLEGROUP(write_iops_sec_max);
>>>> +
>>>> + PARSE_THROTTLEGROUP(size_iops_sec);
>>>> +
>>>> + PARSE_THROTTLEGROUP(total_bytes_sec_max_length);
>>>> + PARSE_THROTTLEGROUP(read_bytes_sec_max_length);
>>>> + PARSE_THROTTLEGROUP(write_bytes_sec_max_length);
>>>> + PARSE_THROTTLEGROUP(total_iops_sec_max_length);
>>>> + PARSE_THROTTLEGROUP(read_iops_sec_max_length);
>>>> + PARSE_THROTTLEGROUP(write_iops_sec_max_length);
>>> I'd prefer if we had only one copy of the code parsing this and the
>>> equivalent disk throttling data so that the code doesn't need to be
>>> duplicated. The cleanup can be done as a followup though.
>> in v4 drafting, I am defining the following common macro to avoid
>> duplication of iterating all options in all parsing and formatting codes for
>> both iotune and throttlegroup:
> I was referring to the fact that the code that parses the old throttling
> XML is not refactored to use this new helper, thus keeping duplications.
>
>> #define FOR_EACH_IOTUNE_ULL_OPTION(process) \
>> process(total_bytes_sec) \
>> process(read_bytes_sec) \
>> process(write_bytes_sec) \
>> process(total_iops_sec) \
>> process(read_iops_sec) \
>> process(write_iops_sec) \
>> process(total_bytes_sec_max) \
>> process(read_bytes_sec_max) \
>> process(write_bytes_sec_max) \
>> process(total_iops_sec_max) \
>> process(read_iops_sec_max) \
>> process(write_iops_sec_max) \
>> process(size_iops_sec) \
>> process(total_bytes_sec_max_length) \
>> process(read_bytes_sec_max_length) \
>> process(write_bytes_sec_max_length) \
>> process(total_iops_sec_max_length) \
>> process(read_iops_sec_max_length) \
>> process(write_iops_sec_max_length)
>>
>> and define different "process" macro for parse and format, and this can be
>> reused by iotune and format by just calling e.g.
>>
>> FOR_EACH_IOTUNE_ULL_OPTION(PARSE_IOTUNE);
>>
>> FOR_EACH_IOTUNE_ULL_OPTION(FORMAT_IOTUNE);
> Please do not do this. Do not hide this any deeper into macros. It's
> hard to understand and harder to debug.
got it, thanks for your info!
--
Thanks and Regards,
Wu
© 2016 - 2024 Red Hat, Inc.