[libvirt] [PATCHv4 0/6] add numatune command

Not quite polished - see my complaints that will require a v5: https://www.redhat.com/archives/libvir-list/2011-December/msg00843.html I'm hoping Hu can help cover the remaining points, as I've already spent quite a bit of time cleaning it up to here. Hu Tao (6): Add functions to set/get cgroup cpuset parameters use cpuset to manage numa add new API virDomain{G, S}etNumaParameters Add virDomain{G, S}etNumaParameters support to the remote driver Implement virDomain{G, S}etNumaParameters for the qemu driver add new command numatune to virsh daemon/remote.c | 64 ++++++++++ include/libvirt/libvirt.h.in | 39 ++++++ python/generator.py | 2 + src/conf/domain_conf.h | 8 -- src/driver.h | 15 +++ src/libvirt.c | 129 +++++++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 6 + src/qemu/qemu_cgroup.c | 19 +++ src/qemu/qemu_driver.c | 262 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 50 ++++++++ src/remote/remote_protocol.x | 24 ++++- src/remote_protocol-structs | 22 ++++ src/util/cgroup.c | 32 +++++ src/util/cgroup.h | 3 + tools/virsh.c | 159 +++++++++++++++++++++++++ tools/virsh.pod | 19 +++ 17 files changed, 846 insertions(+), 9 deletions(-) -- 1.7.7.4

