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> 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> 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> Make the block io tuning params introspectable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 81 +++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 53 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f01146655e..82f5febc6b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15345,57 +15345,29 @@ qemuDomainCheckBlockIoTuneReset(virDomainDiskDef *disk, } -static int -qemuDomainValidateBlockIoTune(virTypedParameterPtr params, - int nparams) -{ - if (virTypedParamsValidate(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, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, - VIR_TYPED_PARAM_STRING, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH, - VIR_TYPED_PARAM_ULLONG, - NULL) < 0) - return -1; - - return 0; -} - +const virTypedParamValidationTemplate qemuDomainBlockIoTuneParametersValidation[] = { + { 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 }, + { VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_GROUP_NAME, VIR_TYPED_PARAM_STRING }, + { VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH, VIR_TYPED_PARAM_ULLONG }, + { "", 0 } +}; static int qemuDomainSetBlockIoTuneFields(virDomainBlockIoTuneInfo *info, @@ -15597,7 +15569,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (qemuDomainValidateBlockIoTune(params, nparams) < 0) + + if (virTypedParamsValidateTemplate(params, nparams, + qemuDomainBlockIoTuneParametersValidation) < 0) return -1; if (!(vm = qemuDomainObjFromDomain(dom))) @@ -20677,7 +20651,8 @@ qemuDomainSetThrottleGroup(virDomainPtr dom, return -1; } - if (qemuDomainValidateBlockIoTune(params, nparams) < 0) + if (virTypedParamsValidateTemplate(params, nparams, + qemuDomainBlockIoTuneParametersValidation) < 0) return -1; if (!(vm = qemuDomainObjFromDomain(dom))) -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> For making introspection possible both qemuDomainSetSchedulerParameters and qemuDomainSetSchedulerParametersFlags need to have the check present. Refactor the flag validation to make it present in both APIs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d64366924c..f0ef6796d3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8991,6 +8991,20 @@ qemuSetIOThreadsBWLive(virDomainObj *vm, virCgroup *cgroup, goto endjob; \ } + +const virTypedParamValidationTemplate qemuDomainSetSchedulerParametersValidation[] = { + { 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 }, + { VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA, VIR_TYPED_PARAM_LLONG }, + { VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, VIR_TYPED_PARAM_LLONG }, + { VIR_DOMAIN_SCHEDULER_IOTHREAD_PERIOD, VIR_TYPED_PARAM_ULLONG }, + { VIR_DOMAIN_SCHEDULER_IOTHREAD_QUOTA, VIR_TYPED_PARAM_LLONG }, + { "", 0 } +}; + static int qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, @@ -9016,26 +9030,9 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if (virTypedParamsValidate(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, - VIR_DOMAIN_SCHEDULER_GLOBAL_PERIOD, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_SCHEDULER_GLOBAL_QUOTA, - VIR_TYPED_PARAM_LLONG, - VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, - VIR_TYPED_PARAM_LLONG, - VIR_DOMAIN_SCHEDULER_IOTHREAD_PERIOD, - VIR_TYPED_PARAM_ULLONG, - VIR_DOMAIN_SCHEDULER_IOTHREAD_QUOTA, - VIR_TYPED_PARAM_LLONG, - NULL) < 0) + + if (virTypedParamsValidateTemplate(params, nparams, + qemuDomainSetSchedulerParametersValidation) < 0) return -1; if (!(vm = qemuDomainObjFromDomain(dom))) @@ -9297,6 +9294,10 @@ qemuDomainSetSchedulerParameters(virDomainPtr dom, virTypedParameterPtr params, int nparams) { + if (virTypedParamsValidateTemplate(params, nparams, + qemuDomainSetSchedulerParametersValidation) < 0) + return -1; + return qemuDomainSetSchedulerParametersFlags(dom, params, nparams, -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Extract information about typed parameters from calls to 'virTypedParamsValidate'/'virTypedParamsValidateTemplate' and expose them in the introspection XML: <api name='virDomainMigrate3'> <flags dec='2097151' hex='0x1fffff'/> <typed-parameters type='input' name='params'> <param name='migrate_uri' type='string'/> <param name='destination_name' type='string'/> <param name='destination_xml' type='string'/> <param name='bandwidth' type='ullong'/> <param name='graphics_uri' type='string'/> <param name='listen_address' type='string'/> <param name='migrate_disks' type='string' multiple='yes'/> <param name='migrate_disks_detect_zeroes' type='string' multiple='yes'/> <param name='migrate_disks_target_zero' type='string' multiple='yes'/> <param name='disks_port' type='int'/> <param name='compression' type='string' multiple='yes'/> <param name='compression.mt.level' type='int'/> <param name='compression.mt.threads' type='int'/> <param name='compression.mt.dthreads' type='int'/> <param name='compression.xbzrle.cache' type='ullong'/> <param name='persistent_xml' type='string'/> <param name='auto_converge.initial' type='int'/> <param name='auto_converge.increment' type='int'/> <param name='bandwidth.postcopy' type='ullong'/> <param name='parallel.connections' type='int'/> <param name='compression.zlib.level' type='int'/> <param name='compression.zstd.level' type='int'/> <param name='tls.destination' type='string'/> <param name='disks_uri' type='string'/> <param name='bandwidth.avail.switchover' type='ullong'/> </typed-parameters> </api> Migration APIs once again required special handling as some typed params are supported even if the backing APIs using typed parameters arend supported because they can be converted to legacy parameters for the lesser APIs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- scripts/genintrospection.py | 127 ++++++++++++++++++++++++++++++++++++ src/util/virintrospection.c | 45 +++++++++++++ src/util/virintrospection.h | 3 + 3 files changed, 175 insertions(+) diff --git a/scripts/genintrospection.py b/scripts/genintrospection.py index 25a17e2b0c..ac51634d10 100644 --- a/scripts/genintrospection.py +++ b/scripts/genintrospection.py @@ -7,6 +7,21 @@ import re import sys +# APIs which fill a user-supplied virTypedParameter pointer -- use it as output +input_params_exceptions = [ + "virDomainGetBlkioParameters", + "virDomainGetMemoryParameters", + "virDomainGetNumaParameters", + "virDomainGetSchedulerParametersFlags", + "virDomainGetSchedulerParameters", + "virDomainBlockStatsFlags", + "virDomainGetInterfaceParameters", + "virDomainGetBlockIoTune", + "virDomainGetCPUStats", + "virNodeGetMemoryParameters", +] + + # driver callbacks needed to infer the introspection for public migration APIs # which do not map directly to any driver API callback. We need a representative # sample of APIs supporting typed parameters and flags @@ -27,6 +42,15 @@ migration_public_api = { "virDomainMigrateToURI3": {"params": True}, } +# list of parameters supported by 'virDomainMigrate3'/'virDomainMigrateToURI3' +# if any migration protocol version is supported by extracting the parameters +migration_public_api_fallback_params = [ + ("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"), +] + def load_public_symbols(filename): """load the public symbol file and return all APIs""" @@ -121,8 +145,13 @@ def parse_api(filename, syms): "callback": f.group("name"), "flags_arg": False, "flags_supported": None, + "input_params": False, + "input_params_supported": None, + "input_params_template": None, } + # find APIs having 'flags' argument and find the corresponding + # virCheckFlags flagarg = re.search(r"\bflags\b", f.group("args")) if flagarg: data["flags_arg"] = True @@ -136,6 +165,54 @@ def parse_api(filename, syms): if flagcheck: data["flags_supported"] = re.sub(r"\s+", " ", flagcheck.group("flags")) + # find APIs supporting typed parameters as input and find the + # corresponding supported flags by matching 'virTypedParamsValidate' + # We're looking for APIs which take typed parameters as input, which + # excludes any API taking a double-pointer. From the rest + # the 'input_params_exceptions' array has APIs which output typed + # parameters into a pre-allocated array and thus are excluded + paramarg = re.search( + r"virTypedParameter(?P<ptrtype>Ptr)?(?P<ptrstar>[ *]+)", f.group("args") + ) + param_nptrs = 0 + + if paramarg: + if paramarg.group("ptrtype"): + param_nptrs += 1 + + for c in paramarg.group("ptrstar"): + if c == "*": + param_nptrs += 1 + + if param_nptrs == 1 and apiname not in input_params_exceptions: + data["input_params"] = True + + paramscheck = re.search( + r"virTypedParamsValidate(?P<template>Template)?(?:Deferred)?\([^,]+,\s*[^,]+,\s*(?P<params>[^)]+)\)", + f.group("impl"), + flags=re.DOTALL | re.MULTILINE, + ) + + if paramscheck: + if paramscheck.group("template"): + data["input_params_template"] = paramscheck.group("params") + + else: + data["input_params_supported"] = [] + + idx = 0 + params = re.sub(r"\s+", "", paramscheck.group("params")).split(",") + + while idx < len(params): + if params[idx] == "NULL": + break + + data["input_params_supported"].append( + (params[idx], params[idx + 1]) + ) + + idx += 2 + if migrationapi: migr_data[apiname] = data else: @@ -148,6 +225,18 @@ def parse_api(filename, syms): if m in migr_data: for apiname, pub in migration_public_api.items(): data = migr_data[m].copy() + + if pub.get("params", False): + if not data["input_params"]: + data["input_params"] = True + data["input_params_supported"] = ( + migration_public_api_fallback_params + ) + else: + data["input_params"] = False + data["input_params_supported"] = None + data["input_params_template"] = None + flagmap[apiname] = data break @@ -184,6 +273,27 @@ with open(args.output, "w") as outfile: """ ) + for api in sorted(introspection.keys()): + data = introspection[api] + + if data.get("input_params_supported", None) is None: + continue + + outfile.write( + f""" +const virTypedParamValidationTemplate {data["callback"]}InputParamValidation[] = {{ +""" + ) + + for param, flag in data["input_params_supported"]: + outfile.write(f" {{ {param}, {flag} }},\n") + + outfile.write( + """ { "", 0 } +}; +""" + ) + outfile.write( """ static const virIntrospectionData driver_api_introspection[] = @@ -201,6 +311,15 @@ static const virIntrospectionData driver_api_introspection[] = else: outfile.write(" .flags_arg = false,\n") + if data.get("input_params_supported", None) is not None: + outfile.write( + f" .input_params = {data["callback"]}InputParamValidation,\n" + ) + elif data.get("input_params_template", None) is not None: + outfile.write(f" .input_params = {data["input_params_template"]},\n") + else: + outfile.write(" .input_params = NULL,\n") + outfile.write(" },\n") epilogue = """ { .api = NULL } @@ -216,5 +335,13 @@ for api, data in introspection.items(): print(f"failed to parse flags for '{api}' in '{data.get("callback", "")}'") fail = True + if data.get("input_params", False) is True and ( + data.get("input_params_supported", None) is None + and data.get("input_params_template", None) is None + ): + print( + f"failed to parse typed params for '{api}' in '{data.get("callback", "")}'" + ) + fail = True if fail: sys.exit(1) diff --git a/src/util/virintrospection.c b/src/util/virintrospection.c index b9fb5beda8..2862e60924 100644 --- a/src/util/virintrospection.c +++ b/src/util/virintrospection.c @@ -8,6 +8,24 @@ #include "virxml.h" #include "virbuffer.h" +/* These strings are exported int he XML */ +VIR_ENUM_DECL(virIntrospectionTypedParam); + +VIR_ENUM_IMPL(virIntrospectionTypedParam, + VIR_TYPED_PARAM_UNSIGNED + 1, + "", + "int", + "uint", + "llong", + "ullong", + "double", + "boolean", + "string", + "", /* VIR_TYPED_PARAM_LAST */ + "ullong", /* VIR_TYPED_PARAM_UNSIGNED */ +); + + char * virIntrospectionGetXML(const virIntrospectionData *d) { @@ -27,6 +45,33 @@ virIntrospectionGetXML(const virIntrospectionData *d) d[i].flags, d[i].flags); } + if (d[i].input_params) { + g_auto(virBuffer) typedparam_attr = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) typedparam_elem = VIR_BUFFER_INIT_CHILD(&api_elem); + size_t j; + + virBufferAddLit(&typedparam_attr, " type='input' name='params'"); + + for (j = 0; d[i].input_params[j].name[0] != '\0'; j++) { + unsigned int typeval = d[i].input_params[j].typeflags & ~VIR_TYPED_PARAM_MULTIPLE; + const char *type = virIntrospectionTypedParamTypeToString(typeval); + + virBufferAsprintf(&typedparam_elem, "<param name='%s'", + d[i].input_params[j].name); + + if (type && type[0] != '\0') + virBufferAsprintf(&typedparam_elem, " type='%s'", type); + + if (d[i].input_params[j].typeflags & VIR_TYPED_PARAM_MULTIPLE) + virBufferAddLit(&typedparam_elem, " multiple='yes'"); + + virBufferAddLit(&typedparam_elem, "/>\n"); + } + + virXMLFormatElement(&api_elem, "typed-parameters", + &typedparam_attr, &typedparam_elem); + } + virXMLFormatElement(&apis, "api", &api_attr, &api_elem); } diff --git a/src/util/virintrospection.h b/src/util/virintrospection.h index f996ce0f07..8d259e36ce 100644 --- a/src/util/virintrospection.h +++ b/src/util/virintrospection.h @@ -6,10 +6,13 @@ #include <stdbool.h> +#include "virtypedparam.h" + struct _virIntrospectionData { const char *api; bool flags_arg; unsigned int flags; + const virTypedParamValidationTemplate *input_params; }; typedef struct _virIntrospectionData virIntrospectionData; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Use the new VIR_DOMAIN_BLOCK_RESIZE_PROBE_FLAGS flag to probe for VIR_DOMAIN_BLOCK_RESIZE_EXTEND and use it if available. This will give users the same protection as with --extend in the default case. IMPORTANT: This patch changes behaviour for users who want to shrink their VM's block device knowingly. Such users, when using a newer daemon will be required to pass --allow-shrink now. I've contemplated either renaming the whole command to preserve functionality for the existing one and deprecating 'virsh blockresize' or adding just the '--extend', but unfortunately neither of those in the end looked appealing to me. As shrinking filesystems is much more invloved and much less common the benefit of preventing accidental data loss should outweigh the breakage from the change. To aleviate issues with scripts virsh now also provides the '--probe-arguments' option to allow safe probe if virsh supports this new argument. Closes: https://gitlab.com/libvirt/libvirt/-/work_items/789 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 11 +++++++- tools/virsh-domain.c | 59 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 01e2ac9d32..e3cf647b51 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -1661,7 +1661,7 @@ blockresize :: - blockresize domain path ([size] | [--capacity]) [--extend] + blockresize domain path ([size] | [--capacity]) [--extend] [--allow-shrink] Resize a block device of domain while the domain is running, *path* specifies the absolute path of the block device; it corresponds @@ -1674,6 +1674,15 @@ smaller than the old size, thus prevents (accidental) shrinking of the block device which may lead to data loss. Users are encouraged to always use this flag unless communicating with an older hypervisor. +As of libvirt 12.4.0 the 'virsh blockresize' command now probes whether the +hypervisor supports enforcing that the device is not shrunk accidentally and +if it is supported this feature will be used as a precaution from accidental +data loss even at the cost of change of behaviour. With older hypervisors +which do not support the feature the protection will not be avaliable. +Users wanting to shrink the block device must use the *--allow-shrink* +flag. In scripts it's possible to look for presence of *--allow-shrink* in +output of ``virsh help blockresize`` to determine if the flag is supported. + For image formats without metadata (raw) stored inside fixed-size storage (e.g. block devices) the --capacity flag can be used to resize the device to the full size of the backing device. diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 08a1ce3953..71bf5f39b0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3313,6 +3313,51 @@ cmdBlockpull(vshControl *ctl, const vshCmd *cmd) return ret; } + +/** + * virshIntrospectFlags: + * @ctl: virsh control data + * @api: name of API to introspect flags + * @flags: filled with flags supported by @api on success + * + * Uses the 'virConnectIntrospection' to introspect @api and return supported + * flags. + * + * Returns 0 on success (@flags are populated), -1 on error. No errors are + * reported by this function. + */ +static int +virshIntrospectFlags(vshControl *ctl, + const char *api, + unsigned int *flags) +{ + virshControl *priv = ctl->privData; + g_autofree char *introspectionxml = NULL; + g_autoptr(xmlDoc) xmldoc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + g_autofree char *xpath = g_strdup_printf("string(//api[@name='%s']/flags/@dec)", + api); + + *flags = 0; + + if (!(introspectionxml = virConnectGetIntrospection(priv->conn, 0))) { + vshResetLibvirtError(); + return -1; + } + + if (!(xmldoc = virXMLParseStringCtxt(introspectionxml, "(introspection xml)", + &ctxt))) + return -1; + + if (virXPathUInt(xpath, ctxt, flags) < 0) + return -1; + + vshDebug(ctl, VSH_ERR_DEBUG, "api:'%s' flags='0x%x'", api, *flags); + + return 0; +} + + /* * "blockresize" command */ @@ -3343,6 +3388,10 @@ static const vshCmdOptDef opts_blockresize[] = { .type = VSH_OT_BOOL, .help = N_("ensure that the new size is larger than actual capacity (prevent shrink)") }, + {.name = "allow-shrink", + .type = VSH_OT_BOOL, + .help = N_("disable size checks so that device can be shrunk") + }, {.name = NULL} }; @@ -3378,6 +3427,16 @@ cmdBlockresize(vshControl *ctl, const vshCmd *cmd) if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; + /* probe for support of VIR_DOMAIN_BLOCK_RESIZE_EXTEND and use it if it's + * supported to prevent mishaps even at the cost of usability change */ + if (!vshCommandOptBool(cmd, "allow-shrink")) { + unsigned int supportedflags = 0; + + if (virshIntrospectFlags(ctl, "virDomainBlockResize", &supportedflags) == 0 && + supportedflags & VIR_DOMAIN_BLOCK_RESIZE_EXTEND) + flags |= VIR_DOMAIN_BLOCK_RESIZE_EXTEND; + } + if (virDomainBlockResize(dom, path, size, flags) < 0) { vshError(ctl, _("Failed to resize block device '%1$s'"), path); return false; -- 2.54.0