Some of the typed parameter APIs are exported publically, but the implementation was intermixed with private functions. Introduce virtypedparam-public.c, move all public API functions there and purge the comments stating that some functions are public. This will decrease the likelyhood of messing up the expectations as well as it will become more clear which of them are actually public. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/Makefile.am | 2 +- docs/apibuild.py | 2 +- src/util/Makefile.inc.am | 1 + src/util/virtypedparam-public.c | 795 ++++++++++++++++++++++++++++++++ src/util/virtypedparam.c | 775 ------------------------------- 5 files changed, 798 insertions(+), 777 deletions(-) create mode 100644 src/util/virtypedparam-public.c diff --git a/docs/Makefile.am b/docs/Makefile.am index 7a5d3450fc..faa5e76555 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -318,7 +318,7 @@ $(APIBUILD_STAMP): $(srcdir)/apibuild.py \ $(top_srcdir)/src/libvirt-admin.c \ $(top_srcdir)/src/util/virerror.c \ $(top_srcdir)/src/util/virevent.c \ - $(top_srcdir)/src/util/virtypedparam.c + $(top_srcdir)/src/util/virtypedparam-public.c $(AM_V_GEN)srcdir=$(srcdir) builddir=$(builddir) $(PYTHON) $(APIBUILD) touch $@ diff --git a/docs/apibuild.py b/docs/apibuild.py index dbdc1c95af..92886e1276 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -52,7 +52,7 @@ included_files = { "libvirt-stream.c": "Stream interfaces for the libvirt library", "virerror.c": "implements error handling and reporting code for libvirt", "virevent.c": "event loop for monitoring file handles", - "virtypedparam.c": "virTypedParameters APIs", + "virtypedparam-public.c": "virTypedParameters APIs", } qemu_included_files = { diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index 46866cf213..f239ab611c 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -207,6 +207,7 @@ UTIL_SOURCES = \ util/virtime.h \ util/virtpm.c \ util/virtpm.h \ + util/virtypedparam-public.c \ util/virtypedparam.c \ util/virtypedparam.h \ util/virusb.c \ diff --git a/src/util/virtypedparam-public.c b/src/util/virtypedparam-public.c new file mode 100644 index 0000000000..585e851f38 --- /dev/null +++ b/src/util/virtypedparam-public.c @@ -0,0 +1,795 @@ +/* + * virtypedparam-public.c: utility functions for dealing with virTypedParameters + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ +#include <config.h> +#include "virtypedparam.h" + +#include "viralloc.h" +#include "virerror.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +/* 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. + * This function does not raise an error, even when returning NULL. + */ +virTypedParameterPtr +virTypedParamsGet(virTypedParameterPtr params, + int nparams, + const char *name) +{ + size_t i; + + /* No need to reset errors, since this function doesn't report any. */ + + 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; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + 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; +} + + +/** + * 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + 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 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 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + goto error; + *maxparams = max; + + if (VIR_STRDUP(str, value) < 0) + 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; +} + +/** + * virTypedParamsAddStringList: + * @params: 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 store values to + * @values: the values to store into the new parameters + * + * Packs NULL-terminated list of strings @values into @params under the + * key @name. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsAddStringList(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char **values) +{ + size_t i; + int rv = -1; + + if (!values) + return 0; + + for (i = 0; values[i]; i++) { + if ((rv = virTypedParamsAddString(params, nparams, maxparams, + name, values[i])) < 0) + break; + } + + return rv; +} + + +/** + * 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 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 + * 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(); + + if (VIR_RESIZE_N(*params, max, n, 1) < 0) + goto error; + *maxparams = max; + + if (virTypedParameterAssignFromStr(*params + n, name, type, value) < 0) + goto error; + + *nparams += 1; + return 0; + + error: + virDispatchError(NULL); + return -1; +} + + +/** + * 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) +{ + size_t 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 + * + * Frees all memory used by string parameters and the memory occupied by + * @params. + * + * Returns nothing. + */ +void +virTypedParamsFree(virTypedParameterPtr params, + int nparams) +{ + virTypedParamsClear(params, nparams); + VIR_FREE(params); +} diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 779714d146..7abf0257ce 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -43,12 +43,6 @@ VIR_ENUM_IMPL(virTypedParameter, "string", ); -/* When editing this file, ensure that public exported functions - * (those in libvirt_public.syms) either trigger no errors, or else - * reset error on entrance and call virDispatchError() on exit; while - * internal utility functions (those in libvirt_private.syms) may - * report errors that the caller will dispatch. */ - static int virTypedParamsSortName(const void *left, const void *right) { @@ -452,40 +446,6 @@ virTypedParamsCopy(virTypedParameterPtr *dst, } -/* 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. - * This function does not raise an error, even when returning NULL. - */ -virTypedParameterPtr -virTypedParamsGet(virTypedParameterPtr params, - int nparams, - const char *name) -{ - size_t i; - - /* No need to reset errors, since this function doesn't report any. */ - - if (!params || !name) - return NULL; - - for (i = 0; i < nparams; i++) { - if (STREQ(params[i].field, name)) - return params + i; - } - - return NULL; -} - - /** * virTypedParamsFilter: * @params: array of typed parameters @@ -528,273 +488,6 @@ virTypedParamsFilter(virTypedParameterPtr params, } -#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; - - virResetLastError(); - - if (!(param = virTypedParamsGet(params, nparams, name))) - return 0; - - 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; -} - - /** * virTypedParamsGetStringList: * @params: array of typed parameters @@ -852,474 +545,6 @@ virTypedParamsGetStringList(virTypedParameterPtr params, } -/** - * 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - 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 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 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - goto error; - *maxparams = max; - - if (VIR_STRDUP(str, value) < 0) - 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; -} - -/** - * virTypedParamsAddStringList: - * @params: 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 store values to - * @values: the values to store into the new parameters - * - * Packs NULL-terminated list of strings @values into @params under the - * key @name. - * - * Returns 0 on success, -1 on error. - */ -int -virTypedParamsAddStringList(virTypedParameterPtr *params, - int *nparams, - int *maxparams, - const char *name, - const char **values) -{ - size_t i; - int rv = -1; - - if (!values) - return 0; - - for (i = 0; values[i]; i++) { - if ((rv = virTypedParamsAddString(params, nparams, maxparams, - name, values[i])) < 0) - break; - } - - return rv; -} - - -/** - * 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 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 - * 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(); - - if (VIR_RESIZE_N(*params, max, n, 1) < 0) - goto error; - *maxparams = max; - - if (virTypedParameterAssignFromStr(*params + n, name, type, value) < 0) - goto error; - - *nparams += 1; - return 0; - - error: - virDispatchError(NULL); - return -1; -} - - -/** - * 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) -{ - size_t 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 - * - * Frees all memory used by string parameters and the memory occupied by - * @params. - * - * Returns nothing. - */ -void -virTypedParamsFree(virTypedParameterPtr params, - int nparams) -{ - virTypedParamsClear(params, nparams); - VIR_FREE(params); -} - /** * virTypedParamsRemoteFree: * @remote_params_val: array of typed parameters as specified by -- 2.21.0

The function is not exported in the public API thus the error dispatching is not required. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 7abf0257ce..acf8c39602 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -514,8 +514,6 @@ virTypedParamsGetStringList(virTypedParameterPtr params, int nfiltered; virTypedParameterPtr *filtered = NULL; - virResetLastError(); - virCheckNonNullArgGoto(values, error); *values = NULL; @@ -540,7 +538,6 @@ virTypedParamsGetStringList(virTypedParameterPtr params, if (values) VIR_FREE(*values); VIR_FREE(filtered); - virDispatchError(NULL); return -1; } -- 2.21.0

The function is only used as a helper in virTypedParamsAddFromString. Make it static and move it to virtypedparam-public.c. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 1 - src/util/virtypedparam-public.c | 95 +++++++++++++++++++++++++++++++++ src/util/virtypedparam.c | 93 -------------------------------- src/util/virtypedparam.h | 6 --- 4 files changed, 95 insertions(+), 100 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 39812227aa..8af9e7c95e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3229,7 +3229,6 @@ virTPMSwtpmSetupFeatureTypeFromString; # util/virtypedparam.h virTypedParameterAssign; -virTypedParameterAssignFromStr; virTypedParameterToString; virTypedParameterTypeFromString; virTypedParameterTypeToString; diff --git a/src/util/virtypedparam-public.c b/src/util/virtypedparam-public.c index 585e851f38..fb7f178c6c 100644 --- a/src/util/virtypedparam-public.c +++ b/src/util/virtypedparam-public.c @@ -25,6 +25,101 @@ #define VIR_FROM_THIS VIR_FROM_NONE +/* Assign name, type, and convert the argument from a const string. + * In case of a string, the string is copied. + * Return 0 on success, -1 after an error message on failure. */ +static int +virTypedParameterAssignFromStr(virTypedParameterPtr param, + const char *name, + int type, + const char *val) +{ + int ret = -1; + + if (!val) { + virReportError(VIR_ERR_INVALID_ARG, _("NULL value for field '%s'"), + name); + goto cleanup; + } + + if (virStrcpyStatic(param->field, name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), + name); + goto cleanup; + } + + param->type = type; + switch (type) { + case VIR_TYPED_PARAM_INT: + if (virStrToLong_i(val, NULL, 10, &param->value.i) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid value for field '%s': expected int"), + name); + goto cleanup; + } + break; + case VIR_TYPED_PARAM_UINT: + if (virStrToLong_ui(val, NULL, 10, &param->value.ui) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid value for field '%s': " + "expected unsigned int"), + name); + goto cleanup; + } + break; + case VIR_TYPED_PARAM_LLONG: + if (virStrToLong_ll(val, NULL, 10, &param->value.l) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid value for field '%s': " + "expected long long"), + name); + goto cleanup; + } + break; + case VIR_TYPED_PARAM_ULLONG: + if (virStrToLong_ull(val, NULL, 10, &param->value.ul) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid value for field '%s': " + "expected unsigned long long"), + name); + goto cleanup; + } + break; + case VIR_TYPED_PARAM_DOUBLE: + if (virStrToDouble(val, NULL, &param->value.d) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid value for field '%s': " + "expected double"), + name); + goto cleanup; + } + break; + case VIR_TYPED_PARAM_BOOLEAN: + if (STRCASEEQ(val, "true") || STREQ(val, "1")) { + param->value.b = true; + } else if (STRCASEEQ(val, "false") || STREQ(val, "0")) { + param->value.b = false; + } else { + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid boolean value for field '%s'"), name); + goto cleanup; + } + break; + case VIR_TYPED_PARAM_STRING: + if (VIR_STRDUP(param->value.s, val) < 0) + goto cleanup; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected type %d for field %s"), type, name); + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + /* The following APIs are public and their signature may never change. */ /** diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index acf8c39602..d9f8203796 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -260,99 +260,6 @@ virTypedParameterAssign(virTypedParameterPtr param, const char *name, return ret; } -/* Assign name, type, and convert the argument from a const string. - * In case of a string, the string is copied. - * Return 0 on success, -1 after an error message on failure. */ -int -virTypedParameterAssignFromStr(virTypedParameterPtr param, const char *name, - int type, const char *val) -{ - int ret = -1; - - if (!val) { - virReportError(VIR_ERR_INVALID_ARG, _("NULL value for field '%s'"), - name); - goto cleanup; - } - - if (virStrcpyStatic(param->field, name) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), - name); - goto cleanup; - } - - param->type = type; - switch (type) { - case VIR_TYPED_PARAM_INT: - if (virStrToLong_i(val, NULL, 10, &param->value.i) < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid value for field '%s': expected int"), - name); - goto cleanup; - } - break; - case VIR_TYPED_PARAM_UINT: - if (virStrToLong_ui(val, NULL, 10, &param->value.ui) < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid value for field '%s': " - "expected unsigned int"), - name); - goto cleanup; - } - break; - case VIR_TYPED_PARAM_LLONG: - if (virStrToLong_ll(val, NULL, 10, &param->value.l) < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid value for field '%s': " - "expected long long"), - name); - goto cleanup; - } - break; - case VIR_TYPED_PARAM_ULLONG: - if (virStrToLong_ull(val, NULL, 10, &param->value.ul) < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid value for field '%s': " - "expected unsigned long long"), - name); - goto cleanup; - } - break; - case VIR_TYPED_PARAM_DOUBLE: - if (virStrToDouble(val, NULL, &param->value.d) < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid value for field '%s': " - "expected double"), - name); - goto cleanup; - } - break; - case VIR_TYPED_PARAM_BOOLEAN: - if (STRCASEEQ(val, "true") || STREQ(val, "1")) { - param->value.b = true; - } else if (STRCASEEQ(val, "false") || STREQ(val, "0")) { - param->value.b = false; - } else { - virReportError(VIR_ERR_INVALID_ARG, - _("Invalid boolean value for field '%s'"), name); - goto cleanup; - } - break; - case VIR_TYPED_PARAM_STRING: - if (VIR_STRDUP(param->value.s, val) < 0) - goto cleanup; - break; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected type %d for field %s"), type, name); - goto cleanup; - } - - ret = 0; - cleanup: - return ret; -} - /** * virTypedParamsReplaceString: diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index f9d22b24fb..cad6953f5d 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -85,12 +85,6 @@ int virTypedParameterAssign(virTypedParameterPtr param, const char *name, int type, /* TYPE arg */ ...) ATTRIBUTE_RETURN_CHECK; -int virTypedParameterAssignFromStr(virTypedParameterPtr param, - const char *name, - int type, - const char *val) - ATTRIBUTE_RETURN_CHECK; - int virTypedParamsReplaceString(virTypedParameterPtr *params, int *nparams, const char *name, -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam-public.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/util/virtypedparam-public.c b/src/util/virtypedparam-public.c index fb7f178c6c..eaa9f433e7 100644 --- a/src/util/virtypedparam-public.c +++ b/src/util/virtypedparam-public.c @@ -34,18 +34,16 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, int type, const char *val) { - int ret = -1; - if (!val) { virReportError(VIR_ERR_INVALID_ARG, _("NULL value for field '%s'"), name); - goto cleanup; + return -1; } if (virStrcpyStatic(param->field, name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), name); - goto cleanup; + return -1; } param->type = type; @@ -55,7 +53,7 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, virReportError(VIR_ERR_INVALID_ARG, _("Invalid value for field '%s': expected int"), name); - goto cleanup; + return -1; } break; case VIR_TYPED_PARAM_UINT: @@ -64,7 +62,7 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, _("Invalid value for field '%s': " "expected unsigned int"), name); - goto cleanup; + return -1; } break; case VIR_TYPED_PARAM_LLONG: @@ -73,7 +71,7 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, _("Invalid value for field '%s': " "expected long long"), name); - goto cleanup; + return -1; } break; case VIR_TYPED_PARAM_ULLONG: @@ -82,7 +80,7 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, _("Invalid value for field '%s': " "expected unsigned long long"), name); - goto cleanup; + return -1; } break; case VIR_TYPED_PARAM_DOUBLE: @@ -91,7 +89,7 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, _("Invalid value for field '%s': " "expected double"), name); - goto cleanup; + return -1; } break; case VIR_TYPED_PARAM_BOOLEAN: @@ -102,22 +100,20 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, } else { virReportError(VIR_ERR_INVALID_ARG, _("Invalid boolean value for field '%s'"), name); - goto cleanup; + return -1; } break; case VIR_TYPED_PARAM_STRING: if (VIR_STRDUP(param->value.s, val) < 0) - goto cleanup; + return -1; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected type %d for field %s"), type, name); - goto cleanup; + return -1; } - ret = 0; - cleanup: - return ret; + return 0; } /* The following APIs are public and their signature may never change. */ -- 2.21.0

The internal helpers are not considered any more so we don't have to ignore them either. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- docs/apibuild.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 92886e1276..b59050f5ce 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -95,13 +95,8 @@ ignored_functions = { "virDomainMigrateConfirm3": "private function for migration", "virDomainMigratePrepareTunnel3": "private function for tunnelled migration", "DllMain": "specific function for Win32", - "virTypedParamsValidate": "internal function in virtypedparam.c", "virTypedParameterValidateSet": "internal function in virtypedparam.c", - "virTypedParameterAssign": "internal function in virtypedparam.c", "virTypedParameterAssignFromStr": "internal function in virtypedparam.c", - "virTypedParameterToString": "internal function in virtypedparam.c", - "virTypedParamsCheck": "internal function in virtypedparam.c", - "virTypedParamsCopy": "internal function in virtypedparam.c", "virDomainMigrateBegin3Params": "private function for migration", "virDomainMigrateFinish3Params": "private function for migration", "virDomainMigratePerform3Params": "private function for migration", -- 2.21.0

The code will be reused in other function. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.c | 55 ++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index d9f8203796..9f86166707 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -205,24 +205,12 @@ virTypedParameterToString(virTypedParameterPtr param) return value; } -/* Assign name, type, and the appropriately typed arg to param; in the - * case of a string, the caller is assumed to have malloc'd a string, - * or can pass NULL to have this function malloc an empty string. - * Return 0 on success, -1 after an error message on failure. */ -int -virTypedParameterAssign(virTypedParameterPtr param, const char *name, - int type, ...) -{ - va_list ap; - int ret = -1; - - va_start(ap, type); - if (virStrcpyStatic(param->field, name) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), - name); - goto cleanup; - } +static int +virTypedParameterAssignValueAP(virTypedParameterPtr param, + int type, + va_list ap) +{ param->type = type; switch (type) { case VIR_TYPED_PARAM_INT: @@ -246,17 +234,40 @@ virTypedParameterAssign(virTypedParameterPtr param, const char *name, case VIR_TYPED_PARAM_STRING: param->value.s = va_arg(ap, char *); if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) - goto cleanup; + return -1; break; default: virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected type %d for field %s"), type, name); - goto cleanup; + _("unexpected type %d for field %s"), type, + NULLSTR(param->field)); + return -1; } - ret = 0; - cleanup: + return 0; +} + + +/* Assign name, type, and the appropriately typed arg to param; in the + * case of a string, the caller is assumed to have malloc'd a string, + * or can pass NULL to have this function malloc an empty string. + * Return 0 on success, -1 after an error message on failure. */ +int +virTypedParameterAssign(virTypedParameterPtr param, const char *name, + int type, ...) +{ + va_list ap; + int ret = -1; + + if (virStrcpyStatic(param->field, name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), + name); + return -1; + } + + va_start(ap, type); + ret = virTypedParameterAssignValueAP(param, type, ap); va_end(ap); + return ret; } -- 2.21.0

Some code paths pass in already pointers to strings which should be added directly as the value of the typed parameter. To allow more universal use of virTypedParameterAssignValue add a flag which allows to copy the value in place. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 9f86166707..720b88dd10 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -209,7 +209,8 @@ virTypedParameterToString(virTypedParameterPtr param) static int virTypedParameterAssignValueAP(virTypedParameterPtr param, int type, - va_list ap) + va_list ap, + bool copystr) { param->type = type; switch (type) { @@ -232,7 +233,13 @@ virTypedParameterAssignValueAP(virTypedParameterPtr param, param->value.b = !!va_arg(ap, int); break; case VIR_TYPED_PARAM_STRING: - param->value.s = va_arg(ap, char *); + if (copystr) { + if (VIR_STRDUP(param->value.s, va_arg(ap, char *)) < 0) + return -1; + } else { + param->value.s = va_arg(ap, char *); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) return -1; break; @@ -265,7 +272,7 @@ virTypedParameterAssign(virTypedParameterPtr param, const char *name, } va_start(ap, type); - ret = virTypedParameterAssignValueAP(param, type, ap); + ret = virTypedParameterAssignValueAP(param, type, ap, false); va_end(ap); return ret; -- 2.21.0

Introduce a new set of helpers including a new data structure which simplifies keeping and construction of lists of typed parameters. The use of VIR_RESIZE_N in the virTypedParamsAdd API has performance benefits but requires passing around 3 arguments. Use of them lead to a set of macros with embedded jumps used in the qemu statistics code. This patch introduces 'virTypedParamList' type which aggregates the necessary list-keeping variables and also a new set of functions to add new typed parameters to a list. These new helpers use printf-like format string and arguments to format the argument name as the stats code often uses indexed typed parameters. The accessor function then allows extracting the typed parameter list in the same format as virTypedParamsAdd* functions would do. One additional benefit is also that the list function can easily be used with VIR_AUTOPTR. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 9 ++ src/util/virtypedparam.c | 222 +++++++++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 61 +++++++++++ 3 files changed, 292 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8af9e7c95e..8f6d0492d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3232,6 +3232,15 @@ virTypedParameterAssign; virTypedParameterToString; virTypedParameterTypeFromString; virTypedParameterTypeToString; +virTypedParamListAddB; +virTypedParamListAddD; +virTypedParamListAddI; +virTypedParamListAddLL; +virTypedParamListAddS; +virTypedParamListAddUI; +virTypedParamListAddULL; +virTypedParamListFree; +virTypedParamListStealParams; virTypedParamsCheck; virTypedParamsCopy; virTypedParamsDeserialize; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 720b88dd10..785495460f 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -254,6 +254,23 @@ virTypedParameterAssignValueAP(virTypedParameterPtr param, } +static int +virTypedParameterAssignValue(virTypedParameterPtr param, + bool copystr, + int type, + ...) +{ + int ret; + va_list ap; + + va_start(ap, type); + ret = virTypedParameterAssignValueAP(param, type, ap, copystr); + va_end(ap); + + return ret; +} + + /* Assign name, type, and the appropriately typed arg to param; in the * case of a string, the caller is assumed to have malloc'd a string, * or can pass NULL to have this function malloc an empty string. @@ -725,3 +742,208 @@ virTypedParamsSerialize(virTypedParameterPtr params, virTypedParamsRemoteFree(params_val, nparams); return rv; } + + +void +virTypedParamListFree(virTypedParamListPtr list) +{ + if (!list) + return; + + virTypedParamsFree(list->par, list->npar); + VIR_FREE(list); +} + + +size_t +virTypedParamListStealParams(virTypedParamListPtr list, + virTypedParameterPtr *params) +{ + size_t ret = list->npar; + + VIR_STEAL_PTR(*params, list->par); + list->npar = 0; + list->par_alloc = 0; + + return ret; +} + + +static int ATTRIBUTE_FMT_PRINTF(2, 0) +virTypedParamSetNameVPrintf(virTypedParameterPtr par, + const char *fmt, + va_list ap) +{ + if (vsnprintf(par->field, VIR_TYPED_PARAM_FIELD_LENGTH, fmt, ap) > VIR_TYPED_PARAM_FIELD_LENGTH) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Field name too long")); + return -1; + } + + return 0; +} + + +static virTypedParameterPtr +virTypedParamListExtend(virTypedParamListPtr list) +{ + if (VIR_RESIZE_N(list->par, list->par_alloc, list->npar, 1) < 0) + return NULL; + + list->npar++; + + return list->par + (list->npar - 1); +} + + +int +virTypedParamListAddI(virTypedParamListPtr list, + int value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_INT, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} + + +int +virTypedParamListAddUI(virTypedParamListPtr list, + unsigned int value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_UINT, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} + + +int +virTypedParamListAddLL(virTypedParamListPtr list, + long long value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_LLONG, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} + + +int +virTypedParamListAddULL(virTypedParamListPtr list, + unsigned long long value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_ULLONG, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} + + +int +virTypedParamListAddS(virTypedParamListPtr list, + const char *value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_STRING, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} + + +int +virTypedParamListAddB(virTypedParamListPtr list, + bool value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_BOOLEAN, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} + + +int +virTypedParamListAddD(virTypedParamListPtr list, + double value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_DOUBLE, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index cad6953f5d..2f6fad9d1a 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -24,6 +24,7 @@ #include "internal.h" #include "virutil.h" #include "virenum.h" +#include "virautoclean.h" /** * VIR_TYPED_PARAM_MULTIPLE: @@ -94,6 +95,15 @@ int virTypedParamsCopy(virTypedParameterPtr *dst, virTypedParameterPtr src, int nparams); + +int virTypedParamsAddPrintf(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *namefmt, + int type, + ...) + ATTRIBUTE_FMT_PRINTF(4, 0) ATTRIBUTE_RETURN_CHECK; + char *virTypedParameterToString(virTypedParameterPtr param); void virTypedParamsRemoteFree(virTypedParameterRemotePtr remote_params_val, @@ -128,3 +138,54 @@ VIR_ENUM_DECL(virTypedParameter); VIR_FREE(_value); \ } \ } while (0) + +typedef struct _virTypedParamList virTypedParamList; +typedef virTypedParamList *virTypedParamListPtr; + +struct _virTypedParamList { + virTypedParameterPtr par; + size_t npar; + size_t par_alloc; +}; + +void virTypedParamListFree(virTypedParamListPtr list); +VIR_DEFINE_AUTOPTR_FUNC(virTypedParamList, virTypedParamListFree); + +size_t virTypedParamListStealParams(virTypedParamListPtr list, + virTypedParameterPtr *params); + +int virTypedParamListAddI(virTypedParamListPtr list, + int value, + const char *namefmt, + ...) + ATTRIBUTE_FMT_PRINTF(3, 4) ATTRIBUTE_RETURN_CHECK; +int virTypedParamListAddUI(virTypedParamListPtr list, + unsigned int value, + const char *namefmt, + ...) + ATTRIBUTE_FMT_PRINTF(3, 4) ATTRIBUTE_RETURN_CHECK; +int virTypedParamListAddLL(virTypedParamListPtr list, + long long value, + const char *namefmt, + ...) + ATTRIBUTE_FMT_PRINTF(3, 4) ATTRIBUTE_RETURN_CHECK; +int virTypedParamListAddULL(virTypedParamListPtr list, + unsigned long long value, + const char *namefmt, + ...) + ATTRIBUTE_FMT_PRINTF(3, 4) ATTRIBUTE_RETURN_CHECK; +int virTypedParamListAddS(virTypedParamListPtr list, + const char *value, + const char *namefmt, + ...) + ATTRIBUTE_FMT_PRINTF(3, 4) ATTRIBUTE_RETURN_CHECK; +int virTypedParamListAddB(virTypedParamListPtr list, + bool value, + const char *namefmt, + ...) + ATTRIBUTE_FMT_PRINTF(3, 4) ATTRIBUTE_RETURN_CHECK; +int virTypedParamListAddD(virTypedParamListPtr list, + double value, + const char *namefmt, + ...) + ATTRIBUTE_FMT_PRINTF(3, 4) ATTRIBUTE_RETURN_CHECK; -- 2.21.0

We no longer use HMP for this API. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e041a8bac..9315b78c48 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11548,10 +11548,6 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, } -/* This uses the 'info blockstats' monitor command which was - * integrated into both qemu & kvm in late 2007. If the command is - * not supported we detect this and return the appropriate error. - */ static int qemuDomainBlockStats(virDomainPtr dom, const char *path, -- 2.21.0

Use VIR_AUTOFREE and get rid of the cleanup label. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e4404f0199..ccf9290838 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2529,8 +2529,7 @@ static qemuBlockStatsPtr qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, int *nstats) { - qemuBlockStatsPtr bstats = NULL; - qemuBlockStatsPtr ret = NULL; + VIR_AUTOFREE(qemuBlockStatsPtr) bstats = NULL; virJSONValuePtr parent; virJSONValuePtr parentstats; virJSONValuePtr stats; @@ -2539,11 +2538,11 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats stats entry was not " "in expected format")); - goto cleanup; + return NULL; } if (VIR_ALLOC(bstats) < 0) - goto cleanup; + return NULL; #define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \ if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \ @@ -2551,7 +2550,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, if (virJSONValueObjectGetNumberLong(stats, NAME, &VAR) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ _("cannot read %s statistic"), NAME); \ - goto cleanup; \ + return NULL; \ } \ } QEMU_MONITOR_BLOCK_STAT_GET("rd_bytes", bstats->rd_bytes, true); @@ -2571,11 +2570,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, bstats->wr_highest_offset_valid = true; } - VIR_STEAL_PTR(ret, bstats); - - cleanup: - VIR_FREE(bstats); - return ret; + VIR_RETURN_PTR(bstats); } -- 2.21.0

Use VIR_AUTOFREE and get rid of the cleanup label. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ccf9290838..8d0ef62c8b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2603,17 +2603,16 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, virHashTablePtr hash, bool backingChain) { - qemuBlockStatsPtr bstats = NULL; - int ret = -1; + VIR_AUTOFREE(qemuBlockStatsPtr) bstats = NULL; int nstats = 0; const char *qdevname = NULL; const char *nodename = NULL; - char *devicename = NULL; + VIR_AUTOFREE(char *) devicename = NULL; virJSONValuePtr backing; if (dev_name && !(devicename = qemuDomainStorageAlias(dev_name, depth))) - goto cleanup; + return -1; qdevname = virJSONValueObjectGetString(dev, "qdev"); nodename = virJSONValueObjectGetString(dev, "node-name"); @@ -2621,35 +2620,31 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, if (!devicename && !qdevname && !nodename) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats device entry was not in expected format")); - goto cleanup; + return -1; } if (!(bstats = qemuMonitorJSONBlockStatsCollectData(dev, &nstats))) - goto cleanup; + return -1; if (devicename && qemuMonitorJSONAddOneBlockStatsInfo(bstats, devicename, hash) < 0) - goto cleanup; + return -1; if (qdevname && STRNEQ_NULLABLE(qdevname, devicename) && qemuMonitorJSONAddOneBlockStatsInfo(bstats, qdevname, hash) < 0) - goto cleanup; + return -1; if (nodename && qemuMonitorJSONAddOneBlockStatsInfo(bstats, nodename, hash) < 0) - goto cleanup; + return -1; if (backingChain && (backing = virJSONValueObjectGetObject(dev, "backing")) && qemuMonitorJSONGetOneBlockStatsInfo(backing, dev_name, depth + 1, hash, true) < 0) - goto cleanup; + return -1; - ret = nstats; - cleanup: - VIR_FREE(bstats); - VIR_FREE(devicename); - return ret; + return nstats; } -- 2.21.0

Use VIR_AUTOPTR and get rid of the cleanup label. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 8d0ef62c8b..9be122a465 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2678,11 +2678,10 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr hash, bool backingChain) { - int ret = -1; int nstats = 0; int rc; size_t i; - virJSONValuePtr devices; + VIR_AUTOPTR(virJSONValue) devices = NULL; if (!(devices = qemuMonitorJSONQueryBlockstats(mon))) return -1; @@ -2695,14 +2694,14 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats device entry was not " "in expected format")); - goto cleanup; + return -1; } if (!(dev_name = virJSONValueObjectGetString(dev, "device"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("blockstats device entry was not " "in expected format")); - goto cleanup; + return -1; } if (*dev_name == '\0') @@ -2712,17 +2711,13 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, backingChain); if (rc < 0) - goto cleanup; + return -1; if (rc > nstats) nstats = rc; } - ret = nstats; - - cleanup: - virJSONValueFree(devices); - return ret; + return nstats; } -- 2.21.0

None of the fields actually return negative values. The internal implementation of BlockAcctStats struct in qemu uses uint64_t and the last place using -1 in libvirt was in the HMP monitor code which was deleted. Change the internal type to unsigned long long and ensure that all public conversions don't overflow. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 35 ++++++++++++++++++++++------------- src/qemu/qemu_monitor.h | 16 ++++++++-------- src/qemu/qemu_monitor_json.c | 2 +- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9315b78c48..ab41e51700 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11416,8 +11416,7 @@ qemuDomainBlockStatsGatherTotals(qemuBlockStatsPtr data, qemuBlockStatsPtr total) { #define QEMU_BLOCK_STAT_TOTAL(NAME) \ - if (data->NAME > 0) \ - total->NAME += data->NAME + total->NAME += data->NAME QEMU_BLOCK_STAT_TOTAL(wr_bytes); QEMU_BLOCK_STAT_TOTAL(wr_req); @@ -11573,10 +11572,14 @@ qemuDomainBlockStats(virDomainPtr dom, if (qemuDomainBlocksStatsGather(driver, vm, path, false, &blockstats) < 0) goto endjob; - stats->rd_req = blockstats->rd_req; - stats->rd_bytes = blockstats->rd_bytes; - stats->wr_req = blockstats->wr_req; - stats->wr_bytes = blockstats->wr_bytes; + if (VIR_ASSIGN_IS_OVERFLOW(stats->rd_req, blockstats->rd_req) || + VIR_ASSIGN_IS_OVERFLOW(stats->rd_bytes, blockstats->rd_bytes) || + VIR_ASSIGN_IS_OVERFLOW(stats->wr_req, blockstats->wr_req) || + VIR_ASSIGN_IS_OVERFLOW(stats->wr_bytes, blockstats->wr_bytes)) { + virReportError(VIR_ERR_OVERFLOW, "%s", _("statistic value too large")); + goto endjob; + } + /* qemu doesn't report the error count */ stats->errs = -1; @@ -11638,9 +11641,15 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, nstats = 0; #define QEMU_BLOCK_STATS_ASSIGN_PARAM(VAR, NAME) \ - if (nstats < *nparams && (blockstats->VAR) != -1) { \ + if (nstats < *nparams) { \ + long long tmp; \ + if (VIR_ASSIGN_IS_OVERFLOW(tmp, (blockstats->VAR))) { \ + virReportError(VIR_ERR_OVERFLOW, \ + _("value of '%s' is too large"), NAME); \ + goto endjob; \ + } \ if (virTypedParameterAssign(params + nstats, NAME, \ - VIR_TYPED_PARAM_LLONG, (blockstats->VAR)) < 0) \ + VIR_TYPED_PARAM_LLONG, tmp) < 0) \ goto endjob; \ nstats++; \ } @@ -21490,11 +21499,11 @@ do { \ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ "block.%zu.%s", num, name); \ - if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ + if (virTypedParamsAddULLong(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + value) < 0) \ goto cleanup; \ } while (0) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 70000a1c72..321ca2b53a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -651,14 +651,14 @@ virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon); typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; struct _qemuBlockStats { - long long rd_req; - long long rd_bytes; - long long wr_req; - long long wr_bytes; - long long rd_total_times; - long long wr_total_times; - long long flush_req; - long long flush_total_times; + unsigned long long rd_req; + unsigned long long rd_bytes; + unsigned long long wr_req; + unsigned long long wr_bytes; + unsigned long long rd_total_times; + unsigned long long wr_total_times; + unsigned long long flush_req; + unsigned long long flush_total_times; unsigned long long capacity; unsigned long long physical; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9be122a465..26cd9057e4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2547,7 +2547,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, #define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \ if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \ (*nstats)++; \ - if (virJSONValueObjectGetNumberLong(stats, NAME, &VAR) < 0) { \ + if (virJSONValueObjectGetNumberUlong(stats, NAME, &VAR) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ _("cannot read %s statistic"), NAME); \ return NULL; \ -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab41e51700..2814d6825a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11415,18 +11415,14 @@ static int qemuDomainBlockStatsGatherTotals(qemuBlockStatsPtr data, qemuBlockStatsPtr total) { -#define QEMU_BLOCK_STAT_TOTAL(NAME) \ - total->NAME += data->NAME - - QEMU_BLOCK_STAT_TOTAL(wr_bytes); - QEMU_BLOCK_STAT_TOTAL(wr_req); - QEMU_BLOCK_STAT_TOTAL(rd_bytes); - QEMU_BLOCK_STAT_TOTAL(rd_req); - QEMU_BLOCK_STAT_TOTAL(flush_req); - QEMU_BLOCK_STAT_TOTAL(wr_total_times); - QEMU_BLOCK_STAT_TOTAL(rd_total_times); - QEMU_BLOCK_STAT_TOTAL(flush_total_times); -#undef QEMU_BLOCK_STAT_TOTAL + total->wr_bytes += data->wr_bytes; + total->wr_req += data->wr_req; + total->rd_bytes += data->rd_bytes; + total->rd_req += data->rd_req; + total->flush_req += data->flush_req; + total->wr_total_times += data->wr_total_times; + total->rd_total_times += data->rd_total_times; + total->flush_total_times += data->flush_total_times; return 0; } -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2814d6825a..98ada96fe7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11411,7 +11411,7 @@ qemuDomainBlockResize(virDomainPtr dom, } -static int +static void qemuDomainBlockStatsGatherTotals(qemuBlockStatsPtr data, qemuBlockStatsPtr total) { @@ -11423,7 +11423,6 @@ qemuDomainBlockStatsGatherTotals(qemuBlockStatsPtr data, total->wr_total_times += data->wr_total_times; total->rd_total_times += data->rd_total_times; total->flush_total_times += data->flush_total_times; - return 0; } -- 2.21.0

Use QEMU_ADD_BLOCK_PARAM_ULL instead since all parameters are now unsigned. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 98ada96fe7..9c24e435e9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21488,20 +21488,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, goto cleanup; \ } while (0) -/* expects a LL, but typed parameter must be ULL */ -#define QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, num, name, value) \ -do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ - "block.%zu.%s", num, name); \ - if (virTypedParamsAddULLong(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ - goto cleanup; \ -} while (0) - #define QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, num, name, value) \ do { \ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ @@ -21680,14 +21666,14 @@ qemuDomainGetStatsBlockExportFrontend(const char *frontendname, goto cleanup; } - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "rd.reqs", entry->rd_req); - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "rd.bytes", entry->rd_bytes); - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "rd.times", entry->rd_total_times); - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "wr.reqs", entry->wr_req); - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "wr.bytes", entry->wr_bytes); - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "wr.times", entry->wr_total_times); - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "fl.reqs", entry->flush_req); - QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "fl.times", entry->flush_total_times); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "rd.reqs", entry->rd_req); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "rd.bytes", entry->rd_bytes); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "rd.times", entry->rd_total_times); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "wr.reqs", entry->wr_req); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "wr.bytes", entry->wr_bytes); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "wr.times", entry->wr_total_times); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "fl.reqs", entry->flush_req); + QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "fl.times", entry->flush_total_times); ret = 0; cleanup: @@ -21882,8 +21868,6 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, return ret; } -#undef QEMU_ADD_BLOCK_PARAM_LL - #undef QEMU_ADD_BLOCK_PARAM_ULL #undef QEMU_ADD_NAME_PARAM -- 2.21.0

The bulk stats functions are specific as they pass around the list into many sub-functions and also a substantial amount of the entries uses formatted names for indexing purposes. This makes them ideal to be converted to the new virTypedParamList helpers. Unfortunately given how the functions are used this requires a big-bang rewrite of all of the calls to add entries to the parameter list. Given that a substantial simplification is achieved as well as a prety significant change to the original code is required some macros which were used only sporradically were replaced by inline calls rather than tweaking the macros first and deleting them later. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 472 ++++++++++++----------------------------- 1 file changed, 139 insertions(+), 333 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9c24e435e9..c33fd6824c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20907,22 +20907,13 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, static int qemuDomainGetStatsState(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags ATTRIBUTE_UNUSED) { - if (virTypedParamsAddInt(&record->params, - &record->nparams, - maxparams, - "state.state", - dom->state.state) < 0) + if (virTypedParamListAddI(params, dom->state.state, "state.state") < 0) return -1; - if (virTypedParamsAddInt(&record->params, - &record->nparams, - maxparams, - "state.reason", - dom->state.reason) < 0) + if (virTypedParamListAddI(params, dom->state.reason, "state.reason") < 0) return -1; return 0; @@ -21063,10 +21054,8 @@ qemuDomainGetResctrlMonData(virQEMUDriverPtr driver, static int qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams) + virTypedParamListPtr params) { - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; virQEMUResctrlMonDataPtr *resdata = NULL; size_t nresdata = 0; size_t i = 0; @@ -21080,49 +21069,29 @@ qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver, VIR_RESCTRL_MONITOR_TYPE_CACHE) < 0) goto cleanup; - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "cpu.cache.monitor.count"); - if (virTypedParamsAddUInt(&record->params, &record->nparams, - maxparams, param_name, nresdata) < 0) + if (virTypedParamListAddUI(params, nresdata, "cpu.cache.monitor.count") < 0) goto cleanup; for (i = 0; i < nresdata; i++) { - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "cpu.cache.monitor.%zu.name", i); - if (virTypedParamsAddString(&record->params, - &record->nparams, - maxparams, - param_name, - resdata[i]->name) < 0) + if (virTypedParamListAddS(params, resdata[i]->name, + "cpu.cache.monitor.%zu.name", i) < 0) goto cleanup; - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "cpu.cache.monitor.%zu.vcpus", i); - if (virTypedParamsAddString(&record->params, &record->nparams, - maxparams, param_name, - resdata[i]->vcpus) < 0) + if (virTypedParamListAddS(params, resdata[i]->vcpus, + "cpu.cache.monitor.%zu.vcpus", i) < 0) goto cleanup; - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "cpu.cache.monitor.%zu.bank.count", i); - if (virTypedParamsAddUInt(&record->params, &record->nparams, - maxparams, param_name, - resdata[i]->nstats) < 0) + if (virTypedParamListAddUI(params, resdata[i]->nstats, + "cpu.cache.monitor.%zu.bank.count", i) < 0) goto cleanup; for (j = 0; j < resdata[i]->nstats; j++) { - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "cpu.cache.monitor.%zu.bank.%zu.id", i, j); - if (virTypedParamsAddUInt(&record->params, &record->nparams, - maxparams, param_name, - resdata[i]->stats[j]->id) < 0) + if (virTypedParamListAddUI(params, resdata[i]->stats[j]->id, + "cpu.cache.monitor.%zu.bank.%zu.id", i, j) < 0) goto cleanup; - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j); - if (virTypedParamsAddUInt(&record->params, &record->nparams, - maxparams, param_name, - resdata[i]->stats[j]->vals[0]) < 0) + if (virTypedParamListAddUI(params, resdata[i]->stats[j]->vals[0], + "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j) < 0) goto cleanup; } } @@ -21138,8 +21107,7 @@ qemuDomainGetStatsCpuCache(virQEMUDriverPtr driver, static int qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams) + virTypedParamListPtr params) { qemuDomainObjPrivatePtr priv = dom->privateData; unsigned long long cpu_time = 0; @@ -21151,25 +21119,13 @@ qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom, return 0; err = virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time); - if (!err && virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - "cpu.time", - cpu_time) < 0) + if (!err && virTypedParamListAddULL(params, cpu_time, "cpu.time") < 0) return -1; err = virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time); - if (!err && virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - "cpu.user", - user_time) < 0) + if (!err && virTypedParamListAddULL(params, user_time, "cpu.user") < 0) return -1; - if (!err && virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - "cpu.system", - sys_time) < 0) + if (!err && virTypedParamListAddULL(params, sys_time, "cpu.system") < 0) return -1; return 0; @@ -21179,14 +21135,13 @@ qemuDomainGetStatsCpuCgroup(virDomainObjPtr dom, static int qemuDomainGetStatsCpu(virQEMUDriverPtr driver, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags ATTRIBUTE_UNUSED) { - if (qemuDomainGetStatsCpuCgroup(dom, record, maxparams) < 0) + if (qemuDomainGetStatsCpuCgroup(dom, params) < 0) return -1; - if (qemuDomainGetStatsCpuCache(driver, dom, record, maxparams) < 0) + if (qemuDomainGetStatsCpuCache(driver, dom, params) < 0) return -1; return 0; @@ -21196,8 +21151,7 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver, static int qemuDomainGetStatsBalloon(virQEMUDriverPtr driver, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags) { virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR]; @@ -21211,18 +21165,11 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver, cur_balloon = dom->def->mem.cur_balloon; } - if (virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - "balloon.current", - cur_balloon) < 0) + if (virTypedParamListAddULL(params, cur_balloon, "balloon.current") < 0) return -1; - if (virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - "balloon.maximum", - virDomainDefGetMemoryTotal(dom->def)) < 0) + if (virTypedParamListAddULL(params, virDomainDefGetMemoryTotal(dom->def), + "balloon.maximum") < 0) return -1; if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) @@ -21235,11 +21182,7 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver, #define STORE_MEM_RECORD(TAG, NAME) \ if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \ - if (virTypedParamsAddULLong(&record->params, \ - &record->nparams, \ - maxparams, \ - "balloon." NAME, \ - stats[i].val) < 0) \ + if (virTypedParamListAddULL(params, stats[i].val, "balloon." NAME) < 0) \ return -1; for (i = 0; i < nr_stats; i++) { @@ -21266,30 +21209,22 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver, static int qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags) { virDomainVcpuDefPtr vcpu; qemuDomainVcpuPrivatePtr vcpupriv; size_t i; int ret = -1; - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; virVcpuInfoPtr cpuinfo = NULL; unsigned long long *cpuwait = NULL; - if (virTypedParamsAddUInt(&record->params, - &record->nparams, - maxparams, - "vcpu.current", - virDomainDefGetVcpus(dom->def)) < 0) + if (virTypedParamListAddUI(params, virDomainDefGetVcpus(dom->def), + "vcpu.current") < 0) return -1; - if (virTypedParamsAddUInt(&record->params, - &record->nparams, - maxparams, - "vcpu.maximum", - virDomainDefGetVcpusMax(dom->def)) < 0) + if (virTypedParamListAddUI(params, virDomainDefGetVcpusMax(dom->def), + "vcpu.maximum") < 0) return -1; if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0 || @@ -21312,34 +21247,20 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, } for (i = 0; i < virDomainDefGetVcpus(dom->def); i++) { - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "vcpu.%u.state", cpuinfo[i].number); - if (virTypedParamsAddInt(&record->params, - &record->nparams, - maxparams, - param_name, - cpuinfo[i].state) < 0) + if (virTypedParamListAddI(params, cpuinfo[i].state, + "vcpu.%u.state", cpuinfo[i].number) < 0) goto cleanup; /* stats below are available only if the VM is alive */ if (!virDomainObjIsActive(dom)) continue; - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "vcpu.%u.time", cpuinfo[i].number); - if (virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - param_name, - cpuinfo[i].cpuTime) < 0) + if (virTypedParamListAddULL(params, cpuinfo[i].cpuTime, + "vcpu.%u.time", cpuinfo[i].number) < 0) goto cleanup; - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "vcpu.%u.wait", cpuinfo[i].number); - if (virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - param_name, - cpuwait[i]) < 0) + + if (virTypedParamListAddULL(params, cpuwait[i], + "vcpu.%u.wait", cpuinfo[i].number) < 0) goto cleanup; /* state below is extracted from the individual vcpu structs */ @@ -21349,13 +21270,10 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu); if (vcpupriv->halted != VIR_TRISTATE_BOOL_ABSENT) { - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "vcpu.%u.halted", cpuinfo[i].number); - if (virTypedParamsAddBoolean(&record->params, - &record->nparams, - maxparams, - param_name, - vcpupriv->halted == VIR_TRISTATE_BOOL_YES) < 0) + if (virTypedParamListAddB(params, + vcpupriv->halted == VIR_TRISTATE_BOOL_YES, + "vcpu.%u.halted", + cpuinfo[i].number) < 0) goto cleanup; } } @@ -21368,49 +21286,15 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, return ret; } -#define QEMU_ADD_COUNT_PARAM(record, maxparams, type, count) \ -do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "%s.count", type); \ - if (virTypedParamsAddUInt(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - count) < 0) \ - goto cleanup; \ -} while (0) - -#define QEMU_ADD_NAME_PARAM(record, maxparams, type, subtype, num, name) \ -do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ - "%s.%zu.%s", type, num, subtype); \ - if (virTypedParamsAddString(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - name) < 0) \ - goto cleanup; \ -} while (0) - -#define QEMU_ADD_NET_PARAM(record, maxparams, num, name, value) \ -do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ - "net.%zu.%s", num, name); \ - if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ - return -1; \ -} while (0) +#define QEMU_ADD_NET_PARAM(params, num, name, value) \ + if (value >= 0 && \ + virTypedParamListAddULL((params), (value), "net.%zu.%s", (num), (name)) < 0) \ + return -1; static int qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags ATTRIBUTE_UNUSED) { size_t i; @@ -21420,7 +21304,8 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, if (!virDomainObjIsActive(dom)) return 0; - QEMU_ADD_COUNT_PARAM(record, maxparams, "net", dom->def->nnets); + if (virTypedParamListAddUI(params, dom->def->nnets, "net.count") < 0) + goto cleanup; /* Check the path is one of the domain's network interfaces. */ for (i = 0; i < dom->def->nnets; i++) { @@ -21434,8 +21319,8 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, actualType = virDomainNetGetActualType(net); - QEMU_ADD_NAME_PARAM(record, maxparams, - "net", "name", i, net->ifname); + if (virTypedParamListAddS(params, net->ifname, "net.%zu.name", i) < 0) + goto cleanup; if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) { @@ -21450,21 +21335,21 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, } } - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "rx.bytes", tmp.rx_bytes); - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "rx.pkts", tmp.rx_packets); - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "rx.errs", tmp.rx_errs); - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "rx.drop", tmp.rx_drop); - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "tx.bytes", tmp.tx_bytes); - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "tx.pkts", tmp.tx_packets); - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "tx.errs", tmp.tx_errs); - QEMU_ADD_NET_PARAM(record, maxparams, i, + QEMU_ADD_NET_PARAM(params, i, "tx.drop", tmp.tx_drop); } @@ -21475,39 +21360,16 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, #undef QEMU_ADD_NET_PARAM -#define QEMU_ADD_BLOCK_PARAM_UI(record, maxparams, num, name, value) \ - do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ - "block.%zu.%s", num, name); \ - if (virTypedParamsAddUInt(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ - goto cleanup; \ - } while (0) - -#define QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, num, name, value) \ -do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ - "block.%zu.%s", num, name); \ - if (virTypedParamsAddULLong(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ - goto cleanup; \ -} while (0) +#define QEMU_ADD_BLOCK_PARAM_ULL(params, num, name, value) \ + if (virTypedParamListAddULL((params), (value), "block.%zu.%s", (num), (name)) < 0) \ + goto cleanup /* refresh information by opening images on the disk */ static int qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, virStorageSourcePtr src, size_t block_idx) { @@ -21522,13 +21384,13 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver, } if (src->allocation) - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, "allocation", src->allocation); if (src->capacity) - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, "capacity", src->capacity); if (src->physical) - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, "physical", src->physical); ret = 0; cleanup: @@ -21576,8 +21438,7 @@ static int qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, const char *entryname, virStorageSourcePtr src, size_t block_idx, @@ -21589,8 +21450,8 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, /* the VM is offline so we have to go and load the stast from the disk by * ourselves */ if (!virDomainObjIsActive(dom)) { - ret = qemuDomainGetStatsOneBlockFallback(driver, cfg, dom, record, - maxparams, src, block_idx); + ret = qemuDomainGetStatsOneBlockFallback(driver, cfg, dom, params, + src, block_idx); goto cleanup; } @@ -21602,18 +21463,18 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, goto cleanup; } - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, "allocation", entry->wr_highest_offset); if (entry->capacity) - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, "capacity", entry->capacity); if (entry->physical) { - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, "physical", entry->physical); } else { if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0) - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, + QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, "physical", src->physical); } @@ -21627,8 +21488,7 @@ static int qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, virHashTablePtr stats, size_t recordnr, - virDomainStatsRecordPtr records, - int *nrecords) + virTypedParamListPtr params) { qemuBlockStats *entry; int ret = -1; @@ -21639,7 +21499,7 @@ qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, } if (entry->write_threshold) - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "threshold", + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "threshold", entry->write_threshold); ret = 0; @@ -21652,8 +21512,7 @@ static int qemuDomainGetStatsBlockExportFrontend(const char *frontendname, virHashTablePtr stats, size_t recordnr, - virDomainStatsRecordPtr records, - int *nrecords) + virTypedParamListPtr params) { qemuBlockStats *entry; int ret = -1; @@ -21666,14 +21525,14 @@ qemuDomainGetStatsBlockExportFrontend(const char *frontendname, goto cleanup; } - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "rd.reqs", entry->rd_req); - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "rd.bytes", entry->rd_bytes); - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "rd.times", entry->rd_total_times); - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "wr.reqs", entry->wr_req); - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "wr.bytes", entry->wr_bytes); - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "wr.times", entry->wr_total_times); - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "fl.reqs", entry->flush_req); - QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "fl.times", entry->flush_total_times); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "rd.reqs", entry->rd_req); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "rd.bytes", entry->rd_bytes); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "rd.times", entry->rd_total_times); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "wr.reqs", entry->wr_req); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "wr.bytes", entry->wr_bytes); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "wr.times", entry->wr_total_times); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "fl.reqs", entry->flush_req); + QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "fl.times", entry->flush_total_times); ret = 0; cleanup: @@ -21685,18 +21544,20 @@ static int qemuDomainGetStatsBlockExportHeader(virDomainDiskDefPtr disk, virStorageSourcePtr src, size_t recordnr, - virDomainStatsRecordPtr records, - int *nrecords) + virTypedParamListPtr params) { int ret = -1; - QEMU_ADD_NAME_PARAM(records, nrecords, "block", "name", recordnr, disk->dst); + if (virTypedParamListAddS(params, disk->dst, "block.%zu.name", recordnr) < 0) + goto cleanup; + + if (virStorageSourceIsLocalStorage(src) && src->path && + virTypedParamListAddS(params, src->path, "block.%zu.path", recordnr) < 0) + goto cleanup; - if (virStorageSourceIsLocalStorage(src) && src->path) - QEMU_ADD_NAME_PARAM(records, nrecords, "block", "path", recordnr, src->path); - if (src->id) - QEMU_ADD_BLOCK_PARAM_UI(records, nrecords, recordnr, "backingIndex", - src->id); + if (src->id && + virTypedParamListAddUI(params, src->id, "block.%zu.backingIndex", recordnr) < 0) + goto cleanup; ret = 0; cleanup: @@ -21708,8 +21569,7 @@ static int qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, virHashTablePtr stats, virHashTablePtr nodestats, - virDomainStatsRecordPtr records, - int *nrecords, + virTypedParamListPtr params, size_t *recordnr, bool visitBacking, virQEMUDriverPtr driver, @@ -21736,7 +21596,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, "skip getting stats", disk->dst); return qemuDomainGetStatsBlockExportHeader(disk, disk->src, *recordnr, - records, nrecords); + params); } for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { @@ -21757,25 +21617,24 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, backendstoragealias = alias; } - if (qemuDomainGetStatsBlockExportHeader(disk, n, *recordnr, - records, nrecords) < 0) + if (qemuDomainGetStatsBlockExportHeader(disk, n, *recordnr, params) < 0) goto cleanup; /* The following stats make sense only for the frontend device */ if (n == disk->src) { if (qemuDomainGetStatsBlockExportFrontend(frontendalias, stats, *recordnr, - records, nrecords) < 0) + params) < 0) goto cleanup; } - if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, + if (qemuDomainGetStatsOneBlock(driver, cfg, dom, params, backendalias, n, *recordnr, stats) < 0) goto cleanup; if (qemuDomainGetStatsBlockExportBackendStorage(backendstoragealias, stats, *recordnr, - records, nrecords) < 0) + params) < 0) goto cleanup; VIR_FREE(alias); @@ -21796,8 +21655,7 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, static int qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags) { size_t i; @@ -21846,18 +21704,19 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, /* When listing backing chains, it's easier to fix up the count * after the iteration than it is to iterate twice; but we still * want count listed first. */ - count_index = record->nparams; - QEMU_ADD_COUNT_PARAM(record, maxparams, "block", 0); + count_index = params->npar; + if (virTypedParamListAddUI(params, 0, "block.count") < 0) + goto cleanup; for (i = 0; i < dom->def->ndisks; i++) { if (qemuDomainGetStatsBlockExportDisk(dom->def->disks[i], stats, nodestats, - record, maxparams, &visited, + params, &visited, visitBacking, driver, cfg, dom, blockdev) < 0) goto cleanup; } - record->params[count_index].value.ui = visited; + params->par[count_index].value.ui = visited; ret = 0; cleanup: @@ -21870,39 +21729,10 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, #undef QEMU_ADD_BLOCK_PARAM_ULL -#undef QEMU_ADD_NAME_PARAM - -#define QEMU_ADD_IOTHREAD_PARAM_UI(record, maxparams, id, name, value) \ - do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ - "iothread.%u.%s", id, name); \ - if (virTypedParamsAddUInt(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ - goto cleanup; \ - } while (0) - -#define QEMU_ADD_IOTHREAD_PARAM_ULL(record, maxparams, id, name, value) \ -do { \ - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ - "iothread.%u.%s", id, name); \ - if (virTypedParamsAddULLong(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ - goto cleanup; \ -} while (0) - static int qemuDomainGetStatsIOThread(virQEMUDriverPtr driver, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags ATTRIBUTE_UNUSED) { qemuDomainObjPrivatePtr priv = dom->privateData; @@ -21923,22 +21753,20 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver, if (niothreads == 0) return 0; - QEMU_ADD_COUNT_PARAM(record, maxparams, "iothread", niothreads); + if (virTypedParamListAddUI(params, niothreads, "iothread.count") < 0) + goto cleanup; for (i = 0; i < niothreads; i++) { if (iothreads[i]->poll_valid) { - QEMU_ADD_IOTHREAD_PARAM_ULL(record, maxparams, - iothreads[i]->iothread_id, - "poll-max-ns", - iothreads[i]->poll_max_ns); - QEMU_ADD_IOTHREAD_PARAM_UI(record, maxparams, - iothreads[i]->iothread_id, - "poll-grow", - iothreads[i]->poll_grow); - QEMU_ADD_IOTHREAD_PARAM_UI(record, maxparams, - iothreads[i]->iothread_id, - "poll-shrink", - iothreads[i]->poll_shrink); + if (virTypedParamListAddULL(params, iothreads[i]->poll_max_ns, + "iothread.%zu.poll-max-ns", i) < 0) + goto cleanup; + if (virTypedParamListAddUI(params, iothreads[i]->poll_grow, + "iothread.%zu.poll-grow", i) < 0) + goto cleanup; + if (virTypedParamListAddUI(params, iothreads[i]->poll_shrink, + "iothread.%zu.poll-shrink", i) < 0) + goto cleanup; } } @@ -21952,32 +21780,19 @@ qemuDomainGetStatsIOThread(virQEMUDriverPtr driver, return ret; } -#undef QEMU_ADD_IOTHREAD_PARAM_UI - -#undef QEMU_ADD_IOTHREAD_PARAM_ULL - -#undef QEMU_ADD_COUNT_PARAM static int qemuDomainGetStatsPerfOneEvent(virPerfPtr perf, virPerfEventType type, - virDomainStatsRecordPtr record, - int *maxparams) + virTypedParamListPtr params) { - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; uint64_t value = 0; if (virPerfReadEvent(perf, type, &value) < 0) return -1; - snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "perf.%s", - virPerfEventTypeToString(type)); - - if (virTypedParamsAddULLong(&record->params, - &record->nparams, - maxparams, - param_name, - value) < 0) + if (virTypedParamListAddULL(params, value, "perf.%s", + virPerfEventTypeToString(type)) < 0) return -1; return 0; @@ -21986,8 +21801,7 @@ qemuDomainGetStatsPerfOneEvent(virPerfPtr perf, static int qemuDomainGetStatsPerf(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr params, unsigned int privflags ATTRIBUTE_UNUSED) { size_t i; @@ -21998,8 +21812,7 @@ qemuDomainGetStatsPerf(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, if (!virPerfEventIsEnabled(priv->perf, i)) continue; - if (qemuDomainGetStatsPerfOneEvent(priv->perf, i, - record, maxparams) < 0) + if (qemuDomainGetStatsPerfOneEvent(priv->perf, i, params) < 0) goto cleanup; } @@ -22012,8 +21825,7 @@ qemuDomainGetStatsPerf(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, typedef int (*qemuDomainGetStatsFunc)(virQEMUDriverPtr driver, virDomainObjPtr dom, - virDomainStatsRecordPtr record, - int *maxparams, + virTypedParamListPtr list, unsigned int flags); struct qemuDomainGetStatsWorker { @@ -22084,37 +21896,31 @@ qemuDomainGetStats(virConnectPtr conn, virDomainStatsRecordPtr *record, unsigned int flags) { - int maxparams = 0; - virDomainStatsRecordPtr tmp; + VIR_AUTOFREE(virDomainStatsRecordPtr) tmp = NULL; + VIR_AUTOPTR(virTypedParamList) params = NULL; size_t i; - int ret = -1; - if (VIR_ALLOC(tmp) < 0) - goto cleanup; + if (VIR_ALLOC(params) < 0) + return -1; for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) { if (stats & qemuDomainGetStatsWorkers[i].stats) { - if (qemuDomainGetStatsWorkers[i].func(conn->privateData, dom, tmp, - &maxparams, flags) < 0) - goto cleanup; + if (qemuDomainGetStatsWorkers[i].func(conn->privateData, dom, params, + flags) < 0) + return -1; } } + if (VIR_ALLOC(tmp) < 0) + return -1; + if (!(tmp->dom = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id))) - goto cleanup; - - *record = tmp; - tmp = NULL; - ret = 0; - - cleanup: - if (tmp) { - virTypedParamsFree(tmp->params, tmp->nparams); - VIR_FREE(tmp); - } + return -1; - return ret; + tmp->nparams = virTypedParamListStealParams(params, &tmp->params); + VIR_STEAL_PTR(*record, tmp); + return 0; } -- 2.21.0

The open-coded version does not take much more space and additionally we get rid of the hidden goto. This also requires us to remove the 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c33fd6824c..b703436744 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21373,8 +21373,6 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver, virStorageSourcePtr src, size_t block_idx) { - int ret = -1; - if (virStorageSourceIsEmpty(src)) return 0; @@ -21383,18 +21381,22 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver, return 0; } - if (src->allocation) - QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, - "allocation", src->allocation); - if (src->capacity) - QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, - "capacity", src->capacity); - if (src->physical) - QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, - "physical", src->physical); - ret = 0; - cleanup: - return ret; + if (src->allocation && + virTypedParamListAddULL(params, src->allocation, + "block.%zu.allocation", block_idx) < 0) + return -1; + + if (src->capacity && + virTypedParamListAddULL(params, src->capacity, + "block.%zu.capacity", block_idx) < 0) + return -1; + + if (src->physical && + virTypedParamListAddULL(params, src->physical, + "block.%zu.physical", block_idx) < 0) + return -1; + + return 0; } -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b703436744..703f07a388 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21447,42 +21447,42 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, virHashTablePtr stats) { qemuBlockStats *entry; - int ret = -1; /* the VM is offline so we have to go and load the stast from the disk by * ourselves */ if (!virDomainObjIsActive(dom)) { - ret = qemuDomainGetStatsOneBlockFallback(driver, cfg, dom, params, - src, block_idx); - goto cleanup; + return qemuDomainGetStatsOneBlockFallback(driver, cfg, dom, params, + src, block_idx); } /* In case where qemu didn't provide the stats we stop here rather than * trying to refresh the stats from the disk. Inability to provide stats is * usually caused by blocked storage so this would make libvirtd hang */ - if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) { - ret = 0; - goto cleanup; - } + if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) + return 0; - QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, - "allocation", entry->wr_highest_offset); + if (virTypedParamListAddULL(params, entry->wr_highest_offset, + "block.%zu.allocation", block_idx) < 0) + return -1; + + if (entry->capacity && + virTypedParamListAddULL(params, entry->capacity, + "block.%zu.capacity", block_idx) < 0) + return -1; - if (entry->capacity) - QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, - "capacity", entry->capacity); if (entry->physical) { - QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, - "physical", entry->physical); + if (virTypedParamListAddULL(params, entry->physical, + "block.%zu.physical", block_idx) < 0) + return -1; } else { - if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0) - QEMU_ADD_BLOCK_PARAM_ULL(params, block_idx, - "physical", src->physical); + if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0) { + if (virTypedParamListAddULL(params, src->physical, + "block.%zu.physical", block_idx) < 0) + return -1; + } } - ret = 0; - cleanup: - return ret; + return 0; } -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 703f07a388..2163d14cea 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21493,20 +21493,16 @@ qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, virTypedParamListPtr params) { qemuBlockStats *entry; - int ret = -1; - if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) { - ret = 0; - goto cleanup; - } + if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) + return 0; - if (entry->write_threshold) - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "threshold", - entry->write_threshold); + if (entry->write_threshold && + virTypedParamListAddULL(params, entry->write_threshold, + "block.%zu.threshold", recordnr) < 0) + return -1; - ret = 0; - cleanup: - return ret; + return 0; } -- 2.21.0

The macro now became unused so it was deleted. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2163d14cea..f0f08dcbfc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21360,10 +21360,6 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, #undef QEMU_ADD_NET_PARAM -#define QEMU_ADD_BLOCK_PARAM_ULL(params, num, name, value) \ - if (virTypedParamListAddULL((params), (value), "block.%zu.%s", (num), (name)) < 0) \ - goto cleanup - /* refresh information by opening images on the disk */ static int qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver, @@ -21509,32 +21505,28 @@ qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, static int qemuDomainGetStatsBlockExportFrontend(const char *frontendname, virHashTablePtr stats, - size_t recordnr, - virTypedParamListPtr params) + size_t idx, + virTypedParamListPtr par) { - qemuBlockStats *entry; - int ret = -1; + qemuBlockStats *en; /* In case where qemu didn't provide the stats we stop here rather than * trying to refresh the stats from the disk. Inability to provide stats is * usually caused by blocked storage so this would make libvirtd hang */ - if (!stats || !frontendname || !(entry = virHashLookup(stats, frontendname))) { - ret = 0; - goto cleanup; - } + if (!stats || !frontendname || !(en = virHashLookup(stats, frontendname))) + return 0; - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "rd.reqs", entry->rd_req); - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "rd.bytes", entry->rd_bytes); - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "rd.times", entry->rd_total_times); - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "wr.reqs", entry->wr_req); - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "wr.bytes", entry->wr_bytes); - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "wr.times", entry->wr_total_times); - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "fl.reqs", entry->flush_req); - QEMU_ADD_BLOCK_PARAM_ULL(params, recordnr, "fl.times", entry->flush_total_times); + if (virTypedParamListAddULL(par, en->rd_req, "block.%zu.rd.reqs", idx) < 0 || + virTypedParamListAddULL(par, en->rd_bytes, "block.%zu.rd.bytes", idx) < 0 || + virTypedParamListAddULL(par, en->rd_total_times, "block.%zu.rd.times", idx) < 0 || + virTypedParamListAddULL(par, en->wr_req, "block.%zu.wr.reqs", idx) < 0 || + virTypedParamListAddULL(par, en->wr_bytes, "block.%zu.wr.bytes", idx) < 0 || + virTypedParamListAddULL(par, en->wr_total_times, "block.%zu.wr.times", idx) < 0 || + virTypedParamListAddULL(par, en->flush_req, "block.%zu.fl.reqs", idx) < 0 || + virTypedParamListAddULL(par, en->flush_total_times, "block.%zu.fl.times", idx) < 0) + return -1; - ret = 0; - cleanup: - return ret; + return 0; } @@ -21725,7 +21717,6 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, return ret; } -#undef QEMU_ADD_BLOCK_PARAM_ULL static int qemuDomainGetStatsIOThread(virQEMUDriverPtr driver, -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f0f08dcbfc..319f2d9476 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21536,22 +21536,18 @@ qemuDomainGetStatsBlockExportHeader(virDomainDiskDefPtr disk, size_t recordnr, virTypedParamListPtr params) { - int ret = -1; - if (virTypedParamListAddS(params, disk->dst, "block.%zu.name", recordnr) < 0) - goto cleanup; + return -1; if (virStorageSourceIsLocalStorage(src) && src->path && virTypedParamListAddS(params, src->path, "block.%zu.path", recordnr) < 0) - goto cleanup; + return -1; if (src->id && virTypedParamListAddUI(params, src->id, "block.%zu.backingIndex", recordnr) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } @@ -21795,20 +21791,16 @@ qemuDomainGetStatsPerf(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, { size_t i; qemuDomainObjPrivatePtr priv = dom->privateData; - int ret = -1; for (i = 0; i < VIR_PERF_EVENT_LAST; i++) { if (!virPerfEventIsEnabled(priv->perf, i)) continue; if (qemuDomainGetStatsPerfOneEvent(priv->perf, i, params) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } typedef int -- 2.21.0