From: Hu Tao <hutao@cn.fujitsu.com> --- src/libvirt_private.syms | 2 ++ src/util/cgroup.c | 32 ++++++++++++++++++++++++++++++++ src/util/cgroup.h | 3 +++ 3 files changed, 37 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a649054..4a86bdc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -78,6 +78,7 @@ virCgroupGetCpuShares; virCgroupGetCpuCfsPeriod; virCgroupGetCpuCfsQuota; virCgroupGetCpuacctUsage; +virCgroupGetCpusetMems; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; @@ -94,6 +95,7 @@ virCgroupSetBlkioWeight; virCgroupSetCpuShares; virCgroupSetCpuCfsPeriod; virCgroupSetCpuCfsQuota; +virCgroupSetCpusetMems; virCgroupSetFreezerState; virCgroupSetMemory; virCgroupSetMemoryHardLimit; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b4d3d8b..25f2691 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -1205,6 +1205,38 @@ int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) } /** + * virCgroupSetCpusetMems: + * + * @group: The cgroup to set cpuset.mems for + * @mems: the numa nodes to set + * + * Returns: 0 on success + */ +int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems) +{ + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.mems", + mems); +} + +/** + * virCgroupGetCpusetMems: + * + * @group: The cgroup to get cpuset.mems for + * @mems: the numa nodes to get + * + * Returns: 0 on success + */ +int virCgroupGetCpusetMems(virCgroupPtr group, char **mems) +{ + return virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.mems", + mems); +} + +/** * virCgroupDenyAllDevices: * * @group: The cgroup to deny all permissions, for all devices diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 70dd392..8d75735 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -119,6 +119,9 @@ int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage); int virCgroupSetFreezerState(virCgroupPtr group, const char *state); int virCgroupGetFreezerState(virCgroupPtr group, char **state); +int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems); +int virCgroupGetCpusetMems(virCgroupPtr group, char **mems); + int virCgroupRemove(virCgroupPtr group); void virCgroupFree(virCgroupPtr *group); -- 1.7.7.4

From: Hu Tao <hutao@cn.fujitsu.com> This patch also sets cgroup cpuset parameters for numatune. --- src/qemu/qemu_cgroup.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index d663798..2d970d6 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -389,6 +389,25 @@ int qemuSetupCgroup(struct qemud_driver *driver, } } + if (vm->def->numatune.memory.nodemask && + vm->def->numatune.memory.mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT && + qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + char *mask = virDomainCpuSetFormat(vm->def->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN); + if (!mask) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to convert memory nodemask")); + goto cleanup; + } + + rc = virCgroupSetCpusetMems(cgroup, mask); + VIR_FREE(mask); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to set cpuset.mems for domain %s"), + vm->def->name); + goto cleanup; + } + } done: virCgroupFree(&cgroup); return 0; -- 1.7.7.4

From: Hu Tao <hutao@cn.fujitsu.com> Set up the types for the numa functions and insert them into the virDriver structure definition. --- include/libvirt/libvirt.h.in | 39 +++++++++++++ python/generator.py | 2 + src/conf/domain_conf.h | 8 --- src/driver.h | 15 +++++ src/libvirt.c | 129 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 6 files changed, 191 insertions(+), 8 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2480add..7f26521 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1344,6 +1344,45 @@ typedef enum { } virDomainMemoryModFlags; +/* Manage numa parameters */ + +/** + * virDomainNumatuneMemMode: + * Representation of the various modes in the <numatune> element of + * a domain. + */ +typedef enum { + VIR_DOMAIN_NUMATUNE_MEM_STRICT = 0, + VIR_DOMAIN_NUMATUNE_MEM_PREFERRED = 1, + VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE = 2, + + /* This constant is subject to change */ + VIR_DOMAIN_NUMATUNE_MEM_LAST +} virDomainNumatuneMemMode; + +/** + * VIR_DOMAIN_NUMA_NODESET: + * + * Macro for typed parameter name that lists the numa nodeset of a + * domain, as a string. + */ +#define VIR_DOMAIN_NUMA_NODESET "numa_nodeset" + +/** + * VIR_DOMAIN_NUMA_MODE: + * + * Macro for typed parameter name that lists the numa mode of a domain, + * as an int containing a virDomainNumatuneMemMode value. + */ +#define VIR_DOMAIN_NUMA_MODE "numa_mode" + +int virDomainSetNumaParameters(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, unsigned int flags); +int virDomainGetNumaParameters(virDomainPtr domain, + virTypedParameterPtr params, + int *nparams, unsigned int flags); + /* * Dynamic control of domains */ diff --git a/python/generator.py b/python/generator.py index 1657f4f..181f70e 100755 --- a/python/generator.py +++ b/python/generator.py @@ -383,6 +383,8 @@ skip_impl = ( 'virDomainGetBlkioParameters', 'virDomainSetMemoryParameters', 'virDomainGetMemoryParameters', + 'virDomainSetNumaParameters', + 'virDomainGetNumaParameters', 'virDomainGetVcpus', 'virDomainPinVcpu', 'virDomainPinVcpuFlags', diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3229a6f..189b8f6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1355,14 +1355,6 @@ virDomainVcpuPinDefPtr virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, int nvcpupin, int vcpu); -enum virDomainNumatuneMemMode { - VIR_DOMAIN_NUMATUNE_MEM_STRICT, - VIR_DOMAIN_NUMATUNE_MEM_PREFERRED, - VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE, - - VIR_DOMAIN_NUMATUNE_MEM_LAST -}; - typedef struct _virDomainNumatuneDef virDomainNumatuneDef; typedef virDomainNumatuneDef *virDomainNumatuneDefPtr; struct _virDomainNumatuneDef { diff --git a/src/driver.h b/src/driver.h index 941ff51..bbd6417 100644 --- a/src/driver.h +++ b/src/driver.h @@ -158,6 +158,19 @@ typedef int int *nparams, unsigned int flags); typedef int + (*virDrvDomainSetNumaParameters) + (virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); +typedef int + (*virDrvDomainGetNumaParameters) + (virDomainPtr domain, + virTypedParameterPtr params, + int *nparams, + unsigned int flags); + +typedef int (*virDrvDomainSetBlkioParameters) (virDomainPtr domain, virTypedParameterPtr params, @@ -816,6 +829,8 @@ struct _virDriver { virDrvDomainSetMemoryFlags domainSetMemoryFlags; virDrvDomainSetMemoryParameters domainSetMemoryParameters; virDrvDomainGetMemoryParameters domainGetMemoryParameters; + virDrvDomainSetNumaParameters domainSetNumaParameters; + virDrvDomainGetNumaParameters domainGetNumaParameters; virDrvDomainSetBlkioParameters domainSetBlkioParameters; virDrvDomainGetBlkioParameters domainGetBlkioParameters; virDrvDomainGetInfo domainGetInfo; diff --git a/src/libvirt.c b/src/libvirt.c index 9977915..574be16 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3769,6 +3769,135 @@ error: } /** + * virDomainSetNumaParameters: + * @domain: pointer to domain object + * @params: pointer to numa parameter objects + * @nparams: number of numa parameters (this value can be the same or + * less than the number of parameters supported) + * @flags: bitwise-OR of virDomainModificationImpact + * + * Change all or a subset of the numa tunables. + * This function may require privileged access to the hypervisor. + * + * Returns -1 in case of error, 0 in case of success. + */ +int +virDomainSetNumaParameters(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=%x", + params, nparams, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + if ((nparams <= 0) || (params == NULL)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (virTypedParameterValidateSet(domain, params, nparams) < 0) + return -1; + + conn = domain->conn; + + if (conn->driver->domainSetNumaParameters) { + int ret; + ret = conn->driver->domainSetNumaParameters(domain, params, nparams, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** + * virDomainGetNumaParameters: + * @domain: pointer to domain object + * @params: pointer to numa parameter object + * (return value, allocated by the caller) + * @nparams: pointer to number of numa parameters + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags + * + * Get all numa parameters. On input, @nparams gives the size of the + * @params array; on output, @nparams gives how many slots were filled + * with parameter information, which might be less but will not exceed + * the input value. + * + * As a special case, calling with @params as NULL and @nparams as 0 on + * input will cause @nparams on output to contain the number of parameters + * supported by the hypervisor. The caller should then allocate @params + * array, i.e. (sizeof(@virTypedParameter) * @nparams) bytes and call the API + * again. + * + * See virDomainGetMemoryParameters() for an equivalent usage example. + * + * This function may require privileged access to the hypervisor. This function + * expects the caller to allocate the @params. + * + * Returns -1 in case of error, 0 in case of success. + */ + +int +virDomainGetNumaParameters(virDomainPtr domain, + virTypedParameterPtr params, + int *nparams, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=%x", + params, (nparams) ? *nparams : -1, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if ((nparams == NULL) || (*nparams < 0) || + (params == NULL && *nparams != 0)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn, + VIR_DRV_FEATURE_TYPED_PARAM_STRING)) + flags |= VIR_TYPED_PARAM_STRING_OKAY; + + conn = domain->conn; + + if (conn->driver->domainGetNumaParameters) { + int ret; + ret = conn->driver->domainGetNumaParameters(domain, params, nparams, + flags); + if (ret < 0) + goto error; + return ret; + } + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainSetBlkioParameters: * @domain: pointer to domain object * @params: pointer to blkio parameter objects diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 164039a..3709f08 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -508,4 +508,10 @@ LIBVIRT_0.9.8 { virNodeSuspendForDuration; } LIBVIRT_0.9.7; +LIBVIRT_0.9.9 { + global: + virDomainGetNumaParameters; + virDomainSetNumaParameters; +} LIBVIRT_0.9.8; + # .... define new API here using predicted next version number .... -- 1.7.7.4

