6:34 p.m.
This patch adds an API for query-stats-schemas and uses it to work with
the pre-existing API for query-stats to display those stats.
[1/6]: This patch adds a simple API for query-stats-schemas and an
extractor function to deserialise it into a GHashTable. The GHashTable
here is a pair of the name of the stat and the schema for it. Some
fields in the latter like the exponent, base and bucket-size are not
utilised in this patchset but they will be useful in the subsequent
patches which add the support for the histograms.
[2/6]: Add query-stats-schemas to the qemu capabilities.
[3/6]: This patch adds the schema hashtable to the
virDomainObjectPrivate. This decision was made due to QEMU upstream not
having vCPUs to generate the schema, so that they could be stored as the
file cache. This might be changed in the future if there is workaround
upstream or if libvirt ends up using a dummy VM just to query the
schema.
[4/6]: This patch simply adds a utility function to traverse the
schemas to find the object that corresponds to the provided QOM path.
[5/6]: This patch adds vCPU stats in addition to the pre-existing ones
using a helper function. Histograms are ignored for now but they will be
added in the next patchset.
[6/6]: This patch adds a new stat worker for QEMU called "Vm" due to the
stats being for the "vm" target. It utilises the same helper function as
above.
Comments are much appreciated.
Amneesh Singh (6):
qemu_monitor: add qemuMonitorQueryStatsSchema
qemu_capabilities: add "query-stats-schemas" QMP command to the QEMU
capabilities
qemu_domain: add statsSchema to qemuDomainObjPrivate
qemu_monitor: add qemuMonitorGetStatsByQOMPath
qemu_driver: add the vCPU stats by KVM to the current stats
qemu_driver: add new stats worker qemuDomainGetStatsVm
include/libvirt/libvirt-domain.h | 1 +
src/libvirt-domain.c | 3 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_domain.c | 41 ++++++
src/qemu/qemu_domain.h | 5 +
src/qemu/qemu_driver.c | 127 ++++++++++++++++++
src/qemu/qemu_monitor.c | 67 +++++++++
src/qemu/qemu_monitor.h | 39 ++++++
src/qemu/qemu_monitor_json.c | 93 +++++++++++++
src/qemu/qemu_monitor_json.h | 4 +
.../caps_7.1.0.x86_64.xml | 1 +
tools/virsh-domain-monitor.c | 7 +
13 files changed, 391 insertions(+)
--
2.37.1
6:34 p.m.
New subject: [RFC PATCH 1/6] qemu_monitor: add qemuMonitorQueryStatsSchema
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(a)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
8:23 p.m.
New subject: [RFC PATCH 1/6] qemu_monitor: add qemuMonitorQueryStatsSchema
Hi,
just a few notes on the code quality.
On Wed, Sep 7, 2022 at 12:34 PM Amneesh Singh <natto(a)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(a)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
8:49 p.m.
New subject: [RFC PATCH 1/6] qemu_monitor: add qemuMonitorQueryStatsSchema
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(a)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(a)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
6:34 p.m.
New subject: [RFC PATCH 2/6] qemu_capabilities: add "query-stats-schemas" QMP command to the QEMU capabilities
Related: https://gitlab.com/libvirt/libvirt/-/issues/276
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml | 1 +
3 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 71018b4f6b..821d5d4ede 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -676,6 +676,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 435 */
"query-stats", /* QEMU_CAPS_QUERY_STATS */
+ "query-stats-schemas", /* QEMU_CAPS_QUERY_STATS_SCHEMAS */
);
@@ -1226,6 +1227,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "sev-inject-launch-secret", QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET },
{ "calc-dirty-rate", QEMU_CAPS_CALC_DIRTY_RATE },
{ "query-stats", QEMU_CAPS_QUERY_STATS },
+ { "query-stats-schemas", QEMU_CAPS_QUERY_STATS_SCHEMAS },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 382c357a78..badc0e82f9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -655,6 +655,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
/* 435 */
QEMU_CAPS_QUERY_STATS, /* accepts query-stats */
+ QEMU_CAPS_QUERY_STATS_SCHEMAS, /* accepts query-stats-schemas */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml
index de961e0569..d602b066a8 100644
--- a/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml
@@ -222,6 +222,7 @@
<flag name='usb-host.guest-resets-all'/>
<flag name='migration.blocked-reasons'/>
<flag name='query-stats'/>
+ <flag name='query-stats-schemas'/>
<version>7001000</version>
<kvmVersion>0</kvmVersion>
<microcodeVersion>43100244</microcodeVersion>
--
2.37.1
6:34 p.m.
New subject: [RFC PATCH 3/6] qemu_domain: add statsSchema to qemuDomainObjPrivate
This patch adds a hashtable for storing the stats schema and a function
to refresh it by querying "query-stats-schemas" using
qemuMonitorQueryStatsSchema
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 5 +++++
2 files changed, 46 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fe3ce023a4..e621e8b25e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1708,6 +1708,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivate *priv)
priv->originalMemlock = 0;
priv->preMigrationMemlock = 0;
+
+ virHashRemoveAll(priv->statsSchema);
}
@@ -1747,6 +1749,9 @@ qemuDomainObjPrivateFree(void *data)
g_object_unref(priv->eventThread);
}
+ if (priv->statsSchema)
+ g_clear_pointer(&priv->statsSchema, g_hash_table_destroy);
+
g_free(priv);
}
@@ -1778,6 +1783,8 @@ qemuDomainObjPrivateAlloc(void *opaque)
priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
priv->driver = opaque;
+ priv->statsSchema = NULL;
+
return g_steal_pointer(&priv);
}
@@ -11744,3 +11751,37 @@ qemuDomainObjWait(virDomainObj *vm)
return 0;
}
+
+
+/**
+ * virDomainRefreshStatsSchema:
+ * @driver: qemu driver data
+ * @vm: Pointer to the vm object
+ *
+ * Load data into dom->privateData->statsSchema if not stored
+ *
+ * Returns -1 on failure, 0 otherwise.
+ */
+int
+qemuDomainRefreshStatsSchema(virDomainObj *dom)
+{
+ qemuDomainObjPrivate *priv = dom->privateData;
+ GHashTable *schema = priv->statsSchema;
+
+ if (schema && g_hash_table_size(schema) > 0)
+ return 0;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_STATS_SCHEMAS))
+ return 0;
+
+ qemuDomainObjEnterMonitor(dom);
+ schema = qemuMonitorQueryStatsSchema(priv->mon,
QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST);
+ qemuDomainObjExitMonitor(dom);
+
+ if (!schema)
+ return -1;
+
+ priv->statsSchema = schema;
+
+ return 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 592ee9805b..cbc65feafc 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -244,6 +244,8 @@ struct _qemuDomainObjPrivate {
unsigned long long originalMemlock; /* Original RLIMIT_MEMLOCK, zero if no
* restore will be required later */
+
+ GHashTable *statsSchema; /* (name, data) pair for stats */
};
#define QEMU_DOMAIN_PRIVATE(vm) \
@@ -1098,3 +1100,6 @@ qemuDomainRemoveLogs(virQEMUDriver *driver,
int
qemuDomainObjWait(virDomainObj *vm);
+
+int
+qemuDomainRefreshStatsSchema(virDomainObj *dom);
--
2.37.1
10:01 p.m.
New subject: [RFC PATCH 3/6] qemu_domain: add statsSchema to qemuDomainObjPrivate
On Wed, Sep 07, 2022 at 04:04:20PM +0530, Amneesh Singh wrote:
This patch adds a hashtable for storing the stats schema and a
function
to refresh it by querying "query-stats-schemas" using
qemuMonitorQueryStatsSchema
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 5 +++++
2 files changed, 46 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fe3ce023a4..e621e8b25e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
[...]
@@ -11744,3 +11751,37 @@ qemuDomainObjWait(virDomainObj *vm)
return 0;
}
+
+
+/**
+ * virDomainRefreshStatsSchema:
+ * @driver: qemu driver data
+ * @vm: Pointer to the vm object
+ *
+ * Load data into dom->privateData->statsSchema if not stored
+ *
+ * Returns -1 on failure, 0 otherwise.
+ */
+int
+qemuDomainRefreshStatsSchema(virDomainObj *dom)
+{
+ qemuDomainObjPrivate *priv = dom->privateData;
+ GHashTable *schema = priv->statsSchema;
+
+ if (schema && g_hash_table_size(schema) > 0)
+ return 0;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_STATS_SCHEMAS))
+ return 0;
So this return value indicates the data were successfully loaded
according to the docs, but it looks like it just reports there was no
error. Even though this cannot technically happen (for now, or rather
in a future patch since there is a restriction on the stats type) it
would be nicer to use in the future.
+
+ qemuDomainObjEnterMonitor(dom);
+ schema = qemuMonitorQueryStatsSchema(priv->mon,
QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST);
+ qemuDomainObjExitMonitor(dom);
+
+ if (!schema)
+ return -1;
+
+ priv->statsSchema = schema;
+
This could override an empty hash table in statsSchema, it should be
free'd first.
[...]
6:34 p.m.
New subject: [RFC PATCH 4/6] qemu_monitor: add qemuMonitorGetStatsByQOMPath
This function returns the virJSONValue object which has the
same qom_path as specified.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_monitor.c | 38 ++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 4 ++++
2 files changed, 42 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9581e90796..3d4302a6ed 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1537,6 +1537,7 @@ qemuMonitorGetCPUInfoLegacy(struct qemuMonitorQueryCpusEntry
*cpuentries,
vcpus[i].tid = cpuentries[i].tid;
vcpus[i].halted = cpuentries[i].halted;
vcpus[i].qemu_id = cpuentries[i].qemu_id;
+ vcpus[i].qom_path = g_strdup(cpuentries[i].qom_path);
}
/* for legacy hotplug to work we need to fake the vcpu count added by
@@ -4446,3 +4447,40 @@ qemuMonitorExtractQueryStats(virJSONValue *info)
return g_steal_pointer(&hash_table);
}
+
+
+/**
+ * qemuMonitorStatsSchemaByQOMPath:
+ * @arr: Array of objects returned by qemuMonitorQueryStats
+ *
+ * Returns the object which matches the QOM path of the vCPU
+ *
+ * Returns NULL on failure.
+ */
+virJSONValue *
+qemuMonitorGetStatsByQOMPath(virJSONValue *arr,
+ char *qom_path)
+{
+ size_t i;
+
+ if (!virJSONValueIsArray(arr) || !qom_path)
+ return NULL;
+
+ for (i = 0; i < virJSONValueArraySize(arr); i++) {
+ virJSONValue *obj = virJSONValueArrayGet(arr, i);
+ const char *test_qom_path = NULL;
+
+ if (!obj)
+ return NULL;
+
+ test_qom_path = virJSONValueObjectGetString(obj, "qom-path");
+
+ if (!test_qom_path)
+ return NULL;
+
+ if (STRCASEEQ(qom_path, test_qom_path))
+ return obj;
+ }
+
+ return NULL;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 4c817dea20..6b0440fdf0 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1564,3 +1564,7 @@ qemuMonitorQueryStats(qemuMonitor *mon,
GHashTable *
qemuMonitorExtractQueryStats(virJSONValue *info);
+
+virJSONValue *
+qemuMonitorGetStatsByQOMPath(virJSONValue *arr,
+ char *qom_path);
--
2.37.1
6:34 p.m.
New subject: [RFC PATCH 5/6] qemu_driver: add the vCPU stats by KVM to the current stats
This patch adds the stats queried by qemuMonitorQueryStats for vCPU and
add them according to their QOM device path
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_driver.c | 86 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ea7d74806c..79146b6bb8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17966,6 +17966,68 @@ qemuDomainGetStatsBalloon(virQEMUDriver *driver G_GNUC_UNUSED,
}
+static void
+qemuDomainAddStatsFromHashTable(GHashTable *stats,
+ GHashTable *schema,
+ const char *prefix,
+ virTypedParamList *params)
+{
+ GHashTableIter iter;
+ virJSONValue *value;
+ char *key;
+
+ if (!stats || !schema)
+ return;
+
+ g_hash_table_iter_init(&iter, stats);
+
+ while (g_hash_table_iter_next(&iter, (gpointer *)&key, (gpointer
*)&value)) {
+ qemuMonitorQueryStatsSchemaData *data = g_hash_table_lookup(schema, key);
+ const char *type = NULL;
+
+ if (!data)
+ continue;
+
+ switch (data->type) {
+ case QEMU_MONITOR_QUERY_STATS_TYPE_CUMULATIVE:
+ type = "sum";
+ break;
+ 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_LAST:
+ default:
+ continue;
+ }
+
+ if (data->unit == QEMU_MONITOR_QUERY_STATS_UNIT_BOOLEAN) {
+ bool stat;
+
+ if (virJSONValueGetBoolean(value, &stat) < 0)
+ continue;
+
+ ignore_value(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));
+ }
+ }
+}
+
+
static int
qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED,
virDomainObj *dom,
@@ -17979,6 +18041,8 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED,
virVcpuInfoPtr cpuinfo = NULL;
g_autofree unsigned long long *cpuwait = NULL;
g_autofree unsigned long long *cpudelay = NULL;
+ qemuDomainObjPrivate *priv = dom->privateData;
+ g_autoptr(virJSONValue) queried_stats = NULL;
if (virTypedParamListAddUInt(params, virDomainDefGetVcpus(dom->def),
"vcpu.current") < 0)
@@ -18007,7 +18071,21 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED,
goto cleanup;
}
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_STATS) &&
+ !qemuDomainRefreshStatsSchema(dom) &&
+ HAVE_JOB(privflags)) {
+ qemuDomainObjEnterMonitor(dom);
+ queried_stats = qemuMonitorQueryStats(priv->mon,
+ QEMU_MONITOR_QUERY_STATS_TARGET_VCPU,
+ NULL, NULL);
+ qemuDomainObjExitMonitor(dom);
+ }
+
for (i = 0; i < virDomainDefGetVcpus(dom->def); i++) {
+ virJSONValue *stat_obj = NULL;
+ 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)
goto cleanup;
@@ -18041,6 +18119,14 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED,
cpuinfo[i].number) < 0)
goto cleanup;
}
+
+ if (!queried_stats)
+ continue;
+
+ stat_obj = qemuMonitorGetStatsByQOMPath(queried_stats, vcpupriv->qomPath);
+ stats = qemuMonitorExtractQueryStats(stat_obj);
+
+ qemuDomainAddStatsFromHashTable(stats, priv->statsSchema, prefix, params);
}
ret = 0;
--
2.37.1
10:02 p.m.
New subject: [RFC PATCH 5/6] qemu_driver: add the vCPU stats by KVM to the current stats
On Wed, Sep 07, 2022 at 04:04:22PM +0530, Amneesh Singh wrote:
This patch adds the stats queried by qemuMonitorQueryStats for vCPU
and
add them according to their QOM device path
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_driver.c | 86 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ea7d74806c..79146b6bb8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17966,6 +17966,68 @@ qemuDomainGetStatsBalloon(virQEMUDriver *driver G_GNUC_UNUSED,
}
+static void
+qemuDomainAddStatsFromHashTable(GHashTable *stats,
+ GHashTable *schema,
+ const char *prefix,
+ virTypedParamList *params)
+{
+ GHashTableIter iter;
+ virJSONValue *value;
+ char *key;
This is "const" char* since there is no need to free it and we don't
really want to modify it anyway.
+
+ if (!stats || !schema)
+ return;
+
+ g_hash_table_iter_init(&iter, stats);
+
+ while (g_hash_table_iter_next(&iter, (gpointer *)&key, (gpointer
*)&value)) {
+ qemuMonitorQueryStatsSchemaData *data = g_hash_table_lookup(schema, key);
+ const char *type = NULL;
+
+ if (!data)
+ continue;
+
+ switch (data->type) {
+ case QEMU_MONITOR_QUERY_STATS_TYPE_CUMULATIVE:
+ type = "sum";
+ break;
+ 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_LAST:
+ default:
By omitting this default branch we get a free check from the compiler
that all the possible types are represented in the switch.
[...]
@@ -18007,7 +18071,21 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver
G_GNUC_UNUSED,
goto cleanup;
}
+ if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_STATS) &&
This function already checks for the capability, but as noted in the
previous patch we could make it behave differently and this usage in
particular could benefit from such change to make this condition.
+ !qemuDomainRefreshStatsSchema(dom) &&
+ HAVE_JOB(privflags)) {
This could call qemuDomainRefreshStatsSchema even without a job and
that's not what we want here since it enters the monitor *and* can
possibly change data in dom->privateData.
6:34 p.m.
New subject: [RFC PATCH 6/6] qemu_driver: add new stats worker qemuDomainGetStatsVm
This patch adds a new worker qemuDomainGetStatsVm which reports the
stats returned by "query-stats" via qemuMonitorQueryStats for the VM
target.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
include/libvirt/libvirt-domain.h | 1 +
src/libvirt-domain.c | 3 +++
src/qemu/qemu_driver.c | 41 ++++++++++++++++++++++++++++++++
tools/virsh-domain-monitor.c | 7 ++++++
4 files changed, 52 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 7430a08619..3b25c8dd77 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2706,6 +2706,7 @@ typedef enum {
VIR_DOMAIN_STATS_IOTHREAD = (1 << 7), /* return iothread poll info (Since:
4.10.0) */
VIR_DOMAIN_STATS_MEMORY = (1 << 8), /* return domain memory info (Since: 6.0.0)
*/
VIR_DOMAIN_STATS_DIRTYRATE = (1 << 9), /* return domain dirty rate info (Since:
7.2.0) */
+ VIR_DOMAIN_STATS_VM = (1 << 10), /* return vm info (Since: 8.6.0) */
} virDomainStatsTypes;
/**
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 80a6f0f269..52fa136186 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12481,6 +12481,9 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
* rate for a virtual cpu as
* unsigned long long.
*
+ * VIR_DOMAIN_STATS_VM:
+ * Return fd-based KVM statistics for the target VM
+ *
* Note that entire stats groups or individual stat fields may be missing from
* the output in case they are not supported by the given hypervisor, are not
* applicable for the current state of the guest domain, or their retrieval
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 79146b6bb8..ce4d4a0f03 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18727,6 +18727,41 @@ qemuDomainGetStatsDirtyRate(virQEMUDriver *driver G_GNUC_UNUSED,
return 0;
}
+
+static int
+qemuDomainGetStatsVm(virQEMUDriver *driver G_GNUC_UNUSED,
+ virDomainObj *dom,
+ virTypedParamList *params,
+ unsigned int privflags)
+{
+ qemuDomainObjPrivate *priv = dom->privateData;
+ g_autoptr(virJSONValue) queried_stats = NULL;
+ g_autoptr(GHashTable) stats = NULL;
+ virJSONValue *stats_obj = NULL;
+
+ if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom))
+ return 0;
+
+ if (qemuDomainRefreshStatsSchema(dom) < 0)
+ return 0;
+
+ qemuDomainObjEnterMonitor(dom);
+ queried_stats = qemuMonitorQueryStats(priv->mon,
+ QEMU_MONITOR_QUERY_STATS_TARGET_VM,
+ NULL, NULL);
+ qemuDomainObjExitMonitor(dom);
+
+ if (!queried_stats || virJSONValueArraySize(queried_stats) != 1)
+ return 0;
+
+ stats_obj = virJSONValueArrayGet(queried_stats, 0);
+ stats = qemuMonitorExtractQueryStats(stats_obj);
+
+ qemuDomainAddStatsFromHashTable(stats, priv->statsSchema, "vm",
params);
+
+ return 0;
+}
+
typedef int
(*qemuDomainGetStatsFunc)(virQEMUDriver *driver,
virDomainObj *dom,
@@ -18751,6 +18786,11 @@ static virQEMUCapsFlags queryDirtyRateRequired[] = {
QEMU_CAPS_LAST
};
+static virQEMUCapsFlags queryVmRequired[] = {
+ QEMU_CAPS_QUERY_STATS,
+ QEMU_CAPS_LAST
+};
+
static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = {
{ qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false, NULL },
{ qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, true, NULL },
@@ -18762,6 +18802,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[]
= {
{ qemuDomainGetStatsIOThread, VIR_DOMAIN_STATS_IOTHREAD, true, queryIOThreadRequired
},
{ qemuDomainGetStatsMemory, VIR_DOMAIN_STATS_MEMORY, false, NULL },
{ qemuDomainGetStatsDirtyRate, VIR_DOMAIN_STATS_DIRTYRATE, true,
queryDirtyRateRequired },
+ { qemuDomainGetStatsVm, VIR_DOMAIN_STATS_VM, true, queryVmRequired },
{ NULL, 0, false, NULL }
};
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index dc5fe13e49..be8f827685 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -2065,6 +2065,10 @@ static const vshCmdOptDef opts_domstats[] = {
.type = VSH_OT_BOOL,
.help = N_("report domain dirty rate information"),
},
+ {.name = "vm",
+ .type = VSH_OT_BOOL,
+ .help = N_("report fd-based VM statistics by KVM"),
+ },
{.name = "list-active",
.type = VSH_OT_BOOL,
.help = N_("list only active domains"),
@@ -2186,6 +2190,9 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "dirtyrate"))
stats |= VIR_DOMAIN_STATS_DIRTYRATE;
+ if (vshCommandOptBool(cmd, "vm"))
+ stats |= VIR_DOMAIN_STATS_VM;
+
if (vshCommandOptBool(cmd, "list-active"))
flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE;
--
2.37.1
10:02 p.m.
On Wed, Sep 07, 2022 at 04:04:17PM +0530, Amneesh Singh wrote:
This patch adds an API for query-stats-schemas and uses it to work
with
the pre-existing API for query-stats to display those stats.
[1/6]: This patch adds a simple API for query-stats-schemas and an
extractor function to deserialise it into a GHashTable. The GHashTable
here is a pair of the name of the stat and the schema for it. Some
fields in the latter like the exponent, base and bucket-size are not
utilised in this patchset but they will be useful in the subsequent
patches which add the support for the histograms.
[2/6]: Add query-stats-schemas to the qemu capabilities.
[3/6]: This patch adds the schema hashtable to the
virDomainObjectPrivate. This decision was made due to QEMU upstream not
having vCPUs to generate the schema, so that they could be stored as the
file cache. This might be changed in the future if there is workaround
upstream or if libvirt ends up using a dummy VM just to query the
schema.
[4/6]: This patch simply adds a utility function to traverse the
schemas to find the object that corresponds to the provided QOM path.
[5/6]: This patch adds vCPU stats in addition to the pre-existing ones
using a helper function. Histograms are ignored for now but they will be
added in the next patchset.
[6/6]: This patch adds a new stat worker for QEMU called "Vm" due to the
stats being for the "vm" target. It utilises the same helper function as
above.
Comments are much appreciated.
Amneesh Singh (6):
qemu_monitor: add qemuMonitorQueryStatsSchema
qemu_capabilities: add "query-stats-schemas" QMP command to the QEMU
capabilities
qemu_domain: add statsSchema to qemuDomainObjPrivate
qemu_monitor: add qemuMonitorGetStatsByQOMPath
qemu_driver: add the vCPU stats by KVM to the current stats
qemu_driver: add new stats worker qemuDomainGetStatsVm
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
There were some details that I missed previously, so I noted most of
them in individual replies (apart from few tiny details), fixed all of
them and pushed the series. Sorry it took so long. Would you mind
rebasing and resending the other series so it does not seem obsolete?
Thank you for the work.
include/libvirt/libvirt-domain.h | 1 +
src/libvirt-domain.c | 3 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_domain.c | 41 ++++++
src/qemu/qemu_domain.h | 5 +
src/qemu/qemu_driver.c | 127 ++++++++++++++++++
src/qemu/qemu_monitor.c | 67 +++++++++
src/qemu/qemu_monitor.h | 39 ++++++
src/qemu/qemu_monitor_json.c | 93 +++++++++++++
src/qemu/qemu_monitor_json.h | 4 +
.../caps_7.1.0.x86_64.xml | 1 +
tools/virsh-domain-monitor.c | 7 +
13 files changed, 391 insertions(+)
--
2.37.1
689
days inactive
731
days old
11 comments
3 participants
participants (3)
-
Amneesh Singh -
Kristina Hanicova -
Martin Kletzander