[libvirt] [PATCH 00/12] Introduce public APIs for dealing with virTypedParameters

Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it. Jiri Denemark (12): Add virTypedParams* APIs virsh: Use virTypedParams* APIs in blkdeviotune virsh: Use virTypedParams* APIs in blkiotune virsh: Use virTypedParams* APIs in domiftune virsh: Use virTypedParams* APIs in schedinfo virsh: Use virTypedParams* APIs in domblkstat virsh: Use virTypedParams* APIs in memtune virsh: Use virTypedParams* APIs in numatune virsh: Use virTypedParams* APIs in node-memory-tune virsh: Use virTypedParams* APIs in cpu-stats Introduce virTypedParamsClear public API python: Avoid freeing uninitialized new_params pointer daemon/remote.c | 34 +- include/libvirt/libvirt.h.in | 95 ++++++ python/generator.py | 20 ++ python/libvirt-override.c | 73 ++-- src/libvirt_private.syms | 1 - src/libvirt_public.syms | 18 + src/remote/remote_driver.c | 4 +- src/rpc/gendispatch.pl | 3 +- src/util/virtypedparam.c | 782 ++++++++++++++++++++++++++++++++++++++++++- src/util/virtypedparam.h | 2 - tools/virsh-domain-monitor.c | 5 +- tools/virsh-domain.c | 542 +++++++++++------------------- tools/virsh-host.c | 78 ++--- tools/virsh.c | 20 -- 14 files changed, 1171 insertions(+), 506 deletions(-) -- 1.8.1.1

Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it. --- include/libvirt/libvirt.h.in | 92 ++++++ python/generator.py | 19 ++ src/libvirt_public.syms | 17 + src/util/virtypedparam.c | 742 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 870 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c1233f6..23f2952 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -519,6 +519,98 @@ struct _virTypedParameter { typedef virTypedParameter *virTypedParameterPtr; +virTypedParameterPtr +virTypedParamsGet (virTypedParameterPtr params, + int nparams, + const char *name); +int +virTypedParamsGetInt (virTypedParameterPtr params, + int nparams, + const char *name, + int *value); +int +virTypedParamsGetUInt (virTypedParameterPtr params, + int nparams, + const char *name, + unsigned int *value); +int +virTypedParamsGetLLong (virTypedParameterPtr params, + int nparams, + const char *name, + long long *value); +int +virTypedParamsGetULLong (virTypedParameterPtr params, + int nparams, + const char *name, + unsigned long long *value); +int +virTypedParamsGetDouble (virTypedParameterPtr params, + int nparams, + const char *name, + double *value); +int +virTypedParamsGetBoolean(virTypedParameterPtr params, + int nparams, + const char *name, + int *value); +int +virTypedParamsGetString (virTypedParameterPtr params, + int nparams, + const char *name, + const char **value); +int +virTypedParamsAddInt (virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + int value); +int +virTypedParamsAddUInt (virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + unsigned int value); +int +virTypedParamsAddLLong (virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + long long value); +int +virTypedParamsAddULLong (virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + unsigned long long value); +int +virTypedParamsAddDouble (virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + double value); +int +virTypedParamsAddBoolean(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + int value); +int +virTypedParamsAddString (virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char *value); +int +virTypedParamsAddFromString(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + int type, + const char *value); +void +virTypedParamsFree (virTypedParameterPtr params, + int nparams); + /* data types related to virNodePtr */ /** diff --git a/python/generator.py b/python/generator.py index 28426c4..00e4858 100755 --- a/python/generator.py +++ b/python/generator.py @@ -527,6 +527,25 @@ skip_function = ( "virNWFilterGetConnect", "virStoragePoolGetConnect", "virStorageVolGetConnect", + + # only useful in C code, python code uses dict for typed parameters + "virTypedParamsAddBoolean", + "virTypedParamsAddDouble", + "virTypedParamsAddFromString", + "virTypedParamsAddInt", + "virTypedParamsAddLLong", + "virTypedParamsAddString", + "virTypedParamsAddUInt", + "virTypedParamsAddULLong", + "virTypedParamsFree", + "virTypedParamsGet", + "virTypedParamsGetBoolean", + "virTypedParamsGetDouble", + "virTypedParamsGetInt", + "virTypedParamsGetLLong", + "virTypedParamsGetString", + "virTypedParamsGetUInt", + "virTypedParamsGetULLong", ) lxc_skip_function = ( diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 2107519..7631b19 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -583,6 +583,23 @@ LIBVIRT_1.0.1 { LIBVIRT_1.0.2 { global: virDomainOpenChannel; + virTypedParamsAddBoolean; + virTypedParamsAddDouble; + virTypedParamsAddFromString; + virTypedParamsAddInt; + virTypedParamsAddLLong; + virTypedParamsAddString; + virTypedParamsAddUInt; + virTypedParamsAddULLong; + virTypedParamsFree; + virTypedParamsGet; + virTypedParamsGetBoolean; + virTypedParamsGetDouble; + virTypedParamsGetInt; + virTypedParamsGetLLong; + virTypedParamsGetString; + virTypedParamsGetUInt; + virTypedParamsGetULLong; } LIBVIRT_1.0.1; # .... define new API here using predicted next version number .... diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 33e6a09..1b153a4 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -276,3 +276,745 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, const char *name, cleanup: return ret; } + + +/* The following APIs are public and their signature may never change. */ + +/** + * virTypedParamsGet: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * + * Finds typed parameter called @name. + * + * Returns pointer to the parameter or NULL if it does not exist in @params. + */ +virTypedParameterPtr +virTypedParamsGet(virTypedParameterPtr params, + int nparams, + const char *name) +{ + int i; + + virResetLastError(); + + if (!params || !name) + return NULL; + + for (i = 0; i < nparams; i++) { + if (STREQ(params[i].field, name)) + return params + i; + } + + return NULL; +} + + +#define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \ + do { if (param->type != check_type) { \ + virReportError(VIR_ERR_INVALID_ARG, \ + _("Invalid type '%s' requested for parameter '%s', " \ + "actual type is '%s'"), \ + virTypedParameterTypeToString(check_type), \ + name, \ + virTypedParameterTypeToString(param->type)); \ + virDispatchError(NULL); \ + return -1; \ + } } while (0) + + +/** + * virTypedParamsGetInt: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its int value in @value. The + * function fails with VIR_ERR_INVALID_ARG error if the parameter does not + * have the expected type. By passing NULL as @value, the function may be + * used to check presence and type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + */ +int +virTypedParamsGetInt(virTypedParameterPtr params, + int nparams, + const char *name, + int *value) +{ + virTypedParameterPtr param; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_INT); + if (value) + *value = param->value.i; + + return 1; +} + + +/** + * virTypedParamsGetUInt: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its unsigned int value in + * @value. The function fails with VIR_ERR_INVALID_ARG error if the parameter + * does not have the expected type. By passing NULL as @value, the function + * may be used to check presence and type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + */ +int +virTypedParamsGetUInt(virTypedParameterPtr params, + int nparams, + const char *name, + unsigned int *value) +{ + virTypedParameterPtr param; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_UINT); + if (value) + *value = param->value.ui; + + return 1; +} + + +/** + * virTypedParamsGetLLong: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its long long int value in + * @value. The function fails with VIR_ERR_INVALID_ARG error if the parameter + * does not have the expected type. By passing NULL as @value, the function + * may be used to check presence and type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + */ +int +virTypedParamsGetLLong(virTypedParameterPtr params, + int nparams, + const char *name, + long long *value) +{ + virTypedParameterPtr param; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_LLONG); + if (value) + *value = param->value.l; + + return 1; +} + + +/** + * virTypedParamsGetULLong: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its unsigned long long int + * value in @value. The function fails with VIR_ERR_INVALID_ARG error if the + * parameter does not have the expected type. By passing NULL as @value, the + * function may be used to check presence and type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + */ +int +virTypedParamsGetULLong(virTypedParameterPtr params, + int nparams, + const char *name, + unsigned long long *value) +{ + virTypedParameterPtr param; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_ULLONG); + if (value) + *value = param->value.ul; + + return 1; +} + + +/** + * virTypedParamsGetDouble: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its double value in @value. + * The function fails with VIR_ERR_INVALID_ARG error if the parameter does not + * have the expected type. By passing NULL as @value, the function may be used + * to check presence and type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + */ +int +virTypedParamsGetDouble(virTypedParameterPtr params, + int nparams, + const char *name, + double *value) +{ + virTypedParameterPtr param; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_DOUBLE); + if (value) + *value = param->value.d; + + return 1; +} + + +/** + * virTypedParamsGetBoolean: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its boolean value in @value. + * The function fails with VIR_ERR_INVALID_ARG error if the parameter does not + * have the expected type. By passing NULL as @value, the function may be used + * to check presence and type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + */ +int +virTypedParamsGetBoolean(virTypedParameterPtr params, + int nparams, + const char *name, + int *value) +{ + virTypedParameterPtr param; + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_BOOLEAN); + if (value) + *value = !!param->value.b; + + return 1; +} + + +/** + * virTypedParamsGetString: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its char * value in @value. + * The function does not create a copy of the string and the caller must not + * free the string @value points to. The function fails with + * VIR_ERR_INVALID_ARG error if the parameter does not have the expected type. + * By passing NULL as @value, the function may be used to check presence and + * type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + */ +int +virTypedParamsGetString(virTypedParameterPtr params, + int nparams, + const char *name, + const char **value) +{ + virTypedParameterPtr param; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_STRING); + if (value) + *value = param->value.s; + + return 1; +} + + +#define VIR_TYPED_PARAM_CHECK() \ + do { if (virTypedParamsGet(*params, n, name)) { \ + virReportError(VIR_ERR_INVALID_ARG, \ + _("Parameter '%s' is already set"), name); \ + goto error; \ + } } while (0) + + +/** + * virTypedParamsAddInt: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with int type and sets its value to @value. + * If @params array points to NULL or to a space that is not large enough to + * accommodate the new parameter (@maxparams < @nparams + 1), the function + * allocates more space for it and updates @maxparams. On success, @nparams + * is incremented by one. The function fails with VIR_ERR_INVALID_ARG error + * if the parameter already exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddInt(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + int value) +{ + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_INT, value) < 0) + goto error; + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsAddUInt: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with unsigned int type and sets its value + * to @value. If @params array points to NULL or to a space that is not large + * enough to accommodate the new parameter (@maxparams < @nparams + 1), the + * function allocates more space for it and updates @maxparams. On success, + * @nparams is incremented by one. The function fails with VIR_ERR_INVALID_ARG + * error if the parameter already exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddUInt(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + unsigned int value) +{ + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_UINT, value) < 0) + goto error; + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsAddLLong: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with long long int type and sets its value + * to @value. If @params array points to NULL or to a space that is not large + * enough to accommodate the new parameter (@maxparams < @nparams + 1), the + * function allocates more space for it and updates @maxparams. On success, + * @nparams is incremented by one. The function fails with VIR_ERR_INVALID_ARG + * error if the parameter already exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddLLong(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + long long value) +{ + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_LLONG, value) < 0) + goto error; + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsAddULLong: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with unsigned long long type and sets its + * value to @value. If @params array points to NULL or to a space that is not + * large enough to accommodate the new parameter (@maxparams < @nparams + 1), + * the function allocates more space for it and updates @maxparams. On success, + * @nparams is incremented by one. The function fails with VIR_ERR_INVALID_ARG + * error if the parameter already exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddULLong(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + unsigned long long value) +{ + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_ULLONG, value) < 0) + goto error; + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsAddDouble: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with double type and sets its value to + * @value. If @params array points to NULL or to a space that is not large + * enough to accommodate the new parameter (@maxparams < @nparams + 1), the + * function allocates more space for it and updates @maxparams. On success, + * @nparams is incremented by one. The function fails with VIR_ERR_INVALID_ARG + * error if the parameter already exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddDouble(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + double value) +{ + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_DOUBLE, value) < 0) + goto error; + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsAddBoolean: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with boolean type and sets its value to + * @value. If @params array points to NULL or to a space that is not large + * enough to accommodate the new parameter (@maxparams < @nparams + 1), the + * function allocates more space for it and updates @maxparams. On success, + * @nparams is incremented by one. The function fails with VIR_ERR_INVALID_ARG + * error if the parameter already exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddBoolean(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + int value) +{ + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_BOOLEAN, value) < 0) + goto error; + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsAddString: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with char * type and sets its value to + * @value. The function creates its own copy of @value string, which needs to + * be freed using virTypedParamsFree. If @params array + * points to NULL or to a space that is not large enough to accommodate the + * new parameter (@maxparams < @nparams + 1), the function allocates more + * space for it and updates @maxparams. On success, @nparams is incremented + * by one. The function fails with VIR_ERR_INVALID_ARG error if the parameter + * already exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddString(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char *value) +{ + char *str = NULL; + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (value && !(str = strdup(value))) { + virReportOOMError(); + goto error; + } + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_STRING, str) < 0) { + VIR_FREE(str); + goto error; + } + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsAddFromString: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @type: type of the parameter + * @value: the value to store into the new parameter encoded as a string + * + * Adds new parameter called @name with the requested @type and parses its + * value from the @value string. If the requested type is string, the function + * creates its own copy of the @value string, which needs to be freed using + * virTypedParamsFree. If @params array points to NULL + * or to a space that is not large enough to accommodate the new parameter + * (@maxparams < @nparams + 1), the function allocates more space for it and + * updates @maxparams. On success, @nparams is incremented by one. The + * function fails with VIR_ERR_INVALID_ARG error if the parameter already + * exists in @params. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddFromString(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + int type, + const char *value) +{ + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_TYPED_PARAM_CHECK(); + if (VIR_RESIZE_N(*params, max, n, 1) < 0) { + virReportOOMError(); + goto error; + } + *maxparams = max; + + if (virTypedParameterAssignFromStr(*params + n, name, type, value) < 0) + goto error; + + *nparams += 1; + return 0; + +error: + virDispatchError(NULL); + return -1; +} + + +/** + * virTypedParamsFree: + * @params: the array of typed parameters + * @nparams: number of parameters in the @params array + * + * Frees all memory used by string parameters and the memory occupied by + * @params. + * + * Returns nothing. + */ +void +virTypedParamsFree(virTypedParameterPtr params, + int nparams) +{ + virResetLastError(); + virTypedParameterArrayClear(params, nparams); + VIR_FREE(params); +} -- 1.8.1.1

--- tools/virsh-domain.c | 134 +++++++++++++++++---------------------------------- 1 file changed, 45 insertions(+), 89 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e3ff5d3..2d57601 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -851,9 +851,9 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; const char *name, *disk; - unsigned long long total_bytes_sec = 0, read_bytes_sec = 0, write_bytes_sec = 0; - unsigned long long total_iops_sec = 0, read_iops_sec = 0, write_iops_sec = 0; + unsigned long long value; int nparams = 0; + int maxparams = 0; virTypedParameterPtr params = NULL; unsigned int flags = 0, i = 0; int rv = 0; @@ -881,62 +881,61 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "device", &disk) < 0) goto cleanup; - if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", - &total_bytes_sec)) < 0) { - vshError(ctl, "%s", - _("Unable to parse integer parameter")); - goto cleanup; + if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", &value)) < 0) { + goto interror; } else if (rv > 0) { - nparams++; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC, + value) < 0) + goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", - &read_bytes_sec)) < 0) { - vshError(ctl, "%s", - _("Unable to parse integer parameter")); - goto cleanup; + if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", &value)) < 0) { + goto interror; } else if (rv > 0) { - nparams++; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC, + value) < 0) + goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", - &write_bytes_sec)) < 0) { - vshError(ctl, "%s", - _("Unable to parse integer parameter")); - goto cleanup; + if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", &value)) < 0) { + goto interror; } else if (rv > 0) { - nparams++; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC, + value) < 0) + goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", - &total_iops_sec)) < 0) { - vshError(ctl, "%s", - _("Unable to parse integer parameter")); - goto cleanup; + if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", &value)) < 0) { + goto interror; } else if (rv > 0) { - nparams++; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC, + value) < 0) + goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", - &read_iops_sec)) < 0) { - vshError(ctl, "%s", - _("Unable to parse integer parameter")); - goto cleanup; + if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", &value)) < 0) { + goto interror; } else if (rv > 0) { - nparams++; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC, + value) < 0) + goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", - &write_iops_sec)) < 0) { - vshError(ctl, "%s", - _("Unable to parse integer parameter")); - goto cleanup; + if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", &value)) < 0) { + goto interror; } else if (rv > 0) { - nparams++; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, + value) < 0) + goto save_error; } if (nparams == 0) { - if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) != 0) { vshError(ctl, "%s", _("Unable to get number of block I/O throttle parameters")); @@ -961,56 +960,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-15s: %s\n", params[i].field, str); VIR_FREE(str); } - - ret = true; - goto cleanup; } else { - /* Set the block I/O throttle, match by opt since parameters can be 0 */ - params = vshCalloc(ctl, nparams, sizeof(*params)); - i = 0; - - if (i < nparams && vshCommandOptBool(cmd, "total-bytes-sec") && - virTypedParameterAssign(¶ms[i++], - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC, - VIR_TYPED_PARAM_ULLONG, - total_bytes_sec) < 0) - goto error; - - if (i < nparams && vshCommandOptBool(cmd, "read-bytes-sec") && - virTypedParameterAssign(¶ms[i++], - VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC, - VIR_TYPED_PARAM_ULLONG, - read_bytes_sec) < 0) - goto error; - - if (i < nparams && vshCommandOptBool(cmd, "write-bytes-sec") && - virTypedParameterAssign(¶ms[i++], - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC, - VIR_TYPED_PARAM_ULLONG, - write_bytes_sec) < 0) - goto error; - - if (i < nparams && vshCommandOptBool(cmd, "total-iops-sec") && - virTypedParameterAssign(¶ms[i++], - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - total_iops_sec) < 0) - goto error; - - if (i < nparams && vshCommandOptBool(cmd, "read-iops-sec") && - virTypedParameterAssign(¶ms[i++], - VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - read_iops_sec) < 0) - goto error; - - if (i < nparams && vshCommandOptBool(cmd, "write-iops-sec") && - virTypedParameterAssign(¶ms[i++], - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - write_iops_sec) < 0) - goto error; - if (virDomainSetBlockIoTune(dom, disk, params, nparams, flags) < 0) goto error; } @@ -1018,13 +968,19 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - VIR_FREE(params); + virTypedParamsFree(params, nparams); virDomainFree(dom); return ret; +save_error: + vshSaveLibvirtError(); error: vshError(ctl, "%s", _("Unable to change block I/O throttle")); goto cleanup; + +interror: + vshError(ctl, "%s", _("Unable to parse integer parameter")); + goto cleanup; } /* -- 1.8.1.1

--- tools/virsh-domain.c | 64 +++++++++++++++++++--------------------------------- 1 file changed, 23 insertions(+), 41 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2d57601..74ea4b4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1014,9 +1014,10 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) const char *device_weight = NULL; int weight = 0; int nparams = 0; + int maxparams = 0; int rv = 0; unsigned int i = 0; - virTypedParameterPtr params = NULL, temp = NULL; + virTypedParameterPtr params = NULL; bool ret = false; unsigned int flags = 0; bool current = vshCommandOptBool(cmd, "current"); @@ -1040,27 +1041,27 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) return false; if ((rv = vshCommandOptInt(cmd, "weight", &weight)) < 0) { - vshError(ctl, "%s", - _("Unable to parse integer parameter")); + vshError(ctl, "%s", _("Unable to parse integer parameter")); goto cleanup; - } - - if (rv > 0) { - nparams++; + } else if (rv > 0) { if (weight <= 0) { vshError(ctl, _("Invalid value of %d for I/O weight"), weight); goto cleanup; } + if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLKIO_WEIGHT, weight) < 0) + goto save_error; } rv = vshCommandOptString(cmd, "device-weights", &device_weight); if (rv < 0) { - vshError(ctl, "%s", - _("Unable to parse string parameter")); + vshError(ctl, "%s", _("Unable to parse string parameter")); goto cleanup; - } - if (rv > 0) { - nparams++; + } else if (rv > 0) { + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, + device_weight) < 0) + goto save_error; } if (nparams == 0) { @@ -1091,41 +1092,22 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) } } else { /* set the blkio parameters */ - params = vshCalloc(ctl, nparams, sizeof(*params)); - - for (i = 0; i < nparams; i++) { - temp = ¶ms[i]; - temp->type = VIR_TYPED_PARAM_UINT; - - if (weight) { - temp->value.ui = weight; - if (!virStrcpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT, - sizeof(temp->field))) - goto cleanup; - weight = 0; - } else if (device_weight) { - temp->value.s = vshStrdup(ctl, device_weight); - temp->type = VIR_TYPED_PARAM_STRING; - if (!virStrcpy(temp->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, - sizeof(temp->field))) - goto cleanup; - device_weight = NULL; - } - } - - if (virDomainSetBlkioParameters(dom, params, nparams, flags) < 0) { - vshError(ctl, "%s", _("Unable to change blkio parameters")); - goto cleanup; - } + if (virDomainSetBlkioParameters(dom, params, nparams, flags) < 0) + goto error; } ret = true; - cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); +cleanup: + virTypedParamsFree(params, nparams); virDomainFree(dom); return ret; + +save_error: + vshSaveLibvirtError(); +error: + vshError(ctl, "%s", _("Unable to change blkio parameters")); + goto cleanup; } typedef enum { -- 1.8.1.1

--- tools/virsh-domain.c | 122 ++++++++++++++++++++------------------------------- 1 file changed, 48 insertions(+), 74 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 74ea4b4..10b0a11 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2093,6 +2093,7 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) *inboundStr = NULL, *outboundStr = NULL; unsigned int flags = 0; int nparams = 0; + int maxparams = 0; virTypedParameterPtr params = NULL; bool ret = false; bool current = vshCommandOptBool(cmd, "current"); @@ -2117,10 +2118,8 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptString(cmd, "interface", &device) <= 0) { - virDomainFree(dom); - return false; - } + if (vshCommandOptString(cmd, "interface", &device) <= 0) + goto cleanup; if (vshCommandOptString(cmd, "inbound", &inboundStr) < 0 || vshCommandOptString(cmd, "outbound", &outboundStr) < 0) { @@ -2140,10 +2139,25 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) vshError(ctl, _("inbound average is mandatory")); goto cleanup; } - nparams++; /* average */ - if (inbound.peak) nparams++; - if (inbound.burst) nparams++; + + if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BANDWIDTH_IN_AVERAGE, + inbound.average) < 0) + goto save_error; + + if (inbound.peak && + virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BANDWIDTH_IN_PEAK, + inbound.peak) < 0) + goto save_error; + + if (inbound.burst && + virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BANDWIDTH_IN_BURST, + inbound.burst) < 0) + goto save_error; } + if (outboundStr) { if (parseRateStr(outboundStr, &outbound) < 0) { vshError(ctl, _("outbound format is incorrect")); @@ -2153,9 +2167,23 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) vshError(ctl, _("outbound average is mandatory")); goto cleanup; } - nparams++; /* average */ - if (outbound.peak) nparams++; - if (outbound.burst) nparams++; + + if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE, + outbound.average) < 0) + goto save_error; + + if (outbound.peak && + virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BANDWIDTH_OUT_PEAK, + outbound.peak) < 0) + goto save_error; + + if (outbound.burst && + virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BANDWIDTH_OUT_BURST, + outbound.burst) < 0) + goto save_error; } if (nparams == 0) { @@ -2174,10 +2202,6 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) /* get all interface parameters */ params = vshCalloc(ctl, nparams, sizeof(*params)); - if (!params) { - virReportOOMError(); - goto cleanup; - } if (virDomainGetInterfaceParameters(dom, device, params, &nparams, flags) != 0) { vshError(ctl, "%s", _("Unable to get interface parameters")); goto cleanup; @@ -2189,73 +2213,23 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) VIR_FREE(str); } } else { - /* set the interface parameters */ - params = vshCalloc(ctl, nparams, sizeof(*params)); - if (!params) { - virReportOOMError(); - goto cleanup; - } - - for (i = 0; i < nparams; i++) - params[i].type = VIR_TYPED_PARAM_UINT; - - i = 0; - if (inbound.average && i < nparams) { - if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE, - sizeof(params[i].field))) - goto cleanup; - params[i].value.ui = inbound.average; - i++; - } - if (inbound.peak && i < nparams) { - if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK, - sizeof(params[i].field))) - goto cleanup; - params[i].value.ui = inbound.peak; - i++; - } - if (inbound.burst && i < nparams) { - if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST, - sizeof(params[i].field))) - goto cleanup; - params[i].value.ui = inbound.burst; - i++; - } - if (outbound.average && i < nparams) { - if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE, - sizeof(params[i].field))) - goto cleanup; - params[i].value.ui = outbound.average; - i++; - } - if (outbound.peak && i < nparams) { - if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK, - sizeof(params[i].field))) - goto cleanup; - params[i].value.ui = outbound.peak; - i++; - } - if (outbound.burst && i < nparams) { - if (!virStrcpy(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST, - sizeof(params[i].field))) - goto cleanup; - params[i].value.ui = outbound.burst; - i++; - } - - if (virDomainSetInterfaceParameters(dom, device, params, nparams, flags) != 0) { - vshError(ctl, "%s", _("Unable to set interface parameters")); - goto cleanup; - } + if (virDomainSetInterfaceParameters(dom, device, params, + nparams, flags) != 0) + goto error; } ret = true; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); virDomainFree(dom); return ret; + +save_error: + vshSaveLibvirtError(); +error: + vshError(ctl, "%s", _("Unable to set interface parameters")); + goto cleanup; } /* -- 1.8.1.1

--- tools/virsh-domain.c | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 10b0a11..20a7a17 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3386,11 +3386,10 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, const char *set_arg; char *set_field = NULL; char *set_val = NULL; - virTypedParameterPtr param; virTypedParameterPtr params = NULL; int nparams = 0; - size_t params_size = 0; + int maxparams = 0; int ret = -1; int rv; int val; @@ -3409,10 +3408,6 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, for (i = 0; i < nsrc_params; i++) { param = &(src_params[i]); - if (VIR_RESIZE_N(params, params_size, nparams, 1) < 0) { - virReportOOMError(); - goto cleanup; - } /* Legacy 'weight' and 'cap' parameter */ if (param->type == VIR_TYPED_PARAM_UINT && @@ -3423,10 +3418,8 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, goto cleanup; } - if (virTypedParameterAssign(&(params[nparams++]), - param->field, - param->type, - val) < 0) { + if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + param->field, val) < 0) { vshSaveLibvirtError(); goto cleanup; } @@ -3434,12 +3427,10 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, continue; } - if (set_field && STREQ(set_field, param->field)) { - if (virTypedParameterAssignFromStr(&(params[nparams++]), - param->field, - param->type, - set_val) < 0) { + if (virTypedParamsAddFromString(¶ms, &nparams, &maxparams, + set_field, param->type, + set_val) < 0) { vshSaveLibvirtError(); goto cleanup; } @@ -3454,8 +3445,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, cleanup: VIR_FREE(set_field); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -3571,10 +3561,8 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) } cleanup: - virTypedParameterArrayClear(params, nparams); - virTypedParameterArrayClear(updates, nupdates); - VIR_FREE(params); - VIR_FREE(updates); + virTypedParamsFree(params, nparams); + virTypedParamsFree(updates, nupdates); virDomainFree(dom); return ret_val; } -- 1.8.1.1

