On Wed, Apr 19, 2023 at 02:04:19PM +0200, Peter Krempa wrote: To ensure compiler protection in switch() statements you should also remove the default case from the switch statements. With that changed: Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com&gt;

On Wed, Apr 19, 2023 at 03:08:03PM +0200, Peter Krempa wrote: Oh, good to know, TIL then. In the meantime I replied as well =)

All changed lines even fit into 80 columns. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 974ec51a79..fe4c04bcea 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -563,28 +563,22 @@ virTypedParamsDeserialize(struct _virTypedParameterRemote *remote_params, param->type = remote_param->value.type; switch ((virTypedParameterType) param->type) { case VIR_TYPED_PARAM_INT: - param->value.i = - remote_param->value.remote_typed_param_value.i; + param->value.i = remote_param->value.remote_typed_param_value.i; break; case VIR_TYPED_PARAM_UINT: - param->value.ui = - remote_param->value.remote_typed_param_value.ui; + param->value.ui = remote_param->value.remote_typed_param_value.ui; break; case VIR_TYPED_PARAM_LLONG: - param->value.l = - remote_param->value.remote_typed_param_value.l; + param->value.l = remote_param->value.remote_typed_param_value.l; break; case VIR_TYPED_PARAM_ULLONG: - param->value.ul = - remote_param->value.remote_typed_param_value.ul; + param->value.ul = remote_param->value.remote_typed_param_value.ul; break; case VIR_TYPED_PARAM_DOUBLE: - param->value.d = - remote_param->value.remote_typed_param_value.d; + param->value.d = remote_param->value.remote_typed_param_value.d; break; case VIR_TYPED_PARAM_BOOLEAN: - param->value.b = - remote_param->value.remote_typed_param_value.b; + param->value.b = remote_param->value.remote_typed_param_value.b; break; case VIR_TYPED_PARAM_STRING: param->value.s = g_strdup(remote_param->value.remote_typed_param_value.s); -- 2.39.2

There are two callers of virTypedParameterAssignValueVArgs. - 'virTypedParameterAssignValue' always uses the correct type, thus doesn't need to be modified. Just use the proper type in the function declaration - 'virTypedParameterAssign' can get improper type, but we can move the validation into it decreasing the scope in which failures need to be propagated. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index fe4c04bcea..f325f3b012 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -203,7 +203,7 @@ virTypedParameterToString(virTypedParameterPtr param) } -static int +static void virTypedParameterAssignValueVArgs(virTypedParameterPtr param, virTypedParameterType type, va_list ap, @@ -240,30 +240,23 @@ virTypedParameterAssignValueVArgs(virTypedParameterPtr param, param->value.s = g_strdup(""); break; case VIR_TYPED_PARAM_LAST: - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected type %1$d for field %2$s"), type, - NULLSTR(param->field)); - return -1; + break; } - - return 0; } static int virTypedParameterAssignValue(virTypedParameterPtr param, - int type, + virTypedParameterType type, ...) { - int ret; va_list ap; va_start(ap, type); - ret = virTypedParameterAssignValueVArgs(param, type, ap, true); + virTypedParameterAssignValueVArgs(param, type, ap, true); va_end(ap); - return ret; + return 0; } @@ -276,7 +269,6 @@ virTypedParameterAssign(virTypedParameterPtr param, const char *name, int type, ...) { va_list ap; - int ret = -1; if (virStrcpyStatic(param->field, name) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Field name '%1$s' too long"), @@ -284,11 +276,18 @@ virTypedParameterAssign(virTypedParameterPtr param, const char *name, return -1; } + if (type < VIR_TYPED_PARAM_INT || + type >= VIR_TYPED_PARAM_LAST) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected type %1$d for field %2$s"), type, name); + return -1; + } + va_start(ap, type); - ret = virTypedParameterAssignValueVArgs(param, type, ap, false); + virTypedParameterAssignValueVArgs(param, type, ap, false); va_end(ap); - return ret; + return 0; } -- 2.39.2

Don't check the return value of 'virTypedParamListExtend' which will always be a valid pointer and 'virTypedParameterAssignValue' always returns 0. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.c | 46 +++++++++++++--------------------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index f325f3b012..1a1a34fdb2 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -245,7 +245,7 @@ virTypedParameterAssignValueVArgs(virTypedParameterPtr param, } -static int +static void virTypedParameterAssignValue(virTypedParameterPtr param, virTypedParameterType type, ...) @@ -255,8 +255,6 @@ virTypedParameterAssignValue(virTypedParameterPtr param, va_start(ap, type); virTypedParameterAssignValueVArgs(param, type, ap, true); va_end(ap); - - return 0; } @@ -776,13 +774,11 @@ virTypedParamListAddInt(virTypedParamList *list, const char *namefmt, ...) { - virTypedParameterPtr par; + virTypedParameterPtr par = virTypedParamListExtend(list); va_list ap; int ret; - if (!(par = virTypedParamListExtend(list)) || - virTypedParameterAssignValue(par, VIR_TYPED_PARAM_INT, value) < 0) - return -1; + virTypedParameterAssignValue(par, VIR_TYPED_PARAM_INT, value); va_start(ap, namefmt); ret = virTypedParamSetNameVPrintf(par, namefmt, ap); @@ -798,13 +794,11 @@ virTypedParamListAddUInt(virTypedParamList *list, const char *namefmt, ...) { - virTypedParameterPtr par; + virTypedParameterPtr par = virTypedParamListExtend(list); va_list ap; int ret; - if (!(par = virTypedParamListExtend(list)) || - virTypedParameterAssignValue(par, VIR_TYPED_PARAM_UINT, value) < 0) - return -1; + virTypedParameterAssignValue(par, VIR_TYPED_PARAM_UINT, value); va_start(ap, namefmt); ret = virTypedParamSetNameVPrintf(par, namefmt, ap); @@ -820,13 +814,11 @@ virTypedParamListAddLLong(virTypedParamList *list, const char *namefmt, ...) { - virTypedParameterPtr par; + virTypedParameterPtr par = virTypedParamListExtend(list); va_list ap; int ret; - if (!(par = virTypedParamListExtend(list)) || - virTypedParameterAssignValue(par, VIR_TYPED_PARAM_LLONG, value) < 0) - return -1; + virTypedParameterAssignValue(par, VIR_TYPED_PARAM_LLONG, value); va_start(ap, namefmt); ret = virTypedParamSetNameVPrintf(par, namefmt, ap); @@ -842,13 +834,11 @@ virTypedParamListAddULLong(virTypedParamList *list, const char *namefmt, ...) { - virTypedParameterPtr par; + virTypedParameterPtr par = virTypedParamListExtend(list); va_list ap; int ret; - if (!(par = virTypedParamListExtend(list)) || - virTypedParameterAssignValue(par, VIR_TYPED_PARAM_ULLONG, value) < 0) - return -1; + virTypedParameterAssignValue(par, VIR_TYPED_PARAM_ULLONG, value); va_start(ap, namefmt); ret = virTypedParamSetNameVPrintf(par, namefmt, ap); @@ -864,13 +854,11 @@ virTypedParamListAddString(virTypedParamList *list, const char *namefmt, ...) { - virTypedParameterPtr par; + virTypedParameterPtr par = virTypedParamListExtend(list); va_list ap; int ret; - if (!(par = virTypedParamListExtend(list)) || - virTypedParameterAssignValue(par, VIR_TYPED_PARAM_STRING, value) < 0) - return -1; + virTypedParameterAssignValue(par, VIR_TYPED_PARAM_STRING, value); va_start(ap, namefmt); ret = virTypedParamSetNameVPrintf(par, namefmt, ap); @@ -886,13 +874,11 @@ virTypedParamListAddBoolean(virTypedParamList *list, const char *namefmt, ...) { - virTypedParameterPtr par; + virTypedParameterPtr par = virTypedParamListExtend(list); va_list ap; int ret; - if (!(par = virTypedParamListExtend(list)) || - virTypedParameterAssignValue(par, VIR_TYPED_PARAM_BOOLEAN, value) < 0) - return -1; + virTypedParameterAssignValue(par, VIR_TYPED_PARAM_BOOLEAN, value); va_start(ap, namefmt); ret = virTypedParamSetNameVPrintf(par, namefmt, ap); @@ -908,13 +894,11 @@ virTypedParamListAddDouble(virTypedParamList *list, const char *namefmt, ...) { - virTypedParameterPtr par; + virTypedParameterPtr par = virTypedParamListExtend(list); va_list ap; int ret; - if (!(par = virTypedParamListExtend(list)) || - virTypedParameterAssignValue(par, VIR_TYPED_PARAM_DOUBLE, value) < 0) - return -1; + virTypedParameterAssignValue(par, VIR_TYPED_PARAM_DOUBLE, value); va_start(ap, namefmt); ret = virTypedParamSetNameVPrintf(par, namefmt, ap); -- 2.39.2

The header uses both styles randomly, switch it to the contemporary style. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.h | 140 ++++++++++++++++++++++----------------- 1 file changed, 80 insertions(+), 60 deletions(-) diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 936dac24ea..fb9815c661 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -55,14 +55,17 @@ struct _virTypedParameterRemote { }; -int virTypedParamsValidate(virTypedParameterPtr params, int nparams, - /* const char *name, int type ... */ ...) +int +virTypedParamsValidate(virTypedParameterPtr params, + int nparams, + /* const char *name, int type ... */ ...) G_GNUC_NULL_TERMINATED G_GNUC_WARN_UNUSED_RESULT; -bool virTypedParamsCheck(virTypedParameterPtr params, - int nparams, - const char **names, - int nnames); +bool +virTypedParamsCheck(virTypedParameterPtr params, + int nparams, + const char **names, + int nnames); int virTypedParamsGetStringList(virTypedParameterPtr params, @@ -77,36 +80,44 @@ virTypedParamsFilter(virTypedParameterPtr params, G_GNUC_WARN_UNUSED_RESULT; -int virTypedParameterAssign(virTypedParameterPtr param, const char *name, - int type, /* TYPE arg */ ...) +int +virTypedParameterAssign(virTypedParameterPtr param, + const char *name, + int type, /* TYPE arg */ ...) G_GNUC_WARN_UNUSED_RESULT; -int virTypedParamsReplaceString(virTypedParameterPtr *params, - int *nparams, - const char *name, - const char *value); +int +virTypedParamsReplaceString(virTypedParameterPtr *params, + int *nparams, + const char *name, + const char *value); -void virTypedParamsCopy(virTypedParameterPtr *dst, - virTypedParameterPtr src, - int nparams); +void +virTypedParamsCopy(virTypedParameterPtr *dst, + virTypedParameterPtr src, + int nparams); -char *virTypedParameterToString(virTypedParameterPtr param); +char * +virTypedParameterToString(virTypedParameterPtr param); -void virTypedParamsRemoteFree(struct _virTypedParameterRemote *remote_params_val, - unsigned int remote_params_len); +void +virTypedParamsRemoteFree(struct _virTypedParameterRemote *remote_params_val, + unsigned int remote_params_len); -int virTypedParamsDeserialize(struct _virTypedParameterRemote *remote_params, - unsigned int remote_params_len, - int limit, - virTypedParameterPtr *params, - int *nparams); +int +virTypedParamsDeserialize(struct _virTypedParameterRemote *remote_params, + unsigned int remote_params_len, + int limit, + virTypedParameterPtr *params, + int *nparams); -int virTypedParamsSerialize(virTypedParameterPtr params, - int nparams, - int limit, - struct _virTypedParameterRemote **remote_params_val, - unsigned int *remote_params_len, - unsigned int flags); +int +virTypedParamsSerialize(virTypedParameterPtr params, + int nparams, + int limit, + struct _virTypedParameterRemote **remote_params_val, + unsigned int *remote_params_len, + unsigned int flags); VIR_ENUM_DECL(virTypedParameter); @@ -132,48 +143,57 @@ struct _virTypedParamList { size_t par_alloc; }; -void virTypedParamListFree(virTypedParamList *list); +void +virTypedParamListFree(virTypedParamList *list); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virTypedParamList, virTypedParamListFree); -size_t virTypedParamListStealParams(virTypedParamList *list, - virTypedParameterPtr *params); +size_t +virTypedParamListStealParams(virTypedParamList *list, + virTypedParameterPtr *params); virTypedParamList * virTypedParamListFromParams(virTypedParameterPtr *params, size_t nparams); -int virTypedParamListAddInt(virTypedParamList *list, - int value, - const char *namefmt, - ...) +int +virTypedParamListAddInt(virTypedParamList *list, + int value, + const char *namefmt, + ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int virTypedParamListAddUInt(virTypedParamList *list, - unsigned int value, - const char *namefmt, - ...) +int +virTypedParamListAddUInt(virTypedParamList *list, + unsigned int value, + const char *namefmt, + ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int virTypedParamListAddLLong(virTypedParamList *list, - long long value, - const char *namefmt, - ...) +int +virTypedParamListAddLLong(virTypedParamList *list, + long long value, + const char *namefmt, + ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int virTypedParamListAddULLong(virTypedParamList *list, - unsigned long long value, - const char *namefmt, - ...) +int +virTypedParamListAddULLong(virTypedParamList *list, + unsigned long long value, + const char *namefmt, + ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int virTypedParamListAddString(virTypedParamList *list, - const char *value, - const char *namefmt, - ...) +int +virTypedParamListAddString(virTypedParamList *list, + const char *value, + const char *namefmt, + ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int virTypedParamListAddBoolean(virTypedParamList *list, - bool value, - const char *namefmt, - ...) +int +virTypedParamListAddBoolean(virTypedParamList *list, + bool value, + const char *namefmt, + ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int virTypedParamListAddDouble(virTypedParamList *list, - double value, - const char *namefmt, - ...) +int +virTypedParamListAddDouble(virTypedParamList *list, + double value, + const char *namefmt, + ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -- 2.39.2

Add an allocator function and refactor all allocations to use it. In upcoming patches 'struct _virTypedParamList' will be made private. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/admin/admin_server.c | 6 +++--- src/libvirt_private.syms | 1 + src/qemu/qemu_domainjob.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/test/test_driver.c | 2 +- src/util/virtypedparam.c | 9 ++++++++- src/util/virtypedparam.h | 1 + 7 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c index 7ed2645f57..394de0dfab 100644 --- a/src/admin/admin_server.c +++ b/src/admin/admin_server.c @@ -76,7 +76,7 @@ adminServerGetThreadPoolParameters(virNetServer *srv, size_t freeWorkers; size_t nPrioWorkers; size_t jobQueueDepth; - g_autoptr(virTypedParamList) paramlist = g_new0(virTypedParamList, 1); + g_autoptr(virTypedParamList) paramlist = virTypedParamListNew(); virCheckFlags(0, -1); @@ -200,7 +200,7 @@ adminClientGetInfo(virNetServerClient *client, bool readonly; g_autofree char *sock_addr = NULL; const char *attr = NULL; - g_autoptr(virTypedParamList) paramlist = g_new0(virTypedParamList, 1); + g_autoptr(virTypedParamList) paramlist = virTypedParamListNew(); g_autoptr(virIdentity) identity = NULL; int rc; @@ -298,7 +298,7 @@ adminServerGetClientLimits(virNetServer *srv, int *nparams, unsigned int flags) { - g_autoptr(virTypedParamList) paramlist = g_new0(virTypedParamList, 1); + g_autoptr(virTypedParamList) paramlist = virTypedParamListNew(); virCheckFlags(0, -1); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1247b67a39..c596ef0f87 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3557,6 +3557,7 @@ virTypedParamListAddUInt; virTypedParamListAddULLong; virTypedParamListFree; virTypedParamListFromParams; +virTypedParamListNew; virTypedParamListStealParams; virTypedParamsCheck; virTypedParamsCopy; diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 6376f928d5..4df1af0ce7 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -478,7 +478,7 @@ qemuDomainBackupJobDataToParams(virDomainJobData *jobData, { qemuDomainJobDataPrivate *priv = jobData->privateData; qemuDomainBackupStats *stats = &priv->stats.backup; - g_autoptr(virTypedParamList) par = g_new0(virTypedParamList, 1); + g_autoptr(virTypedParamList) par = virTypedParamListNew(); if (virTypedParamListAddInt(par, jobData->operation, VIR_DOMAIN_JOB_OPERATION) < 0) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 28e470e4a2..4cf5e8a512 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18447,7 +18447,7 @@ qemuDomainGetStats(virConnectPtr conn, g_autoptr(virTypedParamList) params = NULL; size_t i; - params = g_new0(virTypedParamList, 1); + params = virTypedParamListNew(); for (i = 0; qemuDomainGetStatsWorkers[i].func; i++) { if (stats & qemuDomainGetStatsWorkers[i].stats) { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 7b06896d44..47c74c420c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -9858,7 +9858,7 @@ testDomainGetStats(virConnectPtr conn, g_autoptr(virTypedParamList) params = NULL; size_t i; - params = g_new0(virTypedParamList, 1); + params = virTypedParamListNew(); for (i = 0; testDomainGetStatsWorkers[i].func; i++) { if (stats & testDomainGetStatsWorkers[i].stats) { diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 1a1a34fdb2..de3a4e76b4 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -705,6 +705,13 @@ virTypedParamsSerialize(virTypedParameterPtr params, } +virTypedParamList * +virTypedParamListNew(void) +{ + return g_new0(virTypedParamList, 1); +} + + void virTypedParamListFree(virTypedParamList *list) { @@ -733,7 +740,7 @@ virTypedParamList * virTypedParamListFromParams(virTypedParameterPtr *params, size_t nparams) { - virTypedParamList *l = g_new0(virTypedParamList, 1); + virTypedParamList *l = virTypedParamListNew(); l->par = g_steal_pointer(params); l->npar = nparams; diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index fb9815c661..4aa597bc81 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -146,6 +146,7 @@ struct _virTypedParamList { void virTypedParamListFree(virTypedParamList *list); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virTypedParamList, virTypedParamListFree); +virTypedParamList *virTypedParamListNew(void); size_t virTypedParamListStealParams(virTypedParamList *list, -- 2.39.2

Introduce a helper function to concatenate two virTypedParamLists. This will allow us to refactor qemuDomainGetStatsBlock to not access the list directly. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virtypedparam.c | 24 ++++++++++++++++++++++++ src/util/virtypedparam.h | 4 ++++ 3 files changed, 29 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c596ef0f87..0f42c2de0b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3555,6 +3555,7 @@ virTypedParamListAddLLong; virTypedParamListAddString; virTypedParamListAddUInt; virTypedParamListAddULLong; +virTypedParamListConcat; virTypedParamListFree; virTypedParamListFromParams; virTypedParamListNew; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index de3a4e76b4..f8dce9ed98 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -712,6 +712,30 @@ virTypedParamListNew(void) } +/** + * virTypedParamListConcat: + * + * @to: typed param list to concatenate into + * @fromptr: pointer to pointer to a typed param list to concatenate into @from + * + * Concatenates all params from the virTypedParamList pointed to by @fromptr + * into @to and deallocates the list pointed to by @fromptr and clears the + * variable. + */ +void +virTypedParamListConcat(virTypedParamList *to, + virTypedParamList **fromptr) +{ + g_autoptr(virTypedParamList) from = g_steal_pointer(fromptr); + + VIR_RESIZE_N(to->par, to->par_alloc, to->npar, from->npar); + + memcpy(to->par + to->npar, from->par, sizeof(*(to->par)) * from->npar); + to->npar += from->npar; + from->npar = 0; +} + + void virTypedParamListFree(virTypedParamList *list) { diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 4aa597bc81..45422c2673 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -156,6 +156,10 @@ virTypedParamList * virTypedParamListFromParams(virTypedParameterPtr *params, size_t nparams); +void +virTypedParamListConcat(virTypedParamList *to, + virTypedParamList **fromptr); + int virTypedParamListAddInt(virTypedParamList *list, int value, -- 2.39.2

On Wed, Apr 19, 2023 at 02:04:25PM +0200, Peter Krempa wrote: ^ | These two lines are terrible to read, but I think you meant @to here --------´ Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com&gt;

On a Wednesday in 2023, Peter Krempa wrote: s/0/visited/ Jano

Introduce a helper that fetches the typed parameters from the list while still preserving ownership of the pointer by the list. In the future this will be also able to report errors stored in the list. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com --- src/libvirt_private.syms | 1 + src/util/virtypedparam.c | 26 ++++++++++++++++++++++++++ src/util/virtypedparam.h | 6 ++++++ 3 files changed, 33 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0f42c2de0b..f1999d0e99 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3556,6 +3556,7 @@ virTypedParamListAddString; virTypedParamListAddUInt; virTypedParamListAddULLong; virTypedParamListConcat; +virTypedParamListFetch; virTypedParamListFree; virTypedParamListFromParams; virTypedParamListNew; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index f8dce9ed98..6e837c65bc 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -747,6 +747,32 @@ virTypedParamListFree(virTypedParamList *list) } +/** + * virTypedParamListFetch: + * + * @list: virTypedParamList object + * @par: if not NULL filled with the typed parameters stored in @list + * @npar: if not NULL filled with the number of typed parameters stored in @list + * + * Checks that @list has no errors stored and optionally fills @par and @npar + * with a valid list of typed parametes. The typed parameters still belong to + * @list and will be freed together. + */ +int +virTypedParamListFetch(virTypedParamList *list, + virTypedParameterPtr *par, + size_t *npar) +{ + if (par) + *par = list->par; + + if (npar) + *npar = list->npar; + + return 0; +} + + size_t virTypedParamListStealParams(virTypedParamList *list, virTypedParameterPtr *params) diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 45422c2673..628c951432 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -152,6 +152,12 @@ size_t virTypedParamListStealParams(virTypedParamList *list, virTypedParameterPtr *params); +int +virTypedParamListFetch(virTypedParamList *list, + virTypedParameterPtr *par, + size_t *npar) + G_GNUC_WARN_UNUSED_RESULT; + virTypedParamList * virTypedParamListFromParams(virTypedParameterPtr *params, size_t nparams); -- 2.39.2

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/driver.c | 7 ++++++- src/remote/remote_daemon_dispatch.c | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/driver.c b/src/driver.c index c7a9c2659f..35f8156605 100644 --- a/src/driver.c +++ b/src/driver.c @@ -165,8 +165,13 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char *name) ident = virIdentityGetCurrent(); if (ident) { g_autoptr(virTypedParamList) tmp = virIdentityGetParameters(ident); + virTypedParameterPtr par; + size_t npar; - if (virConnectSetIdentity(conn, tmp->par, tmp->npar, 0) < 0) + if (virTypedParamListFetch(tmp, &par, &npar) < 0) + goto error; + + if (virConnectSetIdentity(conn, par, npar, 0) < 0) goto error; } } diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 4fc83dbd90..7144e9e7ca 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -1824,7 +1824,13 @@ remoteOpenConn(const char *uri, VIR_DEBUG("Opened driver %p", newconn); if (preserveIdentity) { - if (virConnectSetIdentity(newconn, identparams->par, identparams->npar, 0) < 0) + virTypedParameterPtr par; + size_t npar; + + if (virTypedParamListFetch(identparams, &par, &npar) < 0) + return -1; + + if (virConnectSetIdentity(newconn, par, npar, 0) < 0) return -1; VIR_DEBUG("Forwarded current identity to secondary driver"); -- 2.39.2

Return the number of parameters via pointer passed as argument to free up possibility to report errors. Strangely all callers actually use 'int' as type for storing the count of elements, thus this function will use the same. The function is also renamed to virTypedParamListSteal. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/admin/admin_server.c | 10 +++++++--- src/libvirt_private.syms | 2 +- src/qemu/qemu_domainjob.c | 4 +++- src/qemu/qemu_driver.c | 4 +++- src/test/test_driver.c | 4 +++- src/util/virtypedparam.c | 20 ++++++++++++++------ src/util/virtypedparam.h | 7 ++++--- 7 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c index 394de0dfab..1d1ae97f2f 100644 --- a/src/admin/admin_server.c +++ b/src/admin/admin_server.c @@ -113,7 +113,8 @@ adminServerGetThreadPoolParameters(virNetServer *srv, "%s", VIR_THREADPOOL_JOB_QUEUE_DEPTH) < 0) return -1; - *nparams = virTypedParamListStealParams(paramlist, params); + if (virTypedParamListSteal(paramlist, params, nparams) < 0) + return -1; return 0; } @@ -279,7 +280,9 @@ adminClientGetInfo(virNetServerClient *client, "%s", VIR_CLIENT_INFO_SELINUX_CONTEXT) < 0) return -1; - *nparams = virTypedParamListStealParams(paramlist, params); + if (virTypedParamListSteal(paramlist, params, nparams) < 0) + return -1; + return 0; } @@ -322,7 +325,8 @@ adminServerGetClientLimits(virNetServer *srv, "%s", VIR_SERVER_CLIENTS_UNAUTH_CURRENT) < 0) return -1; - *nparams = virTypedParamListStealParams(paramlist, params); + if (virTypedParamListSteal(paramlist, params, nparams) < 0) + return -1; return 0; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f1999d0e99..b58be6aa33 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3560,7 +3560,7 @@ virTypedParamListFetch; virTypedParamListFree; virTypedParamListFromParams; virTypedParamListNew; -virTypedParamListStealParams; +virTypedParamListSteal; virTypedParamsCheck; virTypedParamsCopy; virTypedParamsDeserialize; diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 4df1af0ce7..6be48118b9 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -522,7 +522,9 @@ qemuDomainBackupJobDataToParams(virDomainJobData *jobData, virTypedParamListAddString(par, jobData->errmsg, VIR_DOMAIN_JOB_ERRMSG) < 0) return -1; - *nparams = virTypedParamListStealParams(par, params); + if (virTypedParamListSteal(par, params, nparams) < 0) + return -1; + *type = virDomainJobStatusToType(jobData->status); return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b89bf176bb..9c409948f0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18459,7 +18459,9 @@ qemuDomainGetStats(virConnectPtr conn, dom->def->uuid, dom->def->id))) return -1; - tmp->nparams = virTypedParamListStealParams(params, &tmp->params); + if (virTypedParamListSteal(params, &tmp->params, &tmp->nparams) < 0) + return -1; + *record = g_steal_pointer(&tmp); return 0; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 47c74c420c..ac12488de9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -9873,7 +9873,9 @@ testDomainGetStats(virConnectPtr conn, dom->def->uuid, dom->def->id))) return -1; - tmp->nparams = virTypedParamListStealParams(params, &tmp->params); + if (virTypedParamListSteal(params, &tmp->params, &tmp->nparams) < 0) + return -1; + *record = g_steal_pointer(&tmp); return 0; } diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index e03e112d1e..68698fe583 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -780,19 +780,27 @@ virTypedParamListFetch(virTypedParamList *list, } -size_t -virTypedParamListStealParams(virTypedParamList *list, - virTypedParameterPtr *params) +int +virTypedParamListSteal(virTypedParamList *list, + virTypedParameterPtr *par, + int *npar) { - size_t ret = list->npar; + size_t nparams; + + if (virTypedParamListFetch(list, par, &nparams) < 0) + return -1; - *params = g_steal_pointer(&list->par); + /* most callers expect 'int', so help them out */ + *npar = nparams; + + list->par = NULL; list->npar = 0; list->par_alloc = 0; - return ret; + return 0; } + virTypedParamList * virTypedParamListFromParams(virTypedParameterPtr *params, size_t nparams) diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index b6ad209e25..b2869173e1 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -143,9 +143,10 @@ virTypedParamListFree(virTypedParamList *list); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virTypedParamList, virTypedParamListFree); virTypedParamList *virTypedParamListNew(void); -size_t -virTypedParamListStealParams(virTypedParamList *list, - virTypedParameterPtr *params); +int +virTypedParamListSteal(virTypedParamList *list, + virTypedParameterPtr *par, + int *npar); int virTypedParamListFetch(virTypedParamList *list, -- 2.39.2

The function now return always 0. Refactor the code and remove return values. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/admin/admin_server.c | 108 ++++--------- src/qemu/qemu_domainjob.c | 45 ++---- src/qemu/qemu_driver.c | 330 ++++++++++++-------------------------- src/test/test_driver.c | 28 ++-- src/util/virtypedparam.c | 28 +--- src/util/virtypedparam.h | 28 ++-- 6 files changed, 173 insertions(+), 394 deletions(-) diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c index 1d1ae97f2f..2498a1309a 100644 --- a/src/admin/admin_server.c +++ b/src/admin/admin_server.c @@ -89,29 +89,12 @@ adminServerGetThreadPoolParameters(virNetServer *srv, return -1; } - if (virTypedParamListAddUInt(paramlist, minWorkers, - "%s", VIR_THREADPOOL_WORKERS_MIN) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, maxWorkers, - "%s", VIR_THREADPOOL_WORKERS_MAX) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, nWorkers, - "%s", VIR_THREADPOOL_WORKERS_CURRENT) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, freeWorkers, - "%s", VIR_THREADPOOL_WORKERS_FREE) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, nPrioWorkers, - "%s", VIR_THREADPOOL_WORKERS_PRIORITY) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, jobQueueDepth, - "%s", VIR_THREADPOOL_JOB_QUEUE_DEPTH) < 0) - return -1; + virTypedParamListAddUInt(paramlist, minWorkers, VIR_THREADPOOL_WORKERS_MIN); + virTypedParamListAddUInt(paramlist, maxWorkers, VIR_THREADPOOL_WORKERS_MAX); + virTypedParamListAddUInt(paramlist, nWorkers, VIR_THREADPOOL_WORKERS_CURRENT); + virTypedParamListAddUInt(paramlist, freeWorkers, VIR_THREADPOOL_WORKERS_FREE); + virTypedParamListAddUInt(paramlist, nPrioWorkers, VIR_THREADPOOL_WORKERS_PRIORITY); + virTypedParamListAddUInt(paramlist, jobQueueDepth, VIR_THREADPOOL_JOB_QUEUE_DEPTH); if (virTypedParamListSteal(paramlist, params, nparams) < 0) return -1; @@ -211,74 +194,54 @@ adminClientGetInfo(virNetServerClient *client, &sock_addr, &identity) < 0) return -1; - if (virTypedParamListAddBoolean(paramlist, readonly, - "%s", VIR_CLIENT_INFO_READONLY) < 0) - return -1; + virTypedParamListAddBoolean(paramlist, readonly, VIR_CLIENT_INFO_READONLY); if ((rc = virIdentityGetSASLUserName(identity, &attr)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddString(paramlist, attr, - "%s", VIR_CLIENT_INFO_SASL_USER_NAME) < 0) - return -1; + if (rc == 1) + virTypedParamListAddString(paramlist, attr, VIR_CLIENT_INFO_SASL_USER_NAME); if (!virNetServerClientIsLocal(client)) { - if (virTypedParamListAddString(paramlist, sock_addr, - "%s", VIR_CLIENT_INFO_SOCKET_ADDR) < 0) - return -1; + virTypedParamListAddString(paramlist, sock_addr, VIR_CLIENT_INFO_SOCKET_ADDR); if ((rc = virIdentityGetX509DName(identity, &attr)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddString(paramlist, attr, - "%s", VIR_CLIENT_INFO_X509_DISTINGUISHED_NAME) < 0) - return -1; + if (rc == 1) + virTypedParamListAddString(paramlist, attr, VIR_CLIENT_INFO_X509_DISTINGUISHED_NAME); } else { pid_t pid; uid_t uid; gid_t gid; if ((rc = virIdentityGetUNIXUserID(identity, &uid)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddInt(paramlist, uid, - "%s", VIR_CLIENT_INFO_UNIX_USER_ID) < 0) - return -1; + if (rc == 1) + virTypedParamListAddInt(paramlist, uid, VIR_CLIENT_INFO_UNIX_USER_ID); if ((rc = virIdentityGetUserName(identity, &attr)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddString(paramlist, attr, - "%s", VIR_CLIENT_INFO_UNIX_USER_NAME) < 0) - return -1; + if (rc == 1) + virTypedParamListAddString(paramlist, attr, VIR_CLIENT_INFO_UNIX_USER_NAME); if ((rc = virIdentityGetUNIXGroupID(identity, &gid)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddInt(paramlist, gid, - "%s", VIR_CLIENT_INFO_UNIX_GROUP_ID) < 0) - return -1; + if (rc == 1) + virTypedParamListAddInt(paramlist, gid, VIR_CLIENT_INFO_UNIX_GROUP_ID); if ((rc = virIdentityGetGroupName(identity, &attr)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddString(paramlist, attr, - "%s", VIR_CLIENT_INFO_UNIX_GROUP_NAME) < 0) - return -1; + if (rc == 1) + virTypedParamListAddString(paramlist, attr, VIR_CLIENT_INFO_UNIX_GROUP_NAME); if ((rc = virIdentityGetProcessID(identity, &pid)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddInt(paramlist, pid, - "%s", VIR_CLIENT_INFO_UNIX_PROCESS_ID) < 0) - return -1; + if (rc == 1) + virTypedParamListAddInt(paramlist, pid, VIR_CLIENT_INFO_UNIX_PROCESS_ID); } if ((rc = virIdentityGetSELinuxContext(identity, &attr)) < 0) return -1; - if (rc == 1 && - virTypedParamListAddString(paramlist, attr, - "%s", VIR_CLIENT_INFO_SELINUX_CONTEXT) < 0) - return -1; + if (rc == 1) + virTypedParamListAddString(paramlist, attr, VIR_CLIENT_INFO_SELINUX_CONTEXT); if (virTypedParamListSteal(paramlist, params, nparams) < 0) return -1; @@ -305,25 +268,10 @@ adminServerGetClientLimits(virNetServer *srv, virCheckFlags(0, -1); - if (virTypedParamListAddUInt(paramlist, - virNetServerGetMaxClients(srv), - "%s", VIR_SERVER_CLIENTS_MAX) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, - virNetServerGetCurrentClients(srv), - "%s", VIR_SERVER_CLIENTS_CURRENT) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, - virNetServerGetMaxUnauthClients(srv), - "%s", VIR_SERVER_CLIENTS_UNAUTH_MAX) < 0) - return -1; - - if (virTypedParamListAddUInt(paramlist, - virNetServerGetCurrentUnauthClients(srv), - "%s", VIR_SERVER_CLIENTS_UNAUTH_CURRENT) < 0) - return -1; + virTypedParamListAddUInt(paramlist, virNetServerGetMaxClients(srv), VIR_SERVER_CLIENTS_MAX); + virTypedParamListAddUInt(paramlist, virNetServerGetCurrentClients(srv), VIR_SERVER_CLIENTS_CURRENT); + virTypedParamListAddUInt(paramlist, virNetServerGetMaxUnauthClients(srv), VIR_SERVER_CLIENTS_UNAUTH_MAX); + virTypedParamListAddUInt(paramlist, virNetServerGetCurrentUnauthClients(srv), VIR_SERVER_CLIENTS_UNAUTH_CURRENT); if (virTypedParamListSteal(paramlist, params, nparams) < 0) return -1; diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c index 6be48118b9..245e51f14b 100644 --- a/src/qemu/qemu_domainjob.c +++ b/src/qemu/qemu_domainjob.c @@ -480,47 +480,26 @@ qemuDomainBackupJobDataToParams(virDomainJobData *jobData, qemuDomainBackupStats *stats = &priv->stats.backup; g_autoptr(virTypedParamList) par = virTypedParamListNew(); - if (virTypedParamListAddInt(par, jobData->operation, - VIR_DOMAIN_JOB_OPERATION) < 0) - return -1; - - if (virTypedParamListAddULLong(par, jobData->timeElapsed, - VIR_DOMAIN_JOB_TIME_ELAPSED) < 0) - return -1; + virTypedParamListAddInt(par, jobData->operation, VIR_DOMAIN_JOB_OPERATION); + virTypedParamListAddULLong(par, jobData->timeElapsed, VIR_DOMAIN_JOB_TIME_ELAPSED); if (stats->transferred > 0 || stats->total > 0) { - if (virTypedParamListAddULLong(par, stats->total, - VIR_DOMAIN_JOB_DISK_TOTAL) < 0) - return -1; - - if (virTypedParamListAddULLong(par, stats->transferred, - VIR_DOMAIN_JOB_DISK_PROCESSED) < 0) - return -1; - - if (virTypedParamListAddULLong(par, stats->total - stats->transferred, - VIR_DOMAIN_JOB_DISK_REMAINING) < 0) - return -1; + virTypedParamListAddULLong(par, stats->total, VIR_DOMAIN_JOB_DISK_TOTAL); + virTypedParamListAddULLong(par, stats->transferred, VIR_DOMAIN_JOB_DISK_PROCESSED); + virTypedParamListAddULLong(par, stats->total - stats->transferred, VIR_DOMAIN_JOB_DISK_REMAINING); } if (stats->tmp_used > 0 || stats->tmp_total > 0) { - if (virTypedParamListAddULLong(par, stats->tmp_used, - VIR_DOMAIN_JOB_DISK_TEMP_USED) < 0) - return -1; - - if (virTypedParamListAddULLong(par, stats->tmp_total, - VIR_DOMAIN_JOB_DISK_TEMP_TOTAL) < 0) - return -1; + virTypedParamListAddULLong(par, stats->tmp_used, VIR_DOMAIN_JOB_DISK_TEMP_USED); + virTypedParamListAddULLong(par, stats->tmp_total, VIR_DOMAIN_JOB_DISK_TEMP_TOTAL); } - if (jobData->status != VIR_DOMAIN_JOB_STATUS_ACTIVE && - virTypedParamListAddBoolean(par, - jobData->status == VIR_DOMAIN_JOB_STATUS_COMPLETED, - VIR_DOMAIN_JOB_SUCCESS) < 0) - return -1; + if (jobData->status != VIR_DOMAIN_JOB_STATUS_ACTIVE) + virTypedParamListAddBoolean(par, jobData->status == VIR_DOMAIN_JOB_STATUS_COMPLETED, + VIR_DOMAIN_JOB_SUCCESS); - if (jobData->errmsg && - virTypedParamListAddString(par, jobData->errmsg, VIR_DOMAIN_JOB_ERRMSG) < 0) - return -1; + if (jobData->errmsg) + virTypedParamListAddString(par, jobData->errmsg, VIR_DOMAIN_JOB_ERRMSG); if (virTypedParamListSteal(par, params, nparams) < 0) return -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9c409948f0..d47c1883f0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17020,11 +17020,8 @@ qemuDomainGetStatsState(virQEMUDriver *driver G_GNUC_UNUSED, virTypedParamList *params, unsigned int privflags G_GNUC_UNUSED) { - if (virTypedParamListAddInt(params, dom->state.state, "state.state") < 0) - return -1; - - if (virTypedParamListAddInt(params, dom->state.reason, "state.reason") < 0) - return -1; + virTypedParamListAddInt(params, dom->state.state, "state.state"); + virTypedParamListAddInt(params, dom->state.reason, "state.reason"); return 0; } @@ -17183,33 +17180,16 @@ qemuDomainGetStatsMemoryBandwidth(virQEMUDriver *driver, if (nresdata == 0) return 0; - if (virTypedParamListAddUInt(params, nresdata, - "memory.bandwidth.monitor.count") < 0) - goto cleanup; + virTypedParamListAddUInt(params, nresdata, "memory.bandwidth.monitor.count"); for (i = 0; i < nresdata; i++) { - if (virTypedParamListAddString(params, resdata[i]->name, - "memory.bandwidth.monitor.%zu.name", - i) < 0) - goto cleanup; - - if (virTypedParamListAddString(params, resdata[i]->vcpus, - "memory.bandwidth.monitor.%zu.vcpus", - i) < 0) - goto cleanup; - - if (virTypedParamListAddUInt(params, resdata[i]->nstats, - "memory.bandwidth.monitor.%zu.node.count", - i) < 0) - goto cleanup; - + virTypedParamListAddString(params, resdata[i]->name, "memory.bandwidth.monitor.%zu.name", i); + virTypedParamListAddString(params, resdata[i]->vcpus, "memory.bandwidth.monitor.%zu.vcpus", i); + virTypedParamListAddUInt(params, resdata[i]->nstats, "memory.bandwidth.monitor.%zu.node.count", i); for (j = 0; j < resdata[i]->nstats; j++) { - if (virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, - "memory.bandwidth.monitor.%zu." - "node.%zu.id", - i, j) < 0) - goto cleanup; + virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, + "memory.bandwidth.monitor.%zu.node.%zu.id", i, j); features = resdata[i]->stats[j]->features; @@ -17217,23 +17197,15 @@ qemuDomainGetStatsMemoryBandwidth(virQEMUDriver *driver, if (STREQ(features[k], "mbm_local_bytes")) { /* The accumulative data passing through local memory * controller is recorded with 64 bit counter. */ - if (virTypedParamListAddULLong(params, - resdata[i]->stats[j]->vals[k], - "memory.bandwidth.monitor." - "%zu.node.%zu.bytes.local", - i, j) < 0) - goto cleanup; + virTypedParamListAddULLong(params, resdata[i]->stats[j]->vals[k], + "memory.bandwidth.monitor.%zu.node.%zu.bytes.local", i, j); } if (STREQ(features[k], "mbm_total_bytes")) { /* The accumulative data passing through local and remote * memory controller is recorded with 64 bit counter. */ - if (virTypedParamListAddULLong(params, - resdata[i]->stats[j]->vals[k], - "memory.bandwidth.monitor." - "%zu.node.%zu.bytes.total", - i, j) < 0) - goto cleanup; + virTypedParamListAddULLong(params, resdata[i]->stats[j]->vals[k], + "memory.bandwidth.monitor.%zu.node.%zu.bytes.total", i, j); } } } @@ -17266,26 +17238,16 @@ qemuDomainGetStatsCpuCache(virQEMUDriver *driver, VIR_RESCTRL_MONITOR_TYPE_CACHE) < 0) goto cleanup; - if (virTypedParamListAddUInt(params, nresdata, "cpu.cache.monitor.count") < 0) - goto cleanup; + virTypedParamListAddUInt(params, nresdata, "cpu.cache.monitor.count"); for (i = 0; i < nresdata; i++) { - if (virTypedParamListAddString(params, resdata[i]->name, - "cpu.cache.monitor.%zu.name", i) < 0) - goto cleanup; - - if (virTypedParamListAddString(params, resdata[i]->vcpus, - "cpu.cache.monitor.%zu.vcpus", i) < 0) - goto cleanup; - - if (virTypedParamListAddUInt(params, resdata[i]->nstats, - "cpu.cache.monitor.%zu.bank.count", i) < 0) - goto cleanup; + virTypedParamListAddString(params, resdata[i]->name, "cpu.cache.monitor.%zu.name", i); + virTypedParamListAddString(params, resdata[i]->vcpus, "cpu.cache.monitor.%zu.vcpus", i); + virTypedParamListAddUInt(params, resdata[i]->nstats, "cpu.cache.monitor.%zu.bank.count", i); for (j = 0; j < resdata[i]->nstats; j++) { - if (virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, - "cpu.cache.monitor.%zu.bank.%zu.id", i, j) < 0) - goto cleanup; + virTypedParamListAddUInt(params, resdata[i]->stats[j]->id, + "cpu.cache.monitor.%zu.bank.%zu.id", i, j); /* 'resdata[i]->stats[j]->vals[0]' keeps the value of how many last * level cache in bank j currently occupied by the vcpus listed in @@ -17296,11 +17258,8 @@ qemuDomainGetStatsCpuCache(virQEMUDriver *driver, * than 4G bytes in size, to keep the 'domstats' interface * historically consistent, it is safe to report the value with a * truncated 'UInt' data type here. */ - if (virTypedParamListAddUInt(params, - (unsigned int)resdata[i]->stats[j]->vals[0], - "cpu.cache.monitor.%zu.bank.%zu.bytes", - i, j) < 0) - goto cleanup; + virTypedParamListAddUInt(params, (unsigned int)resdata[i]->stats[j]->vals[0], + "cpu.cache.monitor.%zu.bank.%zu.bytes", i, j); } } @@ -17321,20 +17280,17 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom, unsigned long long cpu_time = 0; unsigned long long user_time = 0; unsigned long long sys_time = 0; - int err = 0; if (!priv->cgroup) return 0; - err = virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time); - if (!err && virTypedParamListAddULLong(params, cpu_time, "cpu.time") < 0) - return -1; + if (virCgroupGetCpuacctUsage(priv->cgroup, &cpu_time) == 0) + virTypedParamListAddULLong(params, cpu_time, "cpu.time"); - err = virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time); - if (!err && virTypedParamListAddULLong(params, user_time, "cpu.user") < 0) - return -1; - if (!err && virTypedParamListAddULLong(params, sys_time, "cpu.system") < 0) - return -1; + if (virCgroupGetCpuacctStat(priv->cgroup, &user_time, &sys_time) == 0) { + virTypedParamListAddULLong(params, user_time, "cpu.user"); + virTypedParamListAddULLong(params, sys_time, "cpu.system"); + } return 0; } @@ -17354,10 +17310,9 @@ qemuDomainGetStatsCpuProc(virDomainObj *vm, return 0; } - if (virTypedParamListAddULLong(params, cpuTime, "cpu.time") < 0 || - virTypedParamListAddULLong(params, userTime, "cpu.user") < 0 || - virTypedParamListAddULLong(params, sysTime, "cpu.system") < 0) - return -1; + virTypedParamListAddULLong(params, cpuTime, "cpu.time"); + virTypedParamListAddULLong(params, userTime, "cpu.user"); + virTypedParamListAddULLong(params, sysTime, "cpu.system"); return 0; } @@ -17445,9 +17400,8 @@ qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom, virHostCPUGetHaltPollTime(dom->pid, &haltPollSuccess, &haltPollFail) < 0) return 0; - if (virTypedParamListAddULLong(params, haltPollSuccess, "cpu.haltpoll.success.time") < 0 || - virTypedParamListAddULLong(params, haltPollFail, "cpu.haltpoll.fail.time") < 0) - return -1; + virTypedParamListAddULLong(params, haltPollSuccess, "cpu.haltpoll.success.time"); + virTypedParamListAddULLong(params, haltPollFail, "cpu.haltpoll.fail.time"); return 0; } @@ -17506,12 +17460,8 @@ qemuDomainGetStatsBalloon(virQEMUDriver *driver G_GNUC_UNUSED, cur_balloon = dom->def->mem.cur_balloon; } - if (virTypedParamListAddULLong(params, cur_balloon, "balloon.current") < 0) - return -1; - - if (virTypedParamListAddULLong(params, virDomainDefGetMemoryTotal(dom->def), - "balloon.maximum") < 0) - return -1; + virTypedParamListAddULLong(params, cur_balloon, "balloon.current"); + virTypedParamListAddULLong(params, virDomainDefGetMemoryTotal(dom->def), "balloon.maximum"); if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) return 0; @@ -17523,8 +17473,7 @@ qemuDomainGetStatsBalloon(virQEMUDriver *driver G_GNUC_UNUSED, #define STORE_MEM_RECORD(TAG, NAME) \ if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \ - if (virTypedParamListAddULLong(params, stats[i].val, "balloon." NAME) < 0) \ - return -1; + virTypedParamListAddULLong(params, stats[i].val, "balloon." NAME); for (i = 0; i < nr_stats; i++) { STORE_MEM_RECORD(SWAP_IN, "swap_in") @@ -17593,16 +17542,14 @@ qemuDomainAddStatsFromHashTable(GHashTable *stats, if (virJSONValueGetBoolean(value, &stat) < 0) continue; - ignore_value(virTypedParamListAddBoolean(params, stat, "%s.%s.%s", - prefix, key, type)); + virTypedParamListAddBoolean(params, stat, "%s.%s.%s", prefix, key, type); } else { unsigned long long stat; if (virJSONValueGetNumberUlong(value, &stat) < 0) continue; - ignore_value(virTypedParamListAddULLong(params, stat, "%s.%s.%s", - prefix, key, type)); + virTypedParamListAddULLong(params, stat, "%s.%s.%s", prefix, key, type); } } } @@ -17623,13 +17570,8 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, qemuDomainObjPrivate *priv = dom->privateData; g_autoptr(virJSONValue) queried_stats = NULL; - if (virTypedParamListAddUInt(params, virDomainDefGetVcpus(dom->def), - "vcpu.current") < 0) - return -1; - - if (virTypedParamListAddUInt(params, virDomainDefGetVcpusMax(dom->def), - "vcpu.maximum") < 0) - return -1; + virTypedParamListAddUInt(params, virDomainDefGetVcpus(dom->def), "vcpu.current"); + virTypedParamListAddUInt(params, virDomainDefGetVcpusMax(dom->def), "vcpu.maximum"); cpuinfo = g_new0(virVcpuInfo, virDomainDefGetVcpus(dom->def)); cpuwait = g_new0(unsigned long long, virDomainDefGetVcpus(dom->def)); @@ -17662,25 +17604,15 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, g_autoptr(GHashTable) stats = NULL; g_autofree char *prefix = g_strdup_printf("vcpu.%u", cpuinfo[i].number); - if (virTypedParamListAddInt(params, cpuinfo[i].state, - "vcpu.%u.state", cpuinfo[i].number) < 0) - return -1; + virTypedParamListAddInt(params, cpuinfo[i].state, "vcpu.%u.state", cpuinfo[i].number); /* stats below are available only if the VM is alive */ if (!virDomainObjIsActive(dom)) continue; - if (virTypedParamListAddULLong(params, cpuinfo[i].cpuTime, - "vcpu.%u.time", cpuinfo[i].number) < 0) - return -1; - - if (virTypedParamListAddULLong(params, cpuwait[i], - "vcpu.%u.wait", cpuinfo[i].number) < 0) - return -1; - - if (virTypedParamListAddULLong(params, cpudelay[i], - "vcpu.%u.delay", cpuinfo[i].number) < 0) - return -1; + virTypedParamListAddULLong(params, cpuinfo[i].cpuTime, "vcpu.%u.time", cpuinfo[i].number); + virTypedParamListAddULLong(params, cpuwait[i], "vcpu.%u.wait", cpuinfo[i].number); + virTypedParamListAddULLong(params, cpudelay[i], "vcpu.%u.delay", cpuinfo[i].number); /* state below is extracted from the individual vcpu structs */ if (!(vcpu = virDomainDefGetVcpu(dom->def, cpuinfo[i].number))) @@ -17689,11 +17621,8 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, vcpupriv = QEMU_DOMAIN_VCPU_PRIVATE(vcpu); if (vcpupriv->halted != VIR_TRISTATE_BOOL_ABSENT) { - if (virTypedParamListAddBoolean(params, - vcpupriv->halted == VIR_TRISTATE_BOOL_YES, - "vcpu.%u.halted", - cpuinfo[i].number) < 0) - return -1; + virTypedParamListAddBoolean(params, vcpupriv->halted == VIR_TRISTATE_BOOL_YES, + "vcpu.%u.halted", cpuinfo[i].number); } if (!queried_stats) @@ -17709,9 +17638,8 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, } #define QEMU_ADD_NET_PARAM(params, num, name, value) \ - if (value >= 0 && \ - virTypedParamListAddULLong((params), (value), "net.%zu.%s", (num), (name)) < 0) \ - return -1; + if (value >= 0)\ + virTypedParamListAddULLong((params), (value), "net.%zu.%s", (num), (name)); static int qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, @@ -17725,8 +17653,7 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, if (!virDomainObjIsActive(dom)) return 0; - if (virTypedParamListAddUInt(params, dom->def->nnets, "net.count") < 0) - return -1; + virTypedParamListAddUInt(params, dom->def->nnets, "net.count"); /* Check the path is one of the domain's network interfaces. */ for (i = 0; i < dom->def->nnets; i++) { @@ -17740,8 +17667,7 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, actualType = virDomainNetGetActualType(net); - if (virTypedParamListAddString(params, net->ifname, "net.%zu.name", i) < 0) - return -1; + virTypedParamListAddString(params, net->ifname, "net.%zu.name", i); if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) { @@ -17756,22 +17682,14 @@ qemuDomainGetStatsInterface(virQEMUDriver *driver G_GNUC_UNUSED, } } - QEMU_ADD_NET_PARAM(params, i, - "rx.bytes", tmp.rx_bytes); - QEMU_ADD_NET_PARAM(params, i, - "rx.pkts", tmp.rx_packets); - QEMU_ADD_NET_PARAM(params, i, - "rx.errs", tmp.rx_errs); - QEMU_ADD_NET_PARAM(params, i, - "rx.drop", tmp.rx_drop); - QEMU_ADD_NET_PARAM(params, i, - "tx.bytes", tmp.tx_bytes); - QEMU_ADD_NET_PARAM(params, i, - "tx.pkts", tmp.tx_packets); - QEMU_ADD_NET_PARAM(params, i, - "tx.errs", tmp.tx_errs); - QEMU_ADD_NET_PARAM(params, i, - "tx.drop", tmp.tx_drop); + QEMU_ADD_NET_PARAM(params, i, "rx.bytes", tmp.rx_bytes); + QEMU_ADD_NET_PARAM(params, i, "rx.pkts", tmp.rx_packets); + QEMU_ADD_NET_PARAM(params, i, "rx.errs", tmp.rx_errs); + QEMU_ADD_NET_PARAM(params, i, "rx.drop", tmp.rx_drop); + QEMU_ADD_NET_PARAM(params, i, "tx.bytes", tmp.tx_bytes); + QEMU_ADD_NET_PARAM(params, i, "tx.pkts", tmp.tx_packets); + QEMU_ADD_NET_PARAM(params, i, "tx.errs", tmp.tx_errs); + QEMU_ADD_NET_PARAM(params, i, "tx.drop", tmp.tx_drop); } return 0; @@ -17799,20 +17717,14 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriver *driver, return 0; } - if (src->allocation && - virTypedParamListAddULLong(params, src->allocation, - "block.%zu.allocation", block_idx) < 0) - return -1; + if (src->allocation) + virTypedParamListAddULLong(params, src->allocation, "block.%zu.allocation", block_idx); - if (src->capacity && - virTypedParamListAddULLong(params, src->capacity, - "block.%zu.capacity", block_idx) < 0) - return -1; + if (src->capacity) + virTypedParamListAddULLong(params, src->capacity, "block.%zu.capacity", block_idx); - if (src->physical && - virTypedParamListAddULLong(params, src->physical, - "block.%zu.physical", block_idx) < 0) - return -1; + if (src->physical) + virTypedParamListAddULLong(params, src->physical, "block.%zu.physical", block_idx); return 0; } @@ -17843,24 +17755,16 @@ qemuDomainGetStatsOneBlock(virQEMUDriver *driver, if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) return 0; - if (virTypedParamListAddULLong(params, entry->wr_highest_offset, - "block.%zu.allocation", block_idx) < 0) - return -1; + virTypedParamListAddULLong(params, entry->wr_highest_offset, "block.%zu.allocation", block_idx); - if (entry->capacity && - virTypedParamListAddULLong(params, entry->capacity, - "block.%zu.capacity", block_idx) < 0) - return -1; + if (entry->capacity) + virTypedParamListAddULLong(params, entry->capacity, "block.%zu.capacity", block_idx); if (entry->physical) { - if (virTypedParamListAddULLong(params, entry->physical, - "block.%zu.physical", block_idx) < 0) - return -1; + virTypedParamListAddULLong(params, entry->physical, "block.%zu.physical", block_idx); } else { if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0) { - if (virTypedParamListAddULLong(params, src->physical, - "block.%zu.physical", block_idx) < 0) - return -1; + virTypedParamListAddULLong(params, src->physical, "block.%zu.physical", block_idx); } } @@ -17879,10 +17783,8 @@ qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) return 0; - if (entry->write_threshold && - virTypedParamListAddULLong(params, entry->write_threshold, - "block.%zu.threshold", recordnr) < 0) - return -1; + if (entry->write_threshold) + virTypedParamListAddULLong(params, entry->write_threshold, "block.%zu.threshold", recordnr); return 0; } @@ -17902,15 +17804,14 @@ qemuDomainGetStatsBlockExportFrontend(const char *frontendname, if (!stats || !frontendname || !(en = virHashLookup(stats, frontendname))) return 0; - if (virTypedParamListAddULLong(par, en->rd_req, "block.%zu.rd.reqs", idx) < 0 || - virTypedParamListAddULLong(par, en->rd_bytes, "block.%zu.rd.bytes", idx) < 0 || - virTypedParamListAddULLong(par, en->rd_total_times, "block.%zu.rd.times", idx) < 0 || - virTypedParamListAddULLong(par, en->wr_req, "block.%zu.wr.reqs", idx) < 0 || - virTypedParamListAddULLong(par, en->wr_bytes, "block.%zu.wr.bytes", idx) < 0 || - virTypedParamListAddULLong(par, en->wr_total_times, "block.%zu.wr.times", idx) < 0 || - virTypedParamListAddULLong(par, en->flush_req, "block.%zu.fl.reqs", idx) < 0 || - virTypedParamListAddULLong(par, en->flush_total_times, "block.%zu.fl.times", idx) < 0) - return -1; + virTypedParamListAddULLong(par, en->rd_req, "block.%zu.rd.reqs", idx); + virTypedParamListAddULLong(par, en->rd_bytes, "block.%zu.rd.bytes", idx); + virTypedParamListAddULLong(par, en->rd_total_times, "block.%zu.rd.times", idx); + virTypedParamListAddULLong(par, en->wr_req, "block.%zu.wr.reqs", idx); + virTypedParamListAddULLong(par, en->wr_bytes, "block.%zu.wr.bytes", idx); + virTypedParamListAddULLong(par, en->wr_total_times, "block.%zu.wr.times", idx); + virTypedParamListAddULLong(par, en->flush_req, "block.%zu.fl.reqs", idx); + virTypedParamListAddULLong(par, en->flush_total_times, "block.%zu.fl.times", idx); return 0; } @@ -17922,16 +17823,13 @@ qemuDomainGetStatsBlockExportHeader(virDomainDiskDef *disk, size_t recordnr, virTypedParamList *params) { - if (virTypedParamListAddString(params, disk->dst, "block.%zu.name", recordnr) < 0) - return -1; + virTypedParamListAddString(params, disk->dst, "block.%zu.name", recordnr); - if (virStorageSourceIsLocalStorage(src) && src->path && - virTypedParamListAddString(params, src->path, "block.%zu.path", recordnr) < 0) - return -1; + if (virStorageSourceIsLocalStorage(src) && src->path) + virTypedParamListAddString(params, src->path, "block.%zu.path", recordnr); - if (src->id && - virTypedParamListAddUInt(params, src->id, "block.%zu.backingIndex", recordnr) < 0) - return -1; + if (src->id) + virTypedParamListAddUInt(params, src->id, "block.%zu.backingIndex", recordnr); return 0; } @@ -18133,9 +18031,7 @@ qemuDomainGetStatsBlock(virQEMUDriver *driver, return -1; } - if (virTypedParamListAddUInt(params, 0, "block.count") < 0) - return -1; - + virTypedParamListAddUInt(params, 0, "block.count"); virTypedParamListConcat(params, &blockparams); return 0; @@ -18166,23 +18062,19 @@ qemuDomainGetStatsIOThread(virQEMUDriver *driver G_GNUC_UNUSED, goto cleanup; } - if (virTypedParamListAddUInt(params, niothreads, "iothread.count") < 0) - goto cleanup; + virTypedParamListAddUInt(params, niothreads, "iothread.count"); for (i = 0; i < niothreads; i++) { if (iothreads[i]->poll_valid) { - if (virTypedParamListAddULLong(params, iothreads[i]->poll_max_ns, - "iothread.%u.poll-max-ns", - iothreads[i]->iothread_id) < 0) - goto cleanup; - if (virTypedParamListAddUInt(params, iothreads[i]->poll_grow, - "iothread.%u.poll-grow", - iothreads[i]->iothread_id) < 0) - goto cleanup; - if (virTypedParamListAddUInt(params, iothreads[i]->poll_shrink, - "iothread.%u.poll-shrink", - iothreads[i]->iothread_id) < 0) - goto cleanup; + virTypedParamListAddULLong(params, iothreads[i]->poll_max_ns, + "iothread.%u.poll-max-ns", + iothreads[i]->iothread_id); + virTypedParamListAddUInt(params, iothreads[i]->poll_grow, + "iothread.%u.poll-grow", + iothreads[i]->iothread_id); + virTypedParamListAddUInt(params, iothreads[i]->poll_shrink, + "iothread.%u.poll-shrink", + iothreads[i]->iothread_id); } } @@ -18207,9 +18099,7 @@ qemuDomainGetStatsPerfOneEvent(virPerf *perf, if (virPerfReadEvent(perf, type, &value) < 0) return -1; - if (virTypedParamListAddULLong(params, value, "perf.%s", - virPerfEventTypeToString(type)) < 0) - return -1; + virTypedParamListAddULLong(params, value, "perf.%s", virPerfEventTypeToString(type)); return 0; } @@ -18262,35 +18152,21 @@ qemuDomainGetStatsDirtyRate(virQEMUDriver *driver G_GNUC_UNUSED, if (qemuDomainGetStatsDirtyRateMon(dom, &info) < 0) return -1; - if (virTypedParamListAddInt(params, info.status, - "dirtyrate.calc_status") < 0) - return -1; - - if (virTypedParamListAddLLong(params, info.startTime, - "dirtyrate.calc_start_time") < 0) - return -1; - - if (virTypedParamListAddInt(params, info.calcTime, - "dirtyrate.calc_period") < 0) - return -1; - - if (virTypedParamListAddString(params, - qemuMonitorDirtyRateCalcModeTypeToString(info.mode), - "dirtyrate.calc_mode") < 0) - return -1; + virTypedParamListAddInt(params, info.status, "dirtyrate.calc_status"); + virTypedParamListAddLLong(params, info.startTime, "dirtyrate.calc_start_time"); + virTypedParamListAddInt(params, info.calcTime, "dirtyrate.calc_period"); + virTypedParamListAddString(params, qemuMonitorDirtyRateCalcModeTypeToString(info.mode), + "dirtyrate.calc_mode"); if (info.status == VIR_DOMAIN_DIRTYRATE_MEASURED) { - if (virTypedParamListAddLLong(params, info.dirtyRate, - "dirtyrate.megabytes_per_second") < 0) - return -1; + virTypedParamListAddLLong(params, info.dirtyRate, "dirtyrate.megabytes_per_second"); if (info.mode == QEMU_MONITOR_DIRTYRATE_CALC_MODE_DIRTY_RING) { size_t i; for (i = 0; i < info.nvcpus; i++) { - if (virTypedParamListAddULLong(params, info.rates[i].value, - "dirtyrate.vcpu.%d.megabytes_per_second", - info.rates[i].idx) < 0) - return -1; + virTypedParamListAddULLong(params, info.rates[i].value, + "dirtyrate.vcpu.%d.megabytes_per_second", + info.rates[i].idx); } } } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ac12488de9..e7fce053b4 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -9784,11 +9784,8 @@ static int testDomainGetStatsState(virDomainObj *dom, virTypedParamList *params) { - if (virTypedParamListAddInt(params, dom->state.state, "state.state") < 0) - return -1; - - if (virTypedParamListAddInt(params, dom->state.reason, "state.reason") < 0) - return -1; + virTypedParamListAddInt(params, dom->state.state, "state.state"); + virTypedParamListAddInt(params, dom->state.reason, "state.reason"); return 0; } @@ -9810,24 +9807,17 @@ testDomainGetStatsIOThread(virDomainObj *dom, return 0; } - if (virTypedParamListAddUInt(params, niothreads, "iothread.count") < 0) - return -1; + virTypedParamListAddUInt(params, niothreads, "iothread.count"); for (i = 0; i < niothreads; i++) { testIOThreadInfo iothread = g_array_index(priv->iothreads, testIOThreadInfo, i); - if (virTypedParamListAddULLong(params, iothread.poll_max_ns, - "iothread.%u.poll-max-ns", - iothread.iothread_id) < 0) - return -1; - if (virTypedParamListAddUInt(params, iothread.poll_grow, - "iothread.%u.poll-grow", - iothread.iothread_id) < 0) - return -1; - if (virTypedParamListAddUInt(params, iothread.poll_shrink, - "iothread.%u.poll-shrink", - iothread.iothread_id) < 0) - return -1; + virTypedParamListAddULLong(params, iothread.poll_max_ns, + "iothread.%u.poll-max-ns", iothread.iothread_id); + virTypedParamListAddUInt(params, iothread.poll_grow, + "iothread.%u.poll-grow", iothread.iothread_id); + virTypedParamListAddUInt(params, iothread.poll_shrink, + "iothread.%u.poll-shrink", iothread.iothread_id); } return 0; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 046164d36c..ee9c6d1c45 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -853,7 +853,7 @@ virTypedParamListExtend(virTypedParamList *list) } -int +void virTypedParamListAddInt(virTypedParamList *list, int value, const char *namefmt, @@ -867,12 +867,10 @@ virTypedParamListAddInt(virTypedParamList *list, va_start(ap, namefmt); virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); - - return 0; } -int +void virTypedParamListAddUInt(virTypedParamList *list, unsigned int value, const char *namefmt, @@ -886,12 +884,10 @@ virTypedParamListAddUInt(virTypedParamList *list, va_start(ap, namefmt); virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); - - return 0; } -int +void virTypedParamListAddLLong(virTypedParamList *list, long long value, const char *namefmt, @@ -905,12 +901,10 @@ virTypedParamListAddLLong(virTypedParamList *list, va_start(ap, namefmt); virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); - - return 0; } -int +void virTypedParamListAddULLong(virTypedParamList *list, unsigned long long value, const char *namefmt, @@ -924,12 +918,10 @@ virTypedParamListAddULLong(virTypedParamList *list, va_start(ap, namefmt); virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); - - return 0; } -int +void virTypedParamListAddString(virTypedParamList *list, const char *value, const char *namefmt, @@ -943,12 +935,10 @@ virTypedParamListAddString(virTypedParamList *list, va_start(ap, namefmt); virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); - - return 0; } -int +void virTypedParamListAddBoolean(virTypedParamList *list, bool value, const char *namefmt, @@ -962,12 +952,10 @@ virTypedParamListAddBoolean(virTypedParamList *list, va_start(ap, namefmt); virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); - - return 0; } -int +void virTypedParamListAddDouble(virTypedParamList *list, double value, const char *namefmt, @@ -981,6 +969,4 @@ virTypedParamListAddDouble(virTypedParamList *list, va_start(ap, namefmt); virTypedParamSetNameVPrintf(list, par, namefmt, ap); va_end(ap); - - return 0; } diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index b2869173e1..7065683297 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -162,45 +162,45 @@ void virTypedParamListConcat(virTypedParamList *to, virTypedParamList **fromptr); -int +void virTypedParamListAddInt(virTypedParamList *list, int value, const char *namefmt, ...) - G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int + G_GNUC_PRINTF(3, 4); +void virTypedParamListAddUInt(virTypedParamList *list, unsigned int value, const char *namefmt, ...) - G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int + G_GNUC_PRINTF(3, 4); +void virTypedParamListAddLLong(virTypedParamList *list, long long value, const char *namefmt, ...) - G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int + G_GNUC_PRINTF(3, 4); +void virTypedParamListAddULLong(virTypedParamList *list, unsigned long long value, const char *namefmt, ...) - G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int + G_GNUC_PRINTF(3, 4); +void virTypedParamListAddString(virTypedParamList *list, const char *value, const char *namefmt, ...) - G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int + G_GNUC_PRINTF(3, 4); +void virTypedParamListAddBoolean(virTypedParamList *list, bool value, const char *namefmt, ...) - G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; -int + G_GNUC_PRINTF(3, 4); +void virTypedParamListAddDouble(virTypedParamList *list, double value, const char *namefmt, ...) - G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; + G_GNUC_PRINTF(3, 4); -- 2.39.2

Add an internal helper for fetching a typed parameter which can be either of the '_UINT' or '_ULONG' type and store it in a unsigned long long variable. Since this is an internal helper it offers less protections against invalid use compared to those we expose as public API. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virtypedparam.c | 51 ++++++++++++++++++++++++++++++++++++++++ src/util/virtypedparam.h | 5 ++++ 3 files changed, 57 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4e21851c53..62c296bf5f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3567,6 +3567,7 @@ virTypedParamsCopy; virTypedParamsDeserialize; virTypedParamsFilter; virTypedParamsGetStringList; +virTypedParamsGetUnsigned; virTypedParamsRemoteFree; virTypedParamsReplaceString; virTypedParamsSerialize; diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 52eea0fc54..d0e6cf6d9b 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -456,6 +456,57 @@ virTypedParamsGetStringList(virTypedParameterPtr params, } +/** + * virTypedParamsGetUnsigned: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @value: where to store the parameter's value + * + * Finds typed parameter called @name and store its 'unsigned long long' or + * 'unsigned int' value in @value. + * + * This is an internal variand which expects that the typed parameters were + * already validated by calling virTypedParamsValidate and the appropriate + * parameter has the expected type. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on invalid usage. + */ +int +virTypedParamsGetUnsigned(virTypedParameterPtr params, + int nparams, + const char *name, + unsigned long long *value) +{ + virTypedParameterPtr param; + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + switch ((virTypedParameterType) param->type) { + case VIR_TYPED_PARAM_UINT: + *value = param->value.ui; + break; + + case VIR_TYPED_PARAM_ULLONG: + *value = param->value.ul; + break; + + case VIR_TYPED_PARAM_INT: + case VIR_TYPED_PARAM_LLONG: + case VIR_TYPED_PARAM_DOUBLE: + case VIR_TYPED_PARAM_BOOLEAN: + case VIR_TYPED_PARAM_STRING: + case VIR_TYPED_PARAM_LAST: + default: + return -1; + } + + return 1; +} + + /** * virTypedParamsRemoteFree: * @remote_params_val: array of typed parameters as specified by diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index 88f810bf78..c2f58e994c 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -79,6 +79,11 @@ virTypedParamsFilter(virTypedParameterPtr params, virTypedParameterPtr **ret) G_GNUC_WARN_UNUSED_RESULT; +int +virTypedParamsGetUnsigned(virTypedParameterPtr params, + int nparams, + const char *name, + unsigned long long *value); int virTypedParameterAssign(virTypedParameterPtr param, -- 2.39.2

For certain typed parameters we want to extend the supproted range by switching to VIR_TYPED_PARAM_ULLONG. To preserve compatibility we've added APIs such as 'virTypedParamsGetUnsigned' and 'virTypedParamListAddUnsigned' which automatically select the bigger type if necessary. This patch adds a new internal macro VIR_TYPED_PARAM_UNSIGNED which is used with virTypedParamsValidate to allow both types and adjusts the code to handle it properly. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virtypedparam.c | 19 ++++++++++++++----- src/util/virtypedparam.h | 9 +++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index c71d4415a1..c3bc1237c1 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -63,7 +63,6 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) size_t j; const char *name; const char *last_name = NULL; - int type; size_t nkeys = 0; size_t nkeysalloc = 0; g_autofree virTypedParameterPtr sorted = NULL; @@ -79,7 +78,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) name = va_arg(ap, const char *); while (name) { - type = va_arg(ap, int); + int type = va_arg(ap, int); VIR_RESIZE_N(keys, nkeysalloc, nkeys, 1); if (virStrcpyStatic(keys[nkeys].field, name) < 0) { @@ -105,6 +104,9 @@ 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, @@ -112,7 +114,15 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) sorted[i].field); return -1; } - if (sorted[i].type != keys[j].type) { + + 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); @@ -120,8 +130,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) badtype = virTypedParameterTypeToString(0); virReportError(VIR_ERR_INVALID_ARG, _("invalid type '%1$s' for parameter '%2$s', expected '%3$s'"), - badtype, sorted[i].field, - virTypedParameterTypeToString(keys[j].type)); + badtype, sorted[i].field, expecttype); return -1; } last_name = sorted[i].field; diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index c2f58e994c..7454ef3ce0 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -24,6 +24,15 @@ #include "internal.h" #include "virenum.h" + +/** + * VIR_TYPED_PARAM_UNSIGNED: + * + * Special typed parameter type only used with virTypedParamsValidate to + * indicate that both VIR_TYPED_PARAM_UINT and VIR_TYPED_PARAM_ULLONG types + * are acceptable for given value. + */ +#define VIR_TYPED_PARAM_UNSIGNED (VIR_TYPED_PARAM_LAST + 1) /** * VIR_TYPED_PARAM_MULTIPLE: * -- 2.39.2

QEMU accepts even values bigger than INT_MAX. The reasoning for these checks was that the QAPI definition declares them as 'int', but in QAPI terms that's any number as it's JSON. Remove the validation as well as the comment missintepreting the QAPI definiton. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 21 --------------------- src/qemu/qemu_monitor_json.c | 7 ------- 2 files changed, 28 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d47c1883f0..23646b7289 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5234,27 +5234,6 @@ qemuDomainIOThreadParseParams(virTypedParameterPtr params, if (rc == 1) iothread->set_thread_pool_max = true; - if (iothread->set_poll_max_ns && iothread->poll_max_ns > INT_MAX) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("poll-max-ns (%1$llu) must be less than or equal to %2$d"), - iothread->poll_max_ns, INT_MAX); - return -1; - } - - if (iothread->set_poll_grow && iothread->poll_grow > INT_MAX) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("poll-grow (%1$u) must be less than or equal to %2$d"), - iothread->poll_grow, INT_MAX); - return -1; - } - - if (iothread->set_poll_shrink && iothread->poll_shrink > INT_MAX) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("poll-shrink (%1$u) must be less than or equal to %2$d"), - iothread->poll_shrink, INT_MAX); - return -1; - } - if (iothread->set_thread_pool_min && iothread->thread_pool_min < -1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("thread_pool_min (%1$d) must be equal to or greater than -1"), diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index b2c0b20a11..3454e85e43 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7128,13 +7128,6 @@ qemuMonitorJSONGetIOThreads(qemuMonitor *mon, goto cleanup; } - /* Fetch poll values (since QEMU 2.9 ) if available. QEMU - * stores these values as int64_t's; however, the qapi type - * is an int. The qapi/misc.json also mis-describes the grow - * and shrink values as pure add/remove values. The source - * util/aio-posix.c function aio_poll uses them as a factor - * or divisor in it's calculation. We will fetch and store - * them as defined in our structures. */ if (virJSONValueObjectGetNumberUlong(child, "poll-max-ns", &info->poll_max_ns) == 0 && virJSONValueObjectGetNumberUint(child, "poll-grow", -- 2.39.2

Convert the internal types to unsigned long long. Luckily we can also covert the external types too: - 'qemuDomainSetIOThreadParams' can accept both _UINT and _ULLONG by converting to 'virTypedParamsGetUnsigned' - querying is handled via the bulk stats API which is flexible: - we use virTypedParamListAddUnsigned to use the bigger type only if necessary - most users don't even notice because the bindings abstract the data types Apart from the code modifications we also improve the documenataion which was missing for the setters. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 4 +++- src/libvirt-domain.c | 14 +++++++---- src/qemu/qemu_driver.c | 28 +++++++++++----------- src/qemu/qemu_monitor.h | 4 ++-- src/qemu/qemu_monitor_json.c | 41 ++++++++++++++++++++------------ 5 files changed, 54 insertions(+), 37 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 3ebb2c6642..20862a69f2 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2482,7 +2482,7 @@ int virDomainDelIOThread(virDomainPtr domain, * poll_grow and poll_shrink parameters provided. A value set too large * will cause more CPU time to be allocated the guest. A value set too * small will not provide enough cycles for the guest to process data. - * The polling interval is not available for statistical purposes. + * Accepted type is VIR_TYPED_PARAM_ULLONG. * * Since: 4.10.0 */ @@ -2495,6 +2495,7 @@ int virDomainDelIOThread(virDomainPtr domain, * use to grow its polling interval up to the poll_max_ns value. A value * of 0 (zero) allows the hypervisor to choose its own value. The algorithm * to use for adjustment is hypervisor specific. + * Accepted type is VIR_TYPED_PARAM_UINT or since 9.3.0 VIR_TYPED_PARAM_ULLONG. * * Since: 4.10.0 */ @@ -2508,6 +2509,7 @@ int virDomainDelIOThread(virDomainPtr domain, * the poll_max_ns value. A value of 0 (zero) allows the hypervisor to * choose its own value. The algorithm to use for adjustment is hypervisor * specific. + * Accepted type is VIR_TYPED_PARAM_UINT or since 9.3.0 VIR_TYPED_PARAM_ULLONG. * * Since: 4.10.0 */ diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 63829bb028..ec42bb9a53 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12449,14 +12449,18 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * "iothread.<id>.poll-max-ns" - maximum polling time in ns as an unsigned * long long. A 0 (zero) means polling is * disabled. - * "iothread.<id>.poll-grow" - polling time factor as an unsigned int. + * "iothread.<id>.poll-grow" - polling time factor as an unsigned int or + * unsigned long long if exceeding range of + * unsigned int. * A 0 (zero) indicates to allow the underlying * hypervisor to choose how to grow the * polling time. - * "iothread.<id>.poll-shrink" - polling time divisor as an unsigned int. - * A 0 (zero) indicates to allow the underlying - * hypervisor to choose how to shrink the - * polling time. + * "iothread.<id>.poll-shrink" - polling time divisor as an unsigned int or + * unsigned long long if exceeding range of + * unsigned int. + * A 0 (zero) indicates to allow the underlying + * hypervisor to choose how to shrink the + * polling time. * * VIR_DOMAIN_STATS_MEMORY: * Return memory bandwidth statistics and the usage information. The typed diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 23646b7289..dafe8af8a6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5189,9 +5189,9 @@ qemuDomainIOThreadParseParams(virTypedParameterPtr params, VIR_DOMAIN_IOTHREAD_POLL_MAX_NS, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_IOTHREAD_POLL_GROW, - VIR_TYPED_PARAM_UINT, + VIR_TYPED_PARAM_UNSIGNED, VIR_DOMAIN_IOTHREAD_POLL_SHRINK, - VIR_TYPED_PARAM_UINT, + VIR_TYPED_PARAM_UNSIGNED, VIR_DOMAIN_IOTHREAD_THREAD_POOL_MIN, VIR_TYPED_PARAM_INT, VIR_DOMAIN_IOTHREAD_THREAD_POOL_MAX, @@ -5206,16 +5206,16 @@ qemuDomainIOThreadParseParams(virTypedParameterPtr params, if (rc == 1) iothread->set_poll_max_ns = true; - if ((rc = virTypedParamsGetUInt(params, nparams, - VIR_DOMAIN_IOTHREAD_POLL_GROW, - &iothread->poll_grow)) < 0) + if ((rc = virTypedParamsGetUnsigned(params, nparams, + VIR_DOMAIN_IOTHREAD_POLL_GROW, + &iothread->poll_grow)) < 0) return -1; if (rc == 1) iothread->set_poll_grow = true; - if ((rc = virTypedParamsGetUInt(params, nparams, - VIR_DOMAIN_IOTHREAD_POLL_SHRINK, - &iothread->poll_shrink)) < 0) + if ((rc = virTypedParamsGetUnsigned(params, nparams, + VIR_DOMAIN_IOTHREAD_POLL_SHRINK, + &iothread->poll_shrink)) < 0) return -1; if (rc == 1) iothread->set_poll_shrink = true; @@ -18048,12 +18048,12 @@ qemuDomainGetStatsIOThread(virQEMUDriver *driver G_GNUC_UNUSED, virTypedParamListAddULLong(params, iothreads[i]->poll_max_ns, "iothread.%u.poll-max-ns", iothreads[i]->iothread_id); - virTypedParamListAddUInt(params, iothreads[i]->poll_grow, - "iothread.%u.poll-grow", - iothreads[i]->iothread_id); - virTypedParamListAddUInt(params, iothreads[i]->poll_shrink, - "iothread.%u.poll-shrink", - iothreads[i]->iothread_id); + virTypedParamListAddUnsigned(params, iothreads[i]->poll_grow, + "iothread.%u.poll-grow", + iothreads[i]->iothread_id); + virTypedParamListAddUnsigned(params, iothreads[i]->poll_shrink, + "iothread.%u.poll-shrink", + iothreads[i]->iothread_id); } } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 72db0c0838..09f22f2328 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1254,8 +1254,8 @@ struct _qemuMonitorIOThreadInfo { int thread_id; bool poll_valid; unsigned long long poll_max_ns; - unsigned int poll_grow; - unsigned int poll_shrink; + unsigned long long poll_grow; + unsigned long long poll_shrink; int thread_pool_min; int thread_pool_max; bool set_poll_max_ns; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3454e85e43..745d83e2b6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7130,10 +7130,10 @@ qemuMonitorJSONGetIOThreads(qemuMonitor *mon, if (virJSONValueObjectGetNumberUlong(child, "poll-max-ns", &info->poll_max_ns) == 0 && - virJSONValueObjectGetNumberUint(child, "poll-grow", - &info->poll_grow) == 0 && - virJSONValueObjectGetNumberUint(child, "poll-shrink", - &info->poll_shrink) == 0) + virJSONValueObjectGetNumberUlong(child, "poll-grow", + &info->poll_grow) == 0 && + virJSONValueObjectGetNumberUlong(child, "poll-shrink", + &info->poll_shrink) == 0) info->poll_valid = true; } @@ -7161,18 +7161,20 @@ qemuMonitorJSONSetIOThread(qemuMonitor *mon, path = g_strdup_printf("/objects/iothread%u", iothreadInfo->iothread_id); -#define VIR_IOTHREAD_SET_PROP(propName, propVal) \ +#define VIR_IOTHREAD_SET_PROP_UL(propName, propVal) \ if (iothreadInfo->set_##propVal) { \ memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); \ - prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \ - prop.val.iv = iothreadInfo->propVal; \ + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_ULONG; \ + prop.val.ul = iothreadInfo->propVal; \ if (qemuMonitorJSONSetObjectProperty(mon, path, propName, &prop) < 0) \ return -1; \ } - VIR_IOTHREAD_SET_PROP("poll-max-ns", poll_max_ns); - VIR_IOTHREAD_SET_PROP("poll-grow", poll_grow); - VIR_IOTHREAD_SET_PROP("poll-shrink", poll_shrink); + VIR_IOTHREAD_SET_PROP_UL("poll-max-ns", poll_max_ns); + VIR_IOTHREAD_SET_PROP_UL("poll-grow", poll_grow); + VIR_IOTHREAD_SET_PROP_UL("poll-shrink", poll_shrink); + +#undef VIR_IOTHREAD_SET_PROP_UL if (iothreadInfo->set_thread_pool_min && iothreadInfo->set_thread_pool_max) { @@ -7192,15 +7194,24 @@ qemuMonitorJSONSetIOThread(qemuMonitor *mon, setMaxFirst = true; } +#define VIR_IOTHREAD_SET_PROP_INT(propName, propVal) \ + if (iothreadInfo->set_##propVal) { \ + memset(&prop, 0, sizeof(qemuMonitorJSONObjectProperty)); \ + prop.type = QEMU_MONITOR_OBJECT_PROPERTY_INT; \ + prop.val.iv = iothreadInfo->propVal; \ + if (qemuMonitorJSONSetObjectProperty(mon, path, propName, &prop) < 0) \ + return -1; \ + } + if (setMaxFirst) { - VIR_IOTHREAD_SET_PROP("thread-pool-max", thread_pool_max); - VIR_IOTHREAD_SET_PROP("thread-pool-min", thread_pool_min); + VIR_IOTHREAD_SET_PROP_INT("thread-pool-max", thread_pool_max); + VIR_IOTHREAD_SET_PROP_INT("thread-pool-min", thread_pool_min); } else { - VIR_IOTHREAD_SET_PROP("thread-pool-min", thread_pool_min); - VIR_IOTHREAD_SET_PROP("thread-pool-max", thread_pool_max); + VIR_IOTHREAD_SET_PROP_INT("thread-pool-min", thread_pool_min); + VIR_IOTHREAD_SET_PROP_INT("thread-pool-max", thread_pool_max); } -#undef VIR_IOTHREAD_SET_PROP +#undef VIR_IOTHREAD_SET_PROP_INT return 0; } -- 2.39.2

The qemu driver now accepts also _ULLONG as type for bigger numbers. Use the 'virTypedParamListAddUnsigned' helper to use the bigger typed parameter type if necessary to allow full range of the values while preserving compatibility. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d37e27000b..db47d63d85 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7853,8 +7853,7 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd) g_autoptr(virTypedParamList) params = virTypedParamListNew(); virTypedParameterPtr par; size_t npar = 0; - unsigned long long poll_max; - unsigned int poll_val; + unsigned long long poll_val; int thread_val; int rc; @@ -7876,20 +7875,20 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd) return false; } - if ((rc = vshCommandOptULongLong(ctl, cmd, "poll-max-ns", &poll_max)) < 0) + if ((rc = vshCommandOptULongLong(ctl, cmd, "poll-max-ns", &poll_val)) < 0) return false; if (rc > 0) - virTypedParamListAddULLong(params, poll_max, VIR_DOMAIN_IOTHREAD_POLL_MAX_NS); + virTypedParamListAddULLong(params, poll_val, VIR_DOMAIN_IOTHREAD_POLL_MAX_NS); - if ((rc = vshCommandOptUInt(ctl, cmd, "poll-grow", &poll_val)) < 0) + if ((rc = vshCommandOptULongLong(ctl, cmd, "poll-grow", &poll_val)) < 0) return false; if (rc > 0) - virTypedParamListAddUInt(params, poll_val, VIR_DOMAIN_IOTHREAD_POLL_GROW); + virTypedParamListAddUnsigned(params, poll_val, VIR_DOMAIN_IOTHREAD_POLL_GROW); - if ((rc = vshCommandOptUInt(ctl, cmd, "poll-shrink", &poll_val)) < 0) + if ((rc = vshCommandOptULongLong(ctl, cmd, "poll-shrink", &poll_val)) < 0) return false; if (rc > 0) - virTypedParamListAddUInt(params, poll_val, VIR_DOMAIN_IOTHREAD_POLL_SHRINK); + virTypedParamListAddUnsigned(params, poll_val, VIR_DOMAIN_IOTHREAD_POLL_SHRINK); if ((rc = vshCommandOptInt(ctl, cmd, "thread-pool-min", &thread_val)) < 0) return false; -- 2.39.2

On a Wednesday in 2023, Peter Krempa wrote: 9.3.0 Jano