On Wed, Jun 12, 2024 at 03:02:09 -0700, wucf(a)linux.ibm.com wrote: We tend to add also a justification/description of the change rather than just mechanically outline what you did: Introduce schema for defining '<throttlegroups>' element which configures throttling groups which can be configured for multiple disks. Or something similar. Don't forget to update to '10.7.0' ... within ``throttlefilters`` sub-element of ``disk`` element ... Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt;

On Wed, Jun 12, 2024 at 03:02:10 -0700, wucf(a)linux.ibm.com wrote: Node names are a qemu driver internal implementation detail and thus must not be noted in documentation.

On Wed, Jun 12, 2024 at 03:02:11 -0700, wucf(a)linux.ibm.com wrote: 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. 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. Please add docs for any function that is being exported.

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; * ThrottleGroup is updated through "qemuMonitorJSONUpdateThrottleGroup" * ThrottleGroup is retrieved through "qemuMonitorJSONGetThrottleGroup" * ThrottleGroup is deleted by reusing "qemuMonitorDelObject" * ThrottleGroup is added by reusing "qemuMonitorAddObject" * "qemuMonitorMakeThrottleGroupLimits" will be used by building qemu cmd as well Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; --- src/qemu/qemu_monitor.c | 34 ++++++++ src/qemu/qemu_monitor.h | 14 ++++ src/qemu/qemu_monitor_json.c | 150 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 14 ++++ 4 files changed, 212 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 34e2ccab97..2f067ab5d6 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2996,6 +2996,40 @@ qemuMonitorGetBlockIoThrottle(qemuMonitor *mon, } +int +qemuMonitorThrottleGroupLimits(virJSONValue *limits, + const virDomainThrottleGroupDef *group) +{ + return qemuMonitorMakeThrottleGroupLimits(limits, group); +} + + +int +qemuMonitorUpdateThrottleGroup(qemuMonitor *mon, + const char *qomid, + virDomainBlockIoTuneInfo *info) +{ + VIR_DEBUG("qomid=%s, info=%p", qomid, info); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONUpdateThrottleGroup(mon, qomid, info); +} + + +int +qemuMonitorGetThrottleGroup(qemuMonitor *mon, + const char *groupname, + virDomainBlockIoTuneInfo *reply) +{ + VIR_DEBUG("throttle-group=%s, reply=%p", groupname, reply); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONGetThrottleGroup(mon, groupname, reply); +} + + int qemuMonitorVMStatusToPausedReason(const char *status) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b78f539c85..6474ce124c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1062,6 +1062,20 @@ int qemuMonitorGetBlockIoThrottle(qemuMonitor *mon, const char *qdevid, virDomainBlockIoTuneInfo *reply); +int +qemuMonitorThrottleGroupLimits(virJSONValue *limits, + const virDomainThrottleGroupDef *group); + +int +qemuMonitorUpdateThrottleGroup(qemuMonitor *mon, + const char *qomid, + virDomainBlockIoTuneInfo *info); + +int +qemuMonitorGetThrottleGroup(qemuMonitor *mon, + const char *groupname, + virDomainBlockIoTuneInfo *reply); + int qemuMonitorSystemWakeup(qemuMonitor *mon); int qemuMonitorGetVersion(qemuMonitor *mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c5e758e7f8..462b40cb6b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4633,6 +4633,156 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitor *mon, return qemuMonitorJSONBlockIoThrottleInfo(devices, qdevid, reply); } + +int +qemuMonitorMakeThrottleGroupLimits(virJSONValue *limits, + const virDomainThrottleGroupDef *group) +{ + if (virJSONValueObjectAdd(&limits, + "P:bps-total", + group->total_bytes_sec, + "P:bps-read", + group->read_bytes_sec, + "P:bps-write", + group->write_bytes_sec, + "P:iops-total", + group->total_iops_sec, + "P:iops-read", + group->read_iops_sec, + "P:iops-write", + group->write_iops_sec, + "P:bps-total-max", + group->total_bytes_sec_max, + "P:bps-read-max", + group->read_bytes_sec_max, + "P:bps-write-max", + group->write_bytes_sec_max, + "P:iops-total-max", + group->total_iops_sec_max, + "P:iops-read-max", + group->read_iops_sec_max, + "P:iops-write-max", + group->write_iops_sec_max, + "P:iops-size", + group->size_iops_sec, + /* avoid error from QEMU: "the burst length cannot be 0" for throttlelimits + * when setting max-length + */ + "P:bps-total-max-length", + group->total_bytes_sec_max_length, + "P:bps-read-max-length", + group->read_bytes_sec_max_length, + "P:bps-write-max-length", + group->write_bytes_sec_max_length, + "P:iops-total-max-length", + group->total_iops_sec_max_length, + "P:iops-read-max-length", + group->read_iops_sec_max_length, + "P:iops-write-max-length", + group->write_iops_sec_max_length, + NULL) < 0) + return -1; + + return 0; +} + + +int +qemuMonitorJSONUpdateThrottleGroup(qemuMonitor *mon, + const char *qomid, + virDomainBlockIoTuneInfo *info) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) result = NULL; + g_autoptr(virJSONValue) limits = virJSONValueNewObject(); + + if (qemuMonitorMakeThrottleGroupLimits(limits, info)<0) + return -1; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-set", + "s:property", "limits", + "s:path", qomid, + "a:value", &limits, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &result) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, result) < 0) + return -1; + + return 0; +} + + +#define GET_THROTTLE_GROUP_VALUE(FIELD, STORE) \ + if (virJSONValueObjectHasKey(ret, FIELD)) { \ + if (virJSONValueObjectGetNumberUlong(ret, FIELD, &reply->STORE) < 0) { \ + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \ + _("throttle group field '%1$s' missing in qemu's output"), \ + #STORE); \ + return -1; \ + } \ + } + + +int +qemuMonitorJSONGetThrottleGroup(qemuMonitor *mon, + const char *gname, + virDomainBlockIoTuneInfo *reply) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) result = NULL; + g_autofree char *groupCopy = NULL; + virJSONValue *ret; + + g_autofree char *path = g_strdup_printf("/objects/%s", gname); + if (!(cmd = qemuMonitorJSONMakeCommand("qom-get", + "s:path", path, + "s:property", "limits", + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &result) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, result) < 0) + return -1; + + if (!(ret = qemuMonitorJSONGetReply(cmd, result, VIR_JSON_TYPE_OBJECT))) + return -1; + + GET_THROTTLE_GROUP_VALUE("bps-total", total_bytes_sec); + GET_THROTTLE_GROUP_VALUE("bps-read", read_bytes_sec); + GET_THROTTLE_GROUP_VALUE("bps-write", write_bytes_sec); + GET_THROTTLE_GROUP_VALUE("iops-total", total_iops_sec); + GET_THROTTLE_GROUP_VALUE("iops-read", read_iops_sec); + GET_THROTTLE_GROUP_VALUE("iops-write", write_iops_sec); + + GET_THROTTLE_GROUP_VALUE("bps-total-max", total_bytes_sec_max); + GET_THROTTLE_GROUP_VALUE("bps-read-max", read_bytes_sec_max); + GET_THROTTLE_GROUP_VALUE("bps-write-max", write_bytes_sec_max); + GET_THROTTLE_GROUP_VALUE("iops-total-max", total_iops_sec_max); + GET_THROTTLE_GROUP_VALUE("iops-read-max", read_iops_sec_max); + GET_THROTTLE_GROUP_VALUE("iops-write-max", write_iops_sec_max); + GET_THROTTLE_GROUP_VALUE("iops-size", size_iops_sec); + + GET_THROTTLE_GROUP_VALUE("bps-total-max-length", total_bytes_sec_max_length); + GET_THROTTLE_GROUP_VALUE("bps-read-max-length", read_bytes_sec_max_length); + GET_THROTTLE_GROUP_VALUE("bps-write-max-length", write_bytes_sec_max_length); + GET_THROTTLE_GROUP_VALUE("iops-total-max-length", total_iops_sec_max_length); + GET_THROTTLE_GROUP_VALUE("iops-read-max-length", read_iops_sec_max_length); + GET_THROTTLE_GROUP_VALUE("iops-write-max-length", write_iops_sec_max_length); + + groupCopy = g_strdup(gname); + reply->group_name = g_steal_pointer(&groupCopy); + + return 0; +} +#undef GET_THROTTLE_GROUP_VALUE + + int qemuMonitorJSONSystemWakeup(qemuMonitor *mon) { g_autoptr(virJSONValue) cmd = NULL; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 9684660d86..fa3138bbf9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -404,6 +404,20 @@ qemuMonitorJSONSetBlockIoThrottle(qemuMonitor *mon, const char *qomid, virDomainBlockIoTuneInfo *info); +int +qemuMonitorMakeThrottleGroupLimits(virJSONValue *limits, + const virDomainThrottleGroupDef *group); + +int +qemuMonitorJSONUpdateThrottleGroup(qemuMonitor *mon, + const char *qomid, + virDomainBlockIoTuneInfo *info); + +int +qemuMonitorJSONGetThrottleGroup(qemuMonitor *mon, + const char *gname, + virDomainBlockIoTuneInfo *reply); + int qemuMonitorJSONGetBlockIoThrottle(qemuMonitor *mon, const char *qdevid, -- 2.34.1

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; Within "testQemuMonitorJSONqemuMonitorJSONUpdateThrottleGroup" * Test qemuMonitorJSONGetThrottleGroup * Test qemuMonitorJSONUpdateThrottleGroup, which updates limits through "qom-set" Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; --- tests/qemumonitorjsontest.c | 86 +++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 45cee23798..7e034234c6 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1853,6 +1853,91 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *opaque) return ret; } + +static int +testQemuMonitorJSONqemuMonitorJSONUpdateThrottleGroup(const void *opaque) +{ + const testGenericData *data = opaque; + virDomainXMLOption *xmlopt = data->xmlopt; + virDomainBlockIoTuneInfo info, expectedInfo; + g_autoptr(qemuMonitorTest) test = NULL; + + if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) + return -1; + + expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, NULL, 15, 16, 17, 18, 19, 20}; + expectedInfo.group_name = g_strdup("limit0"); + + if (qemuMonitorTestAddItem(test, "qom-get", + "{" + " \"return\": {" + " \"bps-total\": 1," + " \"bps-read\": 2," + " \"bps-write\": 3," + " \"iops-total\": 4," + " \"iops-read\": 5," + " \"iops-write\": 6," + " \"bps-total-max\": 7," + " \"bps-read-max\": 8," + " \"bps-write-max\": 9," + " \"iops-total-max\": 10," + " \"iops-read-max\": 11," + " \"iops-write-max\": 12," + " \"iops-size\": 13," + " \"bps-total-max-length\": 15," + " \"bps-read-max-length\": 16," + " \"bps-write-max-length\": 17," + " \"iops-total-max-length\": 18," + " \"iops-read-max-length\": 19," + " \"iops-write-max-length\": 20" + " }," + " \"id\": \"libvirt-12\"" + "}") < 0) + return -1; + + if (qemuMonitorTestAddItemVerbatim(test, + "{\"execute\":\"qom-set\"," + " \"arguments\":{\"property\": \"limits\"," + " \"path\": \"limit1\"," + " \"value\":{\"bps-total\": 1," + " \"bps-read\": 2," + " \"bps-write\": 3," + " \"iops-total\": 4," + " \"iops-read\": 5," + " \"iops-write\": 6," + " \"bps-total-max\": 7," + " \"bps-read-max\": 8," + " \"bps-write-max\": 9," + " \"iops-total-max\": 10," + " \"iops-read-max\": 11," + " \"iops-write-max\": 12," + " \"iops-size\": 13," + " \"bps-total-max-length\": 15," + " \"bps-read-max-length\": 16," + " \"bps-write-max-length\": 17," + " \"iops-total-max-length\": 18," + " \"iops-read-max-length\": 19," + " \"iops-write-max-length\": 20}}," + " \"id\":\"libvirt-2\"}", + NULL, + "{ \"return\" : {}}") < 0) + return -1; + + if (qemuMonitorJSONGetThrottleGroup(qemuMonitorTestGetMonitor(test), + "limit0", &info) < 0) + return -1; + + if (testValidateGetBlockIoThrottle(&info, &expectedInfo) < 0) + return -1; + + if (qemuMonitorJSONUpdateThrottleGroup(qemuMonitorTestGetMonitor(test), + "limit1", &info) < 0) + return -1; + + return 0; +} + + static int testQemuMonitorJSONqemuMonitorJSONGetTargetArch(const void *opaque) { @@ -2899,6 +2984,7 @@ mymain(void) DO_TEST(qemuMonitorJSONGetMigrationStats); DO_TEST(qemuMonitorJSONGetChardevInfo); DO_TEST(qemuMonitorJSONSetBlockIoThrottle); + DO_TEST(qemuMonitorJSONUpdateThrottleGroup); DO_TEST(qemuMonitorJSONGetTargetArch); DO_TEST(qemuMonitorJSONGetMigrationCapabilities); DO_TEST(qemuMonitorJSONQueryCPUsFast); -- 2.34.1

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; Defined new public APIs: * virDomainSetThrottleGroup to add or update throttlegroup within specific domain, it will be referenced by throttlefilter later in disk to do limits * virDomainGetThrottleGroup to get throttlegroup info, old-style is discarded(APIs to query first for the number of parameters and then give it a reasonably-sized pointer), instead, the new approach is adopted that API returns allocated array of fields and number of fileds that are in it. * virDomainDelThrottleGroup to delete throttlegroup, it fails if this throttlegroup is still referenced by some throttlefilter Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; --- include/libvirt/libvirt-domain.h | 21 +++ src/driver-hypervisor.h | 22 ++++ src/libvirt-domain.c | 196 ++++++++++++++++++++++++++++ src/libvirt_private.syms | 9 ++ src/libvirt_public.syms | 7 + src/remote/remote_daemon_dispatch.c | 44 +++++++ src/remote/remote_driver.c | 40 ++++++ src/remote/remote_protocol.x | 48 ++++++- src/remote_protocol-structs | 28 ++++ 9 files changed, 414 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2f5b01bbfe..4f71e2bf18 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6525,4 +6525,25 @@ virDomainGraphicsReload(virDomainPtr domain, unsigned int type, unsigned int flags); + +int +virDomainSetThrottleGroup(virDomainPtr dom, + const char *group, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +int +virDomainGetThrottleGroup(virDomainPtr dom, + const char *group, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + +int +virDomainDelThrottleGroup(virDomainPtr dom, + const char *group, + unsigned int flags); + + #endif /* LIBVIRT_DOMAIN_H */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 4ce8da078d..b3b1912666 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1453,6 +1453,25 @@ typedef int unsigned int type, unsigned int flags); +typedef int +(*virDrvDomainSetThrottleGroup)(virDomainPtr dom, + const char *groupname, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + +typedef int +(*virDrvDomainGetThrottleGroup)(virDomainPtr dom, + const char *groupname, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags); + +typedef int +(*virDrvDomainDelThrottleGroup)(virDomainPtr dom, + const char *groupname, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; /** @@ -1726,4 +1745,7 @@ struct _virHypervisorDriver { virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; virDrvDomainFDAssociate domainFDAssociate; virDrvDomainGraphicsReload domainGraphicsReload; + virDrvDomainSetThrottleGroup domainSetThrottleGroup; + virDrvDomainGetThrottleGroup domainGetThrottleGroup; + virDrvDomainDelThrottleGroup domainDelThrottleGroup; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 7c6b93963c..37ba587b57 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -14162,3 +14162,199 @@ virDomainGraphicsReload(virDomainPtr domain, virDispatchError(domain->conn); return -1; } + + +/** + * virDomainSetThrottleGroup: + * @dom: pointer to domain object + * @group: throttle group name + * @params: Pointer to blkio parameter objects + * @nparams: Number of blkio parameters (this value can be the same or + * less than the number of parameters supported) + * @flags: bitwise-OR of virDomainModificationImpact + * + * Add throttlegroup or change all or a subset of the throttlegroup options + * within specific domain + * + * The @group parameter is the name for new or existing throttlegroup, + * it cannot be NULL, detailed throttlegroup info is included in @params, + * it either creates new throttlegroup with @params or updates existing + * throttlegroup with @params, throttlegroup can be referenced by throttle + * filter in attached disk to do limits, the difference from iotune is that + * multiple throttlegroups can be referenced within attached disk + * + * Returns -1 in case of error, 0 in case of success. + * + * Since: 10.5.0 + */ +int +virDomainSetThrottleGroup(virDomainPtr dom, + const char *group, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "params=%p, nparams=%d, flags=0x%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + conn = dom->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckNonNullArgGoto(group, error); + virCheckPositiveArgGoto(nparams, error); + virCheckNonNullArgGoto(params, error); + + if (virTypedParameterValidateSet(dom->conn, params, nparams) < 0) + goto error; + + if (conn->driver->domainSetThrottleGroup) { + int ret; + ret = conn->driver->domainSetThrottleGroup(dom, group, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} + + +/** + * virDomainGetThrottleGroup: + * @dom: pointer to domain object + * @group: throttle group name + * @params: pointer that will be filled with an array of typed parameters + * @nparams: pointer filled with number of elements in @params + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags + * + * Get all block IO tunable parameters for specific throttle group. @group cannot be NULL. + * @nparams gives how many slots were filled with parameter information + * + * + * Returns -1 in case of error, 0 in case of success. + * + * Since: 10.5.0 + */ +int +virDomainGetThrottleGroup(virDomainPtr dom, + const char *group, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + virConnectPtr conn; + int rc; + + VIR_DOMAIN_DEBUG(dom, "params=%p, nparams=%d, flags=0x%x", + params, (nparams) ? *nparams : -1, flags); + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckNonNullArgGoto(group, error); + virCheckNonNullArgGoto(nparams, error); + virCheckNonNullArgGoto(params, error); + + rc = VIR_DRV_SUPPORTS_FEATURE(dom->conn->driver, dom->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING); + if (rc < 0) + goto error; + if (rc) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE, + VIR_DOMAIN_AFFECT_CONFIG, + error); + + conn = dom->conn; + + if (conn->driver->domainGetThrottleGroup) { + int ret; + ret = conn->driver->domainGetThrottleGroup(dom, group, params, nparams, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} + + +/** + * virDomainDelThrottleGroup: + * @dom: pointer to domain object + * @group: throttle group name + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags + * + * Delete an throttlegroup from the domain. @group cannot be NULL, + * and the @group to be deleted must not have a throttlefilter associated with + * it and can be any of the current valid group. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. + * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain + * and may fail if domain is not alive. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. If neither flag is specified (that is, + * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies + * persistent setup, while an active domain is hypervisor-dependent on whether + * just live or both live and persistent state is changed. + * + * Returns -1 in case of error, 0 in case of success. + * + * Since: 10.5.0 + */ +int +virDomainDelThrottleGroup(virDomainPtr dom, + const char *group, + unsigned int flags) +{ + virConnectPtr conn; + int rc; + + virResetLastError(); + + virCheckDomainReturn(dom, -1); + virCheckNonNullArgGoto(group, error); + + rc = VIR_DRV_SUPPORTS_FEATURE(dom->conn->driver, dom->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING); + if (rc < 0) + goto error; + if (rc) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_AFFECT_LIVE, + VIR_DOMAIN_AFFECT_CONFIG, + error); + + conn = dom->conn; + + if (conn->driver->domainDelThrottleGroup) { + int ret; + ret = conn->driver->domainDelThrottleGroup(dom, group, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(dom->conn); + return -1; +} diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 653c84a520..a438ffa795 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -668,6 +668,15 @@ virDomainTaintMessageTypeFromString; virDomainTaintMessageTypeToString; virDomainTaintTypeFromString; virDomainTaintTypeToString; +virDomainThrottleFilterDefFree; +virDomainThrottleFilterFind; +virDomainThrottleGroupAdd; +virDomainThrottleGroupByName; +virDomainThrottleGroupDefCopy; +virDomainThrottleGroupDefFree; +virDomainThrottleGroupDel; +virDomainThrottleGroupFind; +virDomainThrottleGroupUpdate; virDomainTimerModeTypeFromString; virDomainTimerModeTypeToString; virDomainTimerNameTypeFromString; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 7a3492d9d7..2b37f29a28 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -948,4 +948,11 @@ LIBVIRT_10.2.0 { virDomainGraphicsReload; } LIBVIRT_10.1.0; +LIBVIRT_10.5.0 { + global: + virDomainSetThrottleGroup; + virDomainGetThrottleGroup; + virDomainDelThrottleGroup; +} LIBVIRT_10.2.0; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index cfc1067e40..f5a9e45303 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3614,6 +3614,50 @@ remoteDispatchDomainGetBlockIoTune(virNetServer *server G_GNUC_UNUSED, return rv; } + +static int +remoteDispatchDomainGetThrottleGroup(virNetServer *server G_GNUC_UNUSED, + virNetServerClient *client, + virNetMessage *hdr G_GNUC_UNUSED, + struct virNetMessageError *rerr, + remote_domain_get_throttle_group_args *args, + remote_domain_get_throttle_group_ret *ret) +{ + virDomainPtr dom = NULL; + int rv = -1; + virTypedParameterPtr params = NULL; + int nparams = 0; + virConnectPtr conn = remoteGetHypervisorConn(client); + + if (!conn) + goto cleanup; + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainGetThrottleGroup(dom, args->group ? *args->group : NULL, + &params, &nparams, args->flags) < 0) + goto cleanup; + + /* Serialize the ThrottleGroup parameters. */ + if (virTypedParamsSerialize(params, nparams, + REMOTE_DOMAIN_THROTTLE_GROUP_PARAMETERS_MAX, + (struct _virTypedParameterRemote **) &ret->params.params_val, + &ret->params.params_len, + args->flags) < 0) + goto cleanup; + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virTypedParamsFree(params, nparams); + virObjectUnref(dom); + return rv; +} + + /*-------------------------------------------------------------*/ static int diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e76d9e9ba4..4f2b28e662 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2583,6 +2583,43 @@ static int remoteDomainGetBlockIoTune(virDomainPtr domain, return 0; } + +static int +remoteDomainGetThrottleGroup(virDomainPtr domain, + const char *group, + virTypedParameterPtr *params, + int *nparams, + unsigned int flags) +{ + remote_domain_get_throttle_group_args args = {0}; + g_auto(remote_domain_get_throttle_group_ret) ret = {0}; + struct private_data *priv = domain->conn->privateData; + VIR_LOCK_GUARD lock = remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.group = group ? (char **)&group : NULL; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_THROTTLE_GROUP, + (xdrproc_t) xdr_remote_domain_get_throttle_group_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_get_throttle_group_ret, + (char *) &ret) == -1) { + return -1; + } + + if (virTypedParamsDeserialize((struct _virTypedParameterRemote *) ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_THROTTLE_GROUP_PARAMETERS_MAX, + params, + nparams) < 0) + return -1; + + return 0; + +} + + static int remoteDomainGetCPUStats(virDomainPtr domain, virTypedParameterPtr params, unsigned int nparams, @@ -7842,6 +7879,9 @@ static virHypervisorDriver hypervisor_driver = { .domainSetLaunchSecurityState = remoteDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = remoteDomainFDAssociate, /* 9.0.0 */ .domainGraphicsReload = remoteDomainGraphicsReload, /* 10.2.0 */ + .domainSetThrottleGroup = remoteDomainSetThrottleGroup, /* 10.5.0 */ + .domainGetThrottleGroup = remoteDomainGetThrottleGroup, /* 10.5.0 */ + .domainDelThrottleGroup = remoteDomainDelThrottleGroup, /* 10.5.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 41c045ff78..e7e91dfb07 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -113,6 +113,9 @@ const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16; /* Upper limit on list of blockio tuning parameters. */ const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 32; +/* Upper limit on list of throttle group parameters. */ +const REMOTE_DOMAIN_THROTTLE_GROUP_PARAMETERS_MAX = 32; + /* Upper limit on list of numa parameters. */ const REMOTE_DOMAIN_NUMA_PARAMETERS_MAX = 16; @@ -1475,6 +1478,29 @@ struct remote_domain_get_block_io_tune_ret { int nparams; }; +struct remote_domain_set_throttle_group_args { + remote_nonnull_domain dom; + remote_nonnull_string group; + remote_typed_param params<REMOTE_DOMAIN_THROTTLE_GROUP_PARAMETERS_MAX>; + unsigned int flags; +}; + +struct remote_domain_get_throttle_group_args { + remote_nonnull_domain dom; + remote_string group; + unsigned int flags; +}; + +struct remote_domain_get_throttle_group_ret { + remote_typed_param params<REMOTE_DOMAIN_THROTTLE_GROUP_PARAMETERS_MAX>; +}; + +struct remote_domain_del_throttle_group_args { + remote_nonnull_domain dom; + remote_string group; + unsigned int flags; +}; + struct remote_domain_get_cpu_stats_args { remote_nonnull_domain dom; unsigned int nparams; @@ -7048,5 +7074,25 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448 + REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448, + + /** + * @generate: both + * @acl: domain:write + * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE + * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG + */ + REMOTE_PROC_DOMAIN_SET_THROTTLE_GROUP = 449, + + /** + * @generate: none + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_GET_THROTTLE_GROUP = 450, + + /** + * @generate: both + * @acl: domain:read + */ + REMOTE_PROC_DOMAIN_DEL_THROTTLE_GROUP = 451 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4d3dc2d249..a94e29f9c9 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1050,6 +1050,31 @@ struct remote_domain_get_block_io_tune_ret { } params; int nparams; }; +struct remote_domain_set_throttle_group_args { + remote_nonnull_domain dom; + remote_nonnull_string group; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; +struct remote_domain_get_throttle_group_args { + remote_nonnull_domain dom; + remote_string group; + u_int flags; +}; +struct remote_domain_get_throttle_group_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; +}; +struct remote_domain_del_throttle_group_args { + remote_nonnull_domain dom; + remote_string group; + u_int flags; +}; struct remote_domain_get_cpu_stats_args { remote_nonnull_domain dom; u_int nparams; @@ -3755,4 +3780,7 @@ enum remote_procedure { REMOTE_PROC_NETWORK_EVENT_CALLBACK_METADATA_CHANGE = 446, REMOTE_PROC_NODE_DEVICE_UPDATE = 447, REMOTE_PROC_DOMAIN_GRAPHICS_RELOAD = 448, + REMOTE_PROC_DOMAIN_SET_THROTTLE_GROUP = 449, + REMOTE_PROC_DOMAIN_GET_THROTTLE_GROUP = 450, + REMOTE_PROC_DOMAIN_DEL_THROTTLE_GROUP = 451, }; -- 2.34.1

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; Implement the following methods in qemu driver: * Extract common methods for "qemuDomainSetBlockIoTune" and "qemuDomainSetThrottleGroup": qemuDomainValidateBlockIoTune, qemuDomainSetBlockIoTuneFields, qemuDomainCheckBlockIoTuneMutualExclusion, qemuDomainCheckBlockIoTuneMax. * "qemuDomainSetThrottleGroup", this method is to add("object-add") or update("qom-set") throttlegroup in QOM and update corresponding objects in DOM * "qemuDomainGetThrottleGroup", this method queries throttlegroup info by groupname * "qemuDomainDelThrottleGroup", this method checks if group is referenced by any throttle in disks and delete it if it's not used anymore * Check flag "QEMU_CAPS_OBJECT_JSON" during qemuDomainSetThrottleGroup, throttle group feature requries such flag * "objectAddNoWrap"("props") check is done by reusing qemuMonitorAddObject in qemuDomainSetThrottleGroup Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; --- src/qemu/qemu_driver.c | 611 +++++++++++++++++++++++++++++++++++------ 1 file changed, 528 insertions(+), 83 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e2698c7924..f7e435d6d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14955,35 +14955,8 @@ qemuDomainCheckBlockIoTuneReset(virDomainDiskDef *disk, static int -qemuDomainSetBlockIoTune(virDomainPtr dom, - const char *path, - virTypedParameterPtr params, - int nparams, - unsigned int flags) +qemuDomainValidateBlockIoTune(virTypedParameterPtr params, int nparams) { - virQEMUDriver *driver = dom->conn->privateData; - virDomainObj *vm = NULL; - qemuDomainObjPrivate *priv; - virDomainDef *def = NULL; - virDomainDef *persistentDef = NULL; - virDomainBlockIoTuneInfo info = { 0 }; - virDomainBlockIoTuneInfo conf_info = { 0 }; - int ret = -1; - size_t i; - virDomainDiskDef *conf_disk = NULL; - virDomainDiskDef *disk; - qemuBlockIoTuneSetFlags set_fields = 0; - g_autoptr(virQEMUDriverConfig) cfg = NULL; - virObjectEvent *event = NULL; - virTypedParameterPtr eventParams = NULL; - int eventNparams = 0; - int eventMaxparams = 0; - virDomainBlockIoTuneInfo *cur_info; - virDomainBlockIoTuneInfo *conf_cur_info; - - - 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, @@ -15028,32 +15001,28 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, NULL) < 0) return -1; - if (!(vm = qemuDomainObjFromDomain(dom))) - return -1; - - if (virDomainSetBlockIoTuneEnsureACL(dom->conn, vm->def, flags) < 0) - goto cleanup; - - cfg = virQEMUDriverGetConfig(driver); - - if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) - goto cleanup; - - priv = vm->privateData; + return 0; +} - if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) - goto endjob; - if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams, - VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0) - goto endjob; +static int +qemuDomainSetBlockIoTuneFields(virDomainBlockIoTuneInfo *info, + virTypedParameterPtr params, + int nparams, + qemuBlockIoTuneSetFlags *set_fields, + virTypedParameterPtr *eventParams, + int *eventNparams, + int *eventMaxparams) +{ + size_t i; + int ret = -1; #define SET_IOTUNE_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, \ + 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; \ @@ -15093,10 +15062,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, /* NB: Cannot use macro since this is a value.s not a value.ul */ 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, + 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; @@ -15119,56 +15088,56 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, #undef SET_IOTUNE_FIELD - if ((info.total_bytes_sec && info.read_bytes_sec) || - (info.total_bytes_sec && info.write_bytes_sec)) { + ret = 0; + endjob: + return ret; +} + + +static int +qemuDomainCheckBlockIoTuneMutualExclusion(virDomainBlockIoTuneInfo *info) +{ + 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; + return -1; } - if ((info.total_iops_sec && info.read_iops_sec) || - (info.total_iops_sec && info.write_iops_sec)) { + 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; + return -1; } - if ((info.total_bytes_sec_max && info.read_bytes_sec_max) || - (info.total_bytes_sec_max && info.write_bytes_sec_max)) { + 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; + return -1; } - if ((info.total_iops_sec_max && info.read_iops_sec_max) || - (info.total_iops_sec_max && info.write_iops_sec_max)) { + 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; + return -1; } - virDomainBlockIoTuneInfoCopy(&info, &conf_info); - - if (def) { - if (!(disk = qemuDomainDiskByName(def, path))) - goto endjob; - - if (!qemuDomainDiskBlockIoTuneIsSupported(disk)) - goto endjob; + return 0; +} - cur_info = qemuDomainFindGroupBlockIoTune(def, disk, &info); - if (qemuDomainSetBlockIoTuneDefaults(&info, cur_info, - set_fields) < 0) - goto endjob; - - if (qemuDomainCheckBlockIoTuneReset(disk, &info) < 0) - goto endjob; +static int +qemuDomainCheckBlockIoTuneMax(virDomainBlockIoTuneInfo *info) +{ + int ret = -1; #define CHECK_MAX(val, _bool) \ do { \ - if (info.val##_max) { \ - if (!info.val) { \ + 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"), \ @@ -15180,7 +15149,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } \ goto endjob; \ } \ - if (info.val##_max < info.val) { \ + if (info->val##_max < info->val) { \ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, \ _("value '%1$s' cannot be smaller than '%2$s'"), \ #val "_max", #val); \ @@ -15198,6 +15167,96 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, #undef CHECK_MAX + ret = 0; + endjob: + return ret; +} + + +static int +qemuDomainSetBlockIoTune(virDomainPtr dom, + const char *path, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + virQEMUDriver *driver = dom->conn->privateData; + virDomainObj *vm = NULL; + qemuDomainObjPrivate *priv; + virDomainDef *def = NULL; + virDomainDef *persistentDef = NULL; + virDomainBlockIoTuneInfo info = { 0 }; + virDomainBlockIoTuneInfo conf_info = { 0 }; + int ret = -1; + virDomainDiskDef *conf_disk = NULL; + virDomainDiskDef *disk; + qemuBlockIoTuneSetFlags set_fields = 0; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + virObjectEvent *event = NULL; + virTypedParameterPtr eventParams = NULL; + int eventNparams = 0; + int eventMaxparams = 0; + virDomainBlockIoTuneInfo *cur_info; + virDomainBlockIoTuneInfo *conf_cur_info; + + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + if (qemuDomainValidateBlockIoTune(params, nparams) < 0) + return -1; + + if (!(vm = qemuDomainObjFromDomain(dom))) + return -1; + + if (virDomainSetBlockIoTuneEnsureACL(dom->conn, vm->def, flags) < 0) + goto cleanup; + + cfg = virQEMUDriverGetConfig(driver); + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + priv = vm->privateData; + + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) + goto endjob; + + if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams, + VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0) + goto endjob; + + if (qemuDomainSetBlockIoTuneFields(&info, + params, + nparams, + &set_fields, + &eventParams, + &eventNparams, + &eventMaxparams) < 0) + goto endjob; + + if (qemuDomainCheckBlockIoTuneMutualExclusion(&info) < 0) + goto endjob; + + virDomainBlockIoTuneInfoCopy(&info, &conf_info); + + if (def) { + if (!(disk = qemuDomainDiskByName(def, path))) + goto endjob; + + if (!qemuDomainDiskBlockIoTuneIsSupported(disk)) + goto endjob; + + cur_info = qemuDomainFindGroupBlockIoTune(def, disk, &info); + + if (qemuDomainSetBlockIoTuneDefaults(&info, cur_info, set_fields) < 0) + goto endjob; + + if (qemuDomainCheckBlockIoTuneReset(disk, &info) < 0) + goto endjob; + + if (qemuDomainCheckBlockIoTuneMax(&info) < 0) + goto endjob; + /* blockdev-based qemu doesn't want to set the throttling when a cdrom * is empty. Skip the monitor call here since we will set the throttling * once new media is inserted */ @@ -19995,6 +20054,389 @@ qemuDomainGraphicsReload(virDomainPtr domain, return ret; } + +/** + * qemuDomainCheckThrottleGroupReset: + * @groupname: group to create or update + * @newiotune: Pointer to iotune, which contains detailed items + * + * Check if params within @newiotune contain all zero values, if yes + * and @newiotune specifies the same group name as @groupname, return + * failure since it's meaningless to set all zero values in @newiotune + * + * Returns -1 on failure, or 0 on success. + */ +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); + + 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; + 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(); + qemuDomainObjPrivate *priv = NULL; + virQEMUCaps *qemuCaps = NULL; + + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + if (qemuDomainValidateBlockIoTune(params, nparams) < 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_TUNABLE_BLKDEV_GROUP_NAME, groupname) < 0) + goto endjob; + + if (qemuDomainSetBlockIoTuneFields(&info, + params, + nparams, + &set_fields, + &eventParams, + &eventNparams, + &eventMaxparams) < 0) + goto endjob; + + if (qemuDomainCheckBlockIoTuneMutualExclusion(&info) < 0) + goto endjob; + + virDomainThrottleGroupDefCopy(&info, &conf_info); + + priv = vm->privateData; + qemuCaps = priv->qemuCaps; + /* this throttle group feature requires "QEMU_CAPS_OBJECT_JSON" + * when starting domain later, so check such flag here as well */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_JSON)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU_CAPS_OBJECT_JSON support is required for throttle group creation")); + return -1; + } + + if (def) { + if (qemuDomainCheckThrottleGroupReset(groupname, &info) < 0) + goto endjob; + + if (qemuDomainCheckBlockIoTuneMax(&info) < 0) + goto endjob; + + cur_info = virDomainThrottleGroupByName(def, groupname); + /* Update existing group. */ + if (cur_info != NULL) { + if (qemuDomainSetBlockIoTuneDefaults(&info, cur_info, set_fields) < 0) + goto endjob; + qemuDomainObjEnterMonitor(vm); + rc = qemuMonitorUpdateThrottleGroup(qemuDomainGetMonitor(vm), + groupname, + &info); + qemuDomainObjExitMonitor(vm); + if (rc < 0) + goto endjob; + virDomainThrottleGroupUpdate(def, &info); + } else { + if (qemuMonitorThrottleGroupLimits(limits, &info)<0) + goto endjob; + if (qemuMonitorCreateObjectProps(&props, + "throttle-group", groupname, + "a:limits", &limits, + NULL) < 0) + goto endjob; + qemuDomainObjEnterMonitor(vm); + /* different QMP version has different format for "object-add", reuse + * "qemuMonitorAddObject" to check if "objectAddNoWrap"("props") is + * requried */ + 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 = virDomainThrottleGroupByName(persistentDef, groupname); + + if (qemuDomainCheckThrottleGroupReset(groupname, &conf_info) < 0) + goto endjob; + + if (conf_cur_info != NULL) { + if (qemuDomainSetBlockIoTuneDefaults(&conf_info, conf_cur_info, + set_fields) < 0) + goto endjob; + virDomainThrottleGroupUpdate(persistentDef, &conf_info); + } else { + 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 = 0; + int rc = 0; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + + 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; + + if (def) { + qemuDomainObjEnterMonitor(vm); + rc = qemuMonitorGetThrottleGroup(qemuDomainGetMonitor(vm), groupname, reply); + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + goto endjob; + } + + if (persistentDef) { + reply = virDomainThrottleGroupByName(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 (virTypedParamsAddULLong(params, \ + nparams, \ + &maxparams, \ + VIR_DOMAIN_BLOCK_IOTUNE_ ## name, \ + reply->var) < 0) \ + goto endjob; + + + if (virTypedParamsAddString(params, nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, + 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 +qemuDomainCheckThrottleGroupRef(virDomainDef *def, + const char *group_name) +{ + size_t i; + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDef *disk = def->disks[i]; + if (virDomainThrottleFilterFind(disk, group_name)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("throttle group '%1$s' is still being used by disk %2$s"), + group_name, disk->dst); + return -1; + } + } + return 0; +} + + +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); + + if (!(vm = qemuDomainObjFromDomain(dom))) + return -1; + + if (virDomainDelThrottleGroupEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) + goto endjob; + + if (def) { + int rc = 0; + + /* check if this group is still being used by disks */ + if (qemuDomainCheckThrottleGroupRef(def, groupname) < 0) + goto endjob; + + qemuDomainObjEnterMonitor(vm); + rc = qemuMonitorDelObject(qemuDomainGetMonitor(vm), groupname, true); + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + goto endjob; + + virDomainThrottleGroupDel(def, groupname); + qemuDomainSaveStatus(vm); + } + + if (persistentDef) { + /* check if this group is still being used by disks */ + if (qemuDomainCheckThrottleGroupRef(persistentDef, groupname) < 0) + goto endjob; + + cfg = virQEMUDriverGetConfig(driver); + virDomainThrottleGroupDel(persistentDef, groupname); + if (virDomainDefSave(persistentDef, driver->xmlopt, + cfg->configDir) < 0) + goto endjob; + } + + ret = 0; + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectURIProbe = qemuConnectURIProbe, @@ -20245,6 +20687,9 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = qemuDomainFDAssociate, /* 9.0.0 */ .domainGraphicsReload = qemuDomainGraphicsReload, /* 10.2.0 */ + .domainSetThrottleGroup = qemuDomainSetThrottleGroup, /* 10.5.0 */ + .domainGetThrottleGroup = qemuDomainGetThrottleGroup, /* 10.5.0 */ + .domainDelThrottleGroup = qemuDomainDelThrottleGroup, /* 10.5.0 */ }; -- 2.34.1