--- tools/virsh-domain-monitor.c | 5 ++--- tools/virsh.c | 20 -------------------- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index f4940fa..72d2af1 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -880,9 +880,8 @@ cmdDomblkstat(vshControl *ctl, const vshCmd *cmd) /* at first print all known values in desired order */ for (i = 0; domblkstat_output[i].field != NULL; i++) { - if (!(par = vshFindTypedParamByName(domblkstat_output[i].field, - params, - nparams))) + if (!(par = virTypedParamsGet(params, nparams, + domblkstat_output[i].field))) continue; value = vshGetTypedParamValue(ctl, par); diff --git a/tools/virsh.c b/tools/virsh.c index 669c9c9..31cc5a1 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2028,26 +2028,6 @@ vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) return str; } -virTypedParameterPtr -vshFindTypedParamByName(const char *name, virTypedParameterPtr list, int count) -{ - int i = count; - virTypedParameterPtr found = list; - - if (!list || !name) - return NULL; - - while (i-- > 0) { - if (STREQ(name, found->field)) - return found; - - found++; /* go to next struct in array */ - } - - /* not found */ - return NULL; -} - void vshDebug(vshControl *ctl, int level, const char *format, ...) { -- 1.8.1.1

--- tools/virsh-domain.c | 99 +++++++++++++++++++++------------------------------- 1 file changed, 39 insertions(+), 60 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 20a7a17..ff620b6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6163,8 +6163,9 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd) long long hard_limit = 0, soft_limit = 0, swap_hard_limit = 0; long long min_guarantee = 0; int nparams = 0; + int maxparams = 0; unsigned int i = 0; - virTypedParameterPtr params = NULL, temp = NULL; + virTypedParameterPtr params = NULL; bool ret = false; unsigned int flags = 0; bool current = vshCommandOptBool(cmd, "current"); @@ -6196,17 +6197,41 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (hard_limit) - nparams++; + if (hard_limit) { + if (hard_limit == -1) + hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_MEMORY_HARD_LIMIT, + hard_limit) < 0) + goto save_error; + } - if (soft_limit) - nparams++; + if (soft_limit) { + if (soft_limit == -1) + soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_MEMORY_SOFT_LIMIT, + soft_limit) < 0) + goto save_error; + } - if (swap_hard_limit) - nparams++; + if (swap_hard_limit) { + if (swap_hard_limit == -1) + swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + swap_hard_limit) < 0) + goto save_error; + } - if (min_guarantee) - nparams++; + if (min_guarantee) { + if (min_guarantee == -1) + min_guarantee = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, + VIR_DOMAIN_MEMORY_MIN_GUARANTEE, + min_guarantee) < 0) + goto save_error; + } if (nparams == 0) { /* get the number of memory parameters */ @@ -6239,66 +6264,20 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd) VIR_FREE(str); } } - - ret = true; } else { - /* set the memory 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 (soft_limit) { - if (virTypedParameterAssign(temp, - VIR_DOMAIN_MEMORY_SOFT_LIMIT, - VIR_TYPED_PARAM_ULLONG, - soft_limit) < 0) - goto error; - soft_limit = 0; - } else if (hard_limit) { - if (virTypedParameterAssign(temp, - VIR_DOMAIN_MEMORY_HARD_LIMIT, - VIR_TYPED_PARAM_ULLONG, - hard_limit) < 0) - goto error; - hard_limit = 0; - } else if (swap_hard_limit) { - if (virTypedParameterAssign(temp, - VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, - VIR_TYPED_PARAM_ULLONG, - swap_hard_limit) < 0) - goto error; - swap_hard_limit = 0; - } else if (min_guarantee) { - if (virTypedParameterAssign(temp, - VIR_DOMAIN_MEMORY_MIN_GUARANTEE, - VIR_TYPED_PARAM_ULLONG, - min_guarantee) < 0) - goto error; - min_guarantee = 0; - } - - /* If the user has passed -1, we interpret it as unlimited */ - if (temp->value.ul == -1) - temp->value.ul = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; - } if (virDomainSetMemoryParameters(dom, params, nparams, flags) != 0) goto error; - else - ret = true; } + ret = true; + cleanup: - VIR_FREE(params); + virTypedParamsFree(params, nparams); virDomainFree(dom); return ret; +save_error: + vshSaveLibvirtError(); error: vshError(ctl, "%s", _("Unable to change memory parameters")); goto cleanup; -- 1.8.1.1

