From: Chun Feng Wu <danielwuwy@163.com>
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 <danielwuwy@163.com>
---
include/libvirt/libvirt-domain.h | 21 ++++
src/driver-hypervisor.h | 22 ++++
src/libvirt-domain.c | 174 ++++++++++++++++++++++++++++
src/libvirt_private.syms | 8 ++
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, 391 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 9232ce2e6b..7ff1e50ef5 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -6560,4 +6560,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..ce18d18854 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -14162,3 +14162,177 @@ 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.7.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
+ *
+ * Get all block IO tunable parameters for specific throttle group. @group cannot be NULL.
+ * @nparams gives how many slots were filled with parameter information
+ * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG.
+ * Both flags may be set.
+ *
+ *
+ * Returns -1 in case of error, 0 in case of success.
+ *
+ * Since: 10.7.0
+ */
+int
+virDomainGetThrottleGroup(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) ? *nparams : -1, flags);
+
+ virResetLastError();
+
+ virCheckDomainReturn(dom, -1);
+ virCheckNonNullArgGoto(group, error);
+ virCheckNonNullArgGoto(nparams, error);
+ virCheckNonNullArgGoto(params, 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
+ *
+ * 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.7.0
+ */
+int
+virDomainDelThrottleGroup(virDomainPtr dom,
+ const char *group,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+
+ virResetLastError();
+
+ virCheckDomainReturn(dom, -1);
+ virCheckNonNullArgGoto(group, 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 5b9b44ef96..9727495f3d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -677,6 +677,14 @@ virDomainTaintMessageTypeFromString;
virDomainTaintMessageTypeToString;
virDomainTaintTypeFromString;
virDomainTaintTypeToString;
+virDomainThrottleFilterDefFree;
+virDomainThrottleFilterFind;
+virDomainThrottleGroupAdd;
+virDomainThrottleGroupByName;
+virDomainThrottleGroupDefCopy;
+virDomainThrottleGroupDefFree;
+virDomainThrottleGroupDel;
+virDomainThrottleGroupUpdate;
virDomainTimerModeTypeFromString;
virDomainTimerModeTypeToString;
virDomainTimerNameTypeFromString;
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 7a3492d9d7..f17b6feaef 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.7.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 e812f5c3e9..5d5f286203 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -3618,6 +3618,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,
+ ¶ms, &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..c757156e24 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.7.0 */
+ .domainGetThrottleGroup = remoteDomainGetThrottleGroup, /* 10.7.0 */
+ .domainDelThrottleGroup = remoteDomainDelThrottleGroup, /* 10.7.0 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 41c045ff78..02338b9f02 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:write
+ */
+ 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.39.5 (Apple Git-154)On Mon, Nov 18, 2024 at 19:24:15 +0530, Harikumar R wrote:
> From: Chun Feng Wu <danielwuwy@163.com>
>
> 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 <danielwuwy@163.com>
> ---
> include/libvirt/libvirt-domain.h | 21 ++++
> src/driver-hypervisor.h | 22 ++++
> src/libvirt-domain.c | 174 ++++++++++++++++++++++++++++
> src/libvirt_private.syms | 8 ++
> 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, 391 insertions(+), 1 deletion(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 9232ce2e6b..7ff1e50ef5 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -6560,4 +6560,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);
I'm not exactly a fan of 'getter' functions for configuration which is
also present in the XML, but since we did these historically I'm okay
with this.
[...]
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 7c6b93963c..ce18d18854 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -14162,3 +14162,177 @@ 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.
I don't think we should do the 'change a subset of' semantics any more.
It was proven that it doesn't make sense. Specifically it's not possible
to distinguish between "I don't want to change this field" and "I want
to delete/revert to default for this field".
The users must be instructed to always post the full required config.
> + *
> + * 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.7.0
11.1.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
> + *
> + * Get all block IO tunable parameters for specific throttle group. @group cannot be NULL.
> + * @nparams gives how many slots were filled with parameter information
> + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG.
> + * Both flags may be set.
For getter APIs you can't set both because there's no way the API can
return both configs at once.
> + *
> + *
> + * Returns -1 in case of error, 0 in case of success.
> + *
> + * Since: 10.7.0
> + */
> +int
> +virDomainGetThrottleGroup(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) ? *nparams : -1, flags);
The actual value of 'nparams' at call doesn't matter, thus makes no
sense to be logged. At the same time, 'group' which is a much more
useful info is not logged.
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(dom, -1);
> + virCheckNonNullArgGoto(group, error);
> + virCheckNonNullArgGoto(nparams, error);
> + virCheckNonNullArgGoto(params, 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
> + *
> + * 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.7.0
> + */
> +int
> +virDomainDelThrottleGroup(virDomainPtr dom,
> + const char *group,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(dom, -1);
> + virCheckNonNullArgGoto(group, error);
> +
[...]
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 7a3492d9d7..f17b6feaef 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.7.0 {
This also must correspond to the proper version and this one can't be
fixed later.
> + 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 e812f5c3e9..5d5f286203 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -3618,6 +3618,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,
This is a weird construction, 'group' is supposed to be a nonnull
string, so why should it ever be NULL?
> + ¶ms, &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..c757156e24 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;
Spacing broken. Also 'group' is a mandatory argument, so why it's being
checked? This line makes no sense to me.
> + 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) {
Alignment of args seems to be broken.
> + 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.7.0 */
> + .domainGetThrottleGroup = remoteDomainGetThrottleGroup, /* 10.7.0 */
> + .domainDelThrottleGroup = remoteDomainDelThrottleGroup, /* 10.7.0 */
11.1.0
> };
>
> static virNetworkDriver network_driver = {
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 41c045ff78..02338b9f02 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:write
Deletion of the throttle group needs to have same ACLs as the SET, thus
also the 'save' perms need to be copied.
> + */
> + REMOTE_PROC_DOMAIN_DEL_THROTTLE_GROUP = 451
> };
> On Mon, Nov 18, 2024 at 19:24:15 +0530, Harikumar R wrote: > I don't think we should do the 'change a subset of' semantics any more. > It was proven that it doesn't make sense. Specifically it's not possible > to distinguish between "I don't want to change this field" and "I want > to delete/revert to default for this field". > > The users must be instructed to always post the full required config. If the customer provides only a subset of the throttle group, the other fields will remain unchanged. Deleting/reverting a field may cause data mismatches between the XML and QEMU. We will instruct users to always provide the full required configuration. Can we have a validation to check the config?
On Fri, Feb 07, 2025 at 04:59:49 -0000, Harikumar Rajkumar wrote:
> > On Mon, Nov 18, 2024 at 19:24:15 +0530, Harikumar R wrote:
> > I don't think we should do the 'change a subset of' semantics any more.
> > It was proven that it doesn't make sense. Specifically it's not possible
> > to distinguish between "I don't want to change this field" and "I want
> > to delete/revert to default for this field".
> >
> > The users must be instructed to always post the full required config.
>
> If the customer provides only a subset of the throttle group,
> the other fields will remain unchanged.
First let's focus on the libvirt API design:
(deliberately let's not compare this to the old design)
You have the XML configuration, in which if a filed is omitted the
default value will be assumed.
Now if the API is designed so that omitted fields are not touched you
don't have a way to actually return the configuration to the "default
value" via the API. The users then have to edit the XML and restart the
VM to achieve that config.
Thus the only solution where the setter API allows full range of setting
is the one which fully replaces the configuration with the new fields.
This is also something we started to gravitate towards in the API
backing 'virsh update-device'. Originally it tried to not change
settings that were omitted in the input XML but that simply didn't work
for certain thigns and especially for returning to defualt settings.
>Deleting/reverting a field may cause data mismatches between the XML and QEMU.
This must not happen. Libvirt must always update everything so that the
QEMU config is identical to what is requested in the XML.
It is an implementation detail how that is achieved, based on what qemu
provides:
- explicitly request qemu to revert to default
- query the default and set the same value as default
- refuse to do something that is not possible
> We will instruct users to always provide the full required configuration.
The API needs to allow a way that overwrites the full config including a
way to return to the default.
That said you can have a flag which switches the behaviour to "update
fields which were provided to the provided value and leave everything
else untouched" if that is practical to achieve
> Can we have a validation to check the config?
I don't follow which you mean here. As said, if something is not
possible to set in qemu if the API allows it, it needs to be rejected,
so that the state doesn't desync between qemu and libvirt.
> On Fri, Feb 07, 2025 at 04:59:49 -0000, Harikumar Rajkumar wrote: > It is an implementation detail how that is achieved, based on what qemu > provides: > - explicitly request qemu to revert to default > - query the default and set the same value as default > - refuse to do something that is not possible > > > The API needs to allow a way that overwrites the full config including a > way to return to the default. We have changed the implementation to allowing full config SET. Below six fields are set to default 1 by QEMU if omitted by user, are you suggesting to store this default value in XML by query the default in QEMU while SET? total_bytes_sec_max_length: 1 read_bytes_sec_max_length: 1 write_bytes_sec_max_length: 1 total_iops_sec_max_length: 1 read_iops_sec_max_length: 1 write_iops_sec_max_length: 1
On Mon, Feb 10, 2025 at 11:16:54 -0000, Harikumar Rajkumar wrote: > > On Fri, Feb 07, 2025 at 04:59:49 -0000, Harikumar Rajkumar wrote: > > It is an implementation detail how that is achieved, based on what qemu > > provides: > > - explicitly request qemu to revert to default > > - query the default and set the same value as default > > - refuse to do something that is not possible > > > > > > The API needs to allow a way that overwrites the full config including a > > way to return to the default. > > We have changed the implementation to allowing full config SET. > > Below six fields are set to default 1 by QEMU if omitted by user, are you suggesting to store this default value in XML by query the default in QEMU while SET? Not necessarily. The issue is that if you set a non-default value you'll most likely won't be able to query the original state. This'd require us to instantiate the object with defaults and then set it via monitor during startup. While possible that'll overcomplicate things most likely. > total_bytes_sec_max_length: 1 > read_bytes_sec_max_length: 1 > write_bytes_sec_max_length: 1 > total_iops_sec_max_length: 1 > read_iops_sec_max_length: 1 > write_iops_sec_max_length: 1 Ideally; if you want to express the full power of the API you need a way to re-set this to default in qemu. I didn't have a look if it's possible to do without querying the default first though. Either way, for the initial impl you can also implement a check that will require the users to provide a value if it already was non-default. I just want to leave the possibility open for the API to do full reset semantics if somebody will be willing to implement them.
© 2016 - 2026 Red Hat, Inc.