[RFC PATCH 0/6] qemu: add support for query-stats-schemas
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
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
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
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
Related: https://gitlab.com/libvirt/libvirt/-/issues/276 Signed-off-by: Amneesh Singh <natto@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
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@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
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@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. [...]
This function returns the virJSONValue object which has the same qom_path as specified. Signed-off-by: Amneesh Singh <natto@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
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@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
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@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.
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@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
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@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
participants (3)
-
Amneesh Singh -
Kristina Hanicova -
Martin Kletzander