just a few notes on the code quality.
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