On Thu, Apr 11, 2024 at 19:01:52 -0700, wucf(a)linux.ibm.com wrote:
From: Chun Feng Wu <wucf(a)linux.ibm.com>
Implement the following methods:
* virDomainSetThrottleGroup
* virDomainGetThrottleGroup
* virDomainDelThrottleGroup
Similarly to previous patch, note how you've done this rather than what
you've done, which is visible from the diff.
Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com>
---
As noted in previous patch this really needs to go after the XML and
qemu commandline bits.
src/qemu/qemu_domain.c | 14 ++
src/qemu/qemu_domain.h | 4 +
src/qemu/qemu_driver.c | 523 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 541 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6b869797a8..b676f59c3a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10246,6 +10246,20 @@ qemuDomainDiskByName(virDomainDef *def,
}
+virDomainThrottleGroupDef *
+qemuDomainThrottleGroupByName(virDomainDef *def,
+ const char *name)
+{
+ virDomainThrottleGroupDef *ret;
+
+ if (!(ret = virDomainThrottleGroupByName(def, name))) {
+ return NULL;
This wrapper is really pointless. Use virDomainThrottleGroupByName
directly.
+ }
+
+ return ret;
+}
+
+
int
qemuDomainPrepareChannel(virDomainChrDef *channel,
const char *domainChannelTargetDir)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d01f788aea..da0f9590db 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19995,6 +19995,526 @@ qemuDomainGraphicsReload(virDomainPtr domain,
return ret;
}
+
+/* wrapper of qemuDomainSetBlockIoTuneDefaults for throttle group since they use the
same data structure */
+static int
+qemuDomainSetThrottleGroupDefaults(virDomainBlockIoTuneInfo *newinfo,
+ virDomainBlockIoTuneInfo *oldinfo,
+ qemuBlockIoTuneSetFlags set_fields)
+{
+ return qemuDomainSetBlockIoTuneDefaults(newinfo, oldinfo, set_fields);
+}
Same here, this is a pointless wrapper.
+
+
+static int
+qemuDomainCheckThrottleGroupReset(const char *groupname,
+ virDomainBlockIoTuneInfo *newiotune)
+{
+ if (virDomainBlockIoTuneInfoHasAny(newiotune))
+ return 0;
+
+ if (newiotune->group_name &&
+ STRNEQ_NULLABLE(newiotune->group_name, groupname)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("creating a new group/updating existing with all
parameters zero is not supported"));
+ return -1;
+ }
+
+ /* all zero means remove any throttling and remove from group for qemu */
+ VIR_FREE(newiotune->group_name);
It's not quite obvious what this function is supposed to do and on the
first glance it looks like it's a set of pre-checks. As of such you'll
be better of doing this set of checks
+
+ return 0;
+}
+
+
+static int
+qemuDomainSetThrottleGroup(virDomainPtr dom,
+ const char *groupname,
+ virTypedParameterPtr params,
+ int nparams,
+ unsigned int flags)
+{
+ virQEMUDriver *driver = dom->conn->privateData;
+ virDomainObj *vm = NULL;
+ virDomainDef *def = NULL;
+ virDomainDef *persistentDef = NULL;
+ virDomainThrottleGroupDef info = { 0 };
+ virDomainThrottleGroupDef conf_info = { 0 };
+ int ret = -1;
+ size_t i;
+ qemuBlockIoTuneSetFlags set_fields = 0;
+ g_autoptr(virQEMUDriverConfig) cfg = NULL;
+ virObjectEvent *event = NULL;
+ virTypedParameterPtr eventParams = NULL;
+ int eventNparams = 0;
+ int eventMaxparams = 0;
+ virDomainThrottleGroupDef *cur_info;
+ virDomainThrottleGroupDef *conf_cur_info;
+ int rc = 0;
+ g_autoptr(virJSONValue) props = NULL;
+ g_autoptr(virJSONValue) limits = virJSONValueNewObject();
+
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+ if (virTypedParamsValidate(params, nparams,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME,
+ VIR_TYPED_PARAM_STRING,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ NULL) < 0)
+ return -1;
+
+ if (!(vm = qemuDomainObjFromDomain(dom)))
+ return -1;
+
+ if (virDomainSetThrottleGroupEnsureACL(dom->conn, vm->def, flags) < 0)
+ goto cleanup;
+
+ cfg = virQEMUDriverGetConfig(driver);
+
+ if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+ goto endjob;
+
+ if (virTypedParamsAddString(&eventParams, &eventNparams,
&eventMaxparams,
+ VIR_DOMAIN_THROTTLE_GROUP, groupname) < 0)
+ goto endjob;
So what if the string is already there? It is allowed above.
+
+#define SET_THROTTLE_FIELD(FIELD, BOOL, CONST) \
+ if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_##CONST)) { \
+ info.FIELD = param->value.ul; \
+ set_fields |= QEMU_BLOCK_IOTUNE_SET_##BOOL; \
+ if (virTypedParamsAddULLong(&eventParams, &eventNparams, \
+ &eventMaxparams, \
+ VIR_DOMAIN_TUNABLE_BLKDEV_##CONST, \
+ param->value.ul) < 0) \
+ goto endjob; \
+ continue; \
+ }
+
+ for (i = 0; i < nparams; i++) {
+ virTypedParameterPtr param = ¶ms[i];
+
+ if (param->value.ul > QEMU_BLOCK_IOTUNE_MAX) {
+ virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+ _("throttle group value must be no more than
%1$llu"),
+ QEMU_BLOCK_IOTUNE_MAX);
+ goto endjob;
+ }
+
+ SET_THROTTLE_FIELD(total_bytes_sec, BYTES, TOTAL_BYTES_SEC);
+ SET_THROTTLE_FIELD(read_bytes_sec, BYTES, READ_BYTES_SEC);
+ SET_THROTTLE_FIELD(write_bytes_sec, BYTES, WRITE_BYTES_SEC);
+ SET_THROTTLE_FIELD(total_iops_sec, IOPS, TOTAL_IOPS_SEC);
+ SET_THROTTLE_FIELD(read_iops_sec, IOPS, READ_IOPS_SEC);
+ SET_THROTTLE_FIELD(write_iops_sec, IOPS, WRITE_IOPS_SEC);
+
+ SET_THROTTLE_FIELD(total_bytes_sec_max, BYTES_MAX,
+ TOTAL_BYTES_SEC_MAX);
+ SET_THROTTLE_FIELD(read_bytes_sec_max, BYTES_MAX,
+ READ_BYTES_SEC_MAX);
+ SET_THROTTLE_FIELD(write_bytes_sec_max, BYTES_MAX,
+ WRITE_BYTES_SEC_MAX);
+ SET_THROTTLE_FIELD(total_iops_sec_max, IOPS_MAX,
+ TOTAL_IOPS_SEC_MAX);
+ SET_THROTTLE_FIELD(read_iops_sec_max, IOPS_MAX,
+ READ_IOPS_SEC_MAX);
+ SET_THROTTLE_FIELD(write_iops_sec_max, IOPS_MAX,
+ WRITE_IOPS_SEC_MAX);
+ SET_THROTTLE_FIELD(size_iops_sec, SIZE_IOPS, SIZE_IOPS_SEC);
+
+ if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME)) {
+ info.group_name = g_strdup(param->value.s);
+ set_fields |= QEMU_BLOCK_IOTUNE_SET_GROUP_NAME;
+ if (virTypedParamsAddString(&eventParams, &eventNparams,
+ &eventMaxparams,
+ VIR_DOMAIN_TUNABLE_BLKDEV_GROUP_NAME,
+ param->value.s) < 0)
+ goto endjob;
+ continue;
+ }
+
+ SET_THROTTLE_FIELD(total_bytes_sec_max_length, BYTES_MAX_LENGTH,
+ TOTAL_BYTES_SEC_MAX_LENGTH);
+ SET_THROTTLE_FIELD(read_bytes_sec_max_length, BYTES_MAX_LENGTH,
+ READ_BYTES_SEC_MAX_LENGTH);
+ SET_THROTTLE_FIELD(write_bytes_sec_max_length, BYTES_MAX_LENGTH,
+ WRITE_BYTES_SEC_MAX_LENGTH);
+ SET_THROTTLE_FIELD(total_iops_sec_max_length, IOPS_MAX_LENGTH,
+ TOTAL_IOPS_SEC_MAX_LENGTH);
+ SET_THROTTLE_FIELD(read_iops_sec_max_length, IOPS_MAX_LENGTH,
+ READ_IOPS_SEC_MAX_LENGTH);
+ SET_THROTTLE_FIELD(write_iops_sec_max_length, IOPS_MAX_LENGTH,
+ WRITE_IOPS_SEC_MAX_LENGTH);
+ }
+
+#undef SET_THROTTLE_FIELD
+
+ if ((info.total_bytes_sec && info.read_bytes_sec) ||
+ (info.total_bytes_sec && info.write_bytes_sec)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("total and read/write of bytes_sec cannot be set at the
same time"));
+ goto endjob;
+ }
+
+ if ((info.total_iops_sec && info.read_iops_sec) ||
+ (info.total_iops_sec && info.write_iops_sec)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("total and read/write of iops_sec cannot be set at the
same time"));
+ goto endjob;
+ }
+
+ if ((info.total_bytes_sec_max && info.read_bytes_sec_max) ||
+ (info.total_bytes_sec_max && info.write_bytes_sec_max)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("total and read/write of bytes_sec_max cannot be set at
the same time"));
+ goto endjob;
+ }
+
+ if ((info.total_iops_sec_max && info.read_iops_sec_max) ||
+ (info.total_iops_sec_max && info.write_iops_sec_max)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("total and read/write of iops_sec_max cannot be set at the
same time"));
+ goto endjob;
+ }
+
+ virDomainThrottleGroupDefCopy(&info, &conf_info);
+
+ if (def) {
+ if (qemuDomainCheckThrottleGroupReset(groupname, &info) < 0)
+ goto endjob;
+
+#define CHECK_MAX(val, _bool) \
+ do { \
+ if (info.val##_max) { \
+ if (!info.val) { \
+ if (QEMU_BLOCK_IOTUNE_SET_##_bool) { \
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
+ _("cannot reset '%1$s' when
'%2$s' is set"), \
+ #val, #val "_max"); \
+ } else { \
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
+ _("value '%1$s' cannot be set if
'%2$s' is not set"), \
+ #val "_max", #val); \
+ } \
+ goto endjob; \
+ } \
+ if (info.val##_max < info.val) { \
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \
+ _("value '%1$s' cannot be smaller than
'%2$s'"), \
+ #val "_max", #val); \
+ goto endjob; \
+ } \
+ } \
+ } while (false)
+
+ CHECK_MAX(total_bytes_sec, BYTES);
+ CHECK_MAX(read_bytes_sec, BYTES);
+ CHECK_MAX(write_bytes_sec, BYTES);
+ CHECK_MAX(total_iops_sec, IOPS);
+ CHECK_MAX(read_iops_sec, IOPS);
+ CHECK_MAX(write_iops_sec, IOPS);
All of the above checks are duplicated from qemuDomainSetBlockIoTune.
Refactor them out first and reuse them rather than copy&paste.
+
+#undef CHECK_MAX
+
+ cur_info = qemuDomainThrottleGroupByName(def, groupname);
+ /* Update existing group. */
+ if (cur_info != NULL) {
+ if (qemuDomainSetThrottleGroupDefaults(&info, cur_info,
+ set_fields) < 0)
Spacing is off here ..
+ goto endjob;
+ qemuDomainObjEnterMonitor(vm);
+ rc = qemuMonitorUpdateThrottleGroup(qemuDomainGetMonitor(vm),
+ groupname,
+ &info);
... here ...
+ qemuDomainObjExitMonitor(vm);
+ if (rc < 0)
+ goto endjob;
+ virDomainThrottleGroupUpdate(def, &info);
+ }else{
... here ...
+ if (qemuMonitorThrottleGroupLimits(limits,
&info)<0)
+ goto endjob;
+ if (qemuMonitorCreateObjectProps(&props,
+ "throttle-group", groupname,
+ "a:limits", &limits,
... here ...
+ NULL) < 0)
+ goto endjob;
+ qemuDomainObjEnterMonitor(vm);
+ rc = qemuMonitorAddObject(qemuDomainGetMonitor(vm), &props, NULL);
+ qemuDomainObjExitMonitor(vm);
+ if (rc < 0)
+ goto endjob;
+ virDomainThrottleGroupAdd(def, &info);
+ }
+
+ qemuDomainSaveStatus(vm);
+
+ if (eventNparams) {
+ event = virDomainEventTunableNewFromDom(dom, &eventParams,
eventNparams);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ }
+ }
+
+ if (persistentDef) {
+ conf_cur_info = qemuDomainThrottleGroupByName(persistentDef, groupname);
+
+ if (qemuDomainCheckThrottleGroupReset(groupname, &conf_info) < 0)
+ goto endjob;
+
+ if (conf_cur_info != NULL) {
+ if (qemuDomainSetThrottleGroupDefaults(&conf_info, conf_cur_info,
+ set_fields) < 0)
... here ...
+ goto endjob;
+ virDomainThrottleGroupUpdate(persistentDef, &conf_info);
+ }else{
... here ...
+ virDomainThrottleGroupAdd(persistentDef,
&conf_info);
+ }
+
+
+ if (virDomainDefSave(persistentDef, driver->xmlopt,
+ cfg->configDir) < 0)
+ goto endjob;
+ }
+
+ ret = 0;
+ endjob:
+ virDomainObjEndJob(vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ virTypedParamsFree(eventParams, eventNparams);
+ return ret;
+}
+
+
+static int
+qemuDomainGetThrottleGroup(virDomainPtr dom,
+ const char *groupname,
+ virTypedParameterPtr params,
+ int *nparams,
+ unsigned int flags)
+{
+ virDomainObj *vm = NULL;
+ virDomainDef *def = NULL;
+ virDomainDef *persistentDef = NULL;
+ virDomainThrottleGroupDef groupDef = { 0 };
+ virDomainThrottleGroupDef *reply = &groupDef;
+ int ret = -1;
+ int maxparams;
+ int rc = 0;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ /* We don't return strings, and thus trivially support this flag. */
This is not true as ... [1]
+ flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
+
+ if (!(vm = qemuDomainObjFromDomain(dom)))
+ return -1;
+
+ if (virDomainGetThrottleGroupEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
+ goto cleanup;
+
+ /* the API check guarantees that only one of the definitions will be set */
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+ goto endjob;
+
+ maxparams = QEMU_NB_BLOCK_IO_TUNE_ALL_PARAMS;
+
+ if (*nparams == 0) {
+ *nparams = maxparams;
+ ret = 0;
+ goto endjob;
+ }
+ if (*nparams < maxparams)
+ maxparams = *nparams;
All of this won't make sense once you return the array fully allocated.
+
+ if (def) {
+ qemuDomainObjEnterMonitor(vm);
+ rc = qemuMonitorGetThrottleGroup(qemuDomainGetMonitor(vm), groupname, reply);
+ qemuDomainObjExitMonitor(vm);
+
+ if (rc < 0)
+ goto endjob;
+ }
+
+ if (persistentDef) {
+ reply = qemuDomainThrottleGroupByName(persistentDef, groupname);
+ if (reply == NULL) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("throttle group '%1$s' was not found in the
domain config"),
+ groupname);
+ goto endjob;
+ }
+ reply->group_name = g_strdup(groupname);
+ }
+
+ *nparams = 0;
+
+#define THROTTLE_GROUP_ASSIGN(name, var) \
+ if (*nparams < maxparams && \
+ virTypedParameterAssign(¶ms[(*nparams)++], \
+ VIR_DOMAIN_BLOCK_IOTUNE_ ## name, \
+ VIR_TYPED_PARAM_ULLONG, \
+ reply->var) < 0) \
+ goto endjob;
+
+ if (*nparams < maxparams) {
+ if (virTypedParameterAssign(¶ms[(*nparams)++],
+ VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME,
+ VIR_TYPED_PARAM_STRING,
[1].... you do in fact return a string.
+ reply->group_name) < 0)
+ goto endjob;
+ }
+
+ THROTTLE_GROUP_ASSIGN(TOTAL_BYTES_SEC, total_bytes_sec);
+ THROTTLE_GROUP_ASSIGN(READ_BYTES_SEC, read_bytes_sec);
+ THROTTLE_GROUP_ASSIGN(WRITE_BYTES_SEC, write_bytes_sec);
+
+ THROTTLE_GROUP_ASSIGN(TOTAL_IOPS_SEC, total_iops_sec);
+ THROTTLE_GROUP_ASSIGN(READ_IOPS_SEC, read_iops_sec);
+ THROTTLE_GROUP_ASSIGN(WRITE_IOPS_SEC, write_iops_sec);
+
+ THROTTLE_GROUP_ASSIGN(TOTAL_BYTES_SEC_MAX, total_bytes_sec_max);
+ THROTTLE_GROUP_ASSIGN(READ_BYTES_SEC_MAX, read_bytes_sec_max);
+ THROTTLE_GROUP_ASSIGN(WRITE_BYTES_SEC_MAX, write_bytes_sec_max);
+
+ THROTTLE_GROUP_ASSIGN(TOTAL_IOPS_SEC_MAX, total_iops_sec_max);
+ THROTTLE_GROUP_ASSIGN(READ_IOPS_SEC_MAX, read_iops_sec_max);
+ THROTTLE_GROUP_ASSIGN(WRITE_IOPS_SEC_MAX, write_iops_sec_max);
+
+ THROTTLE_GROUP_ASSIGN(SIZE_IOPS_SEC, size_iops_sec);
+
+ THROTTLE_GROUP_ASSIGN(TOTAL_BYTES_SEC_MAX_LENGTH, total_bytes_sec_max_length);
+ THROTTLE_GROUP_ASSIGN(READ_BYTES_SEC_MAX_LENGTH, read_bytes_sec_max_length);
+ THROTTLE_GROUP_ASSIGN(WRITE_BYTES_SEC_MAX_LENGTH, write_bytes_sec_max_length);
+
+ THROTTLE_GROUP_ASSIGN(TOTAL_IOPS_SEC_MAX_LENGTH, total_iops_sec_max_length);
+ THROTTLE_GROUP_ASSIGN(READ_IOPS_SEC_MAX_LENGTH, read_iops_sec_max_length);
+ THROTTLE_GROUP_ASSIGN(WRITE_IOPS_SEC_MAX_LENGTH, write_iops_sec_max_length);
+#undef THROTTLE_GROUP_ASSIGN
+
+ ret = 0;
+
+ endjob:
+ virDomainObjEndJob(vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}
+
+
+static int
+qemuDomainDelThrottleGroup(virDomainPtr dom,
+ const char *groupname,
+ unsigned int flags)
+{
+ virQEMUDriver *driver = dom->conn->privateData;
+ virDomainObj *vm = NULL;
+ virDomainDef *def = NULL;
+ virDomainDef *persistentDef = NULL;
+ g_autoptr(virQEMUDriverConfig) cfg = NULL;
+ int ret = -1;
+
+ virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+ /* We don't return strings, and thus trivially support this flag. */
+ flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
This function doesn't even accept typed parameters, thus this really
makes no sense here.
+
+ if (!(vm = qemuDomainObjFromDomain(dom)))
+ return -1;
+
+ cfg = virQEMUDriverGetConfig(driver);
+
+ if (virDomainDelThrottleGroupEnsureACL(dom->conn, vm->def) < 0)
+ goto cleanup;
+
+ if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
+ goto cleanup;
Deleting stuff from the definition is definitely _not_ a VIR_JOB_QUERY.
+
+ /* the API check guarantees that only one of the definitions will be set */
Umm so while this comment is true I don't think the deletion API should
refuse the combination of VIR_DOMAIN_AFFECT_LIVE and
VIR_DOMAIN_AFFECT_CONFIG.
While I agree that the combination of the flags can cause problems all
other APIs support this combination.
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef)
< 0)
+ goto endjob;
This is also missing some form of checks that there are no longer any
disks which would make use of the throttling objects any more. I presume
that the monitor command will fail in such case.
+
+ if (def) {
+ int rc = 0;
+
+ qemuDomainObjEnterMonitor(vm);
+ rc = qemuMonitorDelObject(qemuDomainGetMonitor(vm), groupname, true);
+ qemuDomainObjExitMonitor(vm);
+
+ if (rc < 0)
+ goto endjob;
+
+ virDomainThrottleGroupDel(def, groupname);
+ qemuDomainSaveStatus(vm);
+ }
+
+ if (persistentDef) {
You can fetch 'cfg' only in this block.
+ virDomainThrottleGroupDel(persistentDef, groupname);
+ if (virDomainDefSave(persistentDef, driver->xmlopt,
+ cfg->configDir) < 0)
+ goto endjob;
+ }
+
+ ret = 0;
+
+ endjob:
+ virDomainObjEndJob(vm);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return ret;
+}