We had a memory leak on a very arcane OOM situation (unlikely to ever hit in practice, but who knows if libvirt.so would ever be linked into some other program that exhausts all thread-local storage keys?). I found it by code inspection, while analyzing a valgrind report generated by Alex Jia. * src/util/threads.h (virThreadLocalSet): Alter signature. * src/util/threads-pthread.c (virThreadHelper): Reduce allocation lifetime. (virThreadLocalSet): Detect failure. * src/util/threads-win32.c (virThreadLocalSet): Likewise. (virCondWait): Fix caller. * src/util/virterror.c (virLastErrorObject): Likewise. --- src/util/threads-pthread.c | 14 +++++++++++--- src/util/threads-win32.c | 9 ++++++--- src/util/threads.h | 2 +- src/util/virterror.c | 5 +++-- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c index 5b8fd5b..ea64887 100644 --- a/src/util/threads-pthread.c +++ b/src/util/threads-pthread.c @@ -154,8 +154,11 @@ struct virThreadArgs { static void *virThreadHelper(void *data) { struct virThreadArgs *args = data; - args->func(args->opaque); + struct virThreadArgs local = *args; + + /* Free args early, rather than tying it up during the entire thread. */ VIR_FREE(args); + local.func(local.opaque); return NULL; } @@ -249,7 +252,12 @@ void *virThreadLocalGet(virThreadLocalPtr l) return pthread_getspecific(l->key); } -void virThreadLocalSet(virThreadLocalPtr l, void *val) +int virThreadLocalSet(virThreadLocalPtr l, void *val) { - pthread_setspecific(l->key, val); + int err = pthread_setspecific(l->key, val); + if (err) { + errno = err; + return -1; + } + return 0; } diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c index 63f699b..1c33826 100644 --- a/src/util/threads-win32.c +++ b/src/util/threads-win32.c @@ -155,7 +155,10 @@ int virCondWait(virCondPtr c, virMutexPtr m) if (!event) { return -1; } - virThreadLocalSet(&virCondEvent, event); + if (virThreadLocalSet(&virCondEvent, event) < 0) { + CloseHandle(event); + return -1; + } } virMutexLock(&c->lock); @@ -376,7 +379,7 @@ void *virThreadLocalGet(virThreadLocalPtr l) return TlsGetValue(l->key); } -void virThreadLocalSet(virThreadLocalPtr l, void *val) +int virThreadLocalSet(virThreadLocalPtr l, void *val) { - TlsSetValue(l->key, val); + return TlsSetValue(l->key, val) == 0 ? -1 : 0; } diff --git a/src/util/threads.h b/src/util/threads.h index e52f3a9..e5000ea 100644 --- a/src/util/threads.h +++ b/src/util/threads.h @@ -104,7 +104,7 @@ typedef void (*virThreadLocalCleanup)(void *); int virThreadLocalInit(virThreadLocalPtr l, virThreadLocalCleanup c) ATTRIBUTE_RETURN_CHECK; void *virThreadLocalGet(virThreadLocalPtr l); -void virThreadLocalSet(virThreadLocalPtr l, void*); +int virThreadLocalSet(virThreadLocalPtr l, void*) ATTRIBUTE_RETURN_CHECK; # ifdef WIN32 # include "threads-win32.h" diff --git a/src/util/virterror.c b/src/util/virterror.c index 380dc56..8205516 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -267,7 +267,8 @@ virLastErrorObject(void) if (!err) { if (VIR_ALLOC(err) < 0) return NULL; - virThreadLocalSet(&virLastErr, err); + if (virThreadLocalSet(&virLastErr, err) < 0) + VIR_FREE(err); } return err; } @@ -612,7 +613,7 @@ virDispatchError(virConnectPtr conn) virErrorFunc handler = virErrorHandler; void *userData = virUserData; - /* Should never happen, but doesn't hurt to check */ + /* Can only happen on OOM. */ if (!err) return; -- 1.7.7.5

Gnulib claims that there are some classes of warnings that are worth enabling during development, but where silencing those warnings causes code bloat that is not necessary in an optimized build. The code bloat to silence the warnings is only enabled by -Dlint. Follow the lead of coreutils in setting up -Dlint whenever full warnings are requested. * m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Add -Dlint, and move _FORTIFY_SOURCE to config.h instead of CFLAGS. --- m4/virt-compile-warnings.m4 | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index ba388aa..3a428c3 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -100,8 +100,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ gl_WARN_ADD([-Wframe-larger-than=4096]) dnl gl_WARN_ADD([-Wframe-larger-than=256]) + # Silence certain warnings in gnulib, and use improved glibc headers + AC_DEFINE([lint], [1], + [Define to 1 if the compiler is checking for lint.]) + AC_DEFINE([_FORTIFY_SOURCE], [2], + [enable compile-time and run-time bounds-checking, and some warnings]) + # Extra special flags - gl_WARN_ADD([-Wp,-D_FORTIFY_SOURCE=2]) dnl -fstack-protector stuff passes gl_WARN_ADD with gcc dnl on Mingw32, but fails when actually used case $host in -- 1.7.7.5

Based on qemu changes made in commits ae523427 and 659ded58. * src/lxc/lxc_driver.c (lxcSetSchedulerParametersFlags) (lxcGetSchedulerParametersFlags, lxcDomainSetBlkioParameters) (lxcDomainGetBlkioParameters): Use helpers. (lxcDomainSetBlkioParameters): Allow setting live and config at once. --- src/lxc/lxc_driver.c | 160 +++++++++----------------------------------------- 1 files changed, 29 insertions(+), 131 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3baff19..7d6ac59 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_driver.c: linux container driver functions @@ -2749,7 +2749,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainDefPtr vmdef = NULL; int ret = -1; - bool isActive; int rc; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -2765,22 +2764,11 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &vmdef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - lxcError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -2788,12 +2776,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - lxcError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { lxcError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); @@ -2919,12 +2901,12 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, lxc_driver_t *driver = dom->conn->privateData; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef; unsigned long long shares = 0; unsigned long long period = 0; long long quota = 0; int ret = -1; int rc; - bool isActive; bool cpu_bw_status = false; int saved_nparams = 0; @@ -2948,52 +2930,19 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - lxcError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot query persistent config of a transient domain")); - goto cleanup; - } - - if (isActive) { - virDomainDefPtr persistentDef; - - persistentDef = virDomainObjGetPersistentDef(driver->caps, vm); - if (!persistentDef) { - lxcError(VIR_ERR_INTERNAL_ERROR, "%s", - _("can't get persistentDef")); - goto cleanup; - } - shares = persistentDef->cputune.shares; - if (*nparams > 1 && cpu_bw_status) { - period = persistentDef->cputune.period; - quota = persistentDef->cputune.quota; - } - } else { - shares = vm->def->cputune.shares; - if (*nparams > 1 && cpu_bw_status) { - period = vm->def->cputune.period; - quota = vm->def->cputune.quota; - } + shares = persistentDef->cputune.shares; + if (*nparams > 1 && cpu_bw_status) { + period = persistentDef->cputune.period; + quota = persistentDef->cputune.quota; } goto out; } - if (!isActive) { - lxcError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain is not running")); - goto cleanup; - } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { lxcError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); @@ -3091,7 +3040,6 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; - bool isActive; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -3105,22 +3053,11 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - lxcError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { lxcError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't mounted")); goto cleanup; @@ -3131,20 +3068,7 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; } - } - - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - lxcError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - ret = 0; - if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < nparams; i++) { virTypedParameterPtr param = &params[i]; @@ -3153,30 +3077,29 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, if (param->type != VIR_TYPED_PARAM_UINT) { lxcError(VIR_ERR_INVALID_ARG, "%s", _("invalid type for blkio weight tunable, expected a 'unsigned int'")); - ret = -1; - continue; + goto cleanup; } if (params[i].value.ui > 1000 || params[i].value.ui < 100) { lxcError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); - ret = -1; - continue; + goto cleanup; } rc = virCgroupSetBlkioWeight(group, params[i].value.ui); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set blkio weight tunable")); - ret = -1; + goto cleanup; } } else { lxcError(VIR_ERR_INVALID_ARG, _("Parameter `%s' not supported"), param->field); - ret = -1; + goto cleanup; } } - } else if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + } + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Clang can't see that if we get here, persistentDef was set. */ sa_assert(persistentDef); @@ -3187,29 +3110,28 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, if (param->type != VIR_TYPED_PARAM_UINT) { lxcError(VIR_ERR_INVALID_ARG, "%s", _("invalid type for blkio weight tunable, expected a 'unsigned int'")); - ret = -1; - continue; + goto cleanup; } if (params[i].value.ui > 1000 || params[i].value.ui < 100) { lxcError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); - ret = -1; - continue; + goto cleanup; } persistentDef->blkio.weight = params[i].value.ui; } else { lxcError(VIR_ERR_INVALID_ARG, _("Parameter `%s' not supported"), param->field); - ret = -1; + goto cleanup; } } if (virDomainSaveConfig(driver->configDir, persistentDef) < 0) - ret = -1; + goto cleanup; } + ret = 0; cleanup: virCgroupFree(&group); if (vm) @@ -3233,7 +3155,6 @@ static int lxcDomainGetBlkioParameters(virDomainPtr dom, unsigned int val; int ret = -1; int rc; - bool isActive; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -3254,22 +3175,11 @@ static int lxcDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; } - isActive = virDomainObjIsActive(vm); - - if (flags == VIR_DOMAIN_AFFECT_CURRENT) { - if (isActive) - flags = VIR_DOMAIN_AFFECT_LIVE; - else - flags = VIR_DOMAIN_AFFECT_CONFIG; - } + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (!isActive) { - lxcError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; - } - if (!lxcCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { lxcError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't mounted")); goto cleanup; @@ -3280,19 +3190,7 @@ static int lxcDomainGetBlkioParameters(virDomainPtr dom, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; } - } - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - if (!vm->persistent) { - lxcError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change persistent config of a transient domain")); - goto cleanup; - } - if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) - goto cleanup; - } - - if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = &params[i]; val = 0; -- 1.7.7.5

Preparation for another patch that refactors common patterns into the new file for fewer lines of code overall. * src/util/util.h (virTypedParameterArrayClear): Move... * src/util/virtypedparam.h: ...to new file. (virTypedParameterArrayValidate, virTypedParameterAssign): New prototypes. * src/util/util.c (virTypedParameterArrayClear): Likewise. * src/util/virtypedparam.c: New file. * po/POTFILES.in: Mark file for translation. * src/Makefile.am (UTIL_SOURCES): Build it. * src/libvirt_private.syms (util.h): Split... (virtypedparam.h): to new section. (virkeycode.h): Sort. * daemon/remote.c: Adjust callers. * tools/virsh.c: Likewise. --- daemon/remote.c | 1 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 20 +++-- src/util/util.c | 16 +---- src/util/util.h | 4 +- src/util/virtypedparam.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 37 +++++++++ tools/virsh.c | 1 + 9 files changed, 243 insertions(+), 25 deletions(-) create mode 100644 src/util/virtypedparam.c create mode 100644 src/util/virtypedparam.h diff --git a/daemon/remote.c b/daemon/remote.c index a28a754..4db8f91 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -44,6 +44,7 @@ #include "virnetserverservice.h" #include "virnetserver.h" #include "virfile.h" +#include "virtypedparam.h" #include "remote_protocol.h" #include "qemu_protocol.h" diff --git a/po/POTFILES.in b/po/POTFILES.in index 3e8359a..ca1db70 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -135,6 +135,7 @@ src/util/virpidfile.c src/util/virsocketaddr.c src/util/virterror.c src/util/virtime.c +src/util/virtypedparam.c src/util/xml.c src/vbox/vbox_MSCOMGlue.c src/vbox/vbox_XPCOMCGlue.c diff --git a/src/Makefile.am b/src/Makefile.am index 0a1221a..c459f2d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -87,6 +87,7 @@ UTIL_SOURCES = \ util/virfile.c util/virfile.h \ util/virnodesuspend.c util/virnodesuspend.h \ util/virpidfile.c util/virpidfile.h \ + util/virtypedparam.c util/virtypedparam.h \ util/xml.c util/xml.h \ util/virterror.c util/virterror_internal.h \ util/virkeycode.c util/virkeycode.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0dfd4c1..9a7200b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1145,7 +1145,6 @@ virStrToLong_ull; virStrcpy; virStrncpy; virTrimSpaces; -virTypedParameterArrayClear; virVasprintf; @@ -1176,6 +1175,13 @@ virFileFdopen; virFileRewrite; +# virkeycode.h +virKeycodeSetTypeFromString; +virKeycodeSetTypeToString; +virKeycodeValueFromString; +virKeycodeValueTranslate; + + # virnetclient.h virNetClientHasPassFD; @@ -1390,12 +1396,6 @@ virSetError; virSetErrorLogPriorityFunc; virStrerror; -# virkeycode.h -virKeycodeSetTypeToString; -virKeycodeSetTypeFromString; -virKeycodeValueFromString; -virKeycodeValueTranslate; - # virtime.h virTimeFieldsNow; @@ -1410,6 +1410,12 @@ virTimeStringThen; virTimeStringThenRaw; +# virtypedparam.h +virTypedParameterArrayClear; +virTypedParameterArrayValidate; +virTypedParameterAssign; + + # xml.h virXMLChildElementCount; virXMLParseHelper; diff --git a/src/util/util.c b/src/util/util.c index 8663c4d..a60ea21 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1,7 +1,7 @@ /* * utils.c: common, generic utility functions * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain @@ -2575,17 +2575,3 @@ or other application using the libvirt API.\n\ return 0; } - -void -virTypedParameterArrayClear(virTypedParameterPtr params, int nparams) -{ - int i; - - if (!params) - return; - - for (i = 0; i < nparams; i++) { - if (params[i].type == VIR_TYPED_PARAM_STRING) - VIR_FREE(params[i].value.s); - } -} diff --git a/src/util/util.h b/src/util/util.h index 977ab6c..bed2315 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -1,7 +1,7 @@ /* * utils.h: common, generic utility functions * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * @@ -257,6 +257,4 @@ int virEmitXMLWarning(int fd, const char *name, const char *cmd) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); -void virTypedParameterArrayClear(virTypedParameterPtr params, int nparams); - #endif /* __VIR_UTIL_H__ */ diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c new file mode 100644 index 0000000..f71aa25 --- /dev/null +++ b/src/util/virtypedparam.c @@ -0,0 +1,187 @@ +/* + * virtypedparam.c: utility functions for dealing with virTypedParameters + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> +#include "virtypedparam.h" + +#include <stdarg.h> + +#include "memory.h" +#include "util.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +#define virUtilError(code, ...) \ + virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +VIR_ENUM_DECL(virTypedParameter) +VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_STRING + 1, + "unknown", + "int", + "uint", + "llong", + "ullong", + "double", + "boolean", + "string") + +void +virTypedParameterArrayClear(virTypedParameterPtr params, int nparams) +{ + int i; + + if (!params) + return; + + for (i = 0; i < nparams; i++) { + if (params[i].type == VIR_TYPED_PARAM_STRING) + VIR_FREE(params[i].value.s); + } +} + +/* Validate that PARAMS contains only recognized parameter names with + * correct types, and with no duplicates. Pass in as many name/type + * pairs as appropriate, and pass NULL to end the list of accepted + * parameters. Return 0 on success, -1 on failure with error message + * already issued. */ +int +virTypedParameterArrayValidate(virTypedParameterPtr params, int nparams, ...) +{ + va_list ap; + int ret = -1; + int i, j; + const char *name; + int type; + + va_start(ap, nparams); + + /* Yes, this is quadratic, but since we reject duplicates and + * unknowns, it is constrained by the number of var-args passed + * in, which is expected to be small enough to not be + * noticeable. */ + for (i = 0; i < nparams; i++) { + va_end(ap); + va_start(ap, nparams); + + name = va_arg(ap, const char *); + while (name) { + type = va_arg(ap, int); + if (STREQ(params[i].field, name)) { + if (params[i].type != type) { + const char *badtype; + + badtype = virTypedParameterTypeToString(params[i].type); + if (!badtype) + badtype = virTypedParameterTypeToString(0); + virUtilError(VIR_ERR_INVALID_ARG, + _("invalid type '%s' for parameter '%s', " + "expected '%s'"), + badtype, params[i].field, + virTypedParameterTypeToString(type)); + } + break; + } + name = va_arg(ap, const char *); + } + if (!name) { + virUtilError(VIR_ERR_INVALID_ARG, + _("parameter '%s' not supported"), + params[i].field); + goto cleanup; + } + for (j = 0; j < i; j++) { + if (STREQ(params[i].field, params[j].field)) { + virUtilError(VIR_ERR_INVALID_ARG, + _("parameter '%s' occurs multiple times"), + params[i].field); + goto cleanup; + } + } + } + + ret = 0; +cleanup: + 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. + * 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) == NULL) { + virUtilError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), + name); + goto cleanup; + } + param->type = type; + switch (type) + { + case VIR_TYPED_PARAM_INT: + param->value.i = va_arg(ap, int); + break; + case VIR_TYPED_PARAM_UINT: + param->value.ui = va_arg(ap, unsigned int); + break; + case VIR_TYPED_PARAM_LLONG: + param->value.l = va_arg(ap, long long int); + break; + case VIR_TYPED_PARAM_ULLONG: + param->value.ul = va_arg(ap, unsigned long long int); + break; + case VIR_TYPED_PARAM_DOUBLE: + param->value.d = va_arg(ap, double); + break; + case VIR_TYPED_PARAM_BOOLEAN: + param->value.b = !!va_arg(ap, int); + break; + case VIR_TYPED_PARAM_STRING: + param->value.s = va_arg(ap, char *); + if (!param->value.s) + param->value.s = strdup(""); + if (!param->value.s) { + virReportOOMError(); + goto cleanup; + } + break; + default: + virUtilError(VIR_ERR_INTERNAL_ERROR, + _("unexpected type %d for field %s"), type, name); + goto cleanup; + } + + ret = 0; +cleanup: + va_end(ap); + return ret; +} diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h new file mode 100644 index 0000000..52cbe78 --- /dev/null +++ b/src/util/virtypedparam.h @@ -0,0 +1,37 @@ +/* + * virtypedparam.h: managing typed parameters + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + + +#ifndef __VIR_TYPED_PARAM_H_ +# define __VIR_TYPED_PARAM_H_ + +# include "internal.h" + +void virTypedParameterArrayClear(virTypedParameterPtr params, int nparams); + +int virTypedParameterArrayValidate(virTypedParameterPtr params, int nparams, + /* const char *name, int type ... */ ...) + ATTRIBUTE_SENTINEL; + +int virTypedParameterAssign(virTypedParameterPtr param, const char *name, + int type, /* TYPE arg */ ...); + +#endif /* __VIR_TYPED_PARAM_H */ diff --git a/tools/virsh.c b/tools/virsh.c index c511e2a..032a4e0 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -62,6 +62,7 @@ #include "virnetdevbandwidth.h" #include "util/bitmap.h" #include "conf/domain_conf.h" +#include "virtypedparam.h" static char *progname; -- 1.7.7.5

Reusing common code makes things smaller; it also buys us some additional safety, such as now rejecting duplicate parameters during a set operation. * src/qemu/qemu_driver.c (qemuDomainSetBlkioParameters) (qemuDomainSetMemoryParameters, qemuDomainSetNumaParameters) (qemuSetSchedulerParametersFlags) (qemuDomainSetInterfaceParameters, qemuDomainSetBlockIoTune) (qemuDomainGetBlkioParameters, qemuDomainGetMemoryParameters) (qemuDomainGetNumaParameters, qemuGetSchedulerParametersFlags) (qemuDomainBlockStatsFlags, qemuDomainGetInterfaceParameters) (qemuDomainGetBlockIoTune): Use new helpers. * src/esx/esx_driver.c (esxDomainSetSchedulerParametersFlags) (esxDomainSetMemoryParameters) (esxDomainGetSchedulerParametersFlags) (esxDomainGetMemoryParameters): Likewise. * src/libxl/libxl_driver.c (libxlDomainSetSchedulerParametersFlags) (libxlDomainGetSchedulerParametersFlags): Likewise. * src/lxc/lxc_driver.c (lxcDomainSetMemoryParameters) (lxcSetSchedulerParametersFlags, lxcDomainSetBlkioParameters) (lxcDomainGetMemoryParameters, lxcGetSchedulerParametersFlags) (lxcDomainGetBlkioParameters): Likewise. * src/test/test_driver.c (testDomainSetSchedulerParamsFlags) (testDomainGetSchedulerParamsFlags): Likewise. * src/xen/xen_hypervisor.c (xenHypervisorSetSchedulerParameters) (xenHypervisorGetSchedulerParameters): Likewise. --- src/esx/esx_driver.c | 81 +++--- src/libxl/libxl_driver.c | 44 +--- src/lxc/lxc_driver.c | 218 +++++----------- src/qemu/qemu_driver.c | 656 ++++++++++++++-------------------------------- src/test/test_driver.c | 27 +-- src/xen/xen_hypervisor.c | 51 ++-- 6 files changed, 350 insertions(+), 727 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 1e424eb..63cdba5 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -32,6 +32,7 @@ #include "logging.h" #include "uuid.h" #include "vmx.h" +#include "virtypedparam.h" #include "esx_driver.h" #include "esx_interface_driver.h" #include "esx_network_driver.h" @@ -3660,43 +3661,38 @@ esxDomainGetSchedulerParametersFlags(virDomainPtr domain, dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "config.cpuAllocation.reservation") && ! (mask & (1 << 0))) { - snprintf (params[i].field, VIR_TYPED_PARAM_FIELD_LENGTH, "%s", - VIR_DOMAIN_SCHEDULER_RESERVATION); - - params[i].type = VIR_TYPED_PARAM_LLONG; - if (esxVI_AnyType_ExpectType(dynamicProperty->val, esxVI_Type_Long) < 0) { goto cleanup; } - - params[i].value.l = dynamicProperty->val->int64; + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_SCHEDULER_RESERVATION, + VIR_TYPED_PARAM_LLONG, + dynamicProperty->val->int64) < 0) + goto cleanup; mask |= 1 << 0; ++i; } else if (STREQ(dynamicProperty->name, "config.cpuAllocation.limit") && ! (mask & (1 << 1))) { - snprintf (params[i].field, VIR_TYPED_PARAM_FIELD_LENGTH, "%s", - VIR_DOMAIN_SCHEDULER_LIMIT); - - params[i].type = VIR_TYPED_PARAM_LLONG; - if (esxVI_AnyType_ExpectType(dynamicProperty->val, esxVI_Type_Long) < 0) { goto cleanup; } - - params[i].value.l = dynamicProperty->val->int64; + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_SCHEDULER_LIMIT, + VIR_TYPED_PARAM_LLONG, + dynamicProperty->val->int64) < 0) + goto cleanup; mask |= 1 << 1; ++i; } else if (STREQ(dynamicProperty->name, "config.cpuAllocation.shares") && ! (mask & (1 << 2))) { - snprintf (params[i].field, VIR_TYPED_PARAM_FIELD_LENGTH, "%s", - VIR_DOMAIN_SCHEDULER_SHARES); - - params[i].type = VIR_TYPED_PARAM_INT; - + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_SCHEDULER_SHARES, + VIR_TYPED_PARAM_INT, 0) < 0) + goto cleanup; if (esxVI_SharesInfo_CastFromAnyType(dynamicProperty->val, &sharesInfo) < 0) { goto cleanup; @@ -3769,6 +3765,15 @@ esxDomainSetSchedulerParametersFlags(virDomainPtr domain, int i; virCheckFlags(0, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_SCHEDULER_RESERVATION, + VIR_TYPED_PARAM_LLONG, + VIR_DOMAIN_SCHEDULER_LIMIT, + VIR_TYPED_PARAM_LLONG, + VIR_DOMAIN_SCHEDULER_SHARES, + VIR_TYPED_PARAM_INT, + NULL) < 0) + return -1; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -3783,8 +3788,7 @@ esxDomainSetSchedulerParametersFlags(virDomainPtr domain, } for (i = 0; i < nparams; ++i) { - if (STREQ (params[i].field, VIR_DOMAIN_SCHEDULER_RESERVATION) && - params[i].type == VIR_TYPED_PARAM_LLONG) { + if (STREQ(params[i].field, VIR_DOMAIN_SCHEDULER_RESERVATION)) { if (esxVI_Long_Alloc(&spec->cpuAllocation->reservation) < 0) { goto cleanup; } @@ -3797,8 +3801,7 @@ esxDomainSetSchedulerParametersFlags(virDomainPtr domain, } spec->cpuAllocation->reservation->value = params[i].value.l; - } else if (STREQ (params[i].field, VIR_DOMAIN_SCHEDULER_LIMIT) && - params[i].type == VIR_TYPED_PARAM_LLONG) { + } else if(STREQ (params[i].field, VIR_DOMAIN_SCHEDULER_LIMIT)) { if (esxVI_Long_Alloc(&spec->cpuAllocation->limit) < 0) { goto cleanup; } @@ -3812,8 +3815,7 @@ esxDomainSetSchedulerParametersFlags(virDomainPtr domain, } spec->cpuAllocation->limit->value = params[i].value.l; - } else if (STREQ (params[i].field, VIR_DOMAIN_SCHEDULER_SHARES) && - params[i].type == VIR_TYPED_PARAM_INT) { + } else if(STREQ (params[i].field, VIR_DOMAIN_SCHEDULER_SHARES)) { if (esxVI_SharesInfo_Alloc(&sharesInfo) < 0 || esxVI_Int_Alloc(&sharesInfo->shares) < 0) { goto cleanup; @@ -3851,10 +3853,6 @@ esxDomainSetSchedulerParametersFlags(virDomainPtr domain, goto cleanup; } } - } else { - ESX_ERROR(VIR_ERR_INVALID_ARG, _("Unknown field '%s'"), - params[i].field); - goto cleanup; } } @@ -4828,6 +4826,11 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virTypedParameterPtr params, int i; virCheckFlags(0, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_MEMORY_MIN_GUARANTEE, + VIR_TYPED_PARAM_ULLONG, + NULL) < 0) + return -1; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -4842,18 +4845,13 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virTypedParameterPtr params, } for (i = 0; i < nparams; ++i) { - if (STREQ (params[i].field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE) && - params[i].type == VIR_TYPED_PARAM_ULLONG) { + if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { if (esxVI_Long_Alloc(&spec->memoryAllocation->reservation) < 0) { goto cleanup; } spec->memoryAllocation->reservation->value = VIR_DIV_UP(params[i].value.ul, 1024); /* Scale from kilobytes to megabytes */ - } else { - ESX_ERROR(VIR_ERR_INVALID_ARG, _("Unknown field '%s'"), - params[i].field); - goto cleanup; } } @@ -4917,16 +4915,11 @@ esxDomainGetMemoryParameters(virDomainPtr domain, virTypedParameterPtr params, goto cleanup; } - if (virStrcpyStatic(params[0].field, - VIR_DOMAIN_MEMORY_MIN_GUARANTEE) == NULL) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Field %s too big for destination"), - VIR_DOMAIN_MEMORY_MIN_GUARANTEE); + /* Scale from megabytes to kilobytes */ + if (virTypedParameterAssign(params, VIR_DOMAIN_MEMORY_MIN_GUARANTEE, + VIR_TYPED_PARAM_ULLONG, + reservation->value * 1024) < 0) goto cleanup; - } - - params[0].type = VIR_TYPED_PARAM_ULLONG; - params[0].value.ul = reservation->value * 1024; /* Scale from megabytes to kilobytes */ *nparams = 1; result = 0; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 0500ed0..f7f45c7 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -43,7 +43,7 @@ #include "libxl_driver.h" #include "libxl_conf.h" #include "xen_xm.h" - +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -3610,26 +3610,14 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } - params[0].value.ui = sc_info.weight; - params[0].type = VIR_TYPED_PARAM_UINT; - if (virStrcpyStatic(params[0].field, - VIR_DOMAIN_SCHEDULER_WEIGHT) == NULL) { - libxlError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_WEIGHT); + if (virTypedParameterAssign(&params[0], VIR_DOMAIN_SCHEDULER_WEIGHT, + VIR_TYPED_PARAM_UINT, sc_info.weight) < 0) goto cleanup; - } if (*nparams > 1) { - params[1].value.ui = sc_info.cap; - params[1].type = VIR_TYPED_PARAM_UINT; - if (virStrcpyStatic(params[1].field, - VIR_DOMAIN_SCHEDULER_CAP) == NULL) { - libxlError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_CAP); + if (virTypedParameterAssign(&params[0], VIR_DOMAIN_SCHEDULER_CAP, + VIR_TYPED_PARAM_UINT, sc_info.cap) < 0) goto cleanup; - } } if (*nparams > XEN_SCHED_CREDIT_NPARAM) @@ -3664,6 +3652,13 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, int ret = -1; virCheckFlags(0, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_SCHEDULER_WEIGHT, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_SCHEDULER_CAP, + VIR_TYPED_PARAM_UINT, + NULL) < 0) + return -1; libxlDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3705,24 +3700,9 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_WEIGHT)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - libxlError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for weight tunable, expected a 'uint'")); - goto cleanup; - } sc_info.weight = params[i].value.ui; - } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CAP)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - libxlError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for cap tunable, expected a 'uint'")); - goto cleanup; - } sc_info.cap = params[i].value.ui; - } else { - libxlError(VIR_ERR_INVALID_ARG, - _("Invalid parameter '%s'"), param->field); - goto cleanup; } } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 7d6ac59..851028a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -58,6 +58,7 @@ #include "virnetdev.h" #include "virnodesuspend.h" #include "virtime.h" +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -778,18 +779,29 @@ cleanup: return ret; } -static int lxcDomainSetMemoryParameters(virDomainPtr dom, - virTypedParameterPtr params, - int nparams, - unsigned int flags) +static int +lxcDomainSetMemoryParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) { lxc_driver_t *driver = dom->conn->privateData; int i; virCgroupPtr cgroup = NULL; virDomainObjPtr vm = NULL; int ret = -1; + int rc; virCheckFlags(0, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + NULL) < 0) + return -1; lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -813,14 +825,6 @@ static int lxcDomainSetMemoryParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { - int rc; - if (param->type != VIR_TYPED_PARAM_ULLONG) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for memory hard_limit tunable, expected a 'ullong'")); - ret = -1; - continue; - } - rc = virCgroupSetMemoryHardLimit(cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", @@ -828,14 +832,6 @@ static int lxcDomainSetMemoryParameters(virDomainPtr dom, ret = -1; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { - int rc; - if (param->type != VIR_TYPED_PARAM_ULLONG) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for memory soft_limit tunable, expected a 'ullong'")); - ret = -1; - continue; - } - rc = virCgroupSetMemorySoftLimit(cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", @@ -843,28 +839,12 @@ static int lxcDomainSetMemoryParameters(virDomainPtr dom, ret = -1; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { - int rc; - if (param->type != VIR_TYPED_PARAM_ULLONG) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for swap_hard_limit tunable, expected a 'ullong'")); - ret = -1; - continue; - } - rc = virCgroupSetMemSwapHardLimit(cgroup, params[i].value.ul); if (rc != 0) { virReportSystemError(-rc, "%s", _("unable to set swap_hard_limit tunable")); ret = -1; } - } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { - lxcError(VIR_ERR_INVALID_ARG, - _("Memory tunable `%s' not implemented"), param->field); - ret = -1; - } else { - lxcError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), param->field); - ret = -1; } } @@ -877,10 +857,11 @@ cleanup: return ret; } -static int lxcDomainGetMemoryParameters(virDomainPtr dom, - virTypedParameterPtr params, - int *nparams, - unsigned int flags) +static int +lxcDomainGetMemoryParameters(virDomainPtr dom, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) { lxc_driver_t *driver = dom->conn->privateData; int i; @@ -919,8 +900,6 @@ static int lxcDomainGetMemoryParameters(virDomainPtr dom, for (i = 0; i < LXC_NB_MEM_PARAM && i < *nparams; i++) { virTypedParameterPtr param = &params[i]; val = 0; - param->value.ul = 0; - param->type = VIR_TYPED_PARAM_ULLONG; switch(i) { case 0: /* fill memory hard limit here */ @@ -930,14 +909,10 @@ static int lxcDomainGetMemoryParameters(virDomainPtr dom, _("unable to get memory hard limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Field memory hard limit too long for destination")); + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; - } - param->value.ul = val; break; - case 1: /* fill memory soft limit here */ rc = virCgroupGetMemorySoftLimit(cgroup, &val); if (rc != 0) { @@ -945,14 +920,10 @@ static int lxcDomainGetMemoryParameters(virDomainPtr dom, _("unable to get memory soft limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Field memory soft limit too long for destination")); + if (virTypedParameterAssign(param, VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; - } - param->value.ul = val; break; - case 2: /* fill swap hard limit here */ rc = virCgroupGetMemSwapHardLimit(cgroup, &val); if (rc != 0) { @@ -960,12 +931,10 @@ static int lxcDomainGetMemoryParameters(virDomainPtr dom, _("unable to get swap hard limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Field swap hard limit too long for destination")); + if (virTypedParameterAssign(param, + VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; - } - param->value.ul = val; break; default: @@ -2753,6 +2722,15 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_SCHEDULER_CPU_SHARES, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, + VIR_TYPED_PARAM_LLONG, + NULL) < 0) + return -1; lxcDriverLock(driver); @@ -2793,12 +2771,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { - if (param->type != VIR_TYPED_PARAM_ULLONG) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for cpu_shares tunable, expected a 'ullong'")); - goto cleanup; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = virCgroupSetCpuShares(group, params[i].value.ul); if (rc != 0) { @@ -2814,13 +2786,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, vmdef->cputune.shares = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) { - if (param->type != VIR_TYPED_PARAM_ULLONG) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for vcpu_period tunable," - " expected a 'ullong'")); - goto cleanup; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = lxcSetVcpuBWLive(group, params[i].value.ul, 0); if (rc != 0) @@ -2834,13 +2799,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, vmdef->cputune.period = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) { - if (param->type != VIR_TYPED_PARAM_LLONG) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for vcpu_quota tunable," - " expected a 'llong'")); - goto cleanup; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = lxcSetVcpuBWLive(group, 0, params[i].value.l); if (rc != 0) @@ -2853,10 +2811,6 @@ lxcSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { vmdef->cputune.quota = params[i].value.l; } - } else { - lxcError(VIR_ERR_INVALID_ARG, - _("Invalid parameter `%s'"), param->field); - goto cleanup; } } @@ -2968,42 +2922,25 @@ lxcGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } out: - params[0].value.ul = shares; - params[0].type = VIR_TYPED_PARAM_ULLONG; - if (virStrcpyStatic(params[0].field, - VIR_DOMAIN_SCHEDULER_CPU_SHARES) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_CPU_SHARES); + if (virTypedParameterAssign(&params[0], VIR_DOMAIN_SCHEDULER_CPU_SHARES, + VIR_TYPED_PARAM_ULLONG, shares) < 0) goto cleanup; - } - saved_nparams++; if (cpu_bw_status) { if (*nparams > saved_nparams) { - params[1].value.ul = period; - params[1].type = VIR_TYPED_PARAM_ULLONG; - if (virStrcpyStatic(params[1].field, - VIR_DOMAIN_SCHEDULER_VCPU_PERIOD) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_VCPU_PERIOD); + if (virTypedParameterAssign(&params[1], + VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, + VIR_TYPED_PARAM_ULLONG, shares) < 0) goto cleanup; - } saved_nparams++; } if (*nparams > saved_nparams) { - params[2].value.ul = quota; - params[2].type = VIR_TYPED_PARAM_LLONG; - if (virStrcpyStatic(params[2].field, - VIR_DOMAIN_SCHEDULER_VCPU_QUOTA) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_VCPU_QUOTA); + if (virTypedParameterAssign(&params[2], + VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, + VIR_TYPED_PARAM_LLONG, quota) < 0) goto cleanup; - } saved_nparams++; } } @@ -3029,10 +2966,11 @@ lxcGetSchedulerParameters(virDomainPtr domain, } -static int lxcDomainSetBlkioParameters(virDomainPtr dom, - virTypedParameterPtr params, - int nparams, - unsigned int flags) +static int +lxcDomainSetBlkioParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) { lxc_driver_t *driver = dom->conn->privateData; int i; @@ -3043,6 +2981,12 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, + NULL) < 0) + return -1; + lxcDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3074,11 +3018,6 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { int rc; - if (param->type != VIR_TYPED_PARAM_UINT) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for blkio weight tunable, expected a 'unsigned int'")); - goto cleanup; - } if (params[i].value.ui > 1000 || params[i].value.ui < 100) { lxcError(VIR_ERR_INVALID_ARG, "%s", @@ -3092,10 +3031,6 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, _("unable to set blkio weight tunable")); goto cleanup; } - } else { - lxcError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), param->field); - goto cleanup; } } } @@ -3107,12 +3042,6 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - lxcError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for blkio weight tunable, expected a 'unsigned int'")); - goto cleanup; - } - if (params[i].value.ui > 1000 || params[i].value.ui < 100) { lxcError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); @@ -3120,10 +3049,6 @@ static int lxcDomainSetBlkioParameters(virDomainPtr dom, } persistentDef->blkio.weight = params[i].value.ui; - } else { - lxcError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), param->field); - goto cleanup; } } @@ -3142,10 +3067,11 @@ cleanup: #define LXC_NB_BLKIO_PARAM 1 -static int lxcDomainGetBlkioParameters(virDomainPtr dom, - virTypedParameterPtr params, - int *nparams, - unsigned int flags) +static int +lxcDomainGetBlkioParameters(virDomainPtr dom, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) { lxc_driver_t *driver = dom->conn->privateData; int i; @@ -3194,8 +3120,6 @@ static int lxcDomainGetBlkioParameters(virDomainPtr dom, for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = &params[i]; val = 0; - param->value.ui = 0; - param->type = VIR_TYPED_PARAM_UINT; switch (i) { case 0: /* fill blkio weight here */ @@ -3205,13 +3129,9 @@ static int lxcDomainGetBlkioParameters(virDomainPtr dom, _("unable to get blkio weight")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_WEIGHT); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, val) < 0) goto cleanup; - } - param->value.ui = val; break; default: @@ -3222,19 +3142,13 @@ static int lxcDomainGetBlkioParameters(virDomainPtr dom, } else if (flags & VIR_DOMAIN_AFFECT_CONFIG) { for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = &params[i]; - val = 0; - param->value.ui = 0; - param->type = VIR_TYPED_PARAM_UINT; switch (i) { case 0: /* fill blkio weight here */ - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) { - lxcError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_WEIGHT); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, + persistentDef->blkio.weight) < 0) goto cleanup; - } - param->value.ui = persistentDef->blkio.weight; break; default: diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c920bfd..608e82a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -90,6 +90,7 @@ #include "virkeycode.h" #include "virnodesuspend.h" #include "virtime.h" +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -5919,10 +5920,11 @@ qemuDomainMergeDeviceWeights(virBlkioDeviceWeightPtr *def, size_t *def_size, return 0; } -static int qemuDomainSetBlkioParameters(virDomainPtr dom, - virTypedParameterPtr params, - int nparams, - unsigned int flags) +static int +qemuDomainSetBlkioParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; @@ -5933,8 +5935,15 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - qemuDriverLock(driver); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, + VIR_TYPED_PARAM_STRING, + NULL) < 0) + return -1; + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (vm == NULL) { @@ -5949,13 +5958,15 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't mounted")); + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("blkio cgroup isn't mounted")); goto cleanup; } if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); + _("cannot find cgroup for domain %s"), + vm->def->name); goto cleanup; } } @@ -5967,13 +5978,6 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for blkio weight tunable, expected a 'unsigned int'")); - ret = -1; - continue; - } - if (params[i].value.ui > 1000 || params[i].value.ui < 100) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); @@ -5992,14 +5996,6 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, virBlkioDeviceWeightPtr devices = NULL; int j; - if (param->type != VIR_TYPED_PARAM_STRING) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for device_weight tunable, " - "expected a 'char *'")); - ret = -1; - continue; - } - if (qemuDomainParseDeviceWeightStr(params[i].value.s, &devices, &ndevices) < 0) { @@ -6025,11 +6021,6 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, ret = -1; virBlkioDeviceWeightArrayClear(devices, ndevices); VIR_FREE(devices); - } else { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), - param->field); - ret = -1; } } } @@ -6043,13 +6034,6 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for blkio weight tunable, expected a 'unsigned int'")); - ret = -1; - continue; - } - if (params[i].value.ui > 1000 || params[i].value.ui < 100) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); @@ -6061,13 +6045,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { virBlkioDeviceWeightPtr devices = NULL; size_t ndevices; - if (param->type != VIR_TYPED_PARAM_STRING) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for device_weight tunable, " - "expected a 'char *'")); - ret = -1; - continue; - } + if (qemuDomainParseDeviceWeightStr(params[i].value.s, &devices, &ndevices) < 0) { @@ -6080,11 +6058,6 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom, ret = -1; virBlkioDeviceWeightArrayClear(devices, ndevices); VIR_FREE(devices); - } else { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), - param->field); - ret = -1; } } @@ -6100,10 +6073,11 @@ cleanup: return ret; } -static int qemuDomainGetBlkioParameters(virDomainPtr dom, - virTypedParameterPtr params, - int *nparams, - unsigned int flags) +static int +qemuDomainGetBlkioParameters(virDomainPtr dom, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i, j; @@ -6160,8 +6134,6 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, for (i = 0; i < *nparams && i < QEMU_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = &params[i]; val = 0; - param->value.ui = 0; - param->type = VIR_TYPED_PARAM_UINT; switch (i) { case 0: /* fill blkio weight here */ @@ -6171,13 +6143,9 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, _("unable to get blkio weight")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_WEIGHT); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT, + VIR_TYPED_PARAM_UINT, val) < 0) goto cleanup; - } - param->value.ui = val; break; case 1: /* blkiotune.device_weight */ if (vm->def->blkio.ndevices > 0) { @@ -6201,21 +6169,11 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom, } param->value.s = virBufferContentAndReset(&buf); } - if (!param->value.s) { - param->value.s = strdup(""); - if (!param->value.s) { - virReportOOMError(); - goto cleanup; - } - } - param->type = VIR_TYPED_PARAM_STRING; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLKIO_DEVICE_WEIGHT); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) goto cleanup; - } break; default: @@ -6300,10 +6258,11 @@ cleanup: return ret; } -static int qemuDomainSetMemoryParameters(virDomainPtr dom, - virTypedParameterPtr params, - int nparams, - unsigned int flags) +static int +qemuDomainSetMemoryParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; @@ -6311,9 +6270,19 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; + int rc; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + NULL) < 0) + return -1; qemuDriverLock(driver); @@ -6348,14 +6317,6 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { - int rc; - if (param->type != VIR_TYPED_PARAM_ULLONG) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for memory hard_limit tunable, expected a 'ullong'")); - ret = -1; - continue; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = virCgroupSetMemoryHardLimit(group, params[i].value.ul); if (rc != 0) { @@ -6369,14 +6330,6 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, persistentDef->mem.hard_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { - int rc; - if (param->type != VIR_TYPED_PARAM_ULLONG) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for memory soft_limit tunable, expected a 'ullong'")); - ret = -1; - continue; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = virCgroupSetMemorySoftLimit(group, params[i].value.ul); if (rc != 0) { @@ -6390,14 +6343,6 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, persistentDef->mem.soft_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { - int rc; - if (param->type != VIR_TYPED_PARAM_ULLONG) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for swap_hard_limit tunable, expected a 'ullong'")); - ret = -1; - continue; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); if (rc != 0) { @@ -6409,15 +6354,6 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { persistentDef->mem.swap_hard_limit = params[i].value.ul; } - } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Memory tunable `%s' not implemented"), - param->field); - ret = -1; - } else { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), param->field); - ret = -1; } } @@ -6434,17 +6370,17 @@ cleanup: return ret; } -static int qemuDomainGetMemoryParameters(virDomainPtr dom, - virTypedParameterPtr params, - int *nparams, - unsigned int flags) +static int +qemuDomainGetMemoryParameters(virDomainPtr dom, + virTypedParameterPtr params, + int *nparams, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; - unsigned long long val; int ret = -1; int rc; @@ -6493,39 +6429,30 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { for (i = 0; i < *nparams && i < QEMU_NB_MEM_PARAM; i++) { virMemoryParameterPtr param = &params[i]; - val = 0; - param->value.ul = 0; - param->type = VIR_TYPED_PARAM_ULLONG; switch (i) { case 0: /* fill memory hard limit here */ - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_MEMORY_HARD_LIMIT); + if (virTypedParameterAssign(param, + VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + persistentDef->mem.hard_limit) < 0) goto cleanup; - } - param->value.ul = persistentDef->mem.hard_limit; break; case 1: /* fill memory soft limit here */ - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_MEMORY_SOFT_LIMIT); + if (virTypedParameterAssign(param, + VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, + persistentDef->mem.soft_limit) < 0) goto cleanup; - } - param->value.ul = persistentDef->mem.soft_limit; break; case 2: /* fill swap hard limit here */ - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT); + if (virTypedParameterAssign(param, + VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, + persistentDef->mem.swap_hard_limit) < 0) goto cleanup; - } - param->value.ul = persistentDef->mem.swap_hard_limit; break; default: @@ -6538,9 +6465,7 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, for (i = 0; i < *nparams && i < QEMU_NB_MEM_PARAM; i++) { virTypedParameterPtr param = &params[i]; - val = 0; - param->value.ul = 0; - param->type = VIR_TYPED_PARAM_ULLONG; + unsigned long long val = 0; /* Coverity does not realize that if we get here, group is set. */ sa_assert(group); @@ -6553,13 +6478,10 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, _("unable to get memory hard limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_MEMORY_HARD_LIMIT); + if (virTypedParameterAssign(param, + VIR_DOMAIN_MEMORY_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; - } - param->value.ul = val; break; case 1: /* fill memory soft limit here */ @@ -6569,13 +6491,10 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, _("unable to get memory soft limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_MEMORY_SOFT_LIMIT); + if (virTypedParameterAssign(param, + VIR_DOMAIN_MEMORY_SOFT_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; - } - param->value.ul = val; break; case 2: /* fill swap hard limit here */ @@ -6585,13 +6504,10 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, _("unable to get swap hard limit")); goto cleanup; } - if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT); + if (virTypedParameterAssign(param, + VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, + VIR_TYPED_PARAM_ULLONG, val) < 0) goto cleanup; - } - param->value.ul = val; break; default: @@ -6629,6 +6545,13 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_NUMA_MODE, + VIR_TYPED_PARAM_INT, + VIR_DOMAIN_NUMA_NODESET, + VIR_TYPED_PARAM_STRING, + NULL) < 0) + return -1; qemuDriverLock(driver); @@ -6664,14 +6587,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_NUMA_MODE)) { - if (param->type != VIR_TYPED_PARAM_INT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for numa strict tunable, " - "expected an 'int'")); - ret = -1; - continue; - } - if ((flags & VIR_DOMAIN_AFFECT_LIVE) && vm->def->numatune.memory.mode != params[i].value.i) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -6687,13 +6602,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, int rc; bool savedmask; char oldnodemask[VIR_DOMAIN_CPUMASK_LEN]; - if (param->type != VIR_TYPED_PARAM_STRING) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for numa nodeset tunable, " - "expected a 'string'")); - ret = -1; - continue; - } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (vm->def->numatune.memory.mode != @@ -6768,10 +6676,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom, continue; } } - } else { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), param->field); - ret = -1; } } @@ -6852,13 +6756,9 @@ qemuDomainGetNumaParameters(virDomainPtr dom, switch (i) { case 0: /* fill numa mode here */ - if (!virStrcpyStatic(param->field, VIR_DOMAIN_NUMA_MODE)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field '%s' too long for destination"), - VIR_DOMAIN_NUMA_MODE); + if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE, + VIR_TYPED_PARAM_INT, 0) < 0) goto cleanup; - } - param->type = VIR_TYPED_PARAM_INT; if (flags & VIR_DOMAIN_AFFECT_CONFIG) param->value.i = persistentDef->numatune.memory.mode; else @@ -6866,12 +6766,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, break; case 1: /* fill numa nodeset here */ - if (!virStrcpyStatic(param->field, VIR_DOMAIN_NUMA_NODESET)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field '%s' too long for destination"), - VIR_DOMAIN_NUMA_NODESET); - goto cleanup; - } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { char *mask = persistentDef->numatune.memory.nodemask; if (mask) @@ -6887,12 +6781,9 @@ qemuDomainGetNumaParameters(virDomainPtr dom, goto cleanup; } } - if (!nodeset) { - virReportOOMError(); + if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET, + VIR_TYPED_PARAM_STRING, nodeset) < 0) goto cleanup; - } - param->type = VIR_TYPED_PARAM_STRING; - param->value.s = nodeset; break; default: @@ -7020,10 +6911,11 @@ cleanup: return -1; } -static int qemuSetSchedulerParametersFlags(virDomainPtr dom, - virTypedParameterPtr params, - int nparams, - unsigned int flags) +static int +qemuSetSchedulerParametersFlags(virDomainPtr dom, + virTypedParameterPtr params, + int nparams, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; @@ -7035,6 +6927,15 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_SCHEDULER_CPU_SHARES, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, + VIR_TYPED_PARAM_LLONG, + NULL) < 0) + return -1; qemuDriverLock(driver); @@ -7075,12 +6976,6 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) { - if (param->type != VIR_TYPED_PARAM_ULLONG) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for cpu_shares tunable, expected a 'ullong'")); - goto cleanup; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = virCgroupSetCpuShares(group, params[i].value.ul); if (rc != 0) { @@ -7096,13 +6991,6 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, vmdef->cputune.shares = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) { - if (param->type != VIR_TYPED_PARAM_ULLONG) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for vcpu_period tunable," - " expected a 'ullong'")); - goto cleanup; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = qemuSetVcpusBWLive(vm, group, params[i].value.ul, 0); if (rc != 0) @@ -7116,13 +7004,6 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, vmdef->cputune.period = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) { - if (param->type != VIR_TYPED_PARAM_LLONG) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for vcpu_quota tunable," - " expected a 'llong'")); - goto cleanup; - } - if (flags & VIR_DOMAIN_AFFECT_LIVE) { rc = qemuSetVcpusBWLive(vm, group, 0, params[i].value.l); if (rc != 0) @@ -7135,10 +7016,6 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { vmdef->cputune.quota = params[i].value.l; } - } else { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Invalid parameter `%s'"), param->field); - goto cleanup; } } @@ -7166,9 +7043,10 @@ cleanup: return ret; } -static int qemuSetSchedulerParameters(virDomainPtr dom, - virTypedParameterPtr params, - int nparams) +static int +qemuSetSchedulerParameters(virDomainPtr dom, + virTypedParameterPtr params, + int nparams) { return qemuSetSchedulerParametersFlags(dom, params, @@ -7321,42 +7199,25 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, goto cleanup; } out: - params[0].value.ul = shares; - params[0].type = VIR_TYPED_PARAM_ULLONG; - if (virStrcpyStatic(params[0].field, - VIR_DOMAIN_SCHEDULER_CPU_SHARES) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_CPU_SHARES); + if (virTypedParameterAssign(&params[0], VIR_DOMAIN_SCHEDULER_CPU_SHARES, + VIR_TYPED_PARAM_ULLONG, shares) < 0) goto cleanup; - } - saved_nparams++; if (cpu_bw_status) { if (*nparams > saved_nparams) { - params[1].value.ul = period; - params[1].type = VIR_TYPED_PARAM_ULLONG; - if (virStrcpyStatic(params[1].field, - VIR_DOMAIN_SCHEDULER_VCPU_PERIOD) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_VCPU_PERIOD); + if (virTypedParameterAssign(&params[1], + VIR_DOMAIN_SCHEDULER_VCPU_PERIOD, + VIR_TYPED_PARAM_ULLONG, period) < 0) goto cleanup; - } saved_nparams++; } if (*nparams > saved_nparams) { - params[2].value.ul = quota; - params[2].type = VIR_TYPED_PARAM_LLONG; - if (virStrcpyStatic(params[2].field, - VIR_DOMAIN_SCHEDULER_VCPU_QUOTA) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_VCPU_QUOTA); + if (virTypedParameterAssign(&params[2], + VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, + VIR_TYPED_PARAM_LLONG, quota) < 0) goto cleanup; - } saved_nparams++; } } @@ -7648,113 +7509,69 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, if (tmp < *nparams && wr_bytes != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES, + VIR_TYPED_PARAM_LLONG, wr_bytes) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = wr_bytes; tmp++; } if (tmp < *nparams && wr_req != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_WRITE_REQ) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_WRITE_REQ); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_WRITE_REQ, + VIR_TYPED_PARAM_LLONG, wr_req) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = wr_req; tmp++; } if (tmp < *nparams && rd_bytes != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_READ_BYTES) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_READ_BYTES); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_BYTES, + VIR_TYPED_PARAM_LLONG, rd_bytes) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = rd_bytes; tmp++; } if (tmp < *nparams && rd_req != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_READ_REQ) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_READ_REQ); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_READ_REQ, + VIR_TYPED_PARAM_LLONG, rd_req) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = rd_req; tmp++; } if (tmp < *nparams && flush_req != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ); + if (virTypedParameterAssign(param, VIR_DOMAIN_BLOCK_STATS_FLUSH_REQ, + VIR_TYPED_PARAM_LLONG, flush_req) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = flush_req; tmp++; } if (tmp < *nparams && wr_total_times != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_STATS_WRITE_TOTAL_TIMES, + VIR_TYPED_PARAM_LLONG, wr_total_times) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = wr_total_times; tmp++; } if (tmp < *nparams && rd_total_times != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_STATS_READ_TOTAL_TIMES, + VIR_TYPED_PARAM_LLONG, rd_total_times) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = rd_total_times; tmp++; } if (tmp < *nparams && flush_total_times != -1) { param = &params[tmp]; - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_STATS_FLUSH_TOTAL_TIMES, + VIR_TYPED_PARAM_LLONG, + flush_total_times) < 0) goto endjob; - } - param->type = VIR_TYPED_PARAM_LLONG; - param->value.l = flush_total_times; tmp++; } @@ -7852,8 +7669,23 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - qemuDriverLock(driver); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_BANDWIDTH_IN_AVERAGE, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_BANDWIDTH_IN_PEAK, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_BANDWIDTH_IN_BURST, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_BANDWIDTH_OUT_PEAK, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_BANDWIDTH_OUT_BURST, + VIR_TYPED_PARAM_UINT, + NULL) < 0) + return -1; + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (vm == NULL) { @@ -7902,64 +7734,17 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virTypedParameterPtr param = &params[i]; if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for bandwidth average tunable, " - "expected an 'unsigned int'")); - goto cleanup; - } - bandwidth->in->average = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for bandwidth peak tunable, " - "expected an 'unsigned int'")); - goto cleanup; - } - bandwidth->in->peak = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for bandwidth burst tunable, " - "expected an 'unsigned int'")); - goto cleanup; - } - bandwidth->in->burst = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for bandwidth average tunable, " - "expected an 'unsigned int'")); - goto cleanup; - } - bandwidth->out->average = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for bandwidth peak tunable, " - "expected an 'unsigned int'")); - goto cleanup; - } - bandwidth->out->peak = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) { - if (param->type != VIR_TYPED_PARAM_UINT) { - qemuReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid type for bandwidth burst tunable, " - "expected an 'unsigned int'")); - goto cleanup; - } - bandwidth->out->burst = params[i].value.ui; - } else { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Parameter `%s' not supported"), - param->field); - goto cleanup; } } @@ -8106,67 +7891,52 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom, } for (i = 0; i < *nparams && i < QEMU_NB_BANDWIDTH_PARAM; i++) { - params[i].value.ui = 0; - params[i].type = VIR_TYPED_PARAM_UINT; - switch(i) { case 0: /* inbound.average */ - if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BANDWIDTH_IN_AVERAGE); + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_BANDWIDTH_IN_AVERAGE, + VIR_TYPED_PARAM_UINT, 0) < 0) goto cleanup; - } if (net->bandwidth && net->bandwidth->in) params[i].value.ui = net->bandwidth->in->average; break; case 1: /* inbound.peak */ - if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_PEAK) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BANDWIDTH_IN_PEAK); + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_BANDWIDTH_IN_PEAK, + VIR_TYPED_PARAM_UINT, 0) < 0) goto cleanup; - } if (net->bandwidth && net->bandwidth->in) params[i].value.ui = net->bandwidth->in->peak; break; case 2: /* inbound.burst */ - if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_IN_BURST) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BANDWIDTH_IN_BURST); + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_BANDWIDTH_IN_BURST, + VIR_TYPED_PARAM_UINT, 0) < 0) goto cleanup; - } if (net->bandwidth && net->bandwidth->in) params[i].value.ui = net->bandwidth->in->burst; break; case 3: /* outbound.average */ - if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE); + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE, + VIR_TYPED_PARAM_UINT, 0) < 0) goto cleanup; - } if (net->bandwidth && net->bandwidth->out) params[i].value.ui = net->bandwidth->out->average; break; case 4: /* outbound.peak */ - if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BANDWIDTH_OUT_PEAK); + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_BANDWIDTH_OUT_PEAK, + VIR_TYPED_PARAM_UINT, 0) < 0) goto cleanup; - } if (net->bandwidth && net->bandwidth->out) params[i].value.ui = net->bandwidth->out->peak; break; case 5: /* outbound.burst */ - if (virStrcpyStatic(params[i].field, VIR_DOMAIN_BANDWIDTH_OUT_BURST) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BANDWIDTH_OUT_BURST); + if (virTypedParameterAssign(&params[i], + VIR_DOMAIN_BANDWIDTH_OUT_BURST, + VIR_TYPED_PARAM_UINT, 0) < 0) goto cleanup; - } if (net->bandwidth && net->bandwidth->out) params[i].value.ui = net->bandwidth->out->burst; break; @@ -11588,6 +11358,21 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + NULL) < 0) + return -1; memset(&info, 0, sizeof(info)); @@ -11615,13 +11400,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, for (i = 0; i < nparams; i++) { virTypedParameterPtr param = &params[i]; - if (param->type != VIR_TYPED_PARAM_ULLONG) { - qemuReportError(VIR_ERR_INVALID_ARG, - _("expected unsigned long long for parameter %s"), - param->field); - goto endjob; - } - if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) { info.total_bytes_sec = param->value.ul; } else if (STREQ(param->field, @@ -11639,11 +11417,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } else if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) { info.write_iops_sec = param->value.ul; - } else { - qemuReportError(VIR_ERR_INVALID_ARG, - _("Unrecognized parameter %s"), - param->field); - goto endjob; } } @@ -11767,74 +11540,49 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM && i < *nparams; i++) { virTypedParameterPtr param = &params[i]; - param->value.ul = 0; - param->type = VIR_TYPED_PARAM_ULLONG; switch(i) { case 0: - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.total_bytes_sec) < 0) goto endjob; - } - param->value.ul = reply.total_bytes_sec; break; - case 1: - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.read_bytes_sec) < 0) goto endjob; - } - param->value.ul = reply.read_bytes_sec; break; - case 2: - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.write_bytes_sec) < 0) goto endjob; - } - param->value.ul = reply.write_bytes_sec; break; - case 3: - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.total_iops_sec) < 0) goto endjob; - } - param->value.ul = reply.total_iops_sec; break; - case 4: - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.read_iops_sec) < 0) goto endjob; - } - param->value.ul = reply.read_iops_sec; break; - case 5: - if (virStrcpyStatic(param->field, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC) == NULL) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%s' too long"), - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC); + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.write_iops_sec) < 0) goto endjob; - } - param->value.ul = reply.write_iops_sec; break; default: break; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 2a98e7e..c0b2ca6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -50,6 +50,7 @@ #include "threads.h" #include "logging.h" #include "virfile.h" +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_TEST @@ -2691,16 +2692,11 @@ testDomainGetSchedulerParamsFlags(virDomainPtr domain, goto cleanup; } - if (virStrcpyStatic(params[0].field, - VIR_DOMAIN_SCHEDULER_WEIGHT) == NULL) { - testError(VIR_ERR_INTERNAL_ERROR, _("Field name '%s' too long"), - VIR_DOMAIN_SCHEDULER_WEIGHT); + if (virTypedParameterAssign(params, VIR_DOMAIN_SCHEDULER_WEIGHT, + VIR_TYPED_PARAM_UINT, 50) < 0) goto cleanup; - } - params[0].type = VIR_TYPED_PARAM_UINT; /* XXX */ /*params[0].value.ui = privdom->weight;*/ - params[0].value.ui = 50; *nparams = 1; ret = 0; @@ -2730,6 +2726,11 @@ testDomainSetSchedulerParamsFlags(virDomainPtr domain, int ret = -1, i; virCheckFlags(0, -1); + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_SCHEDULER_WEIGHT, + VIR_TYPED_PARAM_UINT, + NULL) < 0) + return -1; testDriverLock(privconn); privdom = virDomainFindByName(&privconn->domains, @@ -2742,16 +2743,10 @@ testDomainSetSchedulerParamsFlags(virDomainPtr domain, } for (i = 0; i < nparams; i++) { - if (STRNEQ(params[i].field, VIR_DOMAIN_SCHEDULER_WEIGHT)) { - testError(VIR_ERR_INVALID_ARG, "field"); - goto cleanup; - } - if (params[i].type != VIR_TYPED_PARAM_UINT) { - testError(VIR_ERR_INVALID_ARG, "type"); - goto cleanup; + if (STREQ(params[i].field, VIR_DOMAIN_SCHEDULER_WEIGHT)) { + /* XXX */ + /*privdom->weight = params[i].value.ui;*/ } - /* XXX */ - /*privdom->weight = params[i].value.ui;*/ } ret = 0; diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index ee3a59c..0c92be8 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1,7 +1,7 @@ /* * xen_internal.c: direct access to Xen hypervisor level * - * Copyright (C) 2005-2011 Red Hat, Inc. + * Copyright (C) 2005-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -67,6 +67,7 @@ #include "memory.h" #include "virfile.h" #include "virnodesuspend.h" +#include "virtypedparam.h" #define VIR_FROM_THIS VIR_FROM_XEN @@ -1335,27 +1336,18 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain, if (ret < 0) return(-1); - if (virStrcpyStatic(params[0].field, - VIR_DOMAIN_SCHEDULER_WEIGHT) == NULL) { - virXenError(VIR_ERR_INTERNAL_ERROR, - "Weight %s too big for destination", - VIR_DOMAIN_SCHEDULER_WEIGHT); + if (virTypedParameterAssign(&params[0], + VIR_DOMAIN_SCHEDULER_WEIGHT, + VIR_TYPED_PARAM_UINT, + op_dom.u.getschedinfo.u.credit.weight) < 0) return -1; - } - params[0].type = VIR_TYPED_PARAM_UINT; - params[0].value.ui = op_dom.u.getschedinfo.u.credit.weight; - - if (*nparams > 1) { - if (virStrcpyStatic(params[1].field, - VIR_DOMAIN_SCHEDULER_CAP) == NULL) { - virXenError(VIR_ERR_INTERNAL_ERROR, - "Cap %s too big for destination", - VIR_DOMAIN_SCHEDULER_CAP); + + if (*nparams > 1 && + virTypedParameterAssign(&params[1], + VIR_DOMAIN_SCHEDULER_CAP, + VIR_TYPED_PARAM_UINT, + op_dom.u.getschedinfo.u.credit.cap) < 0) return -1; - } - params[1].type = VIR_TYPED_PARAM_UINT; - params[1].value.ui = op_dom.u.getschedinfo.u.credit.cap; - } if (*nparams > XEN_SCHED_CRED_NPARAM) *nparams = XEN_SCHED_CRED_NPARAM; @@ -1399,6 +1391,14 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, return 0; } + if (virTypedParameterArrayValidate(params, nparams, + VIR_DOMAIN_SCHEDULER_WEIGHT, + VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_SCHEDULER_CAP, + VIR_TYPED_PARAM_UINT, + NULL) < 0) + return -1; + priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0) { virXenErrorFunc(VIR_ERR_INTERNAL_ERROR, __FUNCTION__, @@ -1453,8 +1453,7 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, for (i = 0; i < nparams; i++) { memset(&buf, 0, sizeof(buf)); - if (STREQ (params[i].field, VIR_DOMAIN_SCHEDULER_WEIGHT) && - params[i].type == VIR_TYPED_PARAM_UINT) { + if (STREQ(params[i].field, VIR_DOMAIN_SCHEDULER_WEIGHT)) { val = params[i].value.ui; if ((val < 1) || (val > USHRT_MAX)) { snprintf(buf, sizeof(buf), _("Credit scheduler weight parameter (%d) is out of range (1-65535)"), val); @@ -1462,8 +1461,7 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, return(-1); } op_dom.u.getschedinfo.u.credit.weight = val; - } else if (STREQ (params[i].field, VIR_DOMAIN_SCHEDULER_CAP) && - params[i].type == VIR_TYPED_PARAM_UINT) { + } else if (STREQ(params[i].field, VIR_DOMAIN_SCHEDULER_CAP)) { val = params[i].value.ui; if (val >= USHRT_MAX) { snprintf(buf, sizeof(buf), _("Credit scheduler cap parameter (%d) is out of range (0-65534)"), val); @@ -1471,11 +1469,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, return(-1); } op_dom.u.getschedinfo.u.credit.cap = val; - } else { - virXenErrorFunc(VIR_ERR_INVALID_ARG, __FUNCTION__, - "Credit scheduler accepts 'cap' and 'weight' integer parameters", - 0); - return(-1); } } -- 1.7.7.5

On 01/19/2012 12:21 PM, Daniel P. Berrange wrote: Thanks for catching that (hard to find minor mistakes like this in a big repetitive patch). Fixed. Yes, I specifically documented and implemented it that way: /* 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. */ as it made life simpler in the callers to pass NULL rather than worrying about OOM checks. Thanks for the speedy review; series pushed. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org