From: Hu Tao <hutao@cn.fujitsu.com> --- daemon/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 50 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 24 +++++++++++++++- src/remote_protocol-structs | 22 ++++++++++++++ 4 files changed, 159 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index e1d208c..20193b1 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1630,6 +1630,70 @@ cleanup: } static int +remoteDispatchDomainGetNumaParameters(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_numa_parameters_args *args, + remote_domain_get_numa_parameters_ret *ret) +{ + virDomainPtr dom = NULL; + virTypedParameterPtr params = NULL; + int nparams = args->nparams; + unsigned int flags; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + flags = args->flags; + + if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; + } + if (VIR_ALLOC_N(params, nparams) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainGetNumaParameters(dom, params, &nparams, flags) < 0) + goto cleanup; + + /* In this case, we need to send back the number of parameters + * supported + */ + if (args->nparams == 0) { + ret->nparams = nparams; + goto success; + } + + if (remoteSerializeTypedParameters(params, nparams, + &ret->params.params_val, + &ret->params.params_len, + flags) < 0) + goto cleanup; + +success: + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + VIR_FREE(params); + return rv; +} + +static int remoteDispatchDomainGetBlkioParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b74b9d0..cd3da53 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1538,6 +1538,54 @@ done: } static int +remoteDomainGetNumaParameters (virDomainPtr domain, + virTypedParameterPtr params, int *nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_numa_parameters_args args; + remote_domain_get_numa_parameters_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + args.nparams = *nparams; + args.flags = flags; + + memset (&ret, 0, sizeof ret); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS, + (xdrproc_t) xdr_remote_domain_get_numa_parameters_args, (char *) &args, + (xdrproc_t) xdr_remote_domain_get_numa_parameters_ret, (char *) &ret) == -1) + goto done; + + /* Handle the case when the caller does not know the number of parameters + * and is asking for the number of parameters supported + */ + if (*nparams == 0) { + *nparams = ret.nparams; + rv = 0; + goto cleanup; + } + + if (remoteDeserializeTypedParameters(ret.params.params_val, + ret.params.params_len, + REMOTE_DOMAIN_NUMA_PARAMETERS_MAX, + params, + nparams) < 0) + goto cleanup; + + rv = 0; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_numa_parameters_ret, + (char *) &ret); +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetBlkioParameters (virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags) @@ -4637,6 +4685,8 @@ static virDriver remote_driver = { .nodeSuspendForDuration = remoteNodeSuspendForDuration, /* 0.9.8 */ .domainSetBlockIoTune = remoteDomainSetBlockIoTune, /* 0.9.8 */ .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ + .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ + .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 509a20b..4843317 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -128,6 +128,9 @@ const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16; /* Upper limit on list of blockio tuning parameters. */ const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 16; +/* Upper limit on list of numa parameters. */ +const REMOTE_DOMAIN_NUMA_PARAMETERS_MAX = 16; + /* Upper limit on list of node cpu stats. */ const REMOTE_NODE_CPU_STATS_MAX = 16; @@ -547,6 +550,23 @@ struct remote_domain_block_resize_args { unsigned int flags; }; +struct remote_domain_set_numa_parameters_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX>; + unsigned int flags; +}; + +struct remote_domain_get_numa_parameters_args { + remote_nonnull_domain dom; + int nparams; + unsigned int flags; +}; + +struct remote_domain_get_numa_parameters_ret { + remote_typed_param params<REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX>; + int nparams; +}; + struct remote_domain_block_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2605,7 +2625,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_RESIZE = 251, /* autogen autogen */ REMOTE_PROC_DOMAIN_SET_BLOCK_IO_TUNE = 252, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 253 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 253, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ + REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index a9d4296..f6d8ff5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -270,6 +270,26 @@ struct remote_domain_block_resize_args { uint64_t size; u_int flags; }; +struct remote_domain_set_numa_parameters_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; +struct remote_domain_get_numa_parameters_args { + remote_nonnull_domain dom; + int nparams; + u_int flags; +}; +struct remote_domain_get_numa_parameters_ret { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + int nparams; +}; struct remote_domain_block_stats_args { remote_nonnull_domain dom; remote_nonnull_string path; @@ -2044,4 +2064,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_RESIZE = 251, REMOTE_PROC_DOMAIN_SET_BLOCK_IO_TUNE = 252, REMOTE_PROC_DOMAIN_GET_BLOCK_IO_TUNE = 253, + REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, + REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, }; -- 1.7.7.4