--- tools/virsh-domain.c | 88 +++++++++++++++++++++------------------------------- 1 file changed, 35 insertions(+), 53 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ff620b6..bcb855f 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6313,8 +6313,9 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd) { virDomainPtr dom; int nparams = 0; + int maxparams = 0; unsigned int i = 0; - virTypedParameterPtr params = NULL, temp = NULL; + virTypedParameterPtr params = NULL; const char *nodeset = NULL; bool ret = false; unsigned int flags = 0; @@ -6341,18 +6342,32 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd) if (vshCommandOptString(cmd, "nodeset", &nodeset) < 0) { vshError(ctl, "%s", _("Unable to parse nodeset.")); - virDomainFree(dom); - return false; + goto cleanup; } - if (nodeset) - nparams++; + if (nodeset && + virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_DOMAIN_NUMA_NODESET, nodeset) < 0) + goto save_error; + if (vshCommandOptString(cmd, "mode", &mode) < 0) { vshError(ctl, "%s", _("Unable to parse mode.")); - virDomainFree(dom); - return false; + goto cleanup; + } + if (mode) { + int m; + /* Accept string or integer, in case server understands newer + * integer than what strings we were compiled with + */ + if ((m = virDomainNumatuneMemModeTypeFromString(mode)) < 0 && + virStrToLong_i(mode, NULL, 0, &m) < 0) { + vshError(ctl, _("Invalid mode: %s"), mode); + goto cleanup; + } + + if (virTypedParamsAddInt(¶ms, &nparams, &maxparams, + VIR_DOMAIN_NUMA_MODE, m) < 0) + goto save_error; } - if (mode) - nparams++; if (nparams == 0) { /* get the number of numa parameters */ @@ -6386,56 +6401,23 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd) 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 && - virStrToLong_i(mode, NULL, 0, &temp->value.i) < 0) { - vshError(ctl, _("Invalid mode: %s"), 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; + goto error; } - cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + ret = true; + +cleanup: + virTypedParamsFree(params, nparams); virDomainFree(dom); return ret; + +save_error: + vshSaveLibvirtError(); +error: + vshError(ctl, "%s", _("Unable to change numa parameters")); + goto cleanup; } /* -- 1.8.1.1

--- tools/virsh-host.c | 78 +++++++++++++++++------------------------------------- 1 file changed, 25 insertions(+), 53 deletions(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 352c49a..80e30df 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -748,42 +748,41 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) { virTypedParameterPtr params = NULL; int nparams = 0; + int maxparams = 0; unsigned int flags = 0; - unsigned int shm_pages_to_scan = 0; - unsigned int shm_sleep_millisecs = 0; - unsigned int shm_merge_across_nodes = 0; - bool has_shm_pages_to_scan = false; - bool has_shm_sleep_millisecs = false; - bool has_shm_merge_across_nodes = false; + unsigned int value; bool ret = false; int rc = -1; int i = 0; - if ((rc = vshCommandOptUInt(cmd, "shm-pages-to-scan", - &shm_pages_to_scan)) < 0) { + if ((rc = vshCommandOptUInt(cmd, "shm-pages-to-scan", &value)) < 0) { vshError(ctl, "%s", _("invalid shm-pages-to-scan number")); - return false; + goto cleanup; } else if (rc > 0) { - nparams++; - has_shm_pages_to_scan = true; + if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN, + value) < 0) + goto save_error; } - if ((rc = vshCommandOptUInt(cmd, "shm-sleep-millisecs", - &shm_sleep_millisecs)) < 0) { + if ((rc = vshCommandOptUInt(cmd, "shm-sleep-millisecs", &value)) < 0) { vshError(ctl, "%s", _("invalid shm-sleep-millisecs number")); - return false; + goto cleanup; } else if (rc > 0) { - nparams++; - has_shm_sleep_millisecs = true; + if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS, + value) < 0) + goto save_error; } - if ((rc = vshCommandOptUInt(cmd, "shm-merge-across-nodes", - &shm_merge_across_nodes)) < 0) { + if ((rc = vshCommandOptUInt(cmd, "shm-merge-across-nodes", &value)) < 0) { vshError(ctl, "%s", _("invalid shm-merge-across-nodes number")); - return false; + goto cleanup; } else if (rc > 0) { - nparams++; - has_shm_merge_across_nodes = true; + if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, + VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES, + value) < 0) + goto save_error; } if (nparams == 0) { @@ -815,46 +814,19 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "\t%-15s %s\n", params[i].field, str); VIR_FREE(str); } - - ret = true; } else { - /* Set the memory parameters */ - params = vshCalloc(ctl, nparams, sizeof(*params)); - - if (i < nparams && has_shm_pages_to_scan) { - if (virTypedParameterAssign(¶ms[i++], - VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN, - VIR_TYPED_PARAM_UINT, - shm_pages_to_scan) < 0) - goto error; - } - - if (i < nparams && has_shm_sleep_millisecs) { - if (virTypedParameterAssign(¶ms[i++], - VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS, - VIR_TYPED_PARAM_UINT, - shm_sleep_millisecs) < 0) - goto error; - } - - if (i < nparams && has_shm_merge_across_nodes) { - if (virTypedParameterAssign(¶ms[i++], - VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES, - VIR_TYPED_PARAM_UINT, - shm_merge_across_nodes) < 0) - goto error; - } - if (virNodeSetMemoryParameters(ctl->conn, params, nparams, flags) != 0) goto error; - else - ret = true; } + ret = true; + cleanup: - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; +save_error: + vshSaveLibvirtError(); error: vshError(ctl, "%s", _("Unable to change memory parameters")); goto cleanup; -- 1.8.1.1

