1. Patchew
  2. Libvirt
  3. [RFC PATCH 0/6] qemu: add support for query-stats-schemas
  4. View patch

[RFC PATCH 1/6] qemu_monitor: add qemuMonitorQueryStatsSchema

Amneesh Singh posted 6 patches 3 years, 5 months ago
[RFC PATCH 1/6] qemu_monitor: add qemuMonitorQueryStatsSchema
Posted by Amneesh Singh 3 years, 5 months ago 
Related: https://gitlab.com/libvirt/libvirt/-/issues/276

This patch adds a simple API for "query-stats-schemas" QMP command

Signed-off-by: Amneesh Singh <natto@weirdnatto.in>
---
 src/qemu/qemu_monitor.c      | 29 +++++++++++
 src/qemu/qemu_monitor.h      | 35 ++++++++++++++
 src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_monitor_json.h |  4 ++
 4 files changed, 161 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c2808c75a3..9581e90796 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4355,6 +4355,35 @@ qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsProviderType provider_type
 }
 
 
+VIR_ENUM_IMPL(qemuMonitorQueryStatsUnit,
+              QEMU_MONITOR_QUERY_STATS_UNIT_LAST,
+              "bytes",
+              "seconds",
+              "cycles",
+              "boolean",
+);
+
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsType,
+              QEMU_MONITOR_QUERY_STATS_TYPE_LAST,
+              "cumulative",
+              "instant",
+              "peak",
+              "linear-histogram",
+              "log2-histogram",
+);
+
+
+GHashTable *
+qemuMonitorQueryStatsSchema(qemuMonitor *mon,
+                            qemuMonitorQueryStatsProviderType provider_type)
+{
+    QEMU_CHECK_MONITOR_NULL(mon);
+
+    return qemuMonitorJSONQueryStatsSchema(mon, provider_type);
+}
+
+
 virJSONValue *
 qemuMonitorQueryStats(qemuMonitor *mon,
                       qemuMonitorQueryStatsTargetType target,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 63269e15bc..4c817dea20 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1521,6 +1521,41 @@ qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsProviderType provider_type
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorQueryStatsProvider,
                               qemuMonitorQueryStatsProviderFree);
 
+typedef enum {
+    QEMU_MONITOR_QUERY_STATS_UNIT_BYTES,
+    QEMU_MONITOR_QUERY_STATS_UNIT_SECONDS,
+    QEMU_MONITOR_QUERY_STATS_UNIT_CYCLES,
+    QEMU_MONITOR_QUERY_STATS_UNIT_BOOLEAN,
+    QEMU_MONITOR_QUERY_STATS_UNIT_LAST,
+} qemuMonitorQueryStatsUnitType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsUnit);
+
+typedef enum {
+    QEMU_MONITOR_QUERY_STATS_TYPE_CUMULATIVE,
+    QEMU_MONITOR_QUERY_STATS_TYPE_INSTANT,
+    QEMU_MONITOR_QUERY_STATS_TYPE_PEAK,
+    QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM,
+    QEMU_MONITOR_QUERY_STATS_TYPE_LOG2_HISTOGRAM,
+    QEMU_MONITOR_QUERY_STATS_TYPE_LAST,
+} qemuMonitorQueryStatsTypeType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsType);
+
+typedef struct _qemuMonitorQueryStatsSchemaData qemuMonitorQueryStatsSchemaData;
+struct _qemuMonitorQueryStatsSchemaData {
+    qemuMonitorQueryStatsTargetType target;
+    qemuMonitorQueryStatsUnitType unit;
+    qemuMonitorQueryStatsTypeType type;
+    unsigned int bucket_size;
+    int base;
+    int exponent;
+};
+
+GHashTable *
+qemuMonitorQueryStatsSchema(qemuMonitor *mon,
+                            qemuMonitorQueryStatsProviderType provider_type);
+
 virJSONValue *
 qemuMonitorQueryStats(qemuMonitor *mon,
                       qemuMonitorQueryStatsTargetType target,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 70fba50e6c..f822a8908c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8575,6 +8575,99 @@ qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
     return qemuMonitorJSONCheckError(cmd, reply);
 }
 
+static GHashTable *
+qemuMonitorJSONExtractQueryStatsSchema(virJSONValue *json)
+{
+    g_autoptr(GHashTable) schema = virHashNew(g_free);
+    size_t i;
+
+    for (i = 0; i < virJSONValueArraySize(json); i++) {
+        virJSONValue *obj, *stats;
+        const char *target_str;
+        int target;
+        size_t j;
+
+        obj = virJSONValueArrayGet(json, i);
+
+        if (!virJSONValueIsObject(obj))
+            continue;
+
+        stats = virJSONValueObjectGetArray(obj, "stats");
+
+        if (!virJSONValueIsArray(stats))
+            continue;
+
+        target_str = virJSONValueObjectGetString(obj, "target");
+        target = qemuMonitorQueryStatsTargetTypeFromString(target_str);
+
+        for (j = 0; j < virJSONValueArraySize(stats); j++) {
+            virJSONValue *stat = virJSONValueArrayGet(stats, j);
+            const char *name, *type_str, *unit_str;
+            qemuMonitorQueryStatsSchemaData *data;
+            int type, unit;
+
+            if (!virJSONValueIsObject(stat))
+                continue;
+
+            name = virJSONValueObjectGetString(stat, "name");
+
+            if (!name)
+                continue;
+
+            type_str = virJSONValueObjectGetString(stat, "type");
+            unit_str = virJSONValueObjectGetString(stat, "unit");
+            type = qemuMonitorQueryStatsTypeTypeFromString(type_str);
+            unit = qemuMonitorQueryStatsUnitTypeFromString(unit_str);
+
+            data = g_new0(qemuMonitorQueryStatsSchemaData, 1);
+            data->target = (target == -1) ? QEMU_MONITOR_QUERY_STATS_TARGET_LAST : target;
+            data->type = (type == -1) ? QEMU_MONITOR_QUERY_STATS_TYPE_LAST : type;
+            data->unit = (unit == -1) ? QEMU_MONITOR_QUERY_STATS_UNIT_LAST : unit;
+
+            if (virJSONValueObjectGetNumberInt(stat, "base", &data->base) < 0 ||
+                virJSONValueObjectGetNumberInt(stat, "exponent", &data->exponent) < 0)
+                data->base = 0, data->exponent = 0;
+
+            /* a base of zero means that there is simply no scale, data->exponent is
+               set to 0 just for safety measures */
+
+            if (data->type == QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM &&
+                virJSONValueObjectGetNumberUint(stat, "bucket-size", &data->bucket_size) < 0)
+                data->bucket_size = 0;
+
+            virHashAddEntry(schema, name, data);
+        }
+    }
+
+    return g_steal_pointer(&schema);
+}
+
+GHashTable *
+qemuMonitorJSONQueryStatsSchema(qemuMonitor *mon,
+                                qemuMonitorQueryStatsProviderType provider_type)
+{
+    g_autoptr(virJSONValue) cmd = NULL;
+    g_autoptr(virJSONValue) reply = NULL;
+    virJSONValue *ret;
+
+    const char *type_str = qemuMonitorQueryStatsProviderTypeToString(provider_type);
+
+    if (!(cmd = qemuMonitorJSONMakeCommand("query-stats-schemas",
+                                           "S:provider", type_str,
+                                           NULL)))
+        return NULL;
+
+    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+        return NULL;
+
+    if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+        return NULL;
+
+    ret = virJSONValueObjectGetArray(reply, "return");
+
+    return qemuMonitorJSONExtractQueryStatsSchema(ret);
+}
+
 
 /**
  * qemuMonitorJSONQueryStats:
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index a53e6423df..c910e46504 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -813,6 +813,10 @@ int
 qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
                               const char *uri);
 
+GHashTable *
+qemuMonitorJSONQueryStatsSchema(qemuMonitor *mon,
+                                qemuMonitorQueryStatsProviderType provider_type);
+
 virJSONValue *
 qemuMonitorJSONQueryStats(qemuMonitor *mon,
                           qemuMonitorQueryStatsTargetType target,
-- 
2.37.1
Re: [RFC PATCH 1/6] qemu_monitor: add qemuMonitorQueryStatsSchema
Posted by Kristina Hanicova 3 years, 5 months ago 
Hi,

just a few notes on the code quality.

On Wed, Sep 7, 2022 at 12:34 PM Amneesh Singh <natto@weirdnatto.in> wrote:

> Related: https://gitlab.com/libvirt/libvirt/-/issues/276
>
> This patch adds a simple API for "query-stats-schemas" QMP command
>
> Signed-off-by: Amneesh Singh <natto@weirdnatto.in>
> ---
>  src/qemu/qemu_monitor.c      | 29 +++++++++++
>  src/qemu/qemu_monitor.h      | 35 ++++++++++++++
>  src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |  4 ++
>  4 files changed, 161 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index c2808c75a3..9581e90796 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4355,6 +4355,35 @@
> qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsProviderType
> provider_type
>  }
>
>
> +VIR_ENUM_IMPL(qemuMonitorQueryStatsUnit,
> +              QEMU_MONITOR_QUERY_STATS_UNIT_LAST,
> +              "bytes",
> +              "seconds",
> +              "cycles",
> +              "boolean",
> +);
> +
> +
> +VIR_ENUM_IMPL(qemuMonitorQueryStatsType,
> +              QEMU_MONITOR_QUERY_STATS_TYPE_LAST,
> +              "cumulative",
> +              "instant",
> +              "peak",
> +              "linear-histogram",
> +              "log2-histogram",
> +);
> +
> +
> +GHashTable *
> +qemuMonitorQueryStatsSchema(qemuMonitor *mon,
> +                            qemuMonitorQueryStatsProviderType
> provider_type)
> +{
> +    QEMU_CHECK_MONITOR_NULL(mon);
> +
> +    return qemuMonitorJSONQueryStatsSchema(mon, provider_type);
> +}
> +
> +
>  virJSONValue *
>  qemuMonitorQueryStats(qemuMonitor *mon,
>                        qemuMonitorQueryStatsTargetType target,
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 63269e15bc..4c817dea20 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1521,6 +1521,41 @@
> qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsProviderType
> provider_type
>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorQueryStatsProvider,
>                                qemuMonitorQueryStatsProviderFree);
>
> +typedef enum {
> +    QEMU_MONITOR_QUERY_STATS_UNIT_BYTES,
> +    QEMU_MONITOR_QUERY_STATS_UNIT_SECONDS,
> +    QEMU_MONITOR_QUERY_STATS_UNIT_CYCLES,
> +    QEMU_MONITOR_QUERY_STATS_UNIT_BOOLEAN,
> +    QEMU_MONITOR_QUERY_STATS_UNIT_LAST,
> +} qemuMonitorQueryStatsUnitType;
> +
> +VIR_ENUM_DECL(qemuMonitorQueryStatsUnit);
> +
> +typedef enum {
> +    QEMU_MONITOR_QUERY_STATS_TYPE_CUMULATIVE,
> +    QEMU_MONITOR_QUERY_STATS_TYPE_INSTANT,
> +    QEMU_MONITOR_QUERY_STATS_TYPE_PEAK,
> +    QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM,
> +    QEMU_MONITOR_QUERY_STATS_TYPE_LOG2_HISTOGRAM,
> +    QEMU_MONITOR_QUERY_STATS_TYPE_LAST,
> +} qemuMonitorQueryStatsTypeType;
> +
> +VIR_ENUM_DECL(qemuMonitorQueryStatsType);
> +
> +typedef struct _qemuMonitorQueryStatsSchemaData
> qemuMonitorQueryStatsSchemaData;
> +struct _qemuMonitorQueryStatsSchemaData {
> +    qemuMonitorQueryStatsTargetType target;
> +    qemuMonitorQueryStatsUnitType unit;
> +    qemuMonitorQueryStatsTypeType type;
> +    unsigned int bucket_size;
> +    int base;
> +    int exponent;
> +};
> +
> +GHashTable *
> +qemuMonitorQueryStatsSchema(qemuMonitor *mon,
> +                            qemuMonitorQueryStatsProviderType
> provider_type);
> +
>  virJSONValue *
>  qemuMonitorQueryStats(qemuMonitor *mon,
>                        qemuMonitorQueryStatsTargetType target,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 70fba50e6c..f822a8908c 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8575,6 +8575,99 @@ qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
>      return qemuMonitorJSONCheckError(cmd, reply);
>  }
>
> +static GHashTable *
> +qemuMonitorJSONExtractQueryStatsSchema(virJSONValue *json)
> +{
> +    g_autoptr(GHashTable) schema = virHashNew(g_free);
> +    size_t i;
> +
> +    for (i = 0; i < virJSONValueArraySize(json); i++) {
> +        virJSONValue *obj, *stats;
> +        const char *target_str;
> +        int target;
> +        size_t j;
> +
> +        obj = virJSONValueArrayGet(json, i);
> +
> +        if (!virJSONValueIsObject(obj))
> +            continue;
> +
> +        stats = virJSONValueObjectGetArray(obj, "stats");
> +
> +        if (!virJSONValueIsArray(stats))
> +            continue;
> +
> +        target_str = virJSONValueObjectGetString(obj, "target");
> +        target = qemuMonitorQueryStatsTargetTypeFromString(target_str);
> +
> +        for (j = 0; j < virJSONValueArraySize(stats); j++) {
> +            virJSONValue *stat = virJSONValueArrayGet(stats, j);
> +            const char *name, *type_str, *unit_str;
> +            qemuMonitorQueryStatsSchemaData *data;
> +            int type, unit;
>

We try to declare each variable on a separate line, even when there is more
of the same type.

In this case, I would also declare (and initialize) the variable 'stat' as
the last, as its value is
checked right after its initialization.


> +
> +            if (!virJSONValueIsObject(stat))
> +                continue;
> +
> +            name = virJSONValueObjectGetString(stat, "name");
> +
>

I prefer checks that look like:

    if (!(name = virJSONValueObjectGetString(stat, "name")))
        continue;

But that's just my personal preference and you can ignore this note if you
do not like it.

+            if (!name)
> +                continue;
> +
> +            type_str = virJSONValueObjectGetString(stat, "type");
> +            unit_str = virJSONValueObjectGetString(stat, "unit");
> +            type = qemuMonitorQueryStatsTypeTypeFromString(type_str);
> +            unit = qemuMonitorQueryStatsUnitTypeFromString(unit_str);
> +
> +            data = g_new0(qemuMonitorQueryStatsSchemaData, 1);
> +            data->target = (target == -1) ?
> QEMU_MONITOR_QUERY_STATS_TARGET_LAST : target;
> +            data->type = (type == -1) ?
> QEMU_MONITOR_QUERY_STATS_TYPE_LAST : type;
> +            data->unit = (unit == -1) ?
> QEMU_MONITOR_QUERY_STATS_UNIT_LAST : unit;
> +
> +            if (virJSONValueObjectGetNumberInt(stat, "base", &data->base)
> < 0 ||
> +                virJSONValueObjectGetNumberInt(stat, "exponent",
> &data->exponent) < 0)
> +                data->base = 0, data->exponent = 0;
>

Definitely split this line into two separate lines please. Operator comma
makes the code very unclean
and should not be used aside from a few exceptions in 'for' and 'while'
loops.


> +
> +            /* a base of zero means that there is simply no scale,
> data->exponent is
> +               set to 0 just for safety measures */
> +
> +            if (data->type ==
> QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM &&
> +                virJSONValueObjectGetNumberUint(stat, "bucket-size",
> &data->bucket_size) < 0)
> +                data->bucket_size = 0;
> +
> +            virHashAddEntry(schema, name, data);
> +        }
> +    }
> +
> +    return g_steal_pointer(&schema);
> +}
> +
> +GHashTable *
> +qemuMonitorJSONQueryStatsSchema(qemuMonitor *mon,
> +                                qemuMonitorQueryStatsProviderType
> provider_type)
> +{
> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) reply = NULL;
> +    virJSONValue *ret;
> +
> +    const char *type_str =
> qemuMonitorQueryStatsProviderTypeToString(provider_type);
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-stats-schemas",
> +                                           "S:provider", type_str,
> +                                           NULL)))

+        return NULL;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        return NULL;
> +
> +    if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
> +        return NULL;
> +
> +    ret = virJSONValueObjectGetArray(reply, "return");
> +
> +    return qemuMonitorJSONExtractQueryStatsSchema(ret);
> +}
> +
>
>  /**
>   * qemuMonitorJSONQueryStats:
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index a53e6423df..c910e46504 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -813,6 +813,10 @@ int
>  qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
>                                const char *uri);
>
> +GHashTable *
> +qemuMonitorJSONQueryStatsSchema(qemuMonitor *mon,
> +                                qemuMonitorQueryStatsProviderType
> provider_type);
> +
>  virJSONValue *
>  qemuMonitorJSONQueryStats(qemuMonitor *mon,
>                            qemuMonitorQueryStatsTargetType target,
> --
> 2.37.1
>
>
Otherwise nice:)

Kristina
Re: [RFC PATCH 1/6] qemu_monitor: add qemuMonitorQueryStatsSchema
Posted by Martin Kletzander 3 years, 5 months ago 
On Wed, Sep 07, 2022 at 02:23:24PM +0200, Kristina Hanicova wrote:
>Hi,
>
>just a few notes on the code quality.
>
>On Wed, Sep 7, 2022 at 12:34 PM Amneesh Singh <natto@weirdnatto.in> wrote:
>
>> Related: https://gitlab.com/libvirt/libvirt/-/issues/276
>>
>> This patch adds a simple API for "query-stats-schemas" QMP command
>>
>> Signed-off-by: Amneesh Singh <natto@weirdnatto.in>
>> ---
>>  src/qemu/qemu_monitor.c      | 29 +++++++++++
>>  src/qemu/qemu_monitor.h      | 35 ++++++++++++++
>>  src/qemu/qemu_monitor_json.c | 93 ++++++++++++++++++++++++++++++++++++
>>  src/qemu/qemu_monitor_json.h |  4 ++
>>  4 files changed, 161 insertions(+)
>>
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index c2808c75a3..9581e90796 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -4355,6 +4355,35 @@
>> qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsProviderType
>> provider_type
>>  }
>>
>>
>> +VIR_ENUM_IMPL(qemuMonitorQueryStatsUnit,
>> +              QEMU_MONITOR_QUERY_STATS_UNIT_LAST,
>> +              "bytes",
>> +              "seconds",
>> +              "cycles",
>> +              "boolean",
>> +);
>> +
>> +
>> +VIR_ENUM_IMPL(qemuMonitorQueryStatsType,
>> +              QEMU_MONITOR_QUERY_STATS_TYPE_LAST,
>> +              "cumulative",
>> +              "instant",
>> +              "peak",
>> +              "linear-histogram",
>> +              "log2-histogram",
>> +);
>> +
>> +
>> +GHashTable *
>> +qemuMonitorQueryStatsSchema(qemuMonitor *mon,
>> +                            qemuMonitorQueryStatsProviderType
>> provider_type)
>> +{
>> +    QEMU_CHECK_MONITOR_NULL(mon);
>> +
>> +    return qemuMonitorJSONQueryStatsSchema(mon, provider_type);
>> +}
>> +
>> +
>>  virJSONValue *
>>  qemuMonitorQueryStats(qemuMonitor *mon,
>>                        qemuMonitorQueryStatsTargetType target,
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index 63269e15bc..4c817dea20 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -1521,6 +1521,41 @@
>> qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsProviderType
>> provider_type
>>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorQueryStatsProvider,
>>                                qemuMonitorQueryStatsProviderFree);
>>
>> +typedef enum {
>> +    QEMU_MONITOR_QUERY_STATS_UNIT_BYTES,
>> +    QEMU_MONITOR_QUERY_STATS_UNIT_SECONDS,
>> +    QEMU_MONITOR_QUERY_STATS_UNIT_CYCLES,
>> +    QEMU_MONITOR_QUERY_STATS_UNIT_BOOLEAN,
>> +    QEMU_MONITOR_QUERY_STATS_UNIT_LAST,
>> +} qemuMonitorQueryStatsUnitType;
>> +
>> +VIR_ENUM_DECL(qemuMonitorQueryStatsUnit);
>> +
>> +typedef enum {
>> +    QEMU_MONITOR_QUERY_STATS_TYPE_CUMULATIVE,
>> +    QEMU_MONITOR_QUERY_STATS_TYPE_INSTANT,
>> +    QEMU_MONITOR_QUERY_STATS_TYPE_PEAK,
>> +    QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM,
>> +    QEMU_MONITOR_QUERY_STATS_TYPE_LOG2_HISTOGRAM,
>> +    QEMU_MONITOR_QUERY_STATS_TYPE_LAST,
>> +} qemuMonitorQueryStatsTypeType;
>> +
>> +VIR_ENUM_DECL(qemuMonitorQueryStatsType);
>> +
>> +typedef struct _qemuMonitorQueryStatsSchemaData
>> qemuMonitorQueryStatsSchemaData;
>> +struct _qemuMonitorQueryStatsSchemaData {
>> +    qemuMonitorQueryStatsTargetType target;
>> +    qemuMonitorQueryStatsUnitType unit;
>> +    qemuMonitorQueryStatsTypeType type;
>> +    unsigned int bucket_size;
>> +    int base;
>> +    int exponent;
>> +};
>> +
>> +GHashTable *
>> +qemuMonitorQueryStatsSchema(qemuMonitor *mon,
>> +                            qemuMonitorQueryStatsProviderType
>> provider_type);
>> +
>>  virJSONValue *
>>  qemuMonitorQueryStats(qemuMonitor *mon,
>>                        qemuMonitorQueryStatsTargetType target,
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 70fba50e6c..f822a8908c 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -8575,6 +8575,99 @@ qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
>>      return qemuMonitorJSONCheckError(cmd, reply);
>>  }
>>
>> +static GHashTable *
>> +qemuMonitorJSONExtractQueryStatsSchema(virJSONValue *json)
>> +{
>> +    g_autoptr(GHashTable) schema = virHashNew(g_free);
>> +    size_t i;
>> +
>> +    for (i = 0; i < virJSONValueArraySize(json); i++) {
>> +        virJSONValue *obj, *stats;
>> +        const char *target_str;
>> +        int target;
>> +        size_t j;
>> +
>> +        obj = virJSONValueArrayGet(json, i);
>> +
>> +        if (!virJSONValueIsObject(obj))
>> +            continue;
>> +
>> +        stats = virJSONValueObjectGetArray(obj, "stats");
>> +
>> +        if (!virJSONValueIsArray(stats))
>> +            continue;
>> +
>> +        target_str = virJSONValueObjectGetString(obj, "target");
>> +        target = qemuMonitorQueryStatsTargetTypeFromString(target_str);
>> +
>> +        for (j = 0; j < virJSONValueArraySize(stats); j++) {
>> +            virJSONValue *stat = virJSONValueArrayGet(stats, j);
>> +            const char *name, *type_str, *unit_str;
>> +            qemuMonitorQueryStatsSchemaData *data;
>> +            int type, unit;
>>
>
>We try to declare each variable on a separate line, even when there is more
>of the same type.
>
>In this case, I would also declare (and initialize) the variable 'stat' as
>the last, as its value is
>checked right after its initialization.
>
>
>> +
>> +            if (!virJSONValueIsObject(stat))
>> +                continue;
>> +
>> +            name = virJSONValueObjectGetString(stat, "name");
>> +
>>
>
>I prefer checks that look like:
>
>    if (!(name = virJSONValueObjectGetString(stat, "name")))
>        continue;
>
>But that's just my personal preference and you can ignore this note if you
>do not like it.
>
>+            if (!name)
>> +                continue;
>> +
>> +            type_str = virJSONValueObjectGetString(stat, "type");
>> +            unit_str = virJSONValueObjectGetString(stat, "unit");
>> +            type = qemuMonitorQueryStatsTypeTypeFromString(type_str);
>> +            unit = qemuMonitorQueryStatsUnitTypeFromString(unit_str);
>> +
>> +            data = g_new0(qemuMonitorQueryStatsSchemaData, 1);
>> +            data->target = (target == -1) ?
>> QEMU_MONITOR_QUERY_STATS_TARGET_LAST : target;
>> +            data->type = (type == -1) ?
>> QEMU_MONITOR_QUERY_STATS_TYPE_LAST : type;
>> +            data->unit = (unit == -1) ?
>> QEMU_MONITOR_QUERY_STATS_UNIT_LAST : unit;
>> +
>> +            if (virJSONValueObjectGetNumberInt(stat, "base", &data->base)
>> < 0 ||
>> +                virJSONValueObjectGetNumberInt(stat, "exponent",
>> &data->exponent) < 0)
>> +                data->base = 0, data->exponent = 0;
>>
>
>Definitely split this line into two separate lines please. Operator comma
>makes the code very unclean
>and should not be used aside from a few exceptions in 'for' and 'while'
>loops.
>

and you can even make it nicer with

data->base = data->exponent = 0;

>
>> +
>> +            /* a base of zero means that there is simply no scale,
>> data->exponent is
>> +               set to 0 just for safety measures */
>> +
>> +            if (data->type ==
>> QEMU_MONITOR_QUERY_STATS_TYPE_LINEAR_HISTOGRAM &&
>> +                virJSONValueObjectGetNumberUint(stat, "bucket-size",
>> &data->bucket_size) < 0)
>> +                data->bucket_size = 0;
>> +
>> +            virHashAddEntry(schema, name, data);
>> +        }
>> +    }
>> +
>> +    return g_steal_pointer(&schema);
>> +}
>> +
>> +GHashTable *
>> +qemuMonitorJSONQueryStatsSchema(qemuMonitor *mon,
>> +                                qemuMonitorQueryStatsProviderType
>> provider_type)
>> +{
>> +    g_autoptr(virJSONValue) cmd = NULL;
>> +    g_autoptr(virJSONValue) reply = NULL;
>> +    virJSONValue *ret;
>> +
>> +    const char *type_str =
>> qemuMonitorQueryStatsProviderTypeToString(provider_type);
>> +
>> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-stats-schemas",
>> +                                           "S:provider", type_str,
>> +                                           NULL)))
>
>+        return NULL;
>> +
>> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
>> +        return NULL;
>> +
>> +    if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
>> +        return NULL;
>> +
>> +    ret = virJSONValueObjectGetArray(reply, "return");
>> +
>> +    return qemuMonitorJSONExtractQueryStatsSchema(ret);
>> +}
>> +
>>
>>  /**
>>   * qemuMonitorJSONQueryStats:
>> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
>> index a53e6423df..c910e46504 100644
>> --- a/src/qemu/qemu_monitor_json.h
>> +++ b/src/qemu/qemu_monitor_json.h
>> @@ -813,6 +813,10 @@ int
>>  qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
>>                                const char *uri);
>>
>> +GHashTable *
>> +qemuMonitorJSONQueryStatsSchema(qemuMonitor *mon,
>> +                                qemuMonitorQueryStatsProviderType
>> provider_type);
>> +
>>  virJSONValue *
>>  qemuMonitorJSONQueryStats(qemuMonitor *mon,
>>                            qemuMonitorQueryStatsTargetType target,
>> --
>> 2.37.1
>>
>>
>Otherwise nice:)
>
>Kristina

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.