[RFC PATCH 0/5] add support for histograms as a virTypedParameterType

This series extends https://listman.redhat.com/archives/libvir-list/2022-September/234197.html to support histogram statistics. This requires adding it as a virTypedParameterType. [1/5]: - add virHistogram as a virTypedParameterType - add utility functions for virHistogram [2/5] - add a global feature for histograms similar to strings [3/5] - add a flag for histograms similar to the one for strings to check compatibility for older clients. [4/5] - add histogram statistics which were previously ignored in the series linked above [5/5] - add RPC support for virHistogram Amneesh Singh (5): virtypedparam: add virHistogram as a virTypedParameterType add a global feature for supporting virHistogram virtypedparams: add VIR_TYPED_PARAM_HISTOGRAM_OKAY qemu_driver: add histograms to the stats remote: add virHistogram support for RPC as a virTypedParameterType include/libvirt/libvirt-common.h.in | 59 +++++++++-- src/admin/admin_server_dispatch.c | 6 +- src/ch/ch_driver.c | 1 + src/driver.c | 1 + src/esx/esx_driver.c | 1 + src/libvirt.c | 46 ++++++--- src/libvirt_internal.h | 5 + src/libvirt_private.syms | 7 ++ src/libvirt_public.syms | 6 ++ src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/network/bridge_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/qemu/qemu_driver.c | 42 +++++++- src/remote/remote_daemon_dispatch.c | 15 ++- src/remote/remote_driver.c | 16 +-- src/remote/remote_protocol.x | 16 +++ src/remote_protocol-structs | 13 +++ src/rpc/gendispatch.pl | 4 +- src/test/test_driver.c | 1 + src/util/virtypedparam-public.c | 101 +++++++++++++++++++ src/util/virtypedparam.c | 145 ++++++++++++++++++++++++++++ src/util/virtypedparam.h | 22 +++++ src/vz/vz_driver.c | 1 + tools/vsh.c | 5 + 25 files changed, 479 insertions(+), 38 deletions(-) -- 2.37.1

This patch adds virHistogram, a list of (x,y) pairs denoting a histogram as a virTypedParameterType. Signed-off-by: Amneesh Singh <natto@weirdnatto.in> --- include/libvirt/libvirt-common.h.in | 54 +++++++++-- src/libvirt_private.syms | 7 ++ src/libvirt_public.syms | 6 ++ src/util/virtypedparam-public.c | 101 ++++++++++++++++++++ src/util/virtypedparam.c | 138 ++++++++++++++++++++++++++++ src/util/virtypedparam.h | 22 +++++ tools/vsh.c | 5 + 7 files changed, 326 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt-common.h.in b/include/libvirt/libvirt-common.h.in index ccdbb2a100..11338d9191 100644 --- a/include/libvirt/libvirt-common.h.in +++ b/include/libvirt/libvirt-common.h.in @@ -128,6 +128,35 @@ typedef enum { # endif } virConnectCloseReason; +/** + * virHistogramBucket: + * + * Simple wrapper for containing the values corresponding to the X and Y axes + * of a histogram entry. + * + * Since: 8.8.0 + */ +typedef struct _virHistogramBucket virHistogramBucket; + +struct _virHistogramBucket { + long long x; + long long y; +}; + +/** + * virHistogram: + * + * Contains a list of virHistogramBuckets to represent a histogram. + * + * Since: 8.8.0 + */ +typedef struct _virHistogram virHistogram; + +struct _virHistogram { + size_t nbuckets; + virHistogramBucket *buckets; +}; + /** * virTypedParameterType: * @@ -136,13 +165,14 @@ typedef enum { * Since: 0.9.2 */ typedef enum { - VIR_TYPED_PARAM_INT = 1, /* integer case (Since: 0.9.2) */ - VIR_TYPED_PARAM_UINT = 2, /* unsigned integer case (Since: 0.9.2) */ - VIR_TYPED_PARAM_LLONG = 3, /* long long case (Since: 0.9.2) */ - VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case (Since: 0.9.2) */ - VIR_TYPED_PARAM_DOUBLE = 5, /* double case (Since: 0.9.2) */ - VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case (Since: 0.9.2) */ - VIR_TYPED_PARAM_STRING = 7, /* string case (Since: 0.9.8) */ + VIR_TYPED_PARAM_INT = 1, /* integer case (Since: 0.9.2) */ + VIR_TYPED_PARAM_UINT = 2, /* unsigned integer case (Since: 0.9.2) */ + VIR_TYPED_PARAM_LLONG = 3, /* long long case (Since: 0.9.2) */ + VIR_TYPED_PARAM_ULLONG = 4, /* unsigned long long case (Since: 0.9.2) */ + VIR_TYPED_PARAM_DOUBLE = 5, /* double case (Since: 0.9.2) */ + VIR_TYPED_PARAM_BOOLEAN = 6, /* boolean(character) case (Since: 0.9.2) */ + VIR_TYPED_PARAM_STRING = 7, /* string case (Since: 0.9.8) */ + VIR_TYPED_PARAM_HISTOGRAM = 8, /* histogram case (Since: 8.8.0) */ # ifdef VIR_ENUM_SENTINELS VIR_TYPED_PARAM_LAST /* (Since: 0.9.10) */ @@ -211,6 +241,7 @@ struct _virTypedParameter { double d; /* type is DOUBLE */ char b; /* type is BOOLEAN */ char *s; /* type is STRING, may not be NULL */ + virHistogram *h; /* type is HISTOGRAM, may not be NULL */ } value; /* parameter value */ }; @@ -254,6 +285,10 @@ int virTypedParamsGetString(virTypedParameterPtr params, int nparams, const char *name, const char **value); +int virTypedParamsGetHistogram(virTypedParameterPtr params, + int nparams, + const char *name, + virHistogram **value); int virTypedParamsAddInt(virTypedParameterPtr *params, int *nparams, @@ -290,6 +325,11 @@ int virTypedParamsAddString(virTypedParameterPtr *params, int *maxparams, const char *name, const char *value); +int virTypedParamsAddHistogram(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + virHistogram *value); int virTypedParamsAddStringList(virTypedParameterPtr *params, int *nparams, int *maxparams, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 25794bc2f4..c60b21babe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3517,12 +3517,19 @@ virTPMSwtpmSetupFeatureTypeFromString; # util/virtypedparam.h +virHistogramBucketToString; +virHistogramCopy; +virHistogramFree; +virHistogramFromString; +virHistogramNew; +virHistogramToString; virTypedParameterAssign; virTypedParameterToString; virTypedParameterTypeFromString; virTypedParameterTypeToString; virTypedParamListAddBoolean; virTypedParamListAddDouble; +virTypedParamListAddHistogram; virTypedParamListAddInt; virTypedParamListAddLLong; virTypedParamListAddString; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 297a2c436a..e5c04bd992 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -927,4 +927,10 @@ LIBVIRT_8.5.0 { virDomainAbortJobFlags; } LIBVIRT_8.4.0; +LIBVIRT_8.8.0 { + global: + virTypedParamsAddHistogram; + virTypedParamsGetHistogram; +} LIBVIRT_8.5.0; + # .... define new API here using predicted next version number .... diff --git a/src/util/virtypedparam-public.c b/src/util/virtypedparam-public.c index 5bd207d1e6..d9336337c3 100644 --- a/src/util/virtypedparam-public.c +++ b/src/util/virtypedparam-public.c @@ -106,6 +106,9 @@ virTypedParameterAssignFromStr(virTypedParameterPtr param, case VIR_TYPED_PARAM_STRING: param->value.s = g_strdup(val); break; + case VIR_TYPED_PARAM_HISTOGRAM: + param->value.h = virHistogramFromString(val); + break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected type %d for field %s"), type, name); @@ -432,6 +435,46 @@ virTypedParamsGetString(virTypedParameterPtr params, } +/** + * virTypedParamsGetHistogram: + * @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 virHistogram * value in @value. + * The function does not create a copy of the string and the caller must not + * free the virHistogram @value points to. The function fails with + * VIR_ERR_INVALID_ARG error if the parameter does not have the expected type. + * By passing NULL as @value, the function may be used to check presence and + * type of the parameter. + * + * Returns 1 on success, 0 when the parameter does not exist in @params, or + * -1 on error. + * + * Since: 8.8.0 + */ +int +virTypedParamsGetHistogram(virTypedParameterPtr params, + int nparams, + const char *name, + virHistogram **value) +{ + virTypedParameterPtr param; + + virResetLastError(); + + if (!(param = virTypedParamsGet(params, nparams, name))) + return 0; + + VIR_TYPED_PARAM_CHECK_TYPE(VIR_TYPED_PARAM_HISTOGRAM); + if (value) + *value = param->value.h; + + return 1; +} + + /** * virTypedParamsAddInt: * @params: pointer to the array of typed parameters @@ -774,6 +817,62 @@ virTypedParamsAddString(virTypedParameterPtr *params, return -1; } + +/** + * virTypedParamsAddHistogram: + * @params: pointer to the array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to find + * @value: the value to store into the new parameter + * + * Adds new parameter called @name with virHistogram * type and sets its value to + * @value. The function creates its own copy of virHistogram, which needs to + * be freed using virTypedParamsFree or virTypedParamsClear. If @params array + * points to NULL or to a space that is not large enough to accommodate the + * new parameter (@maxparams < @nparams + 1), the function allocates more + * space for it and updates @maxparams. On success, @nparams is incremented + * by one. The function fails with VIR_ERR_INVALID_ARG error if the parameter + * already exists in @params. + * + * Returns 0 on success, -1 on error. + * + * Since: 8.8.0 + */ +int +virTypedParamsAddHistogram(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + virHistogram *value) +{ + virHistogram *copy = NULL; + size_t max = *maxparams; + size_t n = *nparams; + + virResetLastError(); + + VIR_RESIZE_N(*params, max, n, 1); + *maxparams = max; + + copy = virHistogramCopy(value); + + if (virTypedParameterAssign(*params + n, name, + VIR_TYPED_PARAM_HISTOGRAM, copy) < 0) { + virHistogramFree(copy); + goto error; + } + + *nparams += 1; + return 0; + + error: + virDispatchError(NULL); + return -1; +} + + /** * virTypedParamsAddStringList: * @params: array of typed parameters @@ -889,6 +988,8 @@ virTypedParamsClear(virTypedParameterPtr params, for (i = 0; i < nparams; i++) { if (params[i].type == VIR_TYPED_PARAM_STRING) VIR_FREE(params[i].value.s); + else if (params[i].type == VIR_TYPED_PARAM_HISTOGRAM) + virHistogramFree(params[i].value.h); } } diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 2d7e4ab354..87fa69271b 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -40,6 +40,7 @@ VIR_ENUM_IMPL(virTypedParameter, "double", "boolean", "string", + "histogram", ); static int @@ -166,6 +167,71 @@ virTypedParamsCheck(virTypedParameterPtr params, return true; } + +char * +virHistogramBucketToString(virHistogramBucket *bucket) { + if (!bucket) + return NULL; + + return g_strdup_printf("(%lld - %lld)", bucket->x, bucket->y); +} + + +char * +virHistogramToString(virHistogram *histogram) +{ + size_t i; + g_autoptr(GString) value = g_string_new(""); + + if (!histogram) + return NULL; + + if (histogram->nbuckets == 0) + return g_strdup(""); + + for (i = 0; i < histogram->nbuckets; i++) { + g_autofree char *bucket_str = virHistogramBucketToString(histogram->buckets + i); + + g_string_append(value, bucket_str); + + if (i != histogram->nbuckets - 1) + g_string_append(value, ", "); + } + + return g_strdup(value->str); +} + + +virHistogram * +virHistogramFromString(const char *str) +{ + g_auto(GStrv) split_str = g_strsplit(str, ",", -1); + g_autoptr(virHistogram) hist = NULL; + unsigned int len; + size_t i; + + if (split_str == NULL) + return NULL; + + len = g_strv_length(split_str); + hist = virHistogramNew(len); + + for (i = 0; i < len; i++) { + virHistogramBucket *bucket = hist->buckets + i; + long long x, y; + + if (sscanf(split_str[i], + " (%lld - %lld)", + &x, &y) < 2) + return NULL; + + bucket->x = x; + bucket->y = y; + } + + return g_steal_pointer(&hist); +} + char * virTypedParameterToString(virTypedParameterPtr param) { @@ -193,6 +259,9 @@ virTypedParameterToString(virTypedParameterPtr param) case VIR_TYPED_PARAM_STRING: value = g_strdup(param->value.s); break; + case VIR_TYPED_PARAM_HISTOGRAM: + value = virHistogramToString(param->value.h); + break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected type %d for field %s"), @@ -239,6 +308,9 @@ virTypedParameterAssignValueVArgs(virTypedParameterPtr param, if (!param->value.s) param->value.s = g_strdup(""); break; + case VIR_TYPED_PARAM_HISTOGRAM: + param->value.h = va_arg(ap, virHistogram *); + break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected type %d for field %s"), type, @@ -347,6 +419,42 @@ virTypedParamsReplaceString(virTypedParameterPtr *params, } +virHistogram * +virHistogramNew(size_t size) +{ + virHistogram *histogram = g_new(virHistogram, 1); + histogram->nbuckets = size; + histogram->buckets = g_new0(virHistogramBucket, size); + + return histogram; +} + + +void +virHistogramFree(virHistogram *histogram) +{ + g_free(histogram->buckets); + g_free(histogram); +} + + +virHistogram * +virHistogramCopy(virHistogram *src) +{ + virHistogram *dst = NULL; + + if (!src) + return NULL; + + dst = g_new0(virHistogram, 1); + dst->nbuckets = src->nbuckets; + dst->buckets = g_new0(virHistogramBucket, dst->nbuckets); + memcpy(dst->buckets, src->buckets, dst->nbuckets * sizeof(virHistogramBucket)); + + return g_steal_pointer(&dst); +} + + int virTypedParamsCopy(virTypedParameterPtr *dst, virTypedParameterPtr src, @@ -365,6 +473,8 @@ virTypedParamsCopy(virTypedParameterPtr *dst, (*dst)[i].type = src[i].type; if (src[i].type == VIR_TYPED_PARAM_STRING) { (*dst)[i].value.s = g_strdup(src[i].value.s); + } else if (src[i].type == VIR_TYPED_PARAM_HISTOGRAM) { + (*dst)[i].value.h = virHistogramCopy(src[i].value.h); } else { (*dst)[i].value = src[i].value; } @@ -486,6 +596,8 @@ virTypedParamsRemoteFree(struct _virTypedParameterRemote *remote_params_val, g_free(remote_params_val[i].field); if (remote_params_val[i].value.type == VIR_TYPED_PARAM_STRING) g_free(remote_params_val[i].value.remote_typed_param_value.s); + else if (remote_params_val[i].value.type == VIR_TYPED_PARAM_HISTOGRAM) + g_free(remote_params_val[i].value.remote_typed_param_value.h); } g_free(remote_params_val); } @@ -591,6 +703,9 @@ virTypedParamsDeserialize(struct _virTypedParameterRemote *remote_params, case VIR_TYPED_PARAM_STRING: param->value.s = g_strdup(remote_param->value.remote_typed_param_value.s); break; + case VIR_TYPED_PARAM_HISTOGRAM: + param->value.h = virHistogramCopy(remote_param->value.remote_typed_param_value.h); + break; default: virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), param->type); @@ -696,6 +811,9 @@ virTypedParamsSerialize(virTypedParameterPtr params, case VIR_TYPED_PARAM_STRING: val->value.remote_typed_param_value.s = g_strdup(param->value.s); break; + case VIR_TYPED_PARAM_HISTOGRAM: + val->value.remote_typed_param_value.h = virHistogramCopy(param->value.h); + break; default: virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), param->type); @@ -929,3 +1047,23 @@ virTypedParamListAddDouble(virTypedParamList *list, return ret; } + +int virTypedParamListAddHistogram(virTypedParamList *list, + virHistogram *value, + const char *namefmt, + ...) +{ + virTypedParameterPtr par; + va_list ap; + int ret; + + if (!(par = virTypedParamListExtend(list)) || + virTypedParameterAssignValue(par, true, VIR_TYPED_PARAM_HISTOGRAM, value) < 0) + return -1; + + va_start(ap, namefmt); + ret = virTypedParamSetNameVPrintf(par, namefmt, ap); + va_end(ap); + + return ret; +} diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h index c4bc58ee8f..8138197493 100644 --- a/src/util/virtypedparam.h +++ b/src/util/virtypedparam.h @@ -45,6 +45,7 @@ struct _virTypedParameterRemoteValue { double d; char b; char *s; + virHistogram *h; } remote_typed_param_value; }; @@ -86,10 +87,25 @@ int virTypedParamsReplaceString(virTypedParameterPtr *params, const char *name, const char *value); +virHistogram *virHistogramNew(size_t size); + +void virHistogramFree(virHistogram *histogram); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virHistogram, + virHistogramFree); + +virHistogram *virHistogramCopy(virHistogram *src); + int virTypedParamsCopy(virTypedParameterPtr *dst, virTypedParameterPtr src, int nparams); +char *virHistogramBucketToString(virHistogramBucket *bucket); + +char *virHistogramToString(virHistogram *histogram); + +virHistogram *virHistogramFromString(const char *str); + char *virTypedParameterToString(virTypedParameterPtr param); void virTypedParamsRemoteFree(struct _virTypedParameterRemote *remote_params_val, @@ -177,3 +193,9 @@ int virTypedParamListAddDouble(virTypedParamList *list, const char *namefmt, ...) G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; + +int virTypedParamListAddHistogram(virTypedParamList *list, + virHistogram *value, + const char *namefmt, + ...) + G_GNUC_PRINTF(3, 4) G_GNUC_WARN_UNUSED_RESULT; diff --git a/tools/vsh.c b/tools/vsh.c index 0066504ebe..d808524568 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -45,6 +45,7 @@ #include "vircommand.h" #include "virstring.h" #include "virutil.h" +#include "virtypedparam.h" #ifdef WITH_READLINE /* For autocompletion */ @@ -1835,6 +1836,10 @@ vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) return g_strdup(item->value.s); break; + case VIR_TYPED_PARAM_HISTOGRAM: + return virHistogramToString(item->value.h); + break; + default: vshError(ctl, _("unimplemented parameter type %d"), item->type); exit(EXIT_FAILURE); -- 2.37.1

Signed-off-by: Amneesh Singh <natto@weirdnatto.in> --- src/ch/ch_driver.c | 1 + src/driver.c | 1 + src/esx/esx_driver.c | 1 + src/libvirt_internal.h | 5 +++++ src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/network/bridge_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_daemon_dispatch.c | 1 + src/test/test_driver.c | 1 + src/vz/vz_driver.c | 1 + 12 files changed, 16 insertions(+) diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c index db2a66d131..80afc4a870 100644 --- a/src/ch/ch_driver.c +++ b/src/ch/ch_driver.c @@ -927,6 +927,7 @@ chConnectSupportsFeature(virConnectPtr conn, case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: case VIR_DRV_FEATURE_FD_PASSING: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/driver.c b/src/driver.c index cea74bdf95..99ff963d2e 100644 --- a/src/driver.c +++ b/src/driver.c @@ -357,6 +357,7 @@ virDriverFeatureIsGlobal(virDrvFeature feat, * At this point everything supports them and thus also drivers need to * always advertise this feature */ case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: /* Feature flag exposes that the accidental switching of order of arguments * in the public API trampoline virNetworkUpdate is known. Updated clients * thus use the correct ordering with an updated server. All drivers must diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 9dc5489411..b8bac39c5b 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1028,6 +1028,7 @@ esxConnectSupportsFeature(virConnectPtr conn, int feature) case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: case VIR_DRV_FEATURE_FD_PASSING: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h index 1ae3e2b2e0..b8b4525e6d 100644 --- a/src/libvirt_internal.h +++ b/src/libvirt_internal.h @@ -132,6 +132,11 @@ typedef enum { * Whether the virNetworkUpdate() API implementation passes arguments to * the driver's callback in correct order. */ VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER = 16, + + /* + * Support for VIR_TYPED_PARAM_HISTOGRAM + */ + VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM = 17, } virDrvFeature; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 1b8b40e9e0..f3d47c83a5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5698,6 +5698,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int feature) case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: case VIR_DRV_FEATURE_FD_PASSING: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index d66c26221c..76cd612447 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1622,6 +1622,7 @@ lxcConnectSupportsFeature(virConnectPtr conn, int feature) case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: case VIR_DRV_FEATURE_FD_PASSING: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7c6430b4e3..fe5ee1f781 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -825,6 +825,7 @@ networkConnectSupportsFeature(virConnectPtr conn, int feature) case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: case VIR_DRV_FEATURE_FD_PASSING: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 5c18f5238b..2a7fbd3bde 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1914,6 +1914,7 @@ openvzConnectSupportsFeature(virConnectPtr conn G_GNUC_UNUSED, int feature) case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: case VIR_DRV_FEATURE_FD_PASSING: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8370b8593b..52e1f88568 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1187,6 +1187,7 @@ qemuConnectSupportsFeature(virConnectPtr conn, int feature) case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: case VIR_DRV_FEATURE_FD_PASSING: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 7efe58b36b..a5837dce26 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -4969,6 +4969,7 @@ static int remoteDispatchConnectSupportsFeature(virNetServer *server G_GNUC_UNUS case VIR_DRV_FEATURE_MIGRATION_V3: case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_XML_MIGRATABLE: case VIR_DRV_FEATURE_MIGRATION_OFFLINE: case VIR_DRV_FEATURE_MIGRATION_PARAMS: diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 686ff051a8..795c8e0798 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1666,6 +1666,7 @@ testConnectSupportsFeature(virConnectPtr conn G_GNUC_UNUSED, switch ((virDrvFeature) feature) { case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER: return 1; case VIR_DRV_FEATURE_MIGRATION_V2: diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 4f5e340d53..bcc4073520 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -3021,6 +3021,7 @@ vzConnectSupportsFeature(virConnectPtr conn G_GNUC_UNUSED, int feature) case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK: case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK: case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + case VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM: case VIR_DRV_FEATURE_XML_MIGRATABLE: default: return 0; -- 2.37.1

This patch adds a flag VIR_TYPED_PARAM_HISTOGRAM_OKAY which works similar to VIR_TYPED_PARAM_STRING_OKAY to maintain server-client compatibility. Signed-off-by: Amneesh Singh <natto@weirdnatto.in> --- include/libvirt/libvirt-common.h.in | 5 ++++ src/admin/admin_server_dispatch.c | 6 +++- src/libvirt.c | 46 +++++++++++++++++++---------- src/remote/remote_daemon_dispatch.c | 14 +++++---- src/remote/remote_driver.c | 16 ++++++---- src/rpc/gendispatch.pl | 4 +-- src/util/virtypedparam.c | 7 +++++ 7 files changed, 69 insertions(+), 29 deletions(-) diff --git a/include/libvirt/libvirt-common.h.in b/include/libvirt/libvirt-common.h.in index 11338d9191..39f96691c3 100644 --- a/include/libvirt/libvirt-common.h.in +++ b/include/libvirt/libvirt-common.h.in @@ -206,6 +206,11 @@ typedef enum { */ VIR_TYPED_PARAM_STRING_OKAY = 1 << 2, + /* Same as VIR_TYPED_PARAM_STRING_OKAY but for histograms + * Since: 8.8.0 + */ + VIR_TYPED_PARAM_HISTOGRAM_OKAY = 1 << 3, + } virTypedParameterFlags; /** diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c index b3e7be8965..0bf852b053 100644 --- a/src/admin/admin_server_dispatch.c +++ b/src/admin/admin_server_dispatch.c @@ -37,6 +37,10 @@ VIR_LOG_INIT("daemon.admin"); +#define VIR_TYPED_PARAM_OKAY_FLAGS \ + VIR_TYPED_PARAM_STRING_OKAY | \ + VIR_TYPED_PARAM_HISTOGRAM_OKAY + typedef struct daemonAdmClientPrivate daemonAdmClientPrivate; /* Separate private data for admin connection */ struct daemonAdmClientPrivate { @@ -322,7 +326,7 @@ adminDispatchClientGetInfo(virNetServer *server G_GNUC_UNUSED, ADMIN_CLIENT_INFO_PARAMETERS_MAX, (struct _virTypedParameterRemote **) &ret->params.params_val, &ret->params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) goto cleanup; rv = 0; diff --git a/src/libvirt.c b/src/libvirt.c index 748f2d8ba0..6dbb11aeb5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1331,12 +1331,36 @@ virTypedParameterValidateSet(virConnectPtr conn, int nparams) { int string_okay; + int histogram_okay; size_t i; string_okay = VIR_DRV_SUPPORTS_FEATURE(conn->driver, conn, VIR_DRV_FEATURE_TYPED_PARAM_STRING); if (string_okay < 0) return -1; + + histogram_okay = VIR_DRV_SUPPORTS_FEATURE(conn->driver, conn, + VIR_DRV_FEATURE_TYPED_PARAM_HISTOGRAM); + if (histogram_okay < 0) + return -1; + +#define VALIDATE_NULLABLE_PARAM_TYPE(LVAR, UVAR, UNIONMEM) \ + if (params[i].type == VIR_TYPED_PARAM_##UVAR) { \ + if (LVAR##_okay) { \ + if (!params[i].value.UNIONMEM) { \ + virReportInvalidArg(params, \ + _("NULL "#LVAR" parameter '%s'"), \ + params[i].field); \ + return -1; \ + } \ + } else { \ + virReportInvalidArg(params, \ + _(#LVAR" parameter '%s' unsupported"), \ + params[i].field); \ + return -1; \ + } \ + } + for (i = 0; i < nparams; i++) { if (strnlen(params[i].field, VIR_TYPED_PARAM_FIELD_LENGTH) == VIR_TYPED_PARAM_FIELD_LENGTH) { @@ -1346,21 +1370,13 @@ virTypedParameterValidateSet(virConnectPtr conn, params[i].field); return -1; } - if (params[i].type == VIR_TYPED_PARAM_STRING) { - if (string_okay) { - if (!params[i].value.s) { - virReportInvalidArg(params, - _("NULL string parameter '%s'"), - params[i].field); - return -1; - } - } else { - virReportInvalidArg(params, - _("string parameter '%s' unsupported"), - params[i].field); - return -1; - } - } + + VALIDATE_NULLABLE_PARAM_TYPE(string, STRING, s); + VALIDATE_NULLABLE_PARAM_TYPE(histogram, HISTOGRAM, h); + } + +#undef VALIDATE_NULLABLE_PARAM_TYPE + return 0; } diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index a5837dce26..888dee10b9 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -69,6 +69,10 @@ VIR_LOG_INIT("daemon.remote"); # define HYPER_TO_ULONG(_to, _from) (_to) = (_from) #endif +#define VIR_TYPED_PARAM_OKAY_FLAGS \ + VIR_TYPED_PARAM_STRING_OKAY | \ + VIR_TYPED_PARAM_HISTOGRAM_OKAY + struct daemonClientEventCallback { virNetServerClient *client; virNetServerProgram *program; @@ -1033,7 +1037,7 @@ remoteRelayDomainEventTunable(virConnectPtr conn, REMOTE_DOMAIN_EVENT_TUNABLE_MAX, (struct _virTypedParameterRemote **) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) return -1; data.callbackID = callback->callbackID; @@ -1173,7 +1177,7 @@ remoteRelayDomainEventJobCompleted(virConnectPtr conn, REMOTE_DOMAIN_JOB_STATS_MAX, (struct _virTypedParameterRemote **) &data.params.params_val, &data.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) return -1; data.callbackID = callback->callbackID; @@ -5468,7 +5472,7 @@ remoteDispatchDomainGetJobStats(virNetServer *server G_GNUC_UNUSED, REMOTE_DOMAIN_JOB_STATS_MAX, (struct _virTypedParameterRemote **) &ret->params.params_val, &ret->params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) goto cleanup; rv = 0; @@ -6757,7 +6761,7 @@ remoteDispatchConnectGetAllDomainStats(virNetServer *server G_GNUC_UNUSED, REMOTE_CONNECT_GET_ALL_DOMAIN_STATS_MAX, (struct _virTypedParameterRemote **) &dst->params.params_val, &dst->params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) goto cleanup; } } else { @@ -7308,7 +7312,7 @@ remoteDispatchDomainGetGuestInfo(virNetServer *server G_GNUC_UNUSED, REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX, (struct _virTypedParameterRemote **) &ret->params.params_val, &ret->params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) goto cleanup; rv = 0; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a4efe101a3..d226bedf19 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -72,6 +72,10 @@ VIR_LOG_INIT("remote.remote_driver"); # define HYPER_TO_ULONG(_to, _from) (_to) = (_from) #endif +#define VIR_TYPED_PARAM_OKAY_FLAGS \ + VIR_TYPED_PARAM_STRING_OKAY | \ + VIR_TYPED_PARAM_HISTOGRAM_OKAY + static bool inside_daemon; static bool monolithic_daemon; @@ -6918,7 +6922,7 @@ remoteDomainMigrateBegin3Params(virDomainPtr domain, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX, (struct _virTypedParameterRemote **) &args.params.params_val, &args.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_begin3_params_args, (char *) &args); goto cleanup; @@ -6980,7 +6984,7 @@ remoteDomainMigratePrepare3Params(virConnectPtr dconn, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX, (struct _virTypedParameterRemote **) &args.params.params_val, &args.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_prepare3_params_args, (char *) &args); goto cleanup; @@ -7062,7 +7066,7 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX, (struct _virTypedParameterRemote **) &args.params.params_val, &args.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel3_params_args, (char *) &args); goto cleanup; @@ -7148,7 +7152,7 @@ remoteDomainMigratePerform3Params(virDomainPtr dom, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX, (struct _virTypedParameterRemote **) &args.params.params_val, &args.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_perform3_params_args, (char *) &args); goto cleanup; @@ -7215,7 +7219,7 @@ remoteDomainMigrateFinish3Params(virConnectPtr dconn, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX, (struct _virTypedParameterRemote **) &args.params.params_val, &args.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_finish3_params_args, (char *) &args); goto cleanup; @@ -7283,7 +7287,7 @@ remoteDomainMigrateConfirm3Params(virDomainPtr domain, REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX, (struct _virTypedParameterRemote **) &args.params.params_val, &args.params.params_len, - VIR_TYPED_PARAM_STRING_OKAY) < 0) { + VIR_TYPED_PARAM_OKAY_FLAGS) < 0) { xdr_free((xdrproc_t) xdr_remote_domain_migrate_confirm3_params_args, (char *) &args); goto cleanup; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 5f2b163ea0..e5615735fd 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -961,7 +961,7 @@ elsif ($mode eq "server") { " $2,\n" . " (struct _virTypedParameterRemote **) &ret->$1.$1_val,\n" . " &ret->$1.$1_len,\n" . - " VIR_TYPED_PARAM_STRING_OKAY) < 0)\n" . + " VIR_TYPED_PARAM_STRING_OKAY | VIR_TYPED_PARAM_HISTOGRAM_OKAY) < 0)\n" . " goto cleanup;\n"); push(@free_list, " virTypedParamsFree($1, $1_len);"); @@ -1445,7 +1445,7 @@ elsif ($mode eq "client") { " $2,\n" . " (struct _virTypedParameterRemote **) &args.$1.$1_val,\n" . " &args.$1.$1_len,\n" . - " VIR_TYPED_PARAM_STRING_OKAY) < 0) {\n" . + " VIR_TYPED_PARAM_STRING_OKAY | VIR_TYPED_PARAM_HISTOGRAM_OKAY) < 0) {\n" . " xdr_free((xdrproc_t)xdr_$call->{args}, (char *)&args);\n" . " goto done;\n" . " }"); diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 87fa69271b..b6ffa64135 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -785,6 +785,13 @@ virTypedParamsSerialize(virTypedParameterPtr params, continue; } + if (!param->type || + (!(flags & VIR_TYPED_PARAM_HISTOGRAM_OKAY) && + param->type == VIR_TYPED_PARAM_HISTOGRAM)) { + --params_len; + continue; + } + /* This will be either freed by virNetServerDispatchCall or call(), * depending on the calling side, i.e. server or client */ val->field = g_strdup(param->field); -- 2.37.1

This patch adds histograms to the stats which were previously ignored due to them not being a virTypedParameterType earlier. Signed-off-by: Amneesh Singh <natto@weirdnatto.in> --- src/qemu/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 52e1f88568..bc6f484d88 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17989,13 +17989,50 @@ qemuDomainAddStatsFromHashTable(GHashTable *stats, case QEMU_MONITOR_QUERY_STATS_TYPE_INSTANT: type = "cur"; break; - case QEMU_MONITOR_QUERY_STATS_TYPE_PEAK: type = "max"; break; - case QEMU_MONITOR_QUERY_STATS_TYPE_LOG2_HISTOGRAM: case QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM: + case QEMU_MONITOR_QUERY_STATS_TYPE_LOG2_HISTOGRAM: { + virHistogram *histogram; + size_t size; + size_t i; + int flag = 0; + + if (!virJSONValueIsArray(value)) + continue; + + if (data->type == QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM && + data->bucket_size == 0) + continue; + + size = virJSONValueArraySize(value); + histogram = virHistogramNew(size); + + for (i = 0; i < size; i++) { + virHistogramBucket *bucket = histogram->buckets + i; + virJSONValue *current = virJSONValueArrayGet(value, i); + + bucket->x = (data->type == QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM ? + (i + 1) * data->bucket_size : + 1LL << i); + + if (virJSONValueGetNumberLong(current, &(bucket->y)) < 0) { + flag = 1; + break; + } + } + + if (flag) { + virHistogramFree(histogram); + continue; + } + + ignore_value(virTypedParamListAddHistogram(params, histogram, "%s.%s", + prefix, key)); + } + case QEMU_MONITOR_QUERY_STATS_TYPE_LAST: default: continue; -- 2.37.1

Signed-off-by: Amneesh Singh <natto@weirdnatto.in> --- src/remote/remote_protocol.x | 16 ++++++++++++++++ src/remote_protocol-structs | 13 +++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 79ffc63f03..c23e184d3a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -418,6 +418,20 @@ struct remote_vcpu_info { int cpu; }; +const REMOTE_HISTOGRAM_BUCKET_MAX = 32; + +struct remote_histogram_bucket { + hyper x; + hyper y; +}; + +struct remote_histogram { + remote_histogram_bucket buckets<REMOTE_HISTOGRAM_BUCKET_MAX>; + unsigned int nbuckets; +}; + +typedef remote_histogram *remote_nonnull_histogram; + /* Wire encoding of virTypedParameter. * Note the enum (type) which must remain binary compatible. */ @@ -436,6 +450,8 @@ union remote_typed_param_value switch (int type) { int b; case VIR_TYPED_PARAM_STRING: remote_nonnull_string s; + case VIR_TYPED_PARAM_HISTOGRAM: + remote_nonnull_histogram h; }; struct remote_typed_param { diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index ca5222439d..95fc4a15fe 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -73,6 +73,18 @@ struct remote_vcpu_info { uint64_t cpu_time; int cpu; }; +struct remote_histogram_bucket { + int64_t x; + int64_t y; +}; +struct remote_histogram { + struct { + u_int buckets_len; + remote_histogram_bucket * buckets_val; + } buckets; + u_int nbuckets; +}; +typedef remote_histogram *remote_nonnull_histogram; struct remote_typed_param_value { int type; union { @@ -83,6 +95,7 @@ struct remote_typed_param_value { double d; int b; remote_nonnull_string s; + remote_nonnull_histogram h; } remote_typed_param_value_u; }; struct remote_typed_param { -- 2.37.1
participants (1)
-
Amneesh Singh