--- tools/virsh-domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bcb855f..9d50298 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5409,8 +5409,7 @@ do_show_total: VIR_FREE(s); } } - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); cleanup: virDomainFree(dom); -- 1.8.1.1

The function is just a renamed public version of former virTypedParameterArrayClear. --- daemon/remote.c | 34 +++++++++---------------- include/libvirt/libvirt.h.in | 3 +++ python/generator.py | 1 + python/libvirt-override.c | 59 +++++++++++++++----------------------------- src/libvirt_private.syms | 1 - src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 4 +-- src/rpc/gendispatch.pl | 3 +-- src/util/virtypedparam.c | 46 +++++++++++++++++++++------------- src/util/virtypedparam.h | 2 -- tools/virsh-domain.c | 2 +- 11 files changed, 70 insertions(+), 86 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 20c3858..51dbd01 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -987,8 +987,8 @@ remoteDeserializeTypedParameters(remote_typed_param *args_params_val, cleanup: if (rv < 0) { - virTypedParameterArrayClear(params, i); - VIR_FREE(params); + virTypedParamsFree(params, i); + params = NULL; } return params; } @@ -1037,8 +1037,7 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server ATTRIBUTE_UNUS cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -1147,8 +1146,7 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server ATTRIBUTE cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -1336,8 +1334,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -1966,8 +1963,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -2031,8 +2027,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -2096,8 +2091,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -2358,8 +2352,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -3862,8 +3855,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); if (dom) virDomainFree(dom); return rv; @@ -3937,8 +3929,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, args->ncpus * args->nparams); - VIR_FREE(params); + virTypedParamsFree(params, args->ncpus * args->nparams); if (dom) virDomainFree(dom); return rv; @@ -4570,8 +4561,7 @@ success: cleanup: if (rv < 0) virNetMessageSaveError(rerr); - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return rv; } diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 23f2952..0983e54 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -608,6 +608,9 @@ virTypedParamsAddFromString(virTypedParameterPtr *params, int type, const char *value); void +virTypedParamsClear (virTypedParameterPtr params, + int nparams); +void virTypedParamsFree (virTypedParameterPtr params, int nparams); diff --git a/python/generator.py b/python/generator.py index 00e4858..f853d77 100755 --- a/python/generator.py +++ b/python/generator.py @@ -537,6 +537,7 @@ skip_function = ( "virTypedParamsAddString", "virTypedParamsAddUInt", "virTypedParamsAddULLong", + "virTypedParamsClear", "virTypedParamsFree", "virTypedParamsGet", "virTypedParamsGetBoolean", diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 91e82c6..f9dff8c 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -367,8 +367,7 @@ libvirt_virDomainBlockStatsFlags(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -456,7 +455,7 @@ libvirt_virDomainGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) start_cpu += queried_ncpus; ncpus -= queried_ncpus; - virTypedParameterArrayClear(params, sumparams); + virTypedParamsClear(params, sumparams); } } else { LIBVIRT_BEGIN_ALLOW_THREADS; @@ -498,13 +497,11 @@ libvirt_virDomainGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) Py_DECREF(total); } - virTypedParameterArrayClear(params, sumparams); - VIR_FREE(params); + virTypedParamsFree(params, sumparams); return ret; error: - virTypedParameterArrayClear(params, sumparams); - VIR_FREE(params); + virTypedParamsFree(params, sumparams); Py_DECREF(ret); return error; } @@ -668,8 +665,7 @@ libvirt_virDomainGetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -717,8 +713,7 @@ libvirt_virDomainGetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -791,8 +786,7 @@ libvirt_virDomainSetSchedulerParameters(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -868,8 +862,7 @@ libvirt_virDomainSetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -943,8 +936,7 @@ libvirt_virDomainSetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -991,8 +983,7 @@ libvirt_virDomainGetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -1065,8 +1056,7 @@ libvirt_virDomainSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -1113,8 +1103,7 @@ libvirt_virDomainGetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -1187,8 +1176,7 @@ libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -1235,8 +1223,7 @@ libvirt_virDomainGetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -1310,8 +1297,7 @@ libvirt_virDomainSetInterfaceParameters(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -1359,8 +1345,7 @@ libvirt_virDomainGetInterfaceParameters(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -4305,8 +4290,7 @@ libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -4354,8 +4338,7 @@ libvirt_virDomainGetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } @@ -6466,8 +6449,7 @@ libvirt_virNodeSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, ret = VIR_PY_INT_SUCCESS; cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); VIR_FREE(new_params); return ret; } @@ -6514,8 +6496,7 @@ libvirt_virNodeGetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, ret = getPyVirTypedParameter(params, nparams); cleanup: - virTypedParameterArrayClear(params, nparams); - VIR_FREE(params); + virTypedParamsFree(params, nparams); return ret; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 521f8e0..fc23adc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1876,7 +1876,6 @@ virTimeStringThenRaw; # virtypedparam.h -virTypedParameterArrayClear; virTypedParameterArrayValidate; virTypedParameterAssign; virTypedParameterAssignFromStr; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 7631b19..9777703 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -591,6 +591,7 @@ LIBVIRT_1.0.2 { virTypedParamsAddString; virTypedParamsAddUInt; virTypedParamsAddULLong; + virTypedParamsClear; virTypedParamsFree; virTypedParamsGet; virTypedParamsGetBoolean; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a9072bb..3555dac 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1612,7 +1612,7 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val, cleanup: if (rv < 0) - virTypedParameterArrayClear(params, i); + virTypedParamsClear(params, i); return rv; } @@ -2754,7 +2754,7 @@ static int remoteDomainGetCPUStats(virDomainPtr domain, rv = ret.nparams; cleanup: if (rv < 0) - virTypedParameterArrayClear(params, nparams * ncpus); + virTypedParamsClear(params, nparams * ncpus); xdr_free((xdrproc_t) xdr_remote_domain_get_cpu_stats_ret, (char *) &ret); diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 899f4bc..f00ed7c 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -490,8 +490,7 @@ elsif ($opt_b) { " $2,\n" . " &n$1)) == NULL)\n" . " goto cleanup;\n"); - push(@free_list, " virTypedParameterArrayClear($1, n$1);"); - push(@free_list, " VIR_FREE($1);"); + push(@free_list, " virTypedParamsFree($1, n$1);"); } elsif ($args_member =~ m/<\S+>;/ or $args_member =~ m/\[\S+\];/) { # just make all other array types fail die "unhandled type for argument value: $args_member"; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 1b153a4..ae2855a 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -41,20 +41,6 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST, "boolean", "string") -void -virTypedParameterArrayClear(virTypedParameterPtr params, int nparams) -{ - int i; - - if (!params) - return; - - for (i = 0; i < nparams; i++) { - if (params[i].type == VIR_TYPED_PARAM_STRING) - VIR_FREE(params[i].value.s); - } -} - /* Validate that PARAMS contains only recognized parameter names with * correct types, and with no duplicates. Pass in as many name/type * pairs as appropriate, and pass NULL to end the list of accepted @@ -897,7 +883,7 @@ error: * * Adds new parameter called @name with char * type and sets its value to * @value. The function creates its own copy of @value string, which needs to - * be freed using virTypedParamsFree. If @params array + * be freed using virTypedParamsFree or virTypedParamsClear. If @params array * points to NULL or to a space that is not large enough to accommodate the * new parameter (@maxparams < @nparams + 1), the function allocates more * space for it and updates @maxparams. On success, @nparams is incremented @@ -959,7 +945,7 @@ error: * Adds new parameter called @name with the requested @type and parses its * value from the @value string. If the requested type is string, the function * creates its own copy of the @value string, which needs to be freed using - * virTypedParamsFree. If @params array points to NULL + * virTypedParamsFree or virTypedParamsClear. If @params array points to NULL * or to a space that is not large enough to accommodate the new parameter * (@maxparams < @nparams + 1), the function allocates more space for it and * updates @maxparams. On success, @nparams is incremented by one. The @@ -1001,6 +987,32 @@ error: /** + * virTypedParamsClear: + * @params: the array of typed parameters + * @nparams: number of parameters in the @params array + * + * Frees all memory used by string parameters. The memory occupied by @params + * is not freed; use virTypedParamsFree if you want it to be freed too. + * + * Returns nothing. + */ +void +virTypedParamsClear(virTypedParameterPtr params, + int nparams) +{ + int i; + + if (!params) + return; + + for (i = 0; i < nparams; i++) { + if (params[i].type == VIR_TYPED_PARAM_STRING) + VIR_FREE(params[i].value.s); + } +} + + +/** * virTypedParamsFree: * @params: the array of typed parameters * @nparams: number of parameters in the @params array @@ -1015,6 +1027,6 @@ virTypedParamsFree(virTypedParameterPtr params, int nparams) { virResetLastError(); - virTypedParameterArrayClear(params, nparams); + virTypedParamsClear(params, nparams); VIR_FREE(params); } diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index d46e9fe..c777a55 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -25,8 +25,6 @@ # include "internal.h" -void virTypedParameterArrayClear(virTypedParameterPtr params, int nparams); - int virTypedParameterArrayValidate(virTypedParameterPtr params, int nparams, /* const char *name, int type ... */ ...) ATTRIBUTE_SENTINEL ATTRIBUTE_RETURN_CHECK; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 9d50298..e84af59 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5369,7 +5369,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } cpu += ncpus; show_count -= ncpus; - virTypedParameterArrayClear(params, nparams * ncpus); + virTypedParamsClear(params, nparams * ncpus); } VIR_FREE(params); -- 1.8.1.1