On 12/19/2011 05:42 PM, Eric Blake wrote:
From: Hu Tao <hutao@cn.fujitsu.com>
--- daemon/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 50 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 24 +++++++++++++++- src/remote_protocol-structs | 22 ++++++++++++++ 4 files changed, 159 insertions(+), 1 deletions(-)
Squash in this, at least. diff --git i/daemon/remote.c w/daemon/remote.c index 20193b1..bf7f02c 100644 --- i/daemon/remote.c +++ w/daemon/remote.c @@ -1652,7 +1652,7 @@ remoteDispatchDomainGetNumaParameters(virNetServerPtr server ATTRIBUTE_UNUSED, flags = args->flags; - if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) { + if (nparams > REMOTE_DOMAIN_NUMA_PARAMETERS_MAX) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; } diff --git i/src/remote/remote_protocol.x w/src/remote/remote_protocol.x index 4843317..18dba8c 100644 --- i/src/remote/remote_protocol.x +++ w/src/remote/remote_protocol.x @@ -552,7 +552,7 @@ struct remote_domain_block_resize_args { struct remote_domain_set_numa_parameters_args { remote_nonnull_domain dom; - remote_typed_param params<REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX>; + remote_typed_param params<REMOTE_DOMAIN_NUMA_PARAMETERS_MAX>; unsigned int flags; }; @@ -563,7 +563,7 @@ struct remote_domain_get_numa_parameters_args { }; struct remote_domain_get_numa_parameters_ret { - remote_typed_param params<REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX>; + remote_typed_param params<REMOTE_DOMAIN_NUMA_PARAMETERS_MAX>; int nparams; }; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 12/19/2011 08:13 PM, Eric Blake wrote:
On 12/19/2011 05:42 PM, Eric Blake wrote:
From: Hu Tao <hutao@cn.fujitsu.com>
--- daemon/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 50 ++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 24 +++++++++++++++- src/remote_protocol-structs | 22 ++++++++++++++ 4 files changed, 159 insertions(+), 1 deletions(-)
Squash in this, at least.
diff --git i/daemon/remote.c w/daemon/remote.c index 20193b1..bf7f02c 100644 --- i/daemon/remote.c +++ w/daemon/remote.c @@ -1652,7 +1652,7 @@ remoteDispatchDomainGetNumaParameters(virNetServerPtr server ATTRIBUTE_UNUSED,
flags = args->flags;
- if (nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) { + if (nparams > REMOTE_DOMAIN_NUMA_PARAMETERS_MAX) { virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); goto cleanup; }
And as long as I'm at it, I found a pretty nasty bug with TYPED_PARAM_STRING (thankfully untriggered - it requires that a driver send a string prior to any other typed parameter to a receiver that won't accept strings, but qemu always sends strings last), as well as some memory leaks. diff --git i/daemon/remote.c w/daemon/remote.c index bf7f02c..8cc475f 100644 --- i/daemon/remote.c +++ w/daemon/remote.c @@ -759,7 +759,7 @@ cleanup: if (val) { for (i = 0; i < nparams; i++) { VIR_FREE(val[i].field); - if (params[i].type == VIR_TYPED_PARAM_STRING) + if (val[i].value.type == VIR_TYPED_PARAM_STRING) VIR_FREE(val[i].value.remote_typed_param_value_u.s); } VIR_FREE(val); @@ -898,9 +898,10 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS cleanup: if (rv < 0) virNetMessageSaveError(rerr); + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); if (dom) virDomainFree(dom); - VIR_FREE(params); return rv; no_memory: @@ -953,9 +954,10 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE cleanup: if (rv < 0) virNetMessageSaveError(rerr); + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); if (dom) virDomainFree(dom); - VIR_FREE(params); return rv; no_memory: @@ -1140,17 +1142,12 @@ success: rv = 0; cleanup: - if (rv < 0) { + if (rv < 0) virNetMessageSaveError(rerr); - if (ret->params.params_val) { - for (i = 0; i < nparams; i++) - VIR_FREE(ret->params.params_val[i].field); - VIR_FREE(ret->params.params_val); - } - } + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); if (dom) virDomainFree(dom); - VIR_FREE(params); return rv; } @@ -1623,9 +1620,10 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); if (dom) virDomainFree(dom); - VIR_FREE(params); return rv; } @@ -1687,9 +1685,10 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); if (dom) virDomainFree(dom); - VIR_FREE(params); return rv; } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Hi Eric, Will you look at the code in xdr_remote_typed_param_value() in file sre/remote/remote_protocol.c that processes VIR_TYPED_PARAM_INT? It looks odd and I'm sure it's the cause of the bug you mentioned. switch (objp->type) { case VIR_TYPED_PARAM_INT: return FALSE; break;

From: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 262 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 660d771..3f8167f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -97,6 +97,8 @@ #define QEMU_NB_BLOCK_IO_TUNE_PARAM 6 +#define QEMU_NB_NUMA_PARAM 2 + #if HAVE_LINUX_KVM_H # include <linux/kvm.h> #endif @@ -6602,6 +6604,264 @@ cleanup: } static int +qemuDomainSetNumaParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virDomainDefPtr persistentDef = NULL; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup cpuset controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), + vm->def->name); + goto cleanup; + } + } + + ret = 0; + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_NUMA_MODE)) { + if (param->type != VIR_TYPED_PARAM_INT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for numa strict tunable, " + "expected an 'int'")); + ret = -1; + continue; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("can't change numa mode for running domain")); + ret = -1; + goto cleanup; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + persistentDef->numatune.memory.mode = params[i].value.i; + } + } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { + int rc; + if (param->type != VIR_TYPED_PARAM_STRING) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for numa nodeset tunable, " + "expected a 'string'")); + ret = -1; + continue; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (vm->def->numatune.memory.mode != + VIR_DOMAIN_NUMATUNE_MEM_STRICT) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("change of nodeset for running domain " + "requires strict numa mode")); + ret = -1; + continue; + } + rc = virCgroupSetCpusetMems(group, params[i].value.s); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set numa tunable")); + ret = -1; + continue; + } + /* XXX FIXME - should we also be updating vm->def + * here? If not, then we need to fix dumpxml to always + * read from cgroups rather than trusting vm->def. */ + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + char oldnodemask[VIR_CPU_MAPLEN(VIR_DOMAIN_CPUMASK_LEN)]; + memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + if (virDomainCpuSetParse(params[i].value.s, + 0, + persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + memcpy(persistentDef->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + ret = -1; + continue; + } + } + } else { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Parameter `%s' not supported"), param->field); + ret = -1; + } + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + ret = -1; + } + +cleanup: + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int +qemuDomainGetNumaParameters(virDomainPtr dom, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; + char *nodeset = NULL; + int ret = -1; + int rc; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + qemuDriverLock(driver); + + /* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ + flags &= ~VIR_TYPED_PARAM_STRING_OKAY; + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if ((*nparams) == 0) { + *nparams = QEMU_NB_NUMA_PARAM; + ret = 0; + goto cleanup; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup memory controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), + vm->def->name); + goto cleanup; + } + } + + for (i = 0; i < QEMU_NB_NUMA_PARAM && i < *nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + + switch (i) { + case 0: /* fill numa mode here */ + if (!virStrcpyStatic(param->field, VIR_DOMAIN_NUMA_MODE)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field '%s' too long for destination"), + VIR_DOMAIN_NUMA_MODE); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_INT; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) + param->value.i = persistentDef->numatune.memory.mode; + else + param->value.i = vm->def->numatune.memory.mode; + break; + + case 1: /* fill numa nodeset here */ + if (!virStrcpyStatic(param->field, VIR_DOMAIN_NUMA_NODESET)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field '%s' too long for destination"), + VIR_DOMAIN_NUMA_NODESET); + goto cleanup; + } + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + char *mask = persistentDef->numatune.memory.nodemask; + if (mask) + nodeset = virDomainCpuSetFormat(mask, + VIR_DOMAIN_CPUMASK_LEN); + else + nodeset = strdup(""); + } else { + rc = virCgroupGetCpusetMems(group, &nodeset); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to get numa nodeset")); + goto cleanup; + } + } + if (!nodeset) { + virReportOOMError(); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_STRING; + param->value.s = nodeset; + break; + + default: + break; + /* should not hit here */ + } + } + + if (*nparams > QEMU_NB_NUMA_PARAM) + *nparams = QEMU_NB_NUMA_PARAM; + ret = 0; + +cleanup: + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, unsigned long long period, long long quota) { @@ -11362,6 +11622,8 @@ static virDriver qemuDriver = { .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */ .domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */ .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */ + .domainSetNumaParameters = qemuDomainSetNumaParameters, /* 0.9.9 */ + .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ }; -- 1.7.7.4

On Mon, Dec 19, 2011 at 05:42:02PM -0700, Eric Blake wrote:
From: Hu Tao <hutao@cn.fujitsu.com>
--- src/qemu/qemu_driver.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 262 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 660d771..3f8167f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -97,6 +97,8 @@
#define QEMU_NB_BLOCK_IO_TUNE_PARAM 6
+#define QEMU_NB_NUMA_PARAM 2 + #if HAVE_LINUX_KVM_H # include <linux/kvm.h> #endif @@ -6602,6 +6604,264 @@ cleanup: }
static int +qemuDomainSetNumaParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virDomainDefPtr persistentDef = NULL; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup cpuset controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), + vm->def->name); + goto cleanup; + } + } + + ret = 0; + for (i = 0; i < nparams; i++) { + virTypedParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_NUMA_MODE)) { + if (param->type != VIR_TYPED_PARAM_INT) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for numa strict tunable, " + "expected an 'int'")); + ret = -1; + continue; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("can't change numa mode for running domain")); + ret = -1; + goto cleanup; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + persistentDef->numatune.memory.mode = params[i].value.i; + } + } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { + int rc; + if (param->type != VIR_TYPED_PARAM_STRING) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for numa nodeset tunable, " + "expected a 'string'")); + ret = -1; + continue; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (vm->def->numatune.memory.mode != + VIR_DOMAIN_NUMATUNE_MEM_STRICT) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("change of nodeset for running domain " + "requires strict numa mode")); + ret = -1; + continue; + } + rc = virCgroupSetCpusetMems(group, params[i].value.s); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set numa tunable")); + ret = -1; + continue; + } + /* XXX FIXME - should we also be updating vm->def + * here? If not, then we need to fix dumpxml to always + * read from cgroups rather than trusting vm->def. */ + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + char oldnodemask[VIR_CPU_MAPLEN(VIR_DOMAIN_CPUMASK_LEN)];
should be char oldnodemask[VIR_DOMAIN_CPUMASK_LEN];
+ memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN); + if (virDomainCpuSetParse(params[i].value.s, + 0, + persistentDef->numatune.memory.nodemask, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + memcpy(persistentDef->numatune.memory.nodemask, + oldnodemask, VIR_DOMAIN_CPUMASK_LEN); + ret = -1; + continue; + } + } + } else { + qemuReportError(VIR_ERR_INVALID_ARG, + _("Parameter `%s' not supported"), param->field); + ret = -1; + } + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) + ret = -1; + } + +cleanup: + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int +qemuDomainGetNumaParameters(virDomainPtr dom, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + int i; + virCgroupPtr group = NULL; + virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; + char *nodeset = NULL; + int ret = -1; + int rc; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + qemuDriverLock(driver); + + /* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ + flags &= ~VIR_TYPED_PARAM_STRING_OKAY; + + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("No such domain %s"), dom->uuid); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + if ((*nparams) == 0) { + *nparams = QEMU_NB_NUMA_PARAM; + ret = 0; + goto cleanup; + } + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup memory controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), + vm->def->name); + goto cleanup; + } + } + + for (i = 0; i < QEMU_NB_NUMA_PARAM && i < *nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + + switch (i) { + case 0: /* fill numa mode here */ + if (!virStrcpyStatic(param->field, VIR_DOMAIN_NUMA_MODE)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field '%s' too long for destination"), + VIR_DOMAIN_NUMA_MODE); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_INT; + if (flags & VIR_DOMAIN_AFFECT_CONFIG) + param->value.i = persistentDef->numatune.memory.mode; + else + param->value.i = vm->def->numatune.memory.mode; + break; + + case 1: /* fill numa nodeset here */ + if (!virStrcpyStatic(param->field, VIR_DOMAIN_NUMA_NODESET)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Field '%s' too long for destination"), + VIR_DOMAIN_NUMA_NODESET); + goto cleanup; + } + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + char *mask = persistentDef->numatune.memory.nodemask; + if (mask) + nodeset = virDomainCpuSetFormat(mask, + VIR_DOMAIN_CPUMASK_LEN); + else + nodeset = strdup(""); + } else { + rc = virCgroupGetCpusetMems(group, &nodeset); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to get numa nodeset")); + goto cleanup; + } + } + if (!nodeset) { + virReportOOMError(); + goto cleanup; + } + param->type = VIR_TYPED_PARAM_STRING; + param->value.s = nodeset; + break; + + default: + break; + /* should not hit here */ + } + } + + if (*nparams > QEMU_NB_NUMA_PARAM) + *nparams = QEMU_NB_NUMA_PARAM; + ret = 0; + +cleanup: + virCgroupFree(&group); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, unsigned long long period, long long quota) { @@ -11362,6 +11622,8 @@ static virDriver qemuDriver = { .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */ .domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */ .domainGetBlockIoTune = qemuDomainGetBlockIoTune, /* 0.9.8 */ + .domainSetNumaParameters = qemuDomainSetNumaParameters, /* 0.9.9 */ + .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ };
-- 1.7.7.4
-- Thanks, Hu Tao

From: Hu Tao <hutao@cn.fujitsu.com> add new command numatune to virsh to get/set numa parameters --- tools/virsh.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 19 +++++++ 2 files changed, 178 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 583ec6d..4264307 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -61,6 +61,7 @@ #include "virkeycode.h" #include "virnetdevbandwidth.h" #include "util/bitmap.h" +#include "conf/domain_conf.h" static char *progname; @@ -5126,6 +5127,163 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) } /* + * "numatune" command + */ +static const vshCmdInfo info_numatune[] = { + {"help", N_("Get or set numa parameters")}, + {"desc", N_("Get or set the current numa parameters for a guest" \ + " domain.\n" \ + " To get the numa parameters use following command: \n\n" \ + " virsh # numatune <domain>")}, + {NULL, NULL} + +}; + +static const vshCmdOptDef opts_numatune[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"mode", VSH_OT_DATA, VSH_OFLAG_NONE, + N_("NUMA mode, one of strict, preferred and interleave")}, + {"nodeset", VSH_OT_DATA, VSH_OFLAG_NONE, + N_("NUMA node selections to set")}, + {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, + {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, + {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdNumatune(vshControl * ctl, const vshCmd * cmd) +{ + virDomainPtr dom; + int nparams = 0; + unsigned int i = 0; + virTypedParameterPtr params = NULL, temp = NULL; + const char *nodeset = NULL; + bool ret = false; + unsigned int flags = 0; + int current = vshCommandOptBool(cmd, "current"); + int config = vshCommandOptBool(cmd, "config"); + int live = vshCommandOptBool(cmd, "live"); + const char *mode = NULL; + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags = VIR_DOMAIN_AFFECT_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + } + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "nodeset", &nodeset) < 0) { + vshError(ctl, "%s", _("Unable to parse nodeset.")); + virDomainFree(dom); + return false; + } + if (nodeset) + nparams++; + if (vshCommandOptString(cmd, "mode", &mode) < 0) { + vshError(ctl, "%s", _("Unable to parse mode.")); + virDomainFree(dom); + return false; + } + if (mode) + nparams++; + + if (nparams == 0) { + /* get the number of numa parameters */ + if (virDomainGetNumaParameters(dom, NULL, &nparams, flags) != 0) { + vshError(ctl, "%s", + _("Unable to get number of memory parameters")); + goto cleanup; + } + + if (nparams == 0) { + /* nothing to output */ + ret = true; + goto cleanup; + } + + /* now go get all the numa parameters */ + params = vshCalloc(ctl, nparams, sizeof(*params)); + if (virDomainGetNumaParameters(dom, params, &nparams, flags) != 0) { + vshError(ctl, "%s", _("Unable to get numa parameters")); + goto cleanup; + } + + for (i = 0; i < nparams; i++) { + if (params[i].type == VIR_TYPED_PARAM_INT && + STREQ(params[i].field, VIR_DOMAIN_NUMA_MODE)) { + vshPrint(ctl, "%-15s: %s\n", params[i].field, + virDomainNumatuneMemModeTypeToString(params[i].value.i)); + } else { + char *str = vshGetTypedParamValue(ctl, ¶ms[i]); + vshPrint(ctl, "%-15s: %s\n", params[i].field, str); + VIR_FREE(str); + } + } + + ret = true; + } else { + /* set the numa parameters */ + params = vshCalloc(ctl, nparams, sizeof(*params)); + + for (i = 0; i < nparams; i++) { + temp = ¶ms[i]; + + /* + * Some magic here, this is used to fill the params structure with + * the valid arguments passed, after filling the particular + * argument we purposely make them 0, so on the next pass it goes + * to the next valid argument and so on. + */ + if (mode) { + /* Accept string or integer, in case server + * understands newer integer than what strings we were + * compiled with */ + if ((temp->value.i = + virDomainNumatuneMemModeTypeFromString(mode)) < 0) { + vshError(ctl, "%s %s", _("Invalid mode"), mode); + goto cleanup; + } + if (!virStrcpy(temp->field, VIR_DOMAIN_NUMA_MODE, + sizeof(temp->field))) + goto cleanup; + temp->type = VIR_TYPED_PARAM_INT; + mode = NULL; + } else if (nodeset) { + temp->value.s = vshStrdup(ctl, nodeset); + temp->type = VIR_TYPED_PARAM_STRING; + if (!virStrcpy(temp->field, VIR_DOMAIN_NUMA_NODESET, + sizeof(temp->field))) + goto cleanup; + nodeset = NULL; + } + } + if (virDomainSetNumaParameters(dom, params, nparams, flags) != 0) + vshError(ctl, "%s", _("Unable to change numa parameters")); + else + ret = true; + } + + cleanup: + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); + virDomainFree(dom); + return ret; +} + +/* * "nodeinfo" command */ static const vshCmdInfo info_nodeinfo[] = { @@ -15237,6 +15395,7 @@ static const vshCmdDef domManagementCmds[] = { opts_migrate_setspeed, info_migrate_setspeed, 0}, {"migrate-getspeed", cmdMigrateGetMaxSpeed, opts_migrate_getspeed, info_migrate_getspeed, 0}, + {"numatune", cmdNumatune, opts_numatune, info_numatune, 0}, {"reboot", cmdReboot, opts_reboot, info_reboot, 0}, {"reset", cmdReset, opts_reset, info_reset, 0}, {"restore", cmdRestore, opts_restore, info_restore, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index dbe5165..2367673 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -832,6 +832,25 @@ migrated to another host. Get the maximum migration bandwidth (in Mbps) for a domain. +=item B<numatune> I<domain> [I<mode>] [I<nodeset>] +[[I<--config>] [I<--live>] | [I<--current>]] + +Set or get a domain's numa parameters, corresponding to the <numatune> +element of domain XML. Without flags, the current settings are +displayed. + +I<mode> can be one of `strict', `interleave' and `preferred'. For a +running domain, the mode can't be changed, and the nodeset can be +changed only if the domain was started with a mode of `strict'. + +I<nodeset> is a list of numa nodes used by the host for running the domain. +Its syntax is a comma separated list, with '-' for ranges and '^' for +excluding a node. + +If I<--live> is specified, set scheduler information of a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect the current guest state. + =item B<reboot> I<domain-id> Reboot a domain. This acts just as if the domain had the B<reboot> -- 1.7.7.4
participants (2)
-
Eric Blake
-
Hu Tao