On 2024/7/26 23:06, Peter Krempa wrote: is it okay to update it as: if (virDomainObjIsActive(vm)) {         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_JSON)) {             virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",                         _("support for '-object' with json format in QEMU command is required when creating throttle group"));             return -1;         } } BTW, may I know if you finished all commits review for v3? -- Thanks and Regards, Wu

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; 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 * Update "qemuDomainDiskGetTopNodename" to take top throttle node name if disk has throttles * Each filter has a nodename, and those filters are chained up in sequence * Filter nodename index is generated by reusing qemuDomainStorageIDNew and current global sequence number is persistented in virDomainObj->privateData(qemuDomainObjPrivate)->nodenameindex * During hotplug, filter is created through QMP request("blockdev-add" with "driver":"throttle") to QEMU * Delete filters by "qemuBlockThrottleFiltersDetach"("blockdev-del") when detaching device * Use "iotune" and "throttlefilters" exclusively for specific disk Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; --- src/conf/domain_validate.c | 8 +++ src/qemu/qemu_block.c | 131 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 53 +++++++++++++++ src/qemu/qemu_command.c | 84 ++++++++++++++++++++++++ src/qemu/qemu_command.h | 9 +++ src/qemu/qemu_domain.c | 39 ++++++++++- src/qemu/qemu_domain.h | 8 +++ src/qemu/qemu_driver.c | 6 ++ src/qemu/qemu_hotplug.c | 33 ++++++++++ 9 files changed, 370 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 395e036e8f..4cc5ed7577 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -942,6 +942,14 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->throttlefilters && (disk->blkdeviotune.group_name || + virDomainBlockIoTuneInfoHasAny(&disk->blkdeviotune))) { + virReportError(VIR_ERR_XML_ERROR, + _("block 'throttlefilters' can't be used together with 'iotune' for disk '%1$s'"), + disk->dst); + return -1; + } + return 0; } diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 738b72d7ea..9b8ff65887 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 +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); +} + + +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 f9e961d85d..9888954ce4 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -214,6 +214,59 @@ qemuBlockStorageSourceCreateDetectSize(GHashTable *blockNamedNodeData, virStorageSource *src, virStorageSource *templ); +void +qemuBlockThrottleFilterSetNodename(virDomainThrottleFilterDef *filter, + char *nodename); + +const char * +qemuBlockThrottleFilterGetNodename(virDomainThrottleFilterDef *filter); + +virJSONValue * +qemuBlockThrottleFilterGetProps(virDomainThrottleFilterDef *filter, + const char *parentNodeName); + +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 2d0eddc79e..5ccae956d3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1582,6 +1582,13 @@ qemuDiskConfigBlkdeviotuneEnabled(const virDomainDiskDef *disk) } +bool +qemuDiskConfigThrottleFiltersEnabled(const virDomainDiskDef *disk) +{ + return disk->nthrottlefilters > 0; +} + + /** * qemuDiskBusIsSD: * @bus: disk bus @@ -11055,6 +11062,83 @@ 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, which is used to build "blockdev-add" QMP request + * + * 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; + g_autofree char *tmp_nodename = NULL; + + data = g_new0(qemuBlockThrottleFiltersData, 1); + /* 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++) { + tmp_nodename = g_strdup(parentNodeName); + if (qemuBuildThrottleFiltersAttachPrepareBlockdevOne(data, disk->throttlefilters[i], tmp_nodename) < 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 dca8877703..ce39acfb2c 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -116,6 +116,15 @@ qemuBlockStorageSourceChainData * qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSource *top, virStorageSource *backingStore); +qemuBlockThrottleFiltersData * +qemuBuildThrottleFiltersAttachPrepareBlockdev(virDomainDiskDef *disk); + +qemuBlockThrottleFiltersData * +qemuBuildThrottleFiltersDetachPrepareBlockdev(virDomainDiskDef *disk); + +bool +qemuDiskConfigThrottleFiltersEnabled(const virDomainDiskDef *disk); + virJSONValue * qemuBuildDiskDeviceProps(const virDomainDef *def, virDomainDiskDef *disk, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7ba2ea4a5e..2831036e23 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7989,7 +7989,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). @@ -8005,6 +8006,10 @@ qemuDomainDiskGetTopNodename(virDomainDiskDef *disk) if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) return priv->nodeCopyOnRead; + /* If disk has throttles, take top throttle node name */ + if (disk->nthrottlefilters > 0) + return disk->throttlefilters[disk->nthrottlefilters-1]->nodename; + return qemuBlockStorageSourceGetEffectiveNodename(disk->src); } @@ -11336,6 +11341,32 @@ qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, } +int +qemuDomainPrepareThrottleFilterBlockdevNodename(virDomainThrottleFilterDef *filter, + const char *nodenameprefix) +{ + char *nodename = g_strdup_printf("%s-filter", nodenameprefix); + + qemuBlockThrottleFilterSetNodename(filter, nodename); + + return 0; +} + + +int +qemuDomainPrepareThrottleFilterBlockdev(virDomainThrottleFilterDef *filter, + qemuDomainObjPrivate *priv) +{ + g_autofree char *nodenameprefix = NULL; + + filter->id = qemuDomainStorageIDNew(priv); + + nodenameprefix = g_strdup_printf("libvirt-%u", filter->id); + + return qemuDomainPrepareThrottleFilterBlockdevNodename(filter, nodenameprefix); +} + + int qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, virStorageSource *src, @@ -11359,6 +11390,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) @@ -11369,6 +11401,11 @@ qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDef *disk, return -1; } + for (i = 0; i < disk->nthrottlefilters; i++) { + if (qemuDomainPrepareThrottleFilterBlockdev(disk->throttlefilters[i], priv) < 0) + return -1; + } + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a3089ea449..38bba4e3ae 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -772,6 +772,14 @@ int qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, qemuDomainObjPrivate *priv, virQEMUDriverConfig *cfg); +int +qemuDomainPrepareThrottleFilterBlockdev(virDomainThrottleFilterDef *filter, + qemuDomainObjPrivate *priv); + +int +qemuDomainPrepareThrottleFilterBlockdevNodename(virDomainThrottleFilterDef *filter, + const char *nodenameprefix); + void qemuDomainCleanupAdd(virDomainObj *vm, qemuDomainCleanupCallback cb); void qemuDomainCleanupRemove(virDomainObj *vm, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f7e435d6d5..60989ae693 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14828,6 +14828,12 @@ qemuDomainDiskBlockIoTuneIsSupported(virDomainDiskDef *disk) return false; } + if (disk->throttlefilters) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("block 'iotune' can't be used together with 'throttlefilters' for disk '%1$s'"), disk->dst); + return false; + } + return true; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4a3f4f657e..103b9e9f00 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -657,6 +657,7 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, virDomainAsyncJob asyncJob) { g_autoptr(qemuBlockStorageSourceChainData) data = NULL; + g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL; qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(virJSONValue) devprops = NULL; bool extensionDeviceAttached = false; @@ -695,6 +696,19 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, if (rc < 0) goto rollback; + /* Setup throttling filters + * add additional "blockdev-add"(throttle filter) between "blockdev-add" (qemuBlockStorageSourceChainAttach) and "device_add" (qemuDomainAttachExtensionDevice) + */ + if ((filterData = qemuBuildThrottleFiltersAttachPrepareBlockdev(disk))) { + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) + return -1; + /* QMP requests("blockdev-add" with "driver":"throttle") to QEMU */ + rc = qemuBlockThrottleFiltersAttach(priv->mon, filterData); + qemuDomainObjExitMonitor(vm); + if (rc < 0) + goto rollback; + } + if (disk->transient) { g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL; g_autoptr(GHashTable) blockNamedNodeData = NULL; @@ -766,6 +780,8 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, if (extensionDeviceAttached) ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info)); + qemuBlockThrottleFiltersDetach(priv->mon, filterData); + qemuBlockStorageSourceChainDetach(priv->mon, data); qemuDomainObjExitMonitor(vm); @@ -921,6 +937,14 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, bool releaseSeclabel = false; int ret = -1; + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { + if (disk->nthrottlefilters > 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("cdrom device with throttle filters isn't supported")); + return -1; + } + } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("floppy device hotplug isn't supported")); @@ -4499,6 +4523,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver, { qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); g_autoptr(qemuBlockStorageSourceChainData) diskBackend = NULL; + g_autoptr(qemuBlockThrottleFiltersData) filterData = NULL; size_t i; qemuDomainObjPrivate *priv = vm->privateData; int ret = -1; @@ -4537,6 +4562,14 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver, } } + qemuDomainObjEnterMonitor(vm); + /* QMP request("blockdev-del") to QEMU to delete throttle filter*/ + if ((filterData = qemuBuildThrottleFiltersDetachPrepareBlockdev(disk))) { + /* "qemuBlockThrottleFiltersDetach" is used in rollback within "qemuDomainAttachDiskGeneric" as well */ + qemuBlockThrottleFiltersDetach(priv->mon, filterData); + } + qemuDomainObjExitMonitor(vm); + qemuDomainObjEnterMonitor(vm); if (diskBackend) -- 2.34.1