--- python/libvirt-override.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index f9dff8c..8154024 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -803,7 +803,7 @@ libvirt_virDomainSetSchedulerParametersFlags(PyObject *self ATTRIBUTE_UNUSED, int nparams = 0; Py_ssize_t size = 0; unsigned int flags; - virTypedParameterPtr params, new_params; + virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainSetScedulerParametersFlags", @@ -878,7 +878,7 @@ libvirt_virDomainSetBlkioParameters(PyObject *self ATTRIBUTE_UNUSED, int nparams = 0; Py_ssize_t size = 0; unsigned int flags; - virTypedParameterPtr params, new_params; + virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainSetBlkioParameters", @@ -998,7 +998,7 @@ libvirt_virDomainSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, int nparams = 0; Py_ssize_t size = 0; unsigned int flags; - virTypedParameterPtr params, new_params; + virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainSetMemoryParameters", @@ -1118,7 +1118,7 @@ libvirt_virDomainSetNumaParameters(PyObject *self ATTRIBUTE_UNUSED, int nparams = 0; Py_ssize_t size = 0; unsigned int flags; - virTypedParameterPtr params, new_params; + virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainSetNumaParameters", @@ -1239,7 +1239,7 @@ libvirt_virDomainSetInterfaceParameters(PyObject *self ATTRIBUTE_UNUSED, Py_ssize_t size = 0; unsigned int flags; const char *device = NULL; - virTypedParameterPtr params, new_params; + virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OzOi:virDomainSetInterfaceParameters", @@ -4233,7 +4233,7 @@ libvirt_virDomainSetBlockIoTune(PyObject *self ATTRIBUTE_UNUSED, Py_ssize_t size = 0; const char *disk; unsigned int flags; - virTypedParameterPtr params, new_params; + virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OzOi:virDomainSetBlockIoTune", &pyobj_domain, &disk, &info, &flags)) @@ -6391,7 +6391,7 @@ libvirt_virNodeSetMemoryParameters(PyObject *self ATTRIBUTE_UNUSED, int nparams = 0; Py_ssize_t size = 0; unsigned int flags; - virTypedParameterPtr params, new_params; + virTypedParameterPtr params, new_params = NULL; if (!PyArg_ParseTuple(args, (char *)"OOi:virNodeSetMemoryParameters", -- 1.8.1.1

On Wed, Jan 16, 2013 at 15:20:17 +0100, Jiri Denemark wrote:
--- python/libvirt-override.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Pushed based on the ACK from DV's reply to the cover letter. Jirka

On Wed, Jan 16, 2013 at 03:20:05PM +0100, Jiri Denemark wrote:
Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it.
Jiri Denemark (12): Add virTypedParams* APIs virsh: Use virTypedParams* APIs in blkdeviotune virsh: Use virTypedParams* APIs in blkiotune virsh: Use virTypedParams* APIs in domiftune virsh: Use virTypedParams* APIs in schedinfo virsh: Use virTypedParams* APIs in domblkstat virsh: Use virTypedParams* APIs in memtune virsh: Use virTypedParams* APIs in numatune virsh: Use virTypedParams* APIs in node-memory-tune virsh: Use virTypedParams* APIs in cpu-stats Introduce virTypedParamsClear public API
In general agreement, the virTypedParams are the most complex construct of our APIs (beside the XML data) and this goes a long way to make them easier to use. We should probably add a small example somewhere in the documentation too (but where ?)
python: Avoid freeing uninitialized new_params pointer
This one can go in asynch, it's a bug fix, ACK on that one Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

On Thu, Jan 17, 2013 at 16:24:43 +0800, Daniel Veillard wrote:
On Wed, Jan 16, 2013 at 03:20:05PM +0100, Jiri Denemark wrote:
Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it.
Jiri Denemark (12): Add virTypedParams* APIs virsh: Use virTypedParams* APIs in blkdeviotune virsh: Use virTypedParams* APIs in blkiotune virsh: Use virTypedParams* APIs in domiftune virsh: Use virTypedParams* APIs in schedinfo virsh: Use virTypedParams* APIs in domblkstat virsh: Use virTypedParams* APIs in memtune virsh: Use virTypedParams* APIs in numatune virsh: Use virTypedParams* APIs in node-memory-tune virsh: Use virTypedParams* APIs in cpu-stats Introduce virTypedParamsClear public API
In general agreement, the virTypedParams are the most complex construct of our APIs (beside the XML data) and this goes a long way to make them easier to use. We should probably add a small example somewhere in the documentation too (but where ?)
Ideally, this would go into the developer guide if it wasn't incomplete and several years old. I think virsh could serve as a documentation until the guide is in a better shape :-)
python: Avoid freeing uninitialized new_params pointer
This one can go in asynch, it's a bug fix, ACK on that one
Thanks, I pushed it. Jirka

On 16.01.2013 15:20, Jiri Denemark wrote:
Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it.
Jiri Denemark (12): Add virTypedParams* APIs virsh: Use virTypedParams* APIs in blkdeviotune virsh: Use virTypedParams* APIs in blkiotune virsh: Use virTypedParams* APIs in domiftune virsh: Use virTypedParams* APIs in schedinfo virsh: Use virTypedParams* APIs in domblkstat virsh: Use virTypedParams* APIs in memtune virsh: Use virTypedParams* APIs in numatune virsh: Use virTypedParams* APIs in node-memory-tune virsh: Use virTypedParams* APIs in cpu-stats Introduce virTypedParamsClear public API python: Avoid freeing uninitialized new_params pointer
daemon/remote.c | 34 +- include/libvirt/libvirt.h.in | 95 ++++++ python/generator.py | 20 ++ python/libvirt-override.c | 73 ++-- src/libvirt_private.syms | 1 - src/libvirt_public.syms | 18 + src/remote/remote_driver.c | 4 +- src/rpc/gendispatch.pl | 3 +- src/util/virtypedparam.c | 782 ++++++++++++++++++++++++++++++++++++++++++- src/util/virtypedparam.h | 2 - tools/virsh-domain-monitor.c | 5 +- tools/virsh-domain.c | 542 +++++++++++------------------- tools/virsh-host.c | 78 ++--- tools/virsh.c | 20 -- 14 files changed, 1171 insertions(+), 506 deletions(-)
ACK series. Michal

On Fri, Jan 18, 2013 at 14:25:09 +0100, Michal Privoznik wrote:
On 16.01.2013 15:20, Jiri Denemark wrote:
Working with virTypedParameters in clients written in C is ugly and requires all clients to duplicate the same code. This set of APIs makes this code for manipulating with virTypedParameters integral part of libvirt so that all clients may benefit from it.
Jiri Denemark (12): Add virTypedParams* APIs virsh: Use virTypedParams* APIs in blkdeviotune virsh: Use virTypedParams* APIs in blkiotune virsh: Use virTypedParams* APIs in domiftune virsh: Use virTypedParams* APIs in schedinfo virsh: Use virTypedParams* APIs in domblkstat virsh: Use virTypedParams* APIs in memtune virsh: Use virTypedParams* APIs in numatune virsh: Use virTypedParams* APIs in node-memory-tune virsh: Use virTypedParams* APIs in cpu-stats Introduce virTypedParamsClear public API python: Avoid freeing uninitialized new_params pointer
ACK series.
Thanks, I pushed the remaining 11 patches. Jirka
participants (3)
-
Daniel Veillard
-
Jiri Denemark
-
Michal Privoznik