From: Peter Krempa <pkrempa@redhat.com> Fixes: a10b3ffebb6c498b357fd546c737d152cdf3e77d Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/remote_protocol-structs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 0f87d13a5a..d11a8f91a9 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1051,13 +1051,13 @@ struct remote_domain_get_block_io_tune_ret { int nparams; }; struct remote_domain_set_throttle_group_args { - remote_nonnull_domain dom; - remote_nonnull_string group; - struct { - u_int params_len; - remote_typed_param * params_val; - } params; - u_int flags; + remote_nonnull_domain dom; + remote_nonnull_string group; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; }; struct remote_domain_del_throttle_group_args { remote_nonnull_domain dom; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Reformat qemu_driver.c to use the contemporary coding style. It will help also for the upcoming script for generating list of supported flags for APIs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 212 +++++++++++++++++++++++++++-------------- 1 file changed, 142 insertions(+), 70 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d227ac58cd..80b21d4650 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1078,10 +1078,11 @@ qemuConnectURIProbe(char **uri) return 0; } -static virDrvOpenStatus qemuConnectOpen(virConnectPtr conn, - virConnectAuthPtr auth G_GNUC_UNUSED, - virConf *conf G_GNUC_UNUSED, - unsigned int flags) +static virDrvOpenStatus +qemuConnectOpen(virConnectPtr conn, + virConnectAuthPtr auth G_GNUC_UNUSED, + virConf *conf G_GNUC_UNUSED, + unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -1123,7 +1124,8 @@ static virDrvOpenStatus qemuConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; } -static int qemuConnectClose(virConnectPtr conn) +static int +qemuConnectClose(virConnectPtr conn) { virQEMUDriver *driver = conn->privateData; @@ -1172,7 +1174,9 @@ qemuConnectSupportsFeature(virConnectPtr conn, int feature) } } -static const char *qemuConnectGetType(virConnectPtr conn) { +static const char * +qemuConnectGetType(virConnectPtr conn) +{ if (virConnectGetTypeEnsureACL(conn) < 0) return NULL; @@ -1180,19 +1184,22 @@ static const char *qemuConnectGetType(virConnectPtr conn) { } -static int qemuConnectIsSecure(virConnectPtr conn G_GNUC_UNUSED) +static int +qemuConnectIsSecure(virConnectPtr conn G_GNUC_UNUSED) { /* Trivially secure, since always inside the daemon */ return 1; } -static int qemuConnectIsEncrypted(virConnectPtr conn G_GNUC_UNUSED) +static int +qemuConnectIsEncrypted(virConnectPtr conn G_GNUC_UNUSED) { /* Not encrypted, but remote driver takes care of that */ return 0; } -static int qemuConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) +static int +qemuConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED) { return 1; } @@ -1221,7 +1228,8 @@ qemuConnectGetSysinfo(virConnectPtr conn, unsigned int flags) } static int -qemuConnectGetMaxVcpus(virConnectPtr conn G_GNUC_UNUSED, const char *type) +qemuConnectGetMaxVcpus(virConnectPtr conn G_GNUC_UNUSED, + const char *type) { if (virConnectGetMaxVcpusEnsureACL(conn) < 0) return -1; @@ -1241,7 +1249,9 @@ qemuConnectGetMaxVcpus(virConnectPtr conn G_GNUC_UNUSED, const char *type) } -static char *qemuConnectGetCapabilities(virConnectPtr conn) { +static char * +qemuConnectGetCapabilities(virConnectPtr conn) +{ virQEMUDriver *driver = conn->privateData; g_autoptr(virCaps) caps = NULL; @@ -1361,8 +1371,9 @@ qemuDomainHelperGetVcpus(virDomainObj *vm, } -static virDomainPtr qemuDomainLookupByID(virConnectPtr conn, - int id) +static virDomainPtr +qemuDomainLookupByID(virConnectPtr conn, + int id) { virQEMUDriver *driver = conn->privateData; virDomainObj *vm; @@ -1386,8 +1397,9 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr conn, return dom; } -static virDomainPtr qemuDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) +static virDomainPtr +qemuDomainLookupByUUID(virConnectPtr conn, + const unsigned char *uuid) { virQEMUDriver *driver = conn->privateData; virDomainObj *vm; @@ -1413,8 +1425,9 @@ static virDomainPtr qemuDomainLookupByUUID(virConnectPtr conn, return dom; } -static virDomainPtr qemuDomainLookupByName(virConnectPtr conn, - const char *name) +static virDomainPtr +qemuDomainLookupByName(virConnectPtr conn, + const char *name) { virQEMUDriver *driver = conn->privateData; virDomainObj *vm; @@ -1439,7 +1452,8 @@ static virDomainPtr qemuDomainLookupByName(virConnectPtr conn, } -static int qemuDomainIsActive(virDomainPtr dom) +static int +qemuDomainIsActive(virDomainPtr dom) { virDomainObj *obj; int ret = -1; @@ -1457,7 +1471,8 @@ static int qemuDomainIsActive(virDomainPtr dom) return ret; } -static int qemuDomainIsPersistent(virDomainPtr dom) +static int +qemuDomainIsPersistent(virDomainPtr dom) { virDomainObj *obj; int ret = -1; @@ -1475,7 +1490,8 @@ static int qemuDomainIsPersistent(virDomainPtr dom) return ret; } -static int qemuDomainIsUpdated(virDomainPtr dom) +static int +qemuDomainIsUpdated(virDomainPtr dom) { virDomainObj *obj; int ret = -1; @@ -1493,7 +1509,9 @@ static int qemuDomainIsUpdated(virDomainPtr dom) return ret; } -static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) +static int +qemuConnectGetVersion(virConnectPtr conn, + unsigned long *version) { virQEMUDriver *driver = conn->privateData; unsigned int qemuVersion = 0; @@ -1515,7 +1533,8 @@ static int qemuConnectGetVersion(virConnectPtr conn, unsigned long *version) } -static char *qemuConnectGetHostname(virConnectPtr conn) +static char * +qemuConnectGetHostname(virConnectPtr conn) { if (virConnectGetHostnameEnsureACL(conn) < 0) return NULL; @@ -1524,7 +1543,10 @@ static char *qemuConnectGetHostname(virConnectPtr conn) } -static int qemuConnectListDomains(virConnectPtr conn, int *ids, int nids) +static int +qemuConnectListDomains(virConnectPtr conn, + int *ids, + int nids) { virQEMUDriver *driver = conn->privateData; @@ -1535,7 +1557,9 @@ static int qemuConnectListDomains(virConnectPtr conn, int *ids, int nids) virConnectListDomainsCheckACL, conn); } -static int qemuConnectNumOfDomains(virConnectPtr conn) + +static int +qemuConnectNumOfDomains(virConnectPtr conn) { virQEMUDriver *driver = conn->privateData; @@ -1547,9 +1571,10 @@ static int qemuConnectNumOfDomains(virConnectPtr conn) } -static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, - const char *xml, - unsigned int flags) +static virDomainPtr +qemuDomainCreateXML(virConnectPtr conn, + const char *xml, + unsigned int flags) { virQEMUDriver *driver = conn->privateData; g_autoptr(virDomainDef) def = NULL; @@ -1637,7 +1662,8 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, } -static int qemuDomainSuspend(virDomainPtr dom) +static int +qemuDomainSuspend(virDomainPtr dom) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm; @@ -1687,7 +1713,8 @@ static int qemuDomainSuspend(virDomainPtr dom) } -static int qemuDomainResume(virDomainPtr dom) +static int +qemuDomainResume(virDomainPtr dom) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm; @@ -1804,7 +1831,9 @@ qemuDomainShutdownFlagsMonitor(virDomainObj *vm, } -static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) +static int +qemuDomainShutdownFlags(virDomainPtr dom, + unsigned int flags) { virDomainObj *vm; int ret = -1; @@ -1862,7 +1891,9 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) return ret; } -static int qemuDomainShutdown(virDomainPtr dom) + +static int +qemuDomainShutdown(virDomainPtr dom) { return qemuDomainShutdownFlags(dom, 0); } @@ -2112,7 +2143,10 @@ qemuDomainDestroy(virDomainPtr dom) return qemuDomainDestroyFlags(dom, 0); } -static char *qemuDomainGetOSType(virDomainPtr dom) { + +static char * +qemuDomainGetOSType(virDomainPtr dom) +{ virDomainObj *vm; char *type = NULL; @@ -2129,6 +2163,7 @@ static char *qemuDomainGetOSType(virDomainPtr dom) { return type; } + /* Returns max memory in kb, 0 if error */ static unsigned long long qemuDomainGetMaxMemory(virDomainPtr dom) @@ -2149,8 +2184,11 @@ qemuDomainGetMaxMemory(virDomainPtr dom) return ret; } -static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, - unsigned int flags) + +static int +qemuDomainSetMemoryFlags(virDomainPtr dom, + unsigned long newmem, + unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; qemuDomainObjPrivate *priv; @@ -2279,18 +2317,27 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, return ret; } -static int qemuDomainSetMemory(virDomainPtr dom, unsigned long newmem) + +static int +qemuDomainSetMemory(virDomainPtr dom, + unsigned long newmem) { return qemuDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE); } -static int qemuDomainSetMaxMemory(virDomainPtr dom, unsigned long memory) + +static int +qemuDomainSetMaxMemory(virDomainPtr dom, + unsigned long memory) { return qemuDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM); } -static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, - unsigned int flags) + +static int +qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, + int period, + unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; qemuDomainObjPrivate *priv; @@ -2360,7 +2407,10 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period, return ret; } -static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) + +static int +qemuDomainInjectNMI(virDomainPtr domain, + unsigned int flags) { virDomainObj *vm = NULL; int ret = -1; @@ -2394,12 +2444,14 @@ static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) return ret; } -static int qemuDomainSendKey(virDomainPtr domain, - unsigned int codeset, - unsigned int holdtime, - unsigned int *keycodes, - int nkeycodes, - unsigned int flags) + +static int +qemuDomainSendKey(virDomainPtr domain, + unsigned int codeset, + unsigned int holdtime, + unsigned int *keycodes, + int nkeycodes, + unsigned int flags) { virDomainObj *vm = NULL; int ret = -1; @@ -5641,7 +5693,9 @@ qemuDomainSetIOThreadParams(virDomainPtr dom, } -static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr seclabel) +static int +qemuDomainGetSecurityLabel(virDomainPtr dom, + virSecurityLabelPtr seclabel) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm; @@ -5682,8 +5736,10 @@ static int qemuDomainGetSecurityLabel(virDomainPtr dom, virSecurityLabelPtr secl return ret; } -static int qemuDomainGetSecurityLabelList(virDomainPtr dom, - virSecurityLabelPtr* seclabels) + +static int +qemuDomainGetSecurityLabelList(virDomainPtr dom, + virSecurityLabelPtr* seclabels) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm; @@ -5744,8 +5800,9 @@ static int qemuDomainGetSecurityLabelList(virDomainPtr dom, } -static int qemuNodeGetSecurityModel(virConnectPtr conn, - virSecurityModelPtr secmodel) +static int +qemuNodeGetSecurityModel(virConnectPtr conn, + virSecurityModelPtr secmodel) { virQEMUDriver *driver = conn->privateData; g_autoptr(virCaps) caps = NULL; @@ -6199,9 +6256,9 @@ qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml, } -static char -*qemuDomainGetXMLDesc(virDomainPtr dom, - unsigned int flags) +static char * +qemuDomainGetXMLDesc(virDomainPtr dom, + unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm; @@ -6245,10 +6302,11 @@ static char } -static char *qemuConnectDomainXMLToNative(virConnectPtr conn, - const char *format, - const char *xmlData, - unsigned int flags) +static char * +qemuConnectDomainXMLToNative(virConnectPtr conn, + const char *format, + const char *xmlData, + unsigned int flags) { virQEMUDriver *driver = conn->privateData; g_autoptr(virDomainObj) vm = NULL; @@ -6308,8 +6366,11 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, } -static int qemuConnectListDefinedDomains(virConnectPtr conn, - char **const names, int nnames) { +static int +qemuConnectListDefinedDomains(virConnectPtr conn, + char **const names, + int nnames) +{ virQEMUDriver *driver = conn->privateData; if (virConnectListDefinedDomainsEnsureACL(conn) < 0) @@ -6320,7 +6381,9 @@ static int qemuConnectListDefinedDomains(virConnectPtr conn, conn); } -static int qemuConnectNumOfDefinedDomains(virConnectPtr conn) + +static int +qemuConnectNumOfDefinedDomains(virConnectPtr conn) { virQEMUDriver *driver = conn->privateData; @@ -7794,14 +7857,18 @@ qemuDomainDetachDeviceAlias(virDomainPtr dom, } -static int qemuDomainDetachDevice(virDomainPtr dom, const char *xml) +static int +qemuDomainDetachDevice(virDomainPtr dom, + const char *xml) { return qemuDomainDetachDeviceFlags(dom, xml, VIR_DOMAIN_AFFECT_LIVE); } -static int qemuDomainGetAutostart(virDomainPtr dom, - int *autostart) + +static int +qemuDomainGetAutostart(virDomainPtr dom, + int *autostart) { virDomainObj *vm; int ret = -1; @@ -7820,8 +7887,10 @@ static int qemuDomainGetAutostart(virDomainPtr dom, return ret; } -static int qemuDomainSetAutostart(virDomainPtr dom, - int autostart) + +static int +qemuDomainSetAutostart(virDomainPtr dom, + int autostart) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm; @@ -7986,8 +8055,9 @@ qemuDomainSetAutostartOnce(virDomainPtr dom, } -static char *qemuDomainGetSchedulerType(virDomainPtr dom, - int *nparams) +static char * +qemuDomainGetSchedulerType(virDomainPtr dom, + int *nparams) { char *ret = NULL; virDomainObj *vm = NULL; @@ -18915,9 +18985,11 @@ qemuDomainRenameCallback(virDomainObj *vm, return ret; } -static int qemuDomainRename(virDomainPtr dom, - const char *new_name, - unsigned int flags) + +static int +qemuDomainRename(virDomainPtr dom, + const char *new_name, + unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; virDomainObj *vm = NULL; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Create 'virTypedParamValidateType' which will use the same logic encapsulated in a function. Use the error message wording from 'virTypedParamsValidate' as it contains less fluff. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam-public.c | 54 +++++++++++++++++++++------------ src/util/virtypedparam.c | 27 +++++++++++++++++ src/util/virtypedparam.h | 4 +++ 3 files changed, 66 insertions(+), 19 deletions(-) diff --git a/src/util/virtypedparam-public.c b/src/util/virtypedparam-public.c index 3486623a9c..8a953ed20f 100644 --- a/src/util/virtypedparam-public.c +++ b/src/util/virtypedparam-public.c @@ -147,18 +147,6 @@ virTypedParamsGet(virTypedParameterPtr params, } -#define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \ - do { if (param->type != check_type) { \ - virReportError(VIR_ERR_INVALID_ARG, \ - _("Invalid type '%1$s' requested for parameter '%2$s', actual type is '%3$s'"), \ - virTypedParameterTypeToString(check_type), \ - name, \ - virTypedParameterTypeToString(param->type)); \ - virDispatchError(NULL); \ - return -1; \ - } } while (0) - - /** * virTypedParamsGetInt: * @params: array of typed parameters @@ -189,7 +177,11 @@ virTypedParamsGetInt(virTypedParameterPtr params, if (!(param = virTypedParamsGet(params, nparams, name))) return 0; - VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_INT); + if (virTypedParamValidateType(param, VIR_TYPED_PARAM_INT) < 0) { + virDispatchError(NULL); + return -1; + } + if (value) *value = param->value.i; @@ -227,7 +219,11 @@ virTypedParamsGetUInt(virTypedParameterPtr params, if (!(param = virTypedParamsGet(params, nparams, name))) return 0; - VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_UINT); + if (virTypedParamValidateType(param, VIR_TYPED_PARAM_UINT) < 0) { + virDispatchError(NULL); + return -1; + } + if (value) *value = param->value.ui; @@ -265,7 +261,11 @@ virTypedParamsGetLLong(virTypedParameterPtr params, if (!(param = virTypedParamsGet(params, nparams, name))) return 0; - VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_LLONG); + if (virTypedParamValidateType(param, VIR_TYPED_PARAM_LLONG) < 0) { + virDispatchError(NULL); + return -1; + } + if (value) *value = param->value.l; @@ -303,7 +303,11 @@ virTypedParamsGetULLong(virTypedParameterPtr params, if (!(param = virTypedParamsGet(params, nparams, name))) return 0; - VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_ULLONG); + if (virTypedParamValidateType(param, VIR_TYPED_PARAM_ULLONG) < 0) { + virDispatchError(NULL); + return -1; + } + if (value) *value = param->value.ul; @@ -341,7 +345,11 @@ virTypedParamsGetDouble(virTypedParameterPtr params, if (!(param = virTypedParamsGet(params, nparams, name))) return 0; - VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_DOUBLE); + if (virTypedParamValidateType(param, VIR_TYPED_PARAM_DOUBLE) < 0) { + virDispatchError(NULL); + return -1; + } + if (value) *value = param->value.d; @@ -379,7 +387,11 @@ virTypedParamsGetBoolean(virTypedParameterPtr params, if (!(param = virTypedParamsGet(params, nparams, name))) return 0; - VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_BOOLEAN); + if (virTypedParamValidateType(param, VIR_TYPED_PARAM_BOOLEAN) < 0) { + virDispatchError(NULL); + return -1; + } + if (value) *value = !!param->value.b; @@ -419,7 +431,11 @@ virTypedParamsGetString(virTypedParameterPtr params, if (!(param = virTypedParamsGet(params, nparams, name))) return 0; - VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_STRING); + if (virTypedParamValidateType(param, VIR_TYPED_PARAM_STRING) < 0) { + virDispatchError(NULL); + return -1; + } + if (value) *value = param->value.s; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index f25530a735..0b40c14f90 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -42,6 +42,33 @@ VIR_ENUM_IMPL(virTypedParameter, "string", ); + +/** + * virTypedParamValidateType: + * @param: typed parameter to validate + * @expected_type: type to look for + * + * Validates that @param is a parameter of @expected type. + * + * Returns 0 on success; -1 on error and reports an error. + */ +int +virTypedParamValidateType(virTypedParameterPtr param, + unsigned int expected_type) +{ + if (param->type != expected_type) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid type '%1$s' for parameter '%2$s', expected '%3$s'"), + virTypedParameterTypeToString(expected_type), + param->field, + virTypedParameterTypeToString(param->type)); + return -1; + } + + return 0; +} + + static int virTypedParamsSortName(const void *left, const void *right, diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 774744244a..c1fc28c612 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -63,6 +63,10 @@ struct _virTypedParameterRemote { virTypedParameterRemoteValue value; }; +int +virTypedParamValidateType(virTypedParameterPtr param, + unsigned int expected_type) + G_GNUC_WARN_UNUSED_RESULT; int virTypedParamsValidate(virTypedParameterPtr params, -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> If the actual type of the typed parameter is an invalid number the type checker would still attempt to convert it to a string resuling in an attempt to print a NULL string. libc saves us from the crash but the error message is still wrong. Fix it. Fixes: 54dd75fd97339dd49a54554e9327e5680c72132b Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 0b40c14f90..6cfdd3276d 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -56,6 +56,13 @@ int virTypedParamValidateType(virTypedParameterPtr param, unsigned int expected_type) { + if (param->type <= 0 || param->type >= VIR_TYPED_PARAM_LAST) { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown type ('%1$d') of parameter '%2$s'"), + param->type, param->field); + return -1; + } + if (param->type != expected_type) { virReportError(VIR_ERR_INVALID_ARG, _("invalid type '%1$s' for parameter '%2$s', expected '%3$s'"), -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Replace the internals of the macro by a function so that 'virTypedParameterToString' doesn't need to be exported as it also adds mappings for values which don't exist in the public API. This change also prevents a NULL to be passed to string formatters in case when the caller sends an unknown typed parameter type as we now also make sure that the type is in range. Fixes: 54dd75fd97339dd49a54554e9327e5680c72132b Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virtypedparam.c | 25 +++++++++++++++++++++++++ src/util/virtypedparam.h | 11 +++++------ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cf0e71cc6a..9f02f74847 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3652,6 +3652,7 @@ virTPMSwtpmSetupFeatureTypeFromString; # util/virtypedparam.h +virTypedParamDebugstr; virTypedParameterAssign; virTypedParameterToString; virTypedParameterTypeFromString; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 6cfdd3276d..77c9279eff 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1109,3 +1109,28 @@ virTypedParamListAddDouble(virTypedParamList *list, virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); } + + +/** + * virTypedParamDebugstr: + * @param: typed parameter + * + * Format @param into a string used for debug prints in public API handlers. + * This must make sure to work on unknown typed parameter types. + * + * Returns the formatted string; caller must free it. + */ +char * +virTypedParamDebugstr(virTypedParameterPtr param) +{ + g_autofree char *value = virTypedParameterToString(param); + int type = param->type; + + if (type < 0 || type > VIR_TYPED_PARAM_LAST) + type = 0; + + return g_strdup_printf("params[\"%s\"]=(%s)%s", + param->field, + virTypedParameterTypeToString(type), + NULLSTR(value)); +} diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index c1fc28c612..3d6f6b999c 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -140,18 +140,17 @@ virTypedParamsSerialize(virTypedParameterPtr params, VIR_ENUM_DECL(virTypedParameter); +char * +virTypedParamDebugstr(virTypedParameterPtr param); + #define VIR_TYPED_PARAMS_DEBUG(params, nparams) \ do { \ int _i; \ if (!params) \ break; \ for (_i = 0; _i < (nparams); _i++) { \ - char *_value = virTypedParameterToString((params) + _i); \ - VIR_DEBUG("params[\"%s\"]=(%s)%s", \ - (params)[_i].field, \ - virTypedParameterTypeToString((params)[_i].type), \ - NULLSTR(_value)); \ - VIR_FREE(_value); \ + g_autofree char *_debugstr = virTypedParamDebugstr((params) + _i); \ + VIR_DEBUG("%s", _debugstr); \ } \ } while (0) -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> 'virTypedParamsValidate' has special logic to handle the internal VIR_TYPED_PARAM_UNSIGNED type, but unfortunately the implementation of the error which is reported is flawed as it only updated the 'expecttype' string when the type actually matched. In cases when it didn't we'd report the following error: error: invalid argument: invalid type 'string' for parameter 'poll_shrink', expected '(null)' To fix it we can re-implement the validation part by using virTypedParamValidateType simply by adding the VIR_TYPED_PARAM_UNSIGNED to 'virTypedParameterTypeToString' handling which is now private and adding logic to allow either one of the unsigned types, which allows us to use the same function in both cases, simplifying the code. Fixes: 07652410a7af98ca03281c4bfe20415ced26a44a Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virtypedparam.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 0b494e60fd..ec8046b998 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -33,7 +33,7 @@ VIR_ENUM_DECL(virTypedParameter); VIR_ENUM_IMPL(virTypedParameter, - VIR_TYPED_PARAM_LAST, + VIR_TYPED_PARAM_UNSIGNED + 1, "unknown", "int", "uint", @@ -42,6 +42,8 @@ VIR_ENUM_IMPL(virTypedParameter, "double", "boolean", "string", + "", /* VIR_TYPED_PARAM_LAST */ + "uint, ullong", /* VIR_TYPED_PARAM_UNSIGNED */ ); @@ -50,7 +52,9 @@ VIR_ENUM_IMPL(virTypedParameter, * @param: typed parameter to validate * @expected_type: type to look for * - * Validates that @param is a parameter of @expected type. + * Validates that @param is a parameter of @expected type. If @expected_type is + * VIR_TYPED_PARAM_UNSIGNED, both VIR_TYPED_PARAM_UINT and VIR_TYPED_PARAM_ULLONG + * are accepted. * * Returns 0 on success; -1 on error and reports an error. */ @@ -65,7 +69,10 @@ virTypedParamValidateType(virTypedParameterPtr param, return -1; } - if (param->type != expected_type) { + if (!(param->type == expected_type || + (expected_type == VIR_TYPED_PARAM_UNSIGNED && + (param->type == VIR_TYPED_PARAM_UINT || + param->type == VIR_TYPED_PARAM_ULLONG)))) { virReportError(VIR_ERR_INVALID_ARG, _("invalid type '%1$s' for parameter '%2$s', expected '%3$s'"), virTypedParameterTypeToString(expected_type), @@ -147,9 +154,6 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) if (STRNEQ(sorted[i].field, keys[j].field)) { j++; } else { - const char *expecttype = virTypedParameterTypeToString(keys[j].type); - int type = sorted[i].type; - if (STREQ_NULLABLE(last_name, sorted[i].field) && !(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) { virReportError(VIR_ERR_INVALID_ARG, @@ -158,24 +162,9 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) return -1; } - if (keys[j].type == VIR_TYPED_PARAM_UNSIGNED && - (type == VIR_TYPED_PARAM_UINT || - type == VIR_TYPED_PARAM_ULLONG)) { - type = VIR_TYPED_PARAM_UNSIGNED; - expecttype = "uint, ullong"; - } - - if (type != keys[j].type) { - const char *badtype; - - badtype = virTypedParameterTypeToString(sorted[i].type); - if (!badtype) - badtype = virTypedParameterTypeToString(0); - virReportError(VIR_ERR_INVALID_ARG, - _("invalid type '%1$s' for parameter '%2$s', expected '%3$s'"), - badtype, sorted[i].field, expecttype); + if (virTypedParamValidateType(sorted + i, keys[j].type) < 0) return -1; - } + last_name = sorted[i].field; i++; } -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> The utility functions which get CPU map and stats don't actually use the flags. Remove the argument and move the 'virCheckFlags' to driver implementation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_driver.c | 8 ++++++-- src/ch/ch_driver.c | 7 +++++-- src/lxc/lxc_driver.c | 8 ++++++-- src/openvz/openvz_driver.c | 8 ++++++-- src/qemu/qemu_driver.c | 8 ++++++-- src/util/virhostcpu.c | 10 ++-------- src/util/virhostcpu.h | 6 ++---- src/vz/vz_driver.c | 8 ++++++-- tests/virhostcputest.c | 2 +- 9 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index c8dd1a728a..2718c074f3 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1145,10 +1145,12 @@ bhyveNodeGetCPUStats(virConnectPtr conn, int *nparams, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUStatsEnsureACL(conn) < 0) return -1; - return virHostCPUGetStats(cpuNum, params, nparams, flags); + return virHostCPUGetStats(cpuNum, params, nparams); } static int @@ -1359,10 +1361,12 @@ bhyveNodeGetCPUMap(virConnectPtr conn, unsigned int *online, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUMapEnsureACL(conn) < 0) return -1; - return virHostCPUGetMap(cpumap, online, flags); + return virHostCPUGetMap(cpumap, online); } static int diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index 662857f88e..874fca2193 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -1642,12 +1642,15 @@ chDomainGetVcpuPinInfo(virDomain *dom, static int chNodeGetCPUMap(virConnectPtr conn, unsigned char **cpumap, - unsigned int *online, unsigned int flags) + unsigned int *online, + unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUMapEnsureACL(conn) < 0) return -1; - return virHostCPUGetMap(cpumap, online, flags); + return virHostCPUGetMap(cpumap, online); } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c0a93c0444..db00288900 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4785,10 +4785,12 @@ lxcNodeGetCPUStats(virConnectPtr conn, int *nparams, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUStatsEnsureACL(conn) < 0) return -1; - return virHostCPUGetStats(cpuNum, params, nparams, flags); + return virHostCPUGetStats(cpuNum, params, nparams); } @@ -4866,10 +4868,12 @@ lxcNodeGetCPUMap(virConnectPtr conn, unsigned int *online, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUMapEnsureACL(conn) < 0) return -1; - return virHostCPUGetMap(cpumap, online, flags); + return virHostCPUGetMap(cpumap, online); } diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 19bdfa37f2..60b8a9534b 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1849,7 +1849,9 @@ openvzNodeGetCPUStats(virConnectPtr conn G_GNUC_UNUSED, int *nparams, unsigned int flags) { - return virHostCPUGetStats(cpuNum, params, nparams, flags); + virCheckFlags(0, -1); + + return virHostCPUGetStats(cpuNum, params, nparams); } @@ -1890,7 +1892,9 @@ openvzNodeGetCPUMap(virConnectPtr conn G_GNUC_UNUSED, unsigned int *online, unsigned int flags) { - return virHostCPUGetMap(cpumap, online, flags); + virCheckFlags(0, -1); + + return virHostCPUGetMap(cpumap, online); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 80b21d4650..52c6ce2f27 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16384,10 +16384,12 @@ qemuNodeGetCPUStats(virConnectPtr conn, int *nparams, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUStatsEnsureACL(conn) < 0) return -1; - return virHostCPUGetStats(cpuNum, params, nparams, flags); + return virHostCPUGetStats(cpuNum, params, nparams); } @@ -16465,10 +16467,12 @@ qemuNodeGetCPUMap(virConnectPtr conn, unsigned int *online, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUMapEnsureACL(conn) < 0) return -1; - return virHostCPUGetMap(cpumap, online, flags); + return virHostCPUGetMap(cpumap, online); } diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 870338edad..60743765a4 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1043,11 +1043,8 @@ virHostCPUGetInfo(virArch hostarch G_GNUC_UNUSED, int virHostCPUGetStats(int cpuNum G_GNUC_UNUSED, virNodeCPUStatsPtr params G_GNUC_UNUSED, - int *nparams G_GNUC_UNUSED, - unsigned int flags) + int *nparams G_GNUC_UNUSED) { - virCheckFlags(0, -1); - #ifdef __linux__ { int ret; @@ -1144,14 +1141,11 @@ virHostCPUGetOnlineBitmap(void) int virHostCPUGetMap(unsigned char **cpumap, - unsigned int *online, - unsigned int flags) + unsigned int *online) { g_autoptr(virBitmap) cpus = NULL; int ncpus = virHostCPUGetCount(); - virCheckFlags(0, -1); - if (!cpumap && !online) return ncpus; diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h index 289ae41439..238054ed34 100644 --- a/src/util/virhostcpu.h +++ b/src/util/virhostcpu.h @@ -36,8 +36,7 @@ struct _virHostCPUTscInfo { int virHostCPUGetStats(int cpuNum, virNodeCPUStatsPtr params, - int *nparams, - unsigned int flags); + int *nparams); bool virHostCPUHasBitmap(void); virBitmap *virHostCPUGetPresentBitmap(void); @@ -49,8 +48,7 @@ int virHostCPUGetCount(void); int virHostCPUGetThreadsPerSubcore(virArch arch) ATTRIBUTE_MOCKABLE; int virHostCPUGetMap(unsigned char **cpumap, - unsigned int *online, - unsigned int flags); + unsigned int *online); int virHostCPUGetInfo(virArch hostarch, unsigned int *cpus, unsigned int *mhz, diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 2d8878fe7f..f8193367c9 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -1000,10 +1000,12 @@ vzNodeGetCPUMap(virConnectPtr conn, unsigned int *online, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUMapEnsureACL(conn) < 0) return -1; - return virHostCPUGetMap(cpumap, online, flags); + return virHostCPUGetMap(cpumap, online); } static int @@ -1964,10 +1966,12 @@ vzNodeGetCPUStats(virConnectPtr conn, int *nparams, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeGetCPUStatsEnsureACL(conn) < 0) return -1; - return virHostCPUGetStats(cpuNum, params, nparams, flags); + return virHostCPUGetStats(cpuNum, params, nparams); } static int diff --git a/tests/virhostcputest.c b/tests/virhostcputest.c index 4b31b79444..e362ee8d39 100644 --- a/tests/virhostcputest.c +++ b/tests/virhostcputest.c @@ -249,7 +249,7 @@ linuxTestHostCPUGetMap(const void *data G_GNUC_UNUSED) { g_autofree unsigned char *cpumap = NULL; - int ncpus = virHostCPUGetMap(&cpumap, NULL, 0); + int ncpus = virHostCPUGetMap(&cpumap, NULL); g_autoptr(virBitmap) actual = virBitmapNewData(cpumap, VIR_DIV_UP(ncpus, 8)); g_autoptr(virBitmap) expected = NULL; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Remove the unused argument and bump the 'virCheckFlags' calls to the top level. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_driver.c | 4 +++- src/qemu/qemu_driver.c | 4 +++- src/util/virnodesuspend.c | 5 +---- src/util/virnodesuspend.h | 3 +-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c51da98777..df8e4e06ec 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4889,10 +4889,12 @@ lxcNodeSuspendForDuration(virConnectPtr conn, unsigned long long duration, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeSuspendForDurationEnsureACL(conn) < 0) return -1; - return virNodeSuspend(target, duration, flags); + return virNodeSuspend(target, duration); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f121a0e788..558c22982d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16488,10 +16488,12 @@ qemuNodeSuspendForDuration(virConnectPtr conn, unsigned long long duration, unsigned int flags) { + virCheckFlags(0, -1); + if (virNodeSuspendForDurationEnsureACL(conn) < 0) return -1; - return virNodeSuspend(target, duration, flags); + return virNodeSuspend(target, duration); } static int diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c index 37659f13ad..5b65643d06 100644 --- a/src/util/virnodesuspend.c +++ b/src/util/virnodesuspend.c @@ -138,16 +138,13 @@ static void virNodeSuspendHelper(void *cmdString) * operation is still in progress. */ int virNodeSuspend(unsigned int target, - unsigned long long duration, - unsigned int flags) + unsigned long long duration) { static virThread thread; const char *cmdString = NULL; unsigned int supported; VIR_LOCK_GUARD lock = { NULL }; - virCheckFlags(0, -1); - if (virNodeSuspendGetTargetMask(&supported) < 0) return -1; diff --git a/src/util/virnodesuspend.h b/src/util/virnodesuspend.h index f767961bff..dc8324df1d 100644 --- a/src/util/virnodesuspend.h +++ b/src/util/virnodesuspend.h @@ -24,7 +24,6 @@ #include "internal.h" int virNodeSuspend(unsigned int target, - unsigned long long duration, - unsigned int flags); + unsigned long long duration); int virNodeSuspendGetTargetMask(unsigned int *bitmask); -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Move the flag check to the top level to allow programatic introspection of supported flags. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8027b5775e..e6ad16f31b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7417,9 +7417,6 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObj *vm, unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE; - virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); - cfg = virQEMUDriverGetConfig(driver); /* The config and live post processing address auto-generation algorithms @@ -7512,6 +7509,9 @@ qemuDomainAttachDeviceFlags(virDomainPtr dom, virDomainObj *vm = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; @@ -7660,9 +7660,6 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriver *driver, unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; g_autoptr(virDomainDef) vmdef = NULL; - virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); - cfg = virQEMUDriverGetConfig(driver); if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && @@ -7796,6 +7793,9 @@ qemuDomainDetachDeviceFlags(virDomainPtr dom, virDomainObj *vm = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Move the flag check to the top level to allow programatic introspection of supported flags. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 11 ----------- src/qemu/qemu_driver.c | 11 +++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index 193cf9a06a..f063b5a5c0 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -582,9 +582,6 @@ qemuCheckpointCreateXML(virDomainPtr domain, g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virDomainCheckpointDef) def = NULL; - virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE | - VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE_VALIDATE, NULL); - if (redefine) { parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE; update_current = false; @@ -783,10 +780,6 @@ qemuCheckpointGetXMLDesc(virDomainObj *vm, virDomainCheckpointDef *chkdef; unsigned int format_flags; - virCheckFlags(VIR_DOMAIN_CHECKPOINT_XML_SECURE | - VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN | - VIR_DOMAIN_CHECKPOINT_XML_SIZE, NULL); - if (!(chk = qemuCheckpointObjFromCheckpoint(vm, checkpoint))) return NULL; @@ -847,10 +840,6 @@ qemuCheckpointDelete(virDomainObj *vm, struct virQEMUCheckpointReparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY); - virCheckFlags(VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN | - VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY | - VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY, -1); - if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 431b3b741e..b0b5c12d47 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13448,6 +13448,9 @@ qemuDomainCheckpointCreateXML(virDomainPtr domain, virDomainObj *vm = NULL; virDomainCheckpointPtr checkpoint = NULL; + virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE | + VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE_VALIDATE, NULL); + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; @@ -13589,6 +13592,10 @@ qemuDomainCheckpointGetXMLDesc(virDomainCheckpointPtr checkpoint, virDomainObj *vm = NULL; char *xml = NULL; + virCheckFlags(VIR_DOMAIN_CHECKPOINT_XML_SECURE | + VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN | + VIR_DOMAIN_CHECKPOINT_XML_SIZE, NULL); + if (!(vm = qemuDomObjFromCheckpoint(checkpoint))) return NULL; @@ -13610,6 +13617,10 @@ qemuDomainCheckpointDelete(virDomainCheckpointPtr checkpoint, virDomainObj *vm = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN | + VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY | + VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY, -1); + if (!(vm = qemuDomObjFromCheckpoint(checkpoint))) return -1; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Move the 'virCheckFlags' invocations to the top level driver function so that they become available for introspection. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 27 +++++++++++++++++++++++++++ src/qemu/qemu_snapshot.c | 27 --------------------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7f8958e45b..a2b5959e55 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13010,6 +13010,24 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virDomainObj *vm = NULL; virDomainSnapshotPtr snapshot = NULL; + virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | + VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | + VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA | + VIR_DOMAIN_SNAPSHOT_CREATE_HALT | + VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY | + VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT | + VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE | + VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC | + VIR_DOMAIN_SNAPSHOT_CREATE_LIVE | + VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, NULL); + + VIR_REQUIRE_FLAG_RET(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, + VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY, + NULL); + VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, + VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, + NULL); + if (!(vm = qemuDomainObjFromDomain(domain))) goto cleanup; @@ -13408,6 +13426,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainObj *vm = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED | + VIR_DOMAIN_SNAPSHOT_REVERT_FORCE | + VIR_DOMAIN_SNAPSHOT_REVERT_RESET_NVRAM, -1); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; @@ -13429,6 +13452,10 @@ qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virDomainObj *vm = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | + VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY | + VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, -1); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) return -1; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 82ae38ca29..07548ca62e 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2222,24 +2222,6 @@ qemuSnapshotCreateXML(virDomainPtr domain, g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virDomainSnapshotDef) def = NULL; - virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE | - VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT | - VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA | - VIR_DOMAIN_SNAPSHOT_CREATE_HALT | - VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY | - VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT | - VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE | - VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC | - VIR_DOMAIN_SNAPSHOT_CREATE_LIVE | - VIR_DOMAIN_SNAPSHOT_CREATE_VALIDATE, NULL); - - VIR_REQUIRE_FLAG_RET(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, - VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY, - NULL); - VIR_EXCLUSIVE_FLAGS_RET(VIR_DOMAIN_SNAPSHOT_CREATE_LIVE, - VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, - NULL); - if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot halt after transient domain snapshot")); @@ -2950,11 +2932,6 @@ qemuSnapshotRevert(virDomainObj *vm, g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); unsigned int start_flags = VIR_QEMU_PROCESS_START_GEN_VMID; - virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | - VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED | - VIR_DOMAIN_SNAPSHOT_REVERT_FORCE | - VIR_DOMAIN_SNAPSHOT_REVERT_RESET_NVRAM, -1); - if (flags & VIR_DOMAIN_SNAPSHOT_REVERT_RESET_NVRAM) start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM; @@ -4489,10 +4466,6 @@ qemuSnapshotDelete(virDomainObj *vm, bool stop_qemu = false; g_autoslist(qemuSnapshotDeleteExternalData) externalData = NULL; - virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | - VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY | - VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY, -1); - if (virDomainObjBeginAsyncJob(vm, VIR_ASYNC_JOB_SNAPSHOT, VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_DELETE, flags) < 0) { -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 781efd612c..8e30857b34 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7742,9 +7742,6 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriver *driver, g_autoptr(virDomainDef) vmdef = NULL; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; - virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); - cfg = virQEMUDriverGetConfig(driver); if ((flags & VIR_DOMAIN_AFFECT_CONFIG) && @@ -7840,6 +7837,9 @@ qemuDomainDetachDeviceAlias(virDomainPtr dom, virDomainObj *vm = NULL; int ret = -1; + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> The implementation uses 'qemuDomainQemuMonitorCommandWithFiles' internally. To have a top level flag validation for the upcoming flag introspection export the supported flags as a macro and add a 'virCheckFlags'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 34d8f0257d..1eef19d3ff 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13720,6 +13720,9 @@ qemuDomainBackupGetXMLDesc(virDomainPtr domain, } +#define QEMU_DOMAIN_MONITOR_COMMAND_FLAGS \ + VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP + static int qemuDomainQemuMonitorCommandWithFiles(virDomainPtr domain, const char *cmd, @@ -13737,7 +13740,7 @@ qemuDomainQemuMonitorCommandWithFiles(virDomainPtr domain, bool hmp; int fd = -1; - virCheckFlags(VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP, -1); + virCheckFlags(QEMU_DOMAIN_MONITOR_COMMAND_FLAGS, -1); /* currently we don't pass back any fds */ if (outfds) @@ -13791,6 +13794,8 @@ qemuDomainQemuMonitorCommand(virDomainPtr domain, char **result, unsigned int flags) { + virCheckFlags(QEMU_DOMAIN_MONITOR_COMMAND_FLAGS, -1); + return qemuDomainQemuMonitorCommandWithFiles(domain, cmd, 0, NULL, NULL, NULL, result, flags); } -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> The implementation uses 'virDomainObjGetMetadata' internally. To have a top level flag validation for the upcoming flag introspection export the supported flags as a macro and add a 'virCheckFlags'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 3 +-- src/conf/domain_conf.h | 4 ++++ src/qemu/qemu_driver.c | 2 ++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3497e84bf5..d73bac5cc5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31557,8 +31557,7 @@ virDomainObjGetMetadata(virDomainObj *vm, virDomainDef *def; char *ret = NULL; - virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, NULL); + virCheckFlags(VIR_DOMAIN_OBJ_GET_METADATA_FLAGS, NULL); if (type >= VIR_DOMAIN_METADATA_LAST) { virReportError(VIR_ERR_INVALID_ARG, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a8f90803da..bdfb99ed1d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -4543,6 +4543,10 @@ const char *virDomainChrSourceDefGetPath(virDomainChrSourceDef *chr); void virDomainChrSourceDefClear(virDomainChrSourceDef *def); +#define VIR_DOMAIN_OBJ_GET_METADATA_FLAGS \ + VIR_DOMAIN_AFFECT_LIVE | \ + VIR_DOMAIN_AFFECT_CONFIG + char *virDomainObjGetMetadata(virDomainObj *vm, int type, const char *uri, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 758dbbe19f..c8975fbbf6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15998,6 +15998,8 @@ qemuDomainGetMetadata(virDomainPtr dom, virDomainObj *vm; char *ret = NULL; + virCheckFlags(VIR_DOMAIN_OBJ_GET_METADATA_FLAGS, NULL); + if (!(vm = qemuDomainObjFromDomain(dom))) return NULL; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 19 ++++++++++++++++++ tools/virsh-host.c | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 80b0ea14a8..01e2ac9d32 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -1097,6 +1097,25 @@ on the host are reported. The option *--all* will report every CPU model known to the hypervisor, including ones that are not supported on the hypervisor (e.g. newer generation models). +introspection +------------- + +**Syntax:** + +:: + + introspection [--xpath EXPRESSION] [--wrap] + +Output the libvirt connection feature introspection XML. + +If the **--xpath** argument provides an XPath expression, it will be +evaluated against the output XML and only those matching nodes will +be printed. The default behaviour is to print each matching node as +a standalone document, however, for ease of additional processing, +the **--wrap** argument will cause the matching node to be wrapped +in a common root node. + + DOMAIN COMMANDS =============== diff --git a/tools/virsh-host.c b/tools/virsh-host.c index ef91e22fed..e1980bc276 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -1864,6 +1864,43 @@ cmdHypervisorCPUModelNames(vshControl *ctl, } +static const vshCmdInfo info_introspection = { + .help = N_("get XML containing connection introspection data"), + .desc = N_("get XML containing connection introspection data"), +}; + +static const vshCmdOptDef opts_introspection[] = { + {.name = "xpath", + .type = VSH_OT_STRING, + .completer = vshCompleteEmpty, + .help = N_("xpath expression to filter the XML document") + }, + {.name = "wrap", + .type = VSH_OT_BOOL, + .help = N_("wrap xpath results in an common root element"), + }, + {.name = NULL} +}; + +static bool +cmdIntrospection(vshControl *ctl, + const vshCmd *cmd) +{ + g_autofree char *xml = NULL; + virshControl *priv = ctl->privData; + bool wrap = vshCommandOptBool(cmd, "wrap"); + const char *xpath = NULL; + + if (vshCommandOptStringQuiet(ctl, cmd, "xpath", &xpath) < 0) + return false; + + if (!(xml = virConnectGetIntrospection(priv->conn, 0))) + return false; + + return virshDumpXML(ctl, xml, "domain", xpath, wrap); +} + + const vshCmdDef hostAndHypervisorCmds[] = { {.name = "allocpages", .handler = cmdAllocpages, @@ -2003,5 +2040,11 @@ const vshCmdDef hostAndHypervisorCmds[] = { .info = &info_version, .flags = 0 }, + {.name = "introspection", + .handler = cmdIntrospection, + .opts = opts_introspection, + .info = &info_introspection, + .flags = 0 + }, {.name = NULL} }; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> While 'virTypedParamsValidate', which uses varargs to pass the template to validate parameters agains, is convenient for single uses we have multiple occasions where we want to validate the same list of parameters in muliple places. We use either a macro which expands to the parameter list in place or a function which encapsulates the validation. For introspection of input typed parameters we'll need to have the list of supported typed parameters in each function which uses them as input and either of the approaches is inconvenient for generating the introspection parts. Refactor 'virTypedParamsValidate', to split the actual validation internals into ''virTypedParamsValidateInternal' and create two wrappers: - 'virTypedParamsValidate' which uses varargs - 'virTypedParamsValidateTemplate' which uses an array of structs containing the template. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virtypedparam.c | 134 +++++++++++++++++++++++++-------------- src/util/virtypedparam.h | 11 ++++ 3 files changed, 100 insertions(+), 46 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 706079b87f..20507af7f7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3684,6 +3684,7 @@ virTypedParamsRemoteFree; virTypedParamsReplaceString; virTypedParamsSerialize; virTypedParamsValidate; +virTypedParamsValidateTemplate; # util/viruri.h diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index ec8046b998..92f25cea39 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -94,75 +94,55 @@ virTypedParamsSortName(const void *left, return strcmp(param_left->field, param_right->field); } +static int +virTypedParamsSortTemplate(const void *left, + const void *right, + void *opaque G_GNUC_UNUSED) +{ + const virTypedParamValidationTemplate *param_left = left; + const virTypedParamValidationTemplate *param_right = right; + return strcmp(param_left->name, param_right->name); +} + /* Validate that PARAMS contains only recognized parameter names with * correct types, and with no duplicates except for parameters * specified with VIR_TYPED_PARAM_MULTIPLE flag in type. * 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 -virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) +static int +virTypedParamsValidateInternal(virTypedParameterPtr params, + size_t nparams, + virTypedParamValidationTemplate *templates, + size_t ntemplates) { - va_list ap; size_t i; size_t j; - const char *name; const char *last_name = NULL; - size_t nkeys = 0; - size_t nkeysalloc = 0; - g_autofree virTypedParameterPtr sorted = NULL; - g_autofree virTypedParameterPtr keys = NULL; + g_autofree virTypedParameterPtr sorted = g_new0(virTypedParameter, nparams); - if (!nparams) { - return 0; - } - - va_start(ap, nparams); - - sorted = g_new0(virTypedParameter, nparams); - - /* Here we intentionally don't copy values */ memcpy(sorted, params, sizeof(*params) * nparams); g_qsort_with_data(sorted, nparams, sizeof(*sorted), virTypedParamsSortName, NULL); - name = va_arg(ap, const char *); - while (name) { - int type = va_arg(ap, int); - VIR_RESIZE_N(keys, nkeysalloc, nkeys, 1); - - if (virStrcpyStatic(keys[nkeys].field, name) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Field name '%1$s' too long"), name); - va_end(ap); - return -1; - } - - keys[nkeys].type = type & ~VIR_TYPED_PARAM_MULTIPLE; - /* Value is not used anyway */ - keys[nkeys].value.i = type & VIR_TYPED_PARAM_MULTIPLE; - - nkeys++; - name = va_arg(ap, const char *); - } - - va_end(ap); - - g_qsort_with_data(keys, nkeys, sizeof(*keys), virTypedParamsSortName, NULL); + g_qsort_with_data(templates, ntemplates, + sizeof(*templates), virTypedParamsSortTemplate, NULL); - for (i = 0, j = 0; i < nparams && j < nkeys;) { - if (STRNEQ(sorted[i].field, keys[j].field)) { + for (i = 0, j = 0; i < nparams && j < ntemplates;) { + if (STRNEQ(sorted[i].field, templates[j].name)) { j++; } else { - if (STREQ_NULLABLE(last_name, sorted[i].field) && - !(keys[j].value.i & VIR_TYPED_PARAM_MULTIPLE)) { + unsigned int expected_type = templates[j].typeflags & ~VIR_TYPED_PARAM_MULTIPLE; + bool multiple = templates[j].typeflags & VIR_TYPED_PARAM_MULTIPLE; + + if (STREQ_NULLABLE(last_name, sorted[i].field) && !multiple) { virReportError(VIR_ERR_INVALID_ARG, _("parameter '%1$s' occurs multiple times"), sorted[i].field); return -1; } - if (virTypedParamValidateType(sorted + i, keys[j].type) < 0) + if (virTypedParamValidateType(sorted + i, expected_type) < 0) return -1; last_name = sorted[i].field; @@ -170,7 +150,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) } } - if (j == nkeys && i != nparams) { + if (j == ntemplates && i != nparams) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, _("parameter '%1$s' not supported"), sorted[i].field); @@ -181,6 +161,68 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) } +/* Validate that PARAMS contains only recognized parameter names with + * correct types, and with no duplicates except for parameters + * specified with VIR_TYPED_PARAM_MULTIPLE flag in type. + * 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 +virTypedParamsValidate(virTypedParameterPtr params, + int nparams, + ...) +{ + va_list ap; + const char *name; + g_autofree virTypedParamValidationTemplate *templates = NULL; + size_t ntemplates = 0; + size_t ntemplatesalloc = 0; + + if (nparams == 0) + return 0; + + va_start(ap, nparams); + + for (name = va_arg(ap, const char *); name; name = va_arg(ap, const char *)) { + VIR_RESIZE_N(templates, ntemplatesalloc, ntemplates, 1); + + if (virStrcpy((char *)templates[ntemplates].name, name, VIR_TYPED_PARAM_FIELD_LENGTH) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%1$s' too long"), name); + va_end(ap); + return -1; + } + + templates[ntemplates].typeflags = va_arg(ap, unsigned int); + ntemplates++; + } + + va_end(ap); + + return virTypedParamsValidateInternal(params, nparams, templates, ntemplates); +} + + +int +virTypedParamsValidateTemplate(virTypedParameterPtr params, + int nparams, + const virTypedParamValidationTemplate *templates) +{ + size_t ntemplates = 0; + g_autofree virTypedParamValidationTemplate *templ_copy = NULL; + + /* we need to copy the list of templates because + * 'virTypedParamsValidateInternal' will need to sort it */ + while (*templates[ntemplates++].name == '\0') + ; + + templ_copy = g_new0(virTypedParamValidationTemplate, ntemplates); + memcpy(templ_copy, templates, sizeof(*templates) * ntemplates); + + return virTypedParamsValidateInternal(params, nparams, templ_copy, ntemplates); +} + + /* Check if params contains only specified parameter names. Return true if * only specified names are present in params, false if params contains any * unspecified parameter name. */ diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 819166ff1b..bee3ecc14d 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -68,6 +68,17 @@ virTypedParamValidateType(virTypedParameterPtr param, unsigned int expected_type) G_GNUC_WARN_UNUSED_RESULT; +struct _virTypedParamValidationTemplate { + const char name[VIR_TYPED_PARAM_FIELD_LENGTH]; /* parameter name */ + unsigned int typeflags; +}; +typedef struct _virTypedParamValidationTemplate virTypedParamValidationTemplate; + +int +virTypedParamsValidateTemplate(virTypedParameterPtr params, + int nparams, + const virTypedParamValidationTemplate *templates) + G_GNUC_WARN_UNUSED_RESULT; int virTypedParamsValidate(virTypedParameterPtr params, int nparams, -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Refactor the code to use the new helper for validating migration params and making them later available for introspection. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++------ src/qemu/qemu_migration.c | 29 +++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 32 +------------------------------- 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 38f87a4bff..f01146655e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11143,7 +11143,7 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, virDomainObj *vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); - if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) + if (virTypedParamsValidateTemplate(params, nparams, qemuMigrationParametersValidation) < 0) return NULL; if (virTypedParamsGetString(params, nparams, @@ -11252,7 +11252,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, const char *nbdURI = NULL; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); - if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) + if (virTypedParamsValidateTemplate(params, nparams, qemuMigrationParametersValidation) < 0) return -1; if (virTypedParamsGetString(params, nparams, @@ -11390,7 +11390,7 @@ qemuDomainMigratePrepareTunnel3Params(virConnectPtr dconn, g_autoptr(qemuMigrationParams) migParams = NULL; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); - if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) + if (virTypedParamsValidateTemplate(params, nparams, qemuMigrationParametersValidation) < 0) return -1; if (virTypedParamsGetString(params, nparams, @@ -11494,7 +11494,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom, int ret = -1; virCheckFlags(QEMU_MIGRATION_FLAGS, -1); - if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) + if (virTypedParamsValidateTemplate(params, nparams, qemuMigrationParametersValidation) < 0) return ret; if (virTypedParamsGetString(params, nparams, @@ -11640,7 +11640,7 @@ qemuDomainMigrateFinish3Params(virConnectPtr dconn, const char *dname = NULL; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); - if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) + if (virTypedParamsValidateTemplate(params, nparams, qemuMigrationParametersValidation) < 0) return NULL; if (virTypedParamsGetString(params, nparams, @@ -11709,7 +11709,7 @@ qemuDomainMigrateConfirm3Params(virDomainPtr domain, virCheckFlags(QEMU_MIGRATION_FLAGS, -1); - if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) + if (virTypedParamsValidateTemplate(params, nparams, qemuMigrationParametersValidation) < 0) return -1; if (!(vm = qemuDomainObjFromDomain(domain))) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ffffeea75c..bc06a100e4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -84,6 +84,35 @@ VIR_ENUM_IMPL(qemuMigrationJobPhase, "finish_resume", ); +const virTypedParamValidationTemplate qemuMigrationParametersValidation[] = { + { VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_DEST_XML, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_BANDWIDTH, VIR_TYPED_PARAM_ULLONG }, + { VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_LISTEN_ADDRESS, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_MIGRATE_DISKS, VIR_TYPED_PARAM_STRING | VIR_TYPED_PARAM_MULTIPLE }, + { VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES, VIR_TYPED_PARAM_STRING | VIR_TYPED_PARAM_MULTIPLE }, + { VIR_MIGRATE_PARAM_MIGRATE_DISKS_TARGET_ZERO, VIR_TYPED_PARAM_STRING | VIR_TYPED_PARAM_MULTIPLE }, + { VIR_MIGRATE_PARAM_DISKS_PORT, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_COMPRESSION, VIR_TYPED_PARAM_STRING | VIR_TYPED_PARAM_MULTIPLE }, + { VIR_MIGRATE_PARAM_COMPRESSION_MT_LEVEL, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_COMPRESSION_MT_THREADS, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_COMPRESSION_MT_DTHREADS, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE, VIR_TYPED_PARAM_ULLONG }, + { VIR_MIGRATE_PARAM_PERSIST_XML, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, VIR_TYPED_PARAM_ULLONG }, + { VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_COMPRESSION_ZLIB_LEVEL, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_COMPRESSION_ZSTD_LEVEL, VIR_TYPED_PARAM_INT }, + { VIR_MIGRATE_PARAM_TLS_DESTINATION, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_DISKS_URI, VIR_TYPED_PARAM_STRING }, + { VIR_MIGRATE_PARAM_BANDWIDTH_AVAIL_SWITCHOVER, VIR_TYPED_PARAM_ULLONG }, + { "", 0 } +}; + static bool ATTRIBUTE_NONNULL(1) qemuMigrationJobIsAllowed(virDomainObj *vm) diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 59f32d2ebf..7e9410e1f7 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -65,38 +65,8 @@ 0) /* All supported migration parameters and their types. */ -#define QEMU_MIGRATION_PARAMETERS \ - VIR_MIGRATE_PARAM_URI, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_DEST_NAME, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_DEST_XML, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_BANDWIDTH, VIR_TYPED_PARAM_ULLONG, \ - VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_LISTEN_ADDRESS, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_MIGRATE_DISKS, VIR_TYPED_PARAM_STRING | \ - VIR_TYPED_PARAM_MULTIPLE, \ - VIR_MIGRATE_PARAM_MIGRATE_DISKS_DETECT_ZEROES, VIR_TYPED_PARAM_STRING | \ - VIR_TYPED_PARAM_MULTIPLE, \ - VIR_MIGRATE_PARAM_MIGRATE_DISKS_TARGET_ZERO, VIR_TYPED_PARAM_STRING | \ - VIR_TYPED_PARAM_MULTIPLE, \ - VIR_MIGRATE_PARAM_DISKS_PORT, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_COMPRESSION, VIR_TYPED_PARAM_STRING | \ - VIR_TYPED_PARAM_MULTIPLE, \ - VIR_MIGRATE_PARAM_COMPRESSION_MT_LEVEL, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_COMPRESSION_MT_THREADS, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_COMPRESSION_MT_DTHREADS, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE, VIR_TYPED_PARAM_ULLONG, \ - VIR_MIGRATE_PARAM_PERSIST_XML, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, VIR_TYPED_PARAM_ULLONG, \ - VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_COMPRESSION_ZLIB_LEVEL, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_COMPRESSION_ZSTD_LEVEL, VIR_TYPED_PARAM_INT, \ - VIR_MIGRATE_PARAM_TLS_DESTINATION, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_DISKS_URI, VIR_TYPED_PARAM_STRING, \ - VIR_MIGRATE_PARAM_BANDWIDTH_AVAIL_SWITCHOVER, VIR_TYPED_PARAM_ULLONG, \ - NULL +extern const virTypedParamValidationTemplate qemuMigrationParametersValidation[]; typedef enum { QEMU_MIGRATION_PHASE_NONE = 0, -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Refactor the validation using 'virTypedParamsValidateTemplate' and export the template so that 'qemuNodeSetMemoryParameters' can expose them via introspection. In addition since 'virHostMemSetParameters' is conditionally compiled, platforms which don't support it will not expose given params. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 4 ++++ src/util/virhostmem.c | 24 ++++++++++++++++-------- src/util/virhostmem.h | 2 ++ 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 20507af7f7..94351fe153 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2605,6 +2605,7 @@ virHostMemGetParameters; virHostMemGetStats; virHostMemGetTHPSize; virHostMemSetParameters; +virHostMemSetParametersValidation; # util/virhostuptime.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 82f5febc6b..d64366924c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16502,6 +16502,10 @@ qemuNodeSetMemoryParameters(virConnectPtr conn, { virCheckFlags(0, -1); + if (virTypedParamsValidateTemplate(params, nparams, + virHostMemSetParametersValidation) < 0) + return -1; + if (virNodeSetMemoryParametersEnsureACL(conn) < 0) return -1; diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c index 7d7deac34b..beff114362 100644 --- a/src/util/virhostmem.c +++ b/src/util/virhostmem.c @@ -351,21 +351,24 @@ virHostMemParametersAreAllSupported(virTypedParameterPtr params, } #endif + #ifdef __linux__ +const virTypedParamValidationTemplate virHostMemSetParametersValidation[] = +{ + { VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN, VIR_TYPED_PARAM_UINT }, + { VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS, VIR_TYPED_PARAM_UINT }, + { VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES, VIR_TYPED_PARAM_UINT }, + { "", 0 } +}; + int virHostMemSetParameters(virTypedParameterPtr params, int nparams) { size_t i; - if (virTypedParamsValidate(params, nparams, - VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN, - VIR_TYPED_PARAM_UINT, - VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS, - VIR_TYPED_PARAM_UINT, - VIR_NODE_MEMORY_SHARED_MERGE_ACROSS_NODES, - VIR_TYPED_PARAM_UINT, - NULL) < 0) + if (virTypedParamsValidateTemplate(params, nparams, + virHostMemSetParametersValidation) < 0) return -1; if (!virHostMemParametersAreAllSupported(params, nparams)) @@ -379,6 +382,11 @@ virHostMemSetParameters(virTypedParameterPtr params, return 0; } #else +const virTypedParamValidationTemplate virHostMemSetParametersValidation[] = +{ + { "", 0 } +}; + int virHostMemSetParameters(virTypedParameterPtr params G_GNUC_UNUSED, int nparams G_GNUC_UNUSED) diff --git a/src/util/virhostmem.h b/src/util/virhostmem.h index 4a0d692402..ebe7d26f21 100644 --- a/src/util/virhostmem.h +++ b/src/util/virhostmem.h @@ -22,6 +22,7 @@ #pragma once #include "internal.h" +#include "virtypedparam.h" int virHostMemGetStats(int cellNum, virNodeMemoryStatsPtr params, @@ -35,6 +36,7 @@ int virHostMemGetInfo(unsigned long long *mem, int virHostMemGetParameters(virTypedParameterPtr params, int *nparams); +extern const virTypedParamValidationTemplate virHostMemSetParametersValidation[]; int virHostMemSetParameters(virTypedParameterPtr params, int nparams); -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> This will allow introspecting them. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f0ef6796d3..718994f5b5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5318,20 +5318,6 @@ qemuDomainIOThreadParseParams(virTypedParameterPtr params, { int rc; - if (virTypedParamsValidate(params, nparams, - VIR_DOMAIN_IOTHREAD_POLL_MAX_NS, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_IOTHREAD_POLL_GROW, - VIR_TYPED_PARAM_UNSIGNED, - VIR_DOMAIN_IOTHREAD_POLL_SHRINK, - VIR_TYPED_PARAM_UNSIGNED, - VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN, - VIR_TYPED_PARAM_INT, - VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX, - VIR_TYPED_PARAM_INT, - NULL) < 0) - return -1; - if ((rc = virTypedParamsGetULLong(params, nparams, VIR_DOMAIN_IOTHREAD_POLL_MAX_NS, &iothread->poll_max_ns)) < 0) @@ -5674,6 +5660,20 @@ qemuDomainSetIOThreadParams(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); + if (virTypedParamsValidate(params, nparams, + VIR_DOMAIN_IOTHREAD_POLL_MAX_NS, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_IOTHREAD_POLL_GROW, + VIR_TYPED_PARAM_UNSIGNED, + VIR_DOMAIN_IOTHREAD_POLL_SHRINK, + VIR_TYPED_PARAM_UNSIGNED, + VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN, + VIR_TYPED_PARAM_INT, + VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX, + VIR_TYPED_PARAM_INT, + NULL) < 0) + return -1; + if (iothread_id == 0) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid value of 0 for iothread_id")); -- 2.54.0