On 2024/8/9 22:04, Peter Krempa wrote: Filter node name is generated by rule "libvirt-nodenameindex-filter" in method "qemuDomainPrepareThrottleFilterBlockdev", which is called by "qemuDomainPrepareDiskSourceBlockdev". it follows the same way like "libvirt-nodenameindex-format" node. And I tested restarting libvirtd, vm with throttle works well in this case. After updating qemuBuildThrottleFiltersAttachPrepareBlockdev and qemuDomainDiskGetTopNodename, order can be adjusted to be "device -> throttle-> copyOnRead Layer-> image chain", and throttle works with copyOnRead, but I am also considering your suggestion about per-storage-source regarding per-storage-source, do you mean xml like below? if so, when specifying "--throttle-groups" in "attach-disk" it apply groups on top source(vm1_disk_2.qcow2) only? is there scenario to apply throttle-groups on backing store source?   <disk type='file' device='disk'>       <driver name='qemu' type='qcow2'/>       <source file='/virt/disks/vm1_disk_2.qcow2' index='1'>           <throttlefilters>             <throttlefilter group='limit0'/>           </throttlefilters>       </source>       <backingStore type='file' index='4'>         <format type='qcow2'/>         <source file='/virt/disks/backing.qcow2'>           <throttlefilters>             <throttlefilter group='limit1'/>           </throttlefilters>         </source>         <backingStore/>       </backingStore>       <target dev='vdc' bus='virtio'/>       <alias name='virtio-disk2'/>       <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>    </disk> -- Thanks and Regards, Wu

On 2024/8/16 23:14, Peter Krempa wrote: sure, then let's stick to current implementation about filter chain(per-disk), about status XML, do you mean <privateData> to store nodename? if so, does the following xml look good to you?   <throttlefilters>     <throttlefilter  group='limit2'>       <privateData>         <nodename type='throttle-filter' name='0123456789ABCDEF0123456789ABCDE'/>       </privateData>     </throttlefilter>   </throttlefilters> -- Thanks and Regards, Wu

On Wed, Jun 12, 2024 at 03:02:19 -0700, wucf(a)linux.ibm.com wrote: ... this is the validation step ... (see below). This valdiation should be also added all together with all the other validation bits that I've seen in other patches and not split randomly. Not used anywhere else, avoid export. coding style. This belongs to the validation step and not to the command line building step. Also same comment as before about not using internal error and internal flag names, which are meaningless to the users. [...]

On Wed, Jun 12, 2024 at 03:02:20 -0700, wucf(a)linux.ibm.com wrote: As noted, please unify all the random bits of validation into single patch. As noted this should be done all at once in one patch. Also as noted I strongly prefer if this is done along with the images and not here. Also as noted before this is ordered wrong for how this is supposed to work as you propose as here the 'copyOnRead' layer is already built.

On Wed, Jun 12, 2024 at 03:02:21 -0700, wucf(a)linux.ibm.com wrote: [...] Please add also one more complex example of a disk referencing multiple groups. Otherwise this'd be identical to what we already support. For completness you can also add one group that is not used by any disk. I also expect at least one negative test case showing how the mixing of throttling via groups and directly you explicitly forbid is handled so that we don't regress in that regard.

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; * Add new cmds: throttlegroupset, throttlegrouplist, throttlegroupinfo, throttlegroupdel Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; --- tools/virsh-domain.c | 428 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 428 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 50e80689a2..f84a65451a 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1509,6 +1509,410 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto cleanup; } + +/* + * "throttlegrouplist" command + */ +static const vshCmdInfo info_throttlegrouplist = { + .help = N_("list all domain throttlegroups."), + .desc = N_("Get the summary of throttle groups for a domain."), +}; + + +static const vshCmdOptDef opts_throttlegrouplist[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "inactive", + .type = VSH_OT_BOOL, + .help = N_("get inactive rather than running configuration") + }, + {.name = NULL} +}; + + +static bool +cmdThrottleGroupList(vshControl *ctl, + const vshCmd *cmd) +{ + unsigned int flags = 0; + size_t i; + g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + g_autofree xmlNodePtr *groups = NULL; + ssize_t ngroups; + g_autoptr(vshTable) table = NULL; + + if (vshCommandOptBool(cmd, "inactive")) + flags |= VIR_DOMAIN_XML_INACTIVE; + + if (virshDomainGetXML(ctl, cmd, flags, &xml, &ctxt) < 0) + return false; + + ngroups = virXPathNodeSet("./throttlegroups/throttlegroup", ctxt, &groups); + if (ngroups < 0) + return false; + + table = vshTableNew(_("Name"), NULL); + + if (!table) + return false; + + for (i = 0; i < ngroups; i++) { + g_autofree char *name = NULL; + ctxt->node = groups[i]; + name = virXPathString("string(./group_name)", ctxt); + if (vshTableRowAppend(table, name, NULL) < 0) + return false; + } + + vshTablePrintToStdout(table, ctl); + + return true; +} + + +/* + * "throttlegroupset" command + */ +static const vshCmdInfo info_throttlegroupset = { + .help = N_("Add or update a throttling group."), + .desc = N_("Add or updte a throttling group."), +}; + + +static const vshCmdOptDef opts_throttlegroupset[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "group-name", + .type = VSH_OT_STRING, + .positional = true, + .required = true, + .completer = virshCompleteEmpty, + .help = N_("throttle group name") + }, + {.name = "total-bytes-sec", + .type = VSH_OT_INT, + .help = N_("total throughput limit, as scaled integer (default bytes)") + }, + {.name = "read-bytes-sec", + .type = VSH_OT_INT, + .help = N_("read throughput limit, as scaled integer (default bytes)") + }, + {.name = "write-bytes-sec", + .type = VSH_OT_INT, + .help = N_("write throughput limit, as scaled integer (default bytes)") + }, + {.name = "total-iops-sec", + .type = VSH_OT_INT, + .help = N_("total I/O operations limit per second") + }, + {.name = "read-iops-sec", + .type = VSH_OT_INT, + .help = N_("read I/O operations limit per second") + }, + {.name = "write-iops-sec", + .type = VSH_OT_INT, + .help = N_("write I/O operations limit per second") + }, + {.name = "total-bytes-sec-max", + .type = VSH_OT_INT, + .help = N_("total max, as scaled integer (default bytes)") + }, + {.name = "read-bytes-sec-max", + .type = VSH_OT_INT, + .help = N_("read max, as scaled integer (default bytes)") + }, + {.name = "write-bytes-sec-max", + .type = VSH_OT_INT, + .help = N_("write max, as scaled integer (default bytes)") + }, + {.name = "total-iops-sec-max", + .type = VSH_OT_INT, + .help = N_("total I/O operations max") + }, + {.name = "read-iops-sec-max", + .type = VSH_OT_INT, + .help = N_("read I/O operations max") + }, + {.name = "write-iops-sec-max", + .type = VSH_OT_INT, + .help = N_("write I/O operations max") + }, + {.name = "size-iops-sec", + .type = VSH_OT_INT, + .help = N_("I/O size in bytes") + }, + {.name = "total-bytes-sec-max-length", + .type = VSH_OT_INT, + .help = N_("duration in seconds to allow total max bytes") + }, + {.name = "read-bytes-sec-max-length", + .type = VSH_OT_INT, + .help = N_("duration in seconds to allow read max bytes") + }, + {.name = "write-bytes-sec-max-length", + .type = VSH_OT_INT, + .help = N_("duration in seconds to allow write max bytes") + }, + {.name = "total-iops-sec-max-length", + .type = VSH_OT_INT, + .help = N_("duration in seconds to allow total I/O operations max") + }, + {.name = "read-iops-sec-max-length", + .type = VSH_OT_INT, + .help = N_("duration in seconds to allow read I/O operations max") + }, + {.name = "write-iops-sec-max-length", + .type = VSH_OT_INT, + .help = N_("duration in seconds to allow write I/O operations max") + }, + VIRSH_COMMON_OPT_DOMAIN_CONFIG, + VIRSH_COMMON_OPT_DOMAIN_LIVE, + VIRSH_COMMON_OPT_DOMAIN_CURRENT, + {.name = NULL} +}; + + +static bool +cmdThrottleGroupSet(vshControl *ctl, + const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + const char *name; + const char *group_name = NULL; + unsigned long long value; + int nparams = 0; + int maxparams = 0; + virTypedParameterPtr params = NULL; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + int rv = 0; + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool ret = false; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) + goto cleanup; + + +#define VSH_SET_THROTTLE_GROUP_SCALED(PARAM, CONST) \ + if ((rv = vshCommandOptScaledInt(ctl, cmd, #PARAM, &value, \ + 1, ULLONG_MAX)) < 0) { \ + goto interror; \ + } else if (rv > 0) { \ + if (virTypedParamsAddULLong(&params, &nparams, &maxparams, \ + VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \ + value) < 0) \ + goto save_error; \ + } + + VSH_SET_THROTTLE_GROUP_SCALED(total-bytes-sec, TOTAL_BYTES_SEC); + VSH_SET_THROTTLE_GROUP_SCALED(read-bytes-sec, READ_BYTES_SEC); + VSH_SET_THROTTLE_GROUP_SCALED(write-bytes-sec, WRITE_BYTES_SEC); + VSH_SET_THROTTLE_GROUP_SCALED(total-bytes-sec-max, TOTAL_BYTES_SEC_MAX); + VSH_SET_THROTTLE_GROUP_SCALED(read-bytes-sec-max, READ_BYTES_SEC_MAX); + VSH_SET_THROTTLE_GROUP_SCALED(write-bytes-sec-max, WRITE_BYTES_SEC_MAX); +#undef VSH_SET_THROTTLE_GROUP_SCALED + +#define VSH_SET_THROTTLE_GROUP(PARAM, CONST) \ + if ((rv = vshCommandOptULongLong(ctl, cmd, #PARAM, &value)) < 0) { \ + goto interror; \ + } else if (rv > 0) { \ + if (virTypedParamsAddULLong(&params, &nparams, &maxparams, \ + VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \ + value) < 0) \ + goto save_error; \ + } + + VSH_SET_THROTTLE_GROUP(total-iops-sec, TOTAL_IOPS_SEC); + VSH_SET_THROTTLE_GROUP(read-iops-sec, READ_IOPS_SEC); + VSH_SET_THROTTLE_GROUP(write-iops-sec, WRITE_IOPS_SEC); + VSH_SET_THROTTLE_GROUP(total-iops-sec-max, TOTAL_IOPS_SEC_MAX); + VSH_SET_THROTTLE_GROUP(read-iops-sec-max, READ_IOPS_SEC_MAX); + VSH_SET_THROTTLE_GROUP(write-iops-sec-max, WRITE_IOPS_SEC_MAX); + VSH_SET_THROTTLE_GROUP(size-iops-sec, SIZE_IOPS_SEC); + + VSH_SET_THROTTLE_GROUP(total-bytes-sec-max-length, TOTAL_BYTES_SEC_MAX_LENGTH); + VSH_SET_THROTTLE_GROUP(read-bytes-sec-max-length, READ_BYTES_SEC_MAX_LENGTH); + VSH_SET_THROTTLE_GROUP(write-bytes-sec-max-length, WRITE_BYTES_SEC_MAX_LENGTH); + VSH_SET_THROTTLE_GROUP(total-iops-sec-max-length, TOTAL_IOPS_SEC_MAX_LENGTH); + VSH_SET_THROTTLE_GROUP(read-iops-sec-max-length, READ_IOPS_SEC_MAX_LENGTH); + VSH_SET_THROTTLE_GROUP(write-iops-sec-max-length, WRITE_IOPS_SEC_MAX_LENGTH); +#undef VSH_SET_THROTTLE_GROUP + + if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) { + goto cleanup; + } + + if (group_name) { + if (virTypedParamsAddString(&params, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, + group_name) < 0) + goto save_error; + } + + if (virDomainSetThrottleGroup(dom, group_name, params, nparams, flags) < 0) + goto error; + vshPrintExtra(ctl, "%s", _("Throttle group set successfully\n")); + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + return ret; + + save_error: + vshSaveLibvirtError(); + error: + vshError(ctl, "%s", _("Unable to change throttle group")); + goto cleanup; + + interror: + vshError(ctl, "%s", _("Unable to parse integer parameter")); + goto cleanup; +} + + +/* + * "throttlegroupdel" command + */ +static const vshCmdInfo info_throttlegroupdel = { + .help = N_("Delete a throttling group."), + .desc = N_("Delete a throttling group."), +}; + + +static const vshCmdOptDef opts_throttlegroupdel[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "group-name", + .type = VSH_OT_STRING, + .positional = true, + .required = true, + .completer = virshCompleteEmpty, + .help = N_("throttle group name") + }, + VIRSH_COMMON_OPT_DOMAIN_CONFIG, + VIRSH_COMMON_OPT_DOMAIN_LIVE, + VIRSH_COMMON_OPT_DOMAIN_CURRENT, + {.name = NULL} +}; + + +static bool +cmdThrottleGroupDel(vshControl *ctl, + const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + const char *group_name = NULL; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) { + return false; + } + + if (virDomainDelThrottleGroup(dom, group_name, flags) < 0) + return false; + vshPrintExtra(ctl, "%s", _("Throttle group deleted successfully\n")); + + return true; +} + + +/* + * "throttlegroupinfo" command + */ +static const vshCmdInfo info_throttlegroupinfo = { + .help = N_("Get a throttling group."), + .desc = N_("Get a throttling group."), +}; + + +static const vshCmdOptDef opts_throttlegroupinfo[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "group-name", + .type = VSH_OT_STRING, + .positional = true, + .required = true, + .completer = virshCompleteEmpty, + .help = N_("throttle group name") + }, + VIRSH_COMMON_OPT_DOMAIN_CONFIG, + VIRSH_COMMON_OPT_DOMAIN_LIVE, + VIRSH_COMMON_OPT_DOMAIN_CURRENT, + {.name = NULL} +}; + + +static bool +cmdThrottleGroupInfo(vshControl *ctl, + const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + const char *name; + const char *group_name = NULL; + int nparams = 0; + virTypedParameterPtr params = NULL; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + size_t i; + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool ret = false; + + VSH_EXCLUSIVE_OPTIONS_VAR(current, live); + VSH_EXCLUSIVE_OPTIONS_VAR(current, config); + + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + + if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) + goto cleanup; + + if (vshCommandOptString(ctl, cmd, "group-name", &group_name) < 0) { + goto cleanup; + } + + if (virDomainGetThrottleGroup(dom, group_name, &params, &nparams, flags) != 0) { + vshError(ctl, "%s", + _("Unable to get throttle group parameters")); + goto cleanup; + } + + for (i = 0; i < nparams; i++) { + g_autofree char *str = vshGetTypedParamValue(ctl, &params[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + } + + ret = true; + + cleanup: + virTypedParamsFree(params, nparams); + return ret; +} + + /* * "blkiotune" command */ @@ -13378,6 +13782,30 @@ const vshCmdDef domManagementCmds[] = { .info = &info_blkdeviotune, .flags = 0 }, + {.name = "throttlegroupset", + .handler = cmdThrottleGroupSet, + .opts = opts_throttlegroupset, + .info = &info_throttlegroupset, + .flags = 0 + }, + {.name = "throttlegroupdel", + .handler = cmdThrottleGroupDel, + .opts = opts_throttlegroupdel, + .info = &info_throttlegroupdel, + .flags = 0 + }, + {.name = "throttlegroupinfo", + .handler = cmdThrottleGroupInfo, + .opts = opts_throttlegroupinfo, + .info = &info_throttlegroupinfo, + .flags = 0 + }, + {.name = "throttlegrouplist", + .handler = cmdThrottleGroupList, + .opts = opts_throttlegrouplist, + .info = &info_throttlegrouplist, + .flags = 0 + }, {.name = "blkiotune", .handler = cmdBlkiotune, .opts = opts_blkiotune, -- 2.34.1

From: Chun Feng Wu <wucf(a)linux.ibm.com&gt; * Update "attach_disk" to support new option: throttle-groups to form filter chain in QEMU for specific disk Signed-off-by: Chun Feng Wu <wucf(a)linux.ibm.com&gt; --- tools/virsh-completer-domain.c | 64 ++++++++++++++++++++++++++++++++++ tools/virsh-completer-domain.h | 5 +++ tools/virsh-domain.c | 25 ++++++++++++- 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 61362224a3..000cf65c99 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -248,6 +248,70 @@ virshDomainMigrateDisksCompleter(vshControl *ctl, } +static char ** +virshDomainThrottleGroupCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + g_autoptr(xmlDoc) xmldoc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + g_autofree xmlNodePtr *groups = NULL; + int ngroups; + size_t i; + g_auto(GStrv) tmp = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0) + return NULL; + + ngroups = virXPathNodeSet("./throttlegroups/throttlegroup", ctxt, &groups); + if (ngroups < 0) + return NULL; + + tmp = g_new0(char *, ngroups + 1); + + for (i = 0; i < ngroups; i++) { + ctxt->node = groups[i]; + if (!(tmp[i] = virXPathString("string(./group_name)", ctxt))) + return NULL; + } + + return g_steal_pointer(&tmp); +} + + +static char ** +virshDomainThrottleGroupListCompleter(vshControl *ctl, + const vshCmd *cmd, + const char *argname) +{ + const char *curval = NULL; + g_auto(GStrv) groups = virshDomainThrottleGroupCompleter(ctl, cmd, 0); + + if (vshCommandOptStringQuiet(ctl, cmd, argname, &curval) < 0) + return NULL; + + if (!groups) + return NULL; + + return virshCommaStringListComplete(curval, (const char **) groups); +} + + +char ** +virshDomainThrottleGroupsCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int completeflags G_GNUC_UNUSED) +{ + return virshDomainThrottleGroupListCompleter(ctl, cmd, "throttle-groups"); +} + + char ** virshDomainUndefineStorageDisksCompleter(vshControl *ctl, const vshCmd *cmd, diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 27cf963912..95ec3e28df 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -41,6 +41,11 @@ virshDomainDiskTargetCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +char ** +virshDomainThrottleGroupsCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int completeflags); + char ** virshDomainInterfaceStateCompleter(vshControl *ctl, const vshCmd *cmd, diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f84a65451a..ec2330c8dd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -522,6 +522,11 @@ static const vshCmdOptDef opts_attach_disk[] = { .type = VSH_OT_STRING, .help = N_("host socket for source of disk device") }, + {.name = "throttle-groups", + .type = VSH_OT_STRING, + .completer = virshDomainThrottleGroupsCompleter, + .help = N_("comma separated list of throttle groups to be applied") + }, VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, VIRSH_COMMON_OPT_DOMAIN_CONFIG, VIRSH_COMMON_OPT_DOMAIN_LIVE, @@ -611,6 +616,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) const char *host_name = NULL; const char *host_transport = NULL; const char *host_socket = NULL; + const char *throttle_groups_str = NULL; + g_autofree char **throttle_groups = NULL; int ret; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; const char *stype = NULL; @@ -622,6 +629,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) g_auto(virBuffer) sourceAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) sourceChildBuf = VIR_BUFFER_INIT_CHILD(&diskChildBuf); g_auto(virBuffer) hostAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) throttleChildBuf = VIR_BUFFER_INITIALIZER; g_autofree char *xml = NULL; struct stat st; bool current = vshCommandOptBool(cmd, "current"); @@ -665,9 +673,14 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptString(ctl, cmd, "source-protocol", &source_protocol) < 0 || vshCommandOptString(ctl, cmd, "source-host-name", &host_name) < 0 || vshCommandOptString(ctl, cmd, "source-host-transport", &host_transport) < 0 || - vshCommandOptString(ctl, cmd, "source-host-socket", &host_socket) < 0) + vshCommandOptString(ctl, cmd, "source-host-socket", &host_socket) < 0 || + vshCommandOptString(ctl, cmd, "throttle-groups", &throttle_groups_str) < 0) return false; + if (throttle_groups_str) { + throttle_groups = g_strsplit(throttle_groups_str, ",", 0); + } + if (stype && (type = virshAttachDiskSourceTypeFromString(stype)) < 0) { vshError(ctl, _("Unknown source type: '%1$s'"), stype); @@ -714,6 +727,16 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virXMLFormatElement(&diskChildBuf, "driver", &driverAttrBuf, NULL); + if (throttle_groups) { + char **iter; + for (iter = throttle_groups; *iter != NULL; iter++) { + g_auto(virBuffer) throttleAttrBuf = VIR_BUFFER_INITIALIZER; + virBufferAsprintf(&throttleAttrBuf, " group='%s'", *iter); + virXMLFormatElement(&throttleChildBuf, "throttlefilter", &throttleAttrBuf, NULL); + } + virXMLFormatElement(&diskChildBuf, "throttlefilters", NULL, &throttleChildBuf); + } + switch ((enum virshAttachDiskSourceType) type) { case VIRSH_ATTACH_DISK_SOURCE_TYPE_FILE: virBufferEscapeString(&sourceAttrBuf, " file='%s'", source); -- 2.34.1