6:26 p.m.
QEMU is gaining introspectable statistics which can be queried via the
"query-stats" QMP command. This patchset aims to add an API for the
same.
The returned JSON for "query-stats" is an array of objects containing
their own respective array of statistics.
Patch 1 adds the API which returns the deserialized JSON in the form of
a GPtrArray of GHashTables.
Patch 2 adds the "query-stats" to QEMU capabilities.
Patch 3 uses the API to query the halt poll success time and the halt
poll failure time.
v1 -> v2
========
I have been tinkering with the v1 patchset and have rewrote the v2
patches a couple of times. I believe the current patchset is still not
perfect and would appreciate some reviews.
I have another patch or two written but they do not make any significant
changes to the current patchset.
[1/3]:
- use virBitmap instead of an array of strings for statistics.
- add enums for the stat names and add
qemuMonitorQueryStatsNameTypeToString to switch between the
"ToString" functions based on the target type.
- change qemuMonitorQueryStatsProviderNew to a variadic function that
takes stat enum values with the sentinel value being -1.
[2/3]:
- No changes
[3/3]:
- Add relevant monitor related checks to check if the domain is
active.
- Acquire and release qemuMonitorObj lock before and after calling
qemuMonitorQueryStats respectively.
- Add the check for privileged access.
Relevant QEMU patches can be found here:
https://lore.kernel.org/all/20220530150714.756954-1-pbonzini@redhat.com/
This patchset is part of the 2022 GSOC contributor project.
Amneesh Singh (3):
qemu_monitor: add qemuMonitorQueryStats
qemu_capabilities: add "query-stats" QMP command to the QEMU
capabilities
qemu_driver: use qemuMonitorQueryStats to extract halt poll time
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 70 ++++++++++++++++---
src/qemu/qemu_monitor.c | 106 ++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 56 +++++++++++++++
src/qemu/qemu_monitor_json.c | 130 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 ++
7 files changed, 363 insertions(+), 8 deletions(-)
--
2.36.1
6:26 p.m.
New subject: [PATCH RFC v2 1/3] qemu_monitor: add qemuMonitorQueryStats
Related: https://gitlab.com/libvirt/libvirt/-/issues/276
This patch adds an API for the "query-stats" QMP command.
The query returns a JSON containing the statistics based on the target,
which can either be vCPU or VM, and the providers. The API deserializes
the query result into an array of GHashMaps, which can later be used to
extract all the query statistics. GHashMaps are used to avoid traversing
the entire array to find the statistics you are looking for. This would
be a singleton array if the target is a VM since the returned JSON is
also a singleton array in that case.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_monitor.c | 106 ++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 56 +++++++++++++++
src/qemu/qemu_monitor_json.c | 130 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 ++
4 files changed, 298 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fda5d2f3..528aed49 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4541,3 +4541,109 @@ qemuMonitorMigrateRecover(qemuMonitor *mon,
return qemuMonitorJSONMigrateRecover(mon, uri);
}
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsTarget,
+ QEMU_MONITOR_QUERY_STATS_TARGET_LAST,
+ "vm",
+ "vcpu",
+);
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsVmName,
+ QEMU_MONITOR_QUERY_STATS_VM_NAME_LAST,
+);
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsVcpuName,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_LAST,
+ "halt_poll_success_ns",
+ "halt_poll_fail_ns"
+);
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsProvider,
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST,
+ "kvm",
+);
+
+const char *
+qemuMonitorQueryStatsNameTypeToString(qemuMonitorQueryStatsTargetType target,
+ int type)
+{
+ switch (target) {
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VM:
+ return qemuMonitorQueryStatsVmNameTypeToString(type);
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VCPU:
+ return qemuMonitorQueryStatsVcpuNameTypeToString(type);
+
+ /* unreachable */
+ case QEMU_MONITOR_QUERY_STATS_TARGET_LAST:
+ ;
+ }
+
+ return NULL;
+}
+
+void
+qemuMonitorQueryStatsProviderFree(qemuMonitorQueryStatsProvider *provider)
+{
+ virBitmapFree(provider->names);
+ g_free(provider);
+}
+
+qemuMonitorQueryStatsProvider *
+qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsTargetType target,
+ qemuMonitorQueryStatsProviderType provider_type,
+ ...)
+{
+ g_autoptr(qemuMonitorQueryStatsProvider) provider = NULL;
+ va_list name_list;
+ int stat;
+
+ provider = g_new0(qemuMonitorQueryStatsProvider, 1);
+ provider->provider = provider_type;
+ provider->names = NULL;
+
+ va_start(name_list, provider_type);
+ stat = va_arg(name_list, int);
+
+ if (stat != -1) {
+ size_t sz = 0;
+
+ switch (target) {
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VM:
+ sz = QEMU_MONITOR_QUERY_STATS_VM_NAME_LAST;
+ break;
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VCPU:
+ sz = QEMU_MONITOR_QUERY_STATS_VCPU_NAME_LAST;
+ break;
+ case QEMU_MONITOR_QUERY_STATS_TARGET_LAST:
+ break;
+ }
+
+ provider->names = virBitmapNew(sz);
+
+ while (stat != -1) {
+ if (virBitmapSetBit(provider->names, stat) < 0)
+ return NULL;
+ stat = va_arg(name_list, int);
+ }
+ }
+
+ va_end(name_list);
+
+ return g_steal_pointer(&provider);
+}
+
+GPtrArray *
+qemuMonitorQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers)
+{
+ VIR_DEBUG("target=%u vcpus=%p providers=%p", target, vcpus, providers);
+
+ QEMU_CHECK_MONITOR_NULL(mon);
+
+ if (target != QEMU_MONITOR_QUERY_STATS_TARGET_VCPU && !vcpus)
+ return NULL;
+
+ return qemuMonitorJSONQueryStats(mon, target, vcpus, providers);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 95267ec6..26df9c63 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1554,3 +1554,59 @@ qemuMonitorChangeMemoryRequestedSize(qemuMonitor *mon,
int
qemuMonitorMigrateRecover(qemuMonitor *mon,
const char *uri);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_TARGET_VM,
+ QEMU_MONITOR_QUERY_STATS_TARGET_VCPU,
+ QEMU_MONITOR_QUERY_STATS_TARGET_LAST,
+} qemuMonitorQueryStatsTargetType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsTarget);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_VM_NAME_LAST,
+} qemuMonitorQueryStatsVmNameType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsVmName);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_SUCCESS_NS,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_FAIL_NS,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_LAST,
+} qemuMonitorQueryStatsVcpuNameType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsVcpuName);
+
+const char *
+qemuMonitorQueryStatsNameTypeToString(qemuMonitorQueryStatsTargetType target,
+ int type);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_KVM,
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST,
+} qemuMonitorQueryStatsProviderType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsProvider);
+
+typedef struct _qemuMonitorQueryStatsProvider qemuMonitorQueryStatsProvider;
+struct _qemuMonitorQueryStatsProvider {
+ qemuMonitorQueryStatsProviderType provider;
+ virBitmap *names;
+};
+
+void
+qemuMonitorQueryStatsProviderFree(qemuMonitorQueryStatsProvider *provider);
+
+qemuMonitorQueryStatsProvider *
+qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsTargetType target,
+ qemuMonitorQueryStatsProviderType provider_type,
+ ...);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorQueryStatsProvider,
+ qemuMonitorQueryStatsProviderFree);
+
+GPtrArray *
+qemuMonitorQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3aad2ab2..8774b1ff 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9006,3 +9006,133 @@ qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
return qemuMonitorJSONCheckError(cmd, reply);
}
+
+static GPtrArray *
+qemuMonitorJSONExtractQueryStats(virJSONValue *arr)
+{
+ g_autoptr(GPtrArray) queried_stats = NULL;
+ size_t nstats = virJSONValueArraySize(arr);
+ size_t i;
+
+ /* Create a GPtrArray for GHashTables */
+ queried_stats = g_ptr_array_new_full(nstats, (GDestroyNotify) g_hash_table_destroy);
+
+ for (i = 0; i < nstats; i++) {
+ virJSONValue *obj = virJSONValueArrayGet(arr, i);
+ virJSONValue *stats = virJSONValueObjectGetArray(obj, "stats");
+ size_t j;
+
+ /* Create a GHashTable for virJSONValues */
+ GHashTable *hash_table = virHashNew((GDestroyNotify) virJSONValueFree);
+
+ for (j = 0; j < virJSONValueArraySize(stats); j++) {
+ virJSONValue *stat = virJSONValueArrayGet(stats, j);
+
+ g_hash_table_insert(hash_table,
+ g_strdup(virJSONValueObjectGetString(stat,
"name")),
+ virJSONValueObjectGet(stat, "value"));
+ }
+
+ g_ptr_array_add(queried_stats, hash_table);
+ }
+
+ return g_steal_pointer(&queried_stats);
+}
+
+
+/**
+ * qemuMonitorJSONQueryStats:
+ * @mon: monitor object
+ * @target: the target type for the query
+ * @vcpus: a list of vCPU QOM paths for filtering the statistics
+ * @providers: an array of providers to filter statistics
+ *
+ * @vcpus is a NULL terminated array of strings. @providers is a GPtrArray
+ * for qemuMonitorQueryStatsProvider.
+ * @vcpus and @providers are optional and can be NULL.
+ *
+ * Queries for the @target based statistics.
+ * Returns a GPtrArray of GHashTables containing the statistics on success or
+ * NULL on failure.
+ */
+
+GPtrArray *
+qemuMonitorJSONQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers)
+{
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
+ g_autoptr(virJSONValue) vcpu_list = NULL;
+ g_autoptr(virJSONValue) provider_list = NULL;
+ virJSONValue *rv = NULL;
+
+ size_t i;
+
+ if (providers) {
+ provider_list = virJSONValueNewArray();
+
+ for (i = 0; i < providers->len; i++) {
+ g_autoptr(virJSONValue) provider_obj = virJSONValueNewObject();
+ qemuMonitorQueryStatsProvider *provider = providers->pdata[i];
+ const char *type_str =
qemuMonitorQueryStatsProviderTypeToString(provider->provider);
+ virBitmap *names = provider->names;
+ int rc;
+
+ rc = virJSONValueObjectAppendString(provider_obj, "provider",
type_str);
+
+ if (rc < 0)
+ return NULL;
+
+ if (names) {
+ g_autoptr(virJSONValue) provider_names = virJSONValueNewArray();
+ int curBit = virBitmapNextSetBit(names, -1);
+
+ while (curBit >= 0) {
+ const char *name = qemuMonitorQueryStatsNameTypeToString(target,
curBit);
+
+ if (virJSONValueArrayAppendString(provider_names, name) < 0)
+ return NULL;
+
+ curBit = virBitmapNextSetBit(names, curBit);
+ }
+
+ rc = virJSONValueObjectAppend(provider_obj, "names",
&provider_names);
+
+ if (rc < 0)
+ return NULL;
+ }
+
+ if (virJSONValueArrayAppend(provider_list, &provider_obj) < 0)
+ return NULL;
+ }
+ }
+
+ if (vcpus) {
+ vcpu_list = virJSONValueNewArray();
+
+ for (i = 0; vcpus[i]; i++)
+ if (virJSONValueArrayAppendString(vcpu_list, vcpus[i]) < 0)
+ return NULL;
+ }
+
+ cmd = qemuMonitorJSONMakeCommand("query-stats",
+ "s:target",
qemuMonitorQueryStatsTargetTypeToString(target),
+ "A:vcpus", &vcpu_list,
+ "A:providers", &provider_list,
+ NULL);
+
+ if (!cmd)
+ return NULL;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ return NULL;
+
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ return NULL;
+
+ rv = virJSONValueObjectStealArray(reply, "return");
+
+ return qemuMonitorJSONExtractQueryStats(rv);
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ad3853ae..db8dec80 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -878,3 +878,9 @@ qemuMonitorJSONChangeMemoryRequestedSize(qemuMonitor *mon,
int
qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
const char *uri);
+
+GPtrArray *
+qemuMonitorJSONQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers);
--
2.36.1
6:21 a.m.
New subject: [PATCH RFC v2 1/3] qemu_monitor: add qemuMonitorQueryStats
On Wed, Jul 06, 2022 at 04:56:26AM +0530, Amneesh Singh wrote:
Related: https://gitlab.com/libvirt/libvirt/-/issues/276
This patch adds an API for the "query-stats" QMP command.
The query returns a JSON containing the statistics based on the target,
which can either be vCPU or VM, and the providers. The API deserializes
the query result into an array of GHashMaps, which can later be used to
extract all the query statistics. GHashMaps are used to avoid traversing
the entire array to find the statistics you are looking for. This would
be a singleton array if the target is a VM since the returned JSON is
also a singleton array in that case.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_monitor.c | 106 ++++++++++++++++++++++++++++
src/qemu/qemu_monitor.h | 56 +++++++++++++++
src/qemu/qemu_monitor_json.c | 130 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.h | 6 ++
4 files changed, 298 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fda5d2f3..528aed49 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4541,3 +4541,109 @@ qemuMonitorMigrateRecover(qemuMonitor *mon,
return qemuMonitorJSONMigrateRecover(mon, uri);
}
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsTarget,
+ QEMU_MONITOR_QUERY_STATS_TARGET_LAST,
+ "vm",
+ "vcpu",
+);
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsVmName,
+ QEMU_MONITOR_QUERY_STATS_VM_NAME_LAST,
+);
+
+VIR_ENUM_IMPL(qemuMonitorQueryStatsVcpuName,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_LAST,
+ "halt_poll_success_ns",
+ "halt_poll_fail_ns"
+);
+
As we discussed these two can be joined together and the code will look
much nicer. I don't think we need to differentiate between those,
definitely not now sice the former one is not even used yet.
+VIR_ENUM_IMPL(qemuMonitorQueryStatsProvider,
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST,
+ "kvm",
+);
+
+const char *
+qemuMonitorQueryStatsNameTypeToString(qemuMonitorQueryStatsTargetType target,
+ int type)
With the merged enums this function will not be needed at all, and there
are other similar cases, but since you suggested yourself you know it
very well too ;)
+{
+ switch (target) {
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VM:
+ return qemuMonitorQueryStatsVmNameTypeToString(type);
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VCPU:
+ return qemuMonitorQueryStatsVcpuNameTypeToString(type);
+
+ /* unreachable */
+ case QEMU_MONITOR_QUERY_STATS_TARGET_LAST:
+ ;
+ }
+
+ return NULL;
+}
+
+void
+qemuMonitorQueryStatsProviderFree(qemuMonitorQueryStatsProvider *provider)
+{
+ virBitmapFree(provider->names);
+ g_free(provider);
+}
+
+qemuMonitorQueryStatsProvider *
+qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsTargetType target,
+ qemuMonitorQueryStatsProviderType provider_type,
+ ...)
+{
+ g_autoptr(qemuMonitorQueryStatsProvider) provider = NULL;
+ va_list name_list;
+ int stat;
+
+ provider = g_new0(qemuMonitorQueryStatsProvider, 1);
+ provider->provider = provider_type;
+ provider->names = NULL;
+
+ va_start(name_list, provider_type);
+ stat = va_arg(name_list, int);
I have a suspicion that sizeof(enum ...) is guaranteed to equal
sizeof(int) everywhere. But you should just be able to do
va_arg(name_list, qemuMonitorQueryStats)
+
+ if (stat != -1) {
and instead of -1 you can pass and check the _LAST value.
+ size_t sz = 0;
+
+ switch (target) {
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VM:
+ sz = QEMU_MONITOR_QUERY_STATS_VM_NAME_LAST;
+ break;
+ case QEMU_MONITOR_QUERY_STATS_TARGET_VCPU:
+ sz = QEMU_MONITOR_QUERY_STATS_VCPU_NAME_LAST;
+ break;
+ case QEMU_MONITOR_QUERY_STATS_TARGET_LAST:
+ break;
+ }
+
+ provider->names = virBitmapNew(sz);
+
+ while (stat != -1) {
+ if (virBitmapSetBit(provider->names, stat) < 0)
+ return NULL;
+ stat = va_arg(name_list, int);
+ }
+ }
+
+ va_end(name_list);
+
+ return g_steal_pointer(&provider);
+}
+
+GPtrArray *
+qemuMonitorQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers)
+{
+ VIR_DEBUG("target=%u vcpus=%p providers=%p", target, vcpus, providers);
+
+ QEMU_CHECK_MONITOR_NULL(mon);
+
+ if (target != QEMU_MONITOR_QUERY_STATS_TARGET_VCPU && !vcpus)
I think you meant:
if (target != QEMU_MONITOR_QUERY_STATS_TARGET_VCPU && vcpus)
+ return NULL;
+
+ return qemuMonitorJSONQueryStats(mon, target, vcpus, providers);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 95267ec6..26df9c63 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1554,3 +1554,59 @@ qemuMonitorChangeMemoryRequestedSize(qemuMonitor *mon,
int
qemuMonitorMigrateRecover(qemuMonitor *mon,
const char *uri);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_TARGET_VM,
+ QEMU_MONITOR_QUERY_STATS_TARGET_VCPU,
+ QEMU_MONITOR_QUERY_STATS_TARGET_LAST,
+} qemuMonitorQueryStatsTargetType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsTarget);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_VM_NAME_LAST,
+} qemuMonitorQueryStatsVmNameType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsVmName);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_SUCCESS_NS,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_FAIL_NS,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_LAST,
+} qemuMonitorQueryStatsVcpuNameType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsVcpuName);
+
+const char *
+qemuMonitorQueryStatsNameTypeToString(qemuMonitorQueryStatsTargetType target,
+ int type);
+
+typedef enum {
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_KVM,
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST,
+} qemuMonitorQueryStatsProviderType;
+
+VIR_ENUM_DECL(qemuMonitorQueryStatsProvider);
+
+typedef struct _qemuMonitorQueryStatsProvider qemuMonitorQueryStatsProvider;
+struct _qemuMonitorQueryStatsProvider {
+ qemuMonitorQueryStatsProviderType provider;
+ virBitmap *names;
+};
+
+void
+qemuMonitorQueryStatsProviderFree(qemuMonitorQueryStatsProvider *provider);
+
+qemuMonitorQueryStatsProvider *
+qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsTargetType target,
+ qemuMonitorQueryStatsProviderType provider_type,
+ ...);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorQueryStatsProvider,
+ qemuMonitorQueryStatsProviderFree);
+
+GPtrArray *
+qemuMonitorQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3aad2ab2..8774b1ff 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9006,3 +9006,133 @@ qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
return qemuMonitorJSONCheckError(cmd, reply);
}
+
+static GPtrArray *
+qemuMonitorJSONExtractQueryStats(virJSONValue *arr)
+{
+ g_autoptr(GPtrArray) queried_stats = NULL;
+ size_t nstats = virJSONValueArraySize(arr);
+ size_t i;
+
+ /* Create a GPtrArray for GHashTables */
+ queried_stats = g_ptr_array_new_full(nstats, (GDestroyNotify) g_hash_table_destroy);
+
+ for (i = 0; i < nstats; i++) {
+ virJSONValue *obj = virJSONValueArrayGet(arr, i);
+ virJSONValue *stats = virJSONValueObjectGetArray(obj, "stats");
+ size_t j;
+
+ /* Create a GHashTable for virJSONValues */
+ GHashTable *hash_table = virHashNew((GDestroyNotify) virJSONValueFree);
+
+ for (j = 0; j < virJSONValueArraySize(stats); j++) {
+ virJSONValue *stat = virJSONValueArrayGet(stats, j);
+
+ g_hash_table_insert(hash_table,
+ g_strdup(virJSONValueObjectGetString(stat,
"name")),
+ virJSONValueObjectGet(stat, "value"));
+ }
+
+ g_ptr_array_add(queried_stats, hash_table);
+ }
+
+ return g_steal_pointer(&queried_stats);
+}
+
+
+/**
+ * qemuMonitorJSONQueryStats:
+ * @mon: monitor object
+ * @target: the target type for the query
+ * @vcpus: a list of vCPU QOM paths for filtering the statistics
+ * @providers: an array of providers to filter statistics
+ *
+ * @vcpus is a NULL terminated array of strings. @providers is a GPtrArray
+ * for qemuMonitorQueryStatsProvider.
+ * @vcpus and @providers are optional and can be NULL.
+ *
+ * Queries for the @target based statistics.
+ * Returns a GPtrArray of GHashTables containing the statistics on success or
+ * NULL on failure.
+ */
+
+GPtrArray *
+qemuMonitorJSONQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers)
+{
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
+ g_autoptr(virJSONValue) vcpu_list = NULL;
+ g_autoptr(virJSONValue) provider_list = NULL;
+ virJSONValue *rv = NULL;
+
+ size_t i;
+
+ if (providers) {
+ provider_list = virJSONValueNewArray();
+
+ for (i = 0; i < providers->len; i++) {
+ g_autoptr(virJSONValue) provider_obj = virJSONValueNewObject();
+ qemuMonitorQueryStatsProvider *provider = providers->pdata[i];
+ const char *type_str =
qemuMonitorQueryStatsProviderTypeToString(provider->provider);
+ virBitmap *names = provider->names;
+ int rc;
+
+ rc = virJSONValueObjectAppendString(provider_obj, "provider",
type_str);
+
+ if (rc < 0)
+ return NULL;
+
+ if (names) {
+ g_autoptr(virJSONValue) provider_names = virJSONValueNewArray();
+ int curBit = virBitmapNextSetBit(names, -1);
+
+ while (curBit >= 0) {
+ const char *name = qemuMonitorQueryStatsNameTypeToString(target,
curBit);
+
+ if (virJSONValueArrayAppendString(provider_names, name) < 0)
+ return NULL;
+
+ curBit = virBitmapNextSetBit(names, curBit);
+ }
+
+ rc = virJSONValueObjectAppend(provider_obj, "names",
&provider_names);
+
+ if (rc < 0)
+ return NULL;
+ }
+
+ if (virJSONValueArrayAppend(provider_list, &provider_obj) < 0)
+ return NULL;
+ }
+ }
+
+ if (vcpus) {
+ vcpu_list = virJSONValueNewArray();
+
+ for (i = 0; vcpus[i]; i++)
+ if (virJSONValueArrayAppendString(vcpu_list, vcpus[i]) < 0)
+ return NULL;
+ }
+
+ cmd = qemuMonitorJSONMakeCommand("query-stats",
+ "s:target",
qemuMonitorQueryStatsTargetTypeToString(target),
+ "A:vcpus", &vcpu_list,
+ "A:providers", &provider_list,
+ NULL);
+
+ if (!cmd)
+ return NULL;
+
+ if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+ return NULL;
+
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ return NULL;
+
+ rv = virJSONValueObjectStealArray(reply, "return");
+
+ return qemuMonitorJSONExtractQueryStats(rv);
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index ad3853ae..db8dec80 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -878,3 +878,9 @@ qemuMonitorJSONChangeMemoryRequestedSize(qemuMonitor *mon,
int
qemuMonitorJSONMigrateRecover(qemuMonitor *mon,
const char *uri);
+
+GPtrArray *
+qemuMonitorJSONQueryStats(qemuMonitor *mon,
+ qemuMonitorQueryStatsTargetType target,
+ char **vcpus,
+ GPtrArray *providers);
--
2.36.1
6:26 p.m.
New subject: [PATCH RFC v2 2/3] qemu_capabilities: add "query-stats" QMP command to the QEMU capabilities
Related: https://gitlab.com/libvirt/libvirt/-/issues/276
This patch adds "query-stats" to the QEMU capabilities.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2c3be3ec..81fc597a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -671,6 +671,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"chardev.qemu-vdagent", /* QEMU_CAPS_CHARDEV_QEMU_VDAGENT */
"display-dbus", /* QEMU_CAPS_DISPLAY_DBUS */
"iothread.thread-pool-max", /* QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX
*/
+ "query-stats", /* QEMU_CAPS_QUERY_STATS */
);
@@ -1225,6 +1226,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-dirty-rate", QEMU_CAPS_QUERY_DIRTY_RATE },
{ "sev-inject-launch-secret", QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET },
{ "calc-dirty-rate", QEMU_CAPS_CALC_DIRTY_RATE },
+ { "query-stats", QEMU_CAPS_QUERY_STATS },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6f35ba14..29e55b3a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -650,6 +650,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
QEMU_CAPS_CHARDEV_QEMU_VDAGENT, /* -chardev qemu-vdagent */
QEMU_CAPS_DISPLAY_DBUS, /* -display dbus */
QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX, /* -object iothread.thread-pool-max */
+ QEMU_CAPS_QUERY_STATS, /*accepts query-stats */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.36.1
6:22 a.m.
New subject: [PATCH RFC v2 2/3] qemu_capabilities: add "query-stats" QMP command to the QEMU capabilities
On Wed, Jul 06, 2022 at 04:56:27AM +0530, Amneesh Singh wrote:
Related: https://gitlab.com/libvirt/libvirt/-/issues/276
This patch adds "query-stats" to the QEMU capabilities.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2c3be3ec..81fc597a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -671,6 +671,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"chardev.qemu-vdagent", /* QEMU_CAPS_CHARDEV_QEMU_VDAGENT */
"display-dbus", /* QEMU_CAPS_DISPLAY_DBUS */
"iothread.thread-pool-max", /* QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX
*/
+ "query-stats", /* QEMU_CAPS_QUERY_STATS */
);
@@ -1225,6 +1226,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-dirty-rate", QEMU_CAPS_QUERY_DIRTY_RATE },
{ "sev-inject-launch-secret", QEMU_CAPS_SEV_INJECT_LAUNCH_SECRET },
{ "calc-dirty-rate", QEMU_CAPS_CALC_DIRTY_RATE },
+ { "query-stats", QEMU_CAPS_QUERY_STATS },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6f35ba14..29e55b3a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -650,6 +650,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
QEMU_CAPS_CHARDEV_QEMU_VDAGENT, /* -chardev qemu-vdagent */
QEMU_CAPS_DISPLAY_DBUS, /* -display dbus */
QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX, /* -object iothread.thread-pool-max */
+ QEMU_CAPS_QUERY_STATS, /*accepts query-stats */
missing space after /*
with that
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.36.1
6:26 p.m.
New subject: [PATCH RFC v2 3/3] qemu_driver: use qemuMonitorQueryStats to extract halt poll time
Related: https://gitlab.com/libvirt/libvirt/-/issues/276
This patch uses qemuMonitorQueryStats to query "halt_poll_success_ns"
and "halt_poll_fail_ns" for every vCPU. The respective values for each
vCPU are then added together.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_driver.c | 70 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 62 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 97c6ed95..30170d5c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18052,15 +18052,69 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom,
}
static int
-qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
- virTypedParamList *params)
+qemuDomainGetStatsCpuHaltPollTime(virQEMUDriver *driver,
+ virDomainObj *dom,
+ virTypedParamList *params,
+ unsigned int privflags)
{
unsigned long long haltPollSuccess = 0;
unsigned long long haltPollFail = 0;
- pid_t pid = dom->pid;
+ qemuDomainObjPrivate *priv = dom->privateData;
+ bool queryStatsCap = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_STATS);
- if (virHostCPUGetHaltPollTime(pid, &haltPollSuccess, &haltPollFail) < 0)
- return 0;
+ if (queryStatsCap && HAVE_JOB(privflags) && virDomainObjIsActive(dom)
&& driver->privileged) {
+ size_t i;
+ qemuMonitorQueryStatsTargetType target = QEMU_MONITOR_QUERY_STATS_TARGET_VCPU;
+ qemuMonitorQueryStatsProvider *provider = NULL;
+ g_autoptr(GPtrArray) providers = NULL;
+ g_autoptr(GPtrArray) queried_stats = NULL;
+ provider = qemuMonitorQueryStatsProviderNew(
+ target,
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_KVM,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_SUCCESS_NS,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_FAIL_NS,
+ -1);
+
+ if (!provider)
+ return 0;
+
+ providers = g_ptr_array_new_full(1, (GDestroyNotify)
qemuMonitorQueryStatsProviderFree);
+ g_ptr_array_add(providers, provider);
+
+ qemuDomainObjEnterMonitor(driver, dom);
+ queried_stats = qemuMonitorQueryStats(priv->mon, target, NULL, providers);
+ qemuDomainObjExitMonitor(dom);
+
+ if (!queried_stats)
+ return 0;
+
+ for (i = 0; i < queried_stats->len; i++) {
+ unsigned long long curHaltPollSuccess, curHaltPollFail;
+ GHashTable *cur_table = queried_stats->pdata[i];
+ virJSONValue *success_obj, *fail_obj;
+ const char *success_str = qemuMonitorQueryStatsVcpuNameTypeToString(
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_SUCCESS_NS);
+ const char *fail_str = qemuMonitorQueryStatsVcpuNameTypeToString(
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_FAIL_NS);
+
+ success_obj = g_hash_table_lookup(cur_table, success_str);
+ fail_obj = g_hash_table_lookup(cur_table, fail_str);
+
+ if (!(success_obj && fail_obj))
+ return 0;
+
+ ignore_value(virJSONValueGetNumberUlong(success_obj,
&curHaltPollSuccess));
+ ignore_value(virJSONValueGetNumberUlong(fail_obj, &curHaltPollFail));
+
+ haltPollSuccess += curHaltPollSuccess;
+ haltPollFail += curHaltPollFail;
+ }
+ } else {
+ pid_t pid = dom->pid;
+
+ if (virHostCPUGetHaltPollTime(pid, &haltPollSuccess, &haltPollFail) <
0)
+ return 0;
+ }
if (virTypedParamListAddULLong(params, haltPollSuccess,
"cpu.haltpoll.success.time") < 0 ||
virTypedParamListAddULLong(params, haltPollFail,
"cpu.haltpoll.fail.time") < 0)
@@ -18073,7 +18127,7 @@ static int
qemuDomainGetStatsCpu(virQEMUDriver *driver,
virDomainObj *dom,
virTypedParamList *params,
- unsigned int privflags G_GNUC_UNUSED)
+ unsigned int privflags)
{
if (qemuDomainGetStatsCpuCgroup(dom, params) < 0)
return -1;
@@ -18081,7 +18135,7 @@ qemuDomainGetStatsCpu(virQEMUDriver *driver,
if (qemuDomainGetStatsCpuCache(driver, dom, params) < 0)
return -1;
- if (qemuDomainGetStatsCpuHaltPollTime(dom, params) < 0)
+ if (qemuDomainGetStatsCpuHaltPollTime(driver, dom, params, privflags) < 0)
return -1;
return 0;
@@ -18915,7 +18969,7 @@ static virQEMUCapsFlags queryDirtyRateRequired[] = {
static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = {
{ qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false, NULL },
- { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false, NULL },
+ { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, true, NULL },
{ qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true, NULL },
{ qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, true, NULL },
{ qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false, NULL },
--
2.36.1
9:26 a.m.
New subject: [PATCH RFC v2 3/3] qemu_driver: use qemuMonitorQueryStats to extract halt poll time
On Wed, Jul 06, 2022 at 04:56:28AM +0530, Amneesh Singh wrote:
Related: https://gitlab.com/libvirt/libvirt/-/issues/276
This patch uses qemuMonitorQueryStats to query "halt_poll_success_ns"
and "halt_poll_fail_ns" for every vCPU. The respective values for each
vCPU are then added together.
Signed-off-by: Amneesh Singh <natto(a)weirdnatto.in>
---
src/qemu/qemu_driver.c | 70 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 62 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 97c6ed95..30170d5c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18052,15 +18052,69 @@ qemuDomainGetStatsCpuCgroup(virDomainObj *dom,
}
static int
-qemuDomainGetStatsCpuHaltPollTime(virDomainObj *dom,
- virTypedParamList *params)
+qemuDomainGetStatsCpuHaltPollTime(virQEMUDriver *driver,
+ virDomainObj *dom,
+ virTypedParamList *params,
+ unsigned int privflags)
{
unsigned long long haltPollSuccess = 0;
unsigned long long haltPollFail = 0;
- pid_t pid = dom->pid;
+ qemuDomainObjPrivate *priv = dom->privateData;
+ bool queryStatsCap = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_STATS);
- if (virHostCPUGetHaltPollTime(pid, &haltPollSuccess, &haltPollFail) < 0)
- return 0;
+ if (queryStatsCap && HAVE_JOB(privflags) && virDomainObjIsActive(dom)
&& driver->privileged) {
Why is there a check for whether the driver is privileged? I thought
this can also work with a session mode.
+ size_t i;
+ qemuMonitorQueryStatsTargetType target = QEMU_MONITOR_QUERY_STATS_TARGET_VCPU;
+ qemuMonitorQueryStatsProvider *provider = NULL;
+ g_autoptr(GPtrArray) providers = NULL;
+ g_autoptr(GPtrArray) queried_stats = NULL;
+ provider = qemuMonitorQueryStatsProviderNew(
+ target,
+ QEMU_MONITOR_QUERY_STATS_PROVIDER_KVM,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_SUCCESS_NS,
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_FAIL_NS,
+ -1);
+
+ if (!provider)
+ return 0;
In this case you can still do the callback. Basically don't put it in
an "else" branch, just get a new condition checking if the stats were
gathered or not (if need be you can get a new variable for that).
+
+ providers = g_ptr_array_new_full(1, (GDestroyNotify)
qemuMonitorQueryStatsProviderFree);
+ g_ptr_array_add(providers, provider);
+
+ qemuDomainObjEnterMonitor(driver, dom);
+ queried_stats = qemuMonitorQueryStats(priv->mon, target, NULL, providers);
+ qemuDomainObjExitMonitor(dom);
+
+ if (!queried_stats)
+ return 0;
+
+ for (i = 0; i < queried_stats->len; i++) {
+ unsigned long long curHaltPollSuccess, curHaltPollFail;
+ GHashTable *cur_table = queried_stats->pdata[i];
+ virJSONValue *success_obj, *fail_obj;
+ const char *success_str = qemuMonitorQueryStatsVcpuNameTypeToString(
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_SUCCESS_NS);
+ const char *fail_str = qemuMonitorQueryStatsVcpuNameTypeToString(
+ QEMU_MONITOR_QUERY_STATS_VCPU_NAME_HALT_POLL_FAIL_NS);
+
+ success_obj = g_hash_table_lookup(cur_table, success_str);
+ fail_obj = g_hash_table_lookup(cur_table, fail_str);
+
If you want it could be nicer to have an extra function for this since
it looks like it'll be repeated. Also have a look if the enums can't be
defined elsewhere/differently so that you can end up with something like
this:
virJSONValue *success_obj = qemuStatsGet(QEMU_STATS_NAME_HALT_POLL_SUCCESS_NS);
+ if (!(success_obj && fail_obj))
+ return 0;
+
This checks if they both exist (although I would prefer (!success_obj ||
!fail_obj) as it is more readable and for me it is similar to how I
would say that in a sentence) but does not check that they really are a
number.
+ ignore_value(virJSONValueGetNumberUlong(success_obj,
&curHaltPollSuccess));
+ ignore_value(virJSONValueGetNumberUlong(fail_obj, &curHaltPollFail));
+
That's why you should also check that the virJSONValueGetNumberUlong
does not return -1. That's why it makes you use the return value.
Also these are another cases where you can execute the fallback instead.
Thanks for taking the time to polish this.
869
days inactive
871
days old
6 comments
2 participants
participants (2)
-
Amneesh Singh -
Martin Kletzander