* [PATCH v4 0/3] Support fd-based KVM stats
@ 2022-02-15 15:04 Mark Kanda
2022-02-15 15:04 ` [PATCH v4 1/3] qmp: Support for querying stats Mark Kanda
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Mark Kanda @ 2022-02-15 15:04 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, armbru
This patchset adds QEMU support for querying fd-based KVM stats. The
kernel support was introduced by:
cb082bfab59a ("KVM: stats: Add fd-based API to read binary stats data")
v4:
- revise and cleanup the API [Daniel, Paolo]
- filtering for multiple providers [Daniel]
- cache KVM stat descriptors [Paolo]
- use g_autofree and other cleanup [Daniel]
v3:
- various QMP API enhancements from review [Daniel, Paolo, Igor]
- general cleanup
v2: [Paolo]
- generalize the interface
- add support for querying stat schema and instances
- add additional HMP semantic processing for a few exponent/unit
combinations (related to seconds and bytes)
Mark Kanda (3):
qmp: Support for querying stats
hmp: Support for querying stats
kvm: Support for querying fd-based stats
accel/kvm/kvm-all.c | 393 ++++++++++++++++++++++++++++++++++++++++
hmp-commands-info.hx | 28 +++
include/monitor/hmp.h | 2 +
include/monitor/stats.h | 51 ++++++
monitor/hmp-cmds.c | 328 +++++++++++++++++++++++++++++++++
monitor/qmp-cmds.c | 219 ++++++++++++++++++++++
qapi/meson.build | 1 +
qapi/qapi-schema.json | 1 +
qapi/stats.json | 259 ++++++++++++++++++++++++++
9 files changed, 1282 insertions(+)
create mode 100644 include/monitor/stats.h
create mode 100644 qapi/stats.json
--
2.27.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/3] qmp: Support for querying stats
2022-02-15 15:04 [PATCH v4 0/3] Support fd-based KVM stats Mark Kanda
@ 2022-02-15 15:04 ` Mark Kanda
2022-03-11 13:06 ` Markus Armbruster
2022-02-15 15:04 ` [PATCH v4 2/3] hmp: " Mark Kanda
2022-02-15 15:04 ` [PATCH v4 3/3] kvm: Support for querying fd-based stats Mark Kanda
2 siblings, 1 reply; 10+ messages in thread
From: Mark Kanda @ 2022-02-15 15:04 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, armbru
Introduce QMP support for querying stats. Provide a framework for adding new
stats and support for the following commands:
- query-stats
Returns a list of all stats per target type (only VM and vCPU to start), with
additional options for specifying stat names, vCPU qom paths, and providers.
- query-stats-schemas
Returns a list of stats included in each target type, with an option for
specifying the provider.
The framework provides a method to register callbacks for these QMP commands.
The first use-case will be for fd-based KVM stats (in an upcoming patch).
Examples (with fd-based KVM stats):
- Query all VM stats:
{ "execute": "query-stats", "arguments" : { "target": "vm" } }
{ "return": {
"vm": [
{ "provider": "kvm",
"stats": [
{ "name": "max_mmu_page_hash_collisions", "value": 0 },
{ "name": "max_mmu_rmap_size", "value": 0 },
{ "name": "nx_lpage_splits", "value": 148 },
...
{ "provider": "xyz",
"stats": [ ...
...
] } }
- Query all vCPU stats:
{ "execute": "query-stats", "arguments" : { "target": "vcpu" } }
{ "return": {
"vcpus": [
{ "path": "/machine/unattached/device[0]"
"providers": [
{ "provider": "kvm",
"stats": [
{ "name": "guest_mode", "value": 0 },
{ "name": "directed_yield_successful", "value": 0 },
{ "name": "directed_yield_attempted", "value": 106 },
...
{ "provider": "xyz",
"stats": [ ...
...
{ "path": "/machine/unattached/device[1]"
"providers": [
{ "provider": "kvm",
"stats": [...
...
} ] } }
- Query 'exits' and 'l1d_flush' KVM stats, and 'somestat' from provider 'xyz'
for vCPUs '/machine/unattached/device[2]' and '/machine/unattached/device[4]':
{ "execute": "query-stats",
"arguments": {
"target": "vcpu",
"vcpus": [ "/machine/unattached/device[2]",
"/machine/unattached/device[4]" ],
"filters": [
{ "provider": "kvm",
"fields": [ "l1d_flush", "exits" ] },
{ "provider": "xyz",
"fields": [ "somestat" ] } ] } }
{ "return": {
"vcpus": [
{ "path": "/machine/unattached/device[2]"
"providers": [
{ "provider": "kvm",
"stats": [ { "name": "l1d_flush", "value": 41213 },
{ "name": "exits", "value": 74291 } ] },
{ "provider": "xyz",
"stats": [ ... ] } ] },
{ "path": "/machine/unattached/device[4]"
"providers": [
{ "provider": "kvm",
"stats": [ { "name": "l1d_flush", "value": 16132 },
{ "name": "exits", "value": 57922 } ] },
{ "provider": "xyz",
"stats": [ ... ] } ] } ] } }
- Query stats schemas:
{ "execute": "query-stats-schemas" }
{ "return": {
"vcpu": [
{ "provider": "kvm",
"stats": [
{ "name": "guest_mode",
"unit": "none",
"base": 10,
"exponent": 0,
"type": "instant" },
{ "name": "directed_yield_successful",
"unit": "none",
"base": 10,
"exponent": 0,
"type": "cumulative" },
...
{ "provider": "xyz",
...
"vm": [
{ "provider": "kvm",
"stats": [
{ "name": "max_mmu_page_hash_collisions",
"unit": "none",
"base": 10,
"exponent": 0,
"type": "peak" },
{ "provider": "xyz",
...
Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
include/monitor/stats.h | 51 ++++++++
monitor/qmp-cmds.c | 219 +++++++++++++++++++++++++++++++++
qapi/meson.build | 1 +
qapi/qapi-schema.json | 1 +
qapi/stats.json | 259 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 531 insertions(+)
create mode 100644 include/monitor/stats.h
create mode 100644 qapi/stats.json
diff --git a/include/monitor/stats.h b/include/monitor/stats.h
new file mode 100644
index 0000000000..172dc01a4d
--- /dev/null
+++ b/include/monitor/stats.h
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2022 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef STATS_H
+#define STATS_H
+
+#include "qapi/qapi-types-stats.h"
+
+/*
+ * Add QMP stats callbacks to the stats_callbacks list.
+ *
+ * @provider: stats provider
+ *
+ * @stats_fn: routine to query stats:
+ * void (*stats_fn)(StatsResults *results, StatsFilter *filter, Error **errp)
+ *
+ * @schema_fn: routine to query stat schemas:
+ * void (*schemas_fn)(StatsSchemaResult *results, Error **errp)
+ */
+void add_stats_callbacks(StatsProvider provider,
+ void (*stats_fn)(StatsResults *, StatsFilter *,
+ Error **),
+ void (*schemas_fn)(StatsSchemaResults *, Error **));
+
+/*
+ * Helper routines for adding stats entries to the results lists.
+ */
+void add_vm_stats_entry(StatsList *, StatsResults *, StatsProvider);
+void add_vcpu_stats_entry(StatsList *, StatsResults *, StatsProvider, char *);
+void add_vm_stats_schema(StatsSchemaValueList *, StatsSchemaResults *,
+ StatsProvider);
+void add_vcpu_stats_schema(StatsSchemaValueList *, StatsSchemaResults *,
+ StatsProvider);
+
+/*
+ * True if a stat name and provider match a filter or if no corresponding
+ * filters are defined. False otherwise.
+ */
+bool stats_requested_name(const char *, StatsProvider, StatsFilter *);
+
+/*
+ * True if a vcpu qom path and provider match a filter or if no corresponding
+ * filters are defined. False otherwise.
+ */
+bool stats_requested_vcpu(const char *, StatsProvider, StatsFilter *);
+
+#endif /* STATS_H */
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index db4d186448..07f1e683be 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -36,6 +36,7 @@
#include "qapi/qapi-commands-control.h"
#include "qapi/qapi-commands-machine.h"
#include "qapi/qapi-commands-misc.h"
+#include "qapi/qapi-commands-stats.h"
#include "qapi/qapi-commands-ui.h"
#include "qapi/type-helpers.h"
#include "qapi/qmp/qerror.h"
@@ -44,6 +45,7 @@
#include "hw/acpi/acpi_dev_interface.h"
#include "hw/intc/intc.h"
#include "hw/rdma/rdma.h"
+#include "monitor/stats.h"
NameInfo *qmp_query_name(Error **errp)
{
@@ -448,3 +450,220 @@ HumanReadableText *qmp_x_query_irq(Error **errp)
return human_readable_text_from_str(buf);
}
+
+typedef struct StatsCallbacks {
+ StatsProvider provider;
+ void (*stats_cb)(StatsResults *, StatsFilter *, Error **);
+ void (*schemas_cb)(StatsSchemaResults *, Error **);
+ QTAILQ_ENTRY(StatsCallbacks) next;
+} StatsCallbacks;
+
+static QTAILQ_HEAD(, StatsCallbacks) stats_callbacks =
+ QTAILQ_HEAD_INITIALIZER(stats_callbacks);
+
+void add_stats_callbacks(StatsProvider provider,
+ void (*stats_fn)(StatsResults *, StatsFilter*,
+ Error **),
+ void (*schemas_fn)(StatsSchemaResults *, Error **))
+{
+ StatsCallbacks *entry = g_malloc0(sizeof(*entry));
+ entry->provider = provider;
+ entry->stats_cb = stats_fn;
+ entry->schemas_cb = schemas_fn;
+
+ QTAILQ_INSERT_TAIL(&stats_callbacks, entry, next);
+}
+
+static StatsRequestList *stats_target_filter(StatsFilter *filter)
+{
+ switch (filter->target) {
+ case STATS_TARGET_VM:
+ if (!filter->u.vm.has_filters) {
+ return NULL;
+ }
+ return filter->u.vm.filters;
+ case STATS_TARGET_VCPU:
+ if (!filter->u.vcpu.has_filters) {
+ return NULL;
+ }
+ return filter->u.vcpu.filters;
+ break;
+ default:
+ return NULL;
+ }
+}
+
+static bool stats_provider_match(StatsProvider provider,
+ StatsRequestList *request)
+{
+ return (!request->value->has_provider ||
+ (request->value->provider == provider));
+}
+
+static bool stats_requested_provider(StatsProvider provider,
+ StatsFilter *filter)
+{
+ StatsRequestList *request = stats_target_filter(filter);
+
+ if (!request) {
+ return true;
+ }
+ for (; request; request = request->next) {
+ if (stats_provider_match(provider, request)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+StatsResults *qmp_query_stats(StatsFilter *filter, Error **errp)
+{
+ StatsResults *stats_results = g_malloc0(sizeof(*stats_results));
+ StatsCallbacks *entry;
+
+ QTAILQ_FOREACH(entry, &stats_callbacks, next) {
+ if (stats_requested_provider(entry->provider, filter)) {
+ entry->stats_cb(stats_results, filter, errp);
+ }
+ }
+
+ return stats_results;
+}
+
+StatsSchemaResults *qmp_query_stats_schemas(bool has_provider,
+ StatsProvider provider,
+ Error **errp)
+{
+ StatsSchemaResults *stats_results = g_malloc0(sizeof(*stats_results));
+ StatsCallbacks *entry;
+
+ QTAILQ_FOREACH(entry, &stats_callbacks, next) {
+ if (has_provider && (provider != entry->provider)) {
+ continue;
+ }
+ entry->schemas_cb(stats_results, errp);
+ }
+
+ return stats_results;
+}
+
+void add_vm_stats_entry(StatsList *stats_list, StatsResults *stats_results,
+ StatsProvider provider)
+{
+ StatsResultsEntry *entry = g_malloc0(sizeof(*entry));
+ entry->provider = provider;
+ entry->stats = stats_list;
+
+ QAPI_LIST_PREPEND(stats_results->vm, entry);
+ stats_results->has_vm = true;
+}
+
+void add_vcpu_stats_entry(StatsList *stats_list, StatsResults *stats_results,
+ StatsProvider provider, char *path)
+{
+ StatsResultsVCPUEntry *value;
+ StatsResultsEntry *entry;
+ StatsResultsVCPUEntryList **tailp, *tail;
+
+ entry = g_malloc0(sizeof(*entry));
+ entry->provider = provider;
+ entry->stats = stats_list;
+
+ /* Find the vCPU entry and add to its list; else create it */
+ tailp = &stats_results->vcpus;
+
+ for (tail = *tailp; tail; tail = tail->next) {
+ if (g_str_equal(tail->value->path, path)) {
+ /* Add to the existing vCPU list */
+ QAPI_LIST_PREPEND(tail->value->providers, entry);
+ return;
+ }
+ tailp = &tail->next;
+ }
+
+ /* Create and populate a new vCPU entry */
+ value = g_malloc0(sizeof(*value));
+ value->path = g_strdup(path);
+ value->providers = g_malloc0(sizeof(*value->providers));
+ value->providers->value = entry;
+ QAPI_LIST_APPEND(tailp, value);
+ stats_results->has_vcpus = true;
+}
+
+void add_vm_stats_schema(StatsSchemaValueList *stats_list,
+ StatsSchemaResults *schema_results,
+ StatsProvider provider)
+{
+ StatsSchemaProvider *entry = g_malloc0(sizeof(*entry));
+
+ entry->provider = provider;
+ entry->stats = stats_list;
+ QAPI_LIST_PREPEND(schema_results->vm, entry);
+ schema_results->has_vm = true;
+}
+
+void add_vcpu_stats_schema(StatsSchemaValueList *stats_list,
+ StatsSchemaResults *schema_results,
+ StatsProvider provider)
+{
+ StatsSchemaProvider *entry = g_malloc0(sizeof(*entry));
+
+ entry->provider = provider;
+ entry->stats = stats_list;
+ QAPI_LIST_PREPEND(schema_results->vcpu, entry);
+ schema_results->has_vcpu = true;
+}
+
+static bool str_in_list(const char *name, strList *list)
+{
+ strList *str_list = NULL;
+
+ for (str_list = list; str_list; str_list = str_list->next) {
+ if (g_str_equal(name, str_list->value)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+bool stats_requested_name(const char *name, StatsProvider provider,
+ StatsFilter *filter)
+{
+ StatsRequestList *request = stats_target_filter(filter);
+
+ if (!request) {
+ return true;
+ }
+ for (; request; request = request->next) {
+ if (!stats_provider_match(provider, request)) {
+ continue;
+ }
+ if (!request->value->has_fields ||
+ str_in_list(name, request->value->fields)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+bool stats_requested_vcpu(const char *path, StatsProvider provider,
+ StatsFilter *filter)
+{
+ StatsRequestList *request = stats_target_filter(filter);
+
+ if (!request) {
+ return true;
+ }
+ if (!filter->u.vcpu.has_filters) {
+ return true;
+ }
+ if (filter->u.vcpu.has_vcpus && !str_in_list(path, filter->u.vcpu.vcpus)) {
+ return false;
+ }
+ for (; request; request = request->next) {
+ if (stats_provider_match(provider, request)) {
+ return true;
+ }
+ }
+ return false;
+}
diff --git a/qapi/meson.build b/qapi/meson.build
index 656ef0e039..fd5c93d643 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -46,6 +46,7 @@ qapi_all_modules = [
'replay',
'run-state',
'sockets',
+ 'stats',
'trace',
'transaction',
'yank',
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 4912b9744e..92d7ecc52c 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -93,3 +93,4 @@
{ 'include': 'audio.json' }
{ 'include': 'acpi.json' }
{ 'include': 'pci.json' }
+{ 'include': 'stats.json' }
diff --git a/qapi/stats.json b/qapi/stats.json
new file mode 100644
index 0000000000..ae5dc3ee2c
--- /dev/null
+++ b/qapi/stats.json
@@ -0,0 +1,259 @@
+# -*- Mode: Python -*-
+# vim: filetype=python
+#
+# Copyright (c) 2022 Oracle and/or its affiliates.
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+##
+# = Stats
+##
+
+##
+# @StatsType:
+#
+# Enumeration of stats types
+# @cumulative: stat is cumulative; value can only increase.
+# @instant: stat is instantaneous; value can increase or decrease.
+# @peak: stat is the peak value; value can only increase.
+# @linear-hist: stat is a linear histogram.
+# @log-hist: stat is a logarithmic histogram.
+#
+# Since: 7.0
+##
+{ 'enum' : 'StatsType',
+ 'data' : [ 'cumulative', 'instant', 'peak', 'linear-hist', 'log-hist' ] }
+
+##
+# @StatsUnit:
+#
+# Enumeration of stats units
+# @bytes: stat reported in bytes.
+# @seconds: stat reported in seconds.
+# @cycles: stat reported in clock cycles.
+# @none: no unit for this stat.
+#
+# Since: 7.0
+##
+{ 'enum' : 'StatsUnit',
+ 'data' : [ 'bytes', 'seconds', 'cycles', 'none' ] }
+
+##
+# @StatsBase:
+#
+# Enumeration of stats base
+# @pow10: scale is based on power of 10.
+# @pow2: scale is based on power of 2.
+#
+# Since: 7.0
+##
+{ 'enum' : 'StatsBase',
+ 'data' : [ 'pow10', 'pow2' ] }
+
+##
+# @StatsProvider:
+#
+# Enumeration of stats providers.
+#
+# Since: 7.0
+##
+{ 'enum': 'StatsProvider',
+ 'data': [ ] }
+
+##
+# @StatsTarget:
+#
+# Enumeration of stats targets.
+# @vm: stat is per vm.
+# @vcpu: stat is per vcpu.
+#
+# Since: 7.0
+##
+{ 'enum': 'StatsTarget',
+ 'data': [ 'vm', 'vcpu' ] }
+
+##
+# @StatsRequest:
+#
+# Stats filter element.
+# @provider: stat provider.
+# @fields: list of stat names.
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsRequest',
+ 'data': { '*provider': 'StatsProvider',
+ '*fields': [ 'str' ] } }
+
+##
+# @StatsRequestArray:
+#
+# @filters: filters for this request.
+##
+{ 'struct': 'StatsRequestArray',
+ 'data': { '*filters': [ 'StatsRequest' ] } }
+
+##
+# @StatsVCPURequestArray:
+#
+# @vcpus: list of qom paths.
+##
+{ 'struct': 'StatsVCPURequestArray',
+ 'base': 'StatsRequestArray',
+ 'data': { '*vcpus': [ 'str' ] } }
+
+##
+# @StatsFilter:
+#
+# Target specific filter.
+#
+# Since: 7.0
+##
+{ 'union': 'StatsFilter',
+ 'base': { 'target': 'StatsTarget' },
+ 'discriminator': 'target',
+ 'data': { 'vcpu': 'StatsVCPURequestArray',
+ 'vm': 'StatsRequestArray' } }
+
+##
+# @StatsValueArray:
+#
+# @values: uint64 list.
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsValueArray',
+ 'data': { 'values' : [ 'uint64' ] } }
+
+##
+# @StatsValue:
+#
+# @scalar: single uint64.
+# @list: list of uint64.
+#
+# Since: 7.0
+##
+{ 'alternate': 'StatsValue',
+ 'data': { 'scalar': 'uint64',
+ 'list': 'StatsValueArray' } }
+
+##
+# @Stats:
+#
+# @name: name of stat.
+# @value: stat value.
+#
+# Since: 7.0
+##
+{ 'struct': 'Stats',
+ 'data': { 'name': 'str',
+ 'value' : 'StatsValue' } }
+
+##
+# @StatsResultsEntry:
+#
+# @provider: stat provider.
+# @stats: list of stats.
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsResultsEntry',
+ 'data': { 'provider': 'StatsProvider',
+ 'stats': [ 'Stats' ] } }
+
+##
+# @StatsResultsVCPUEntry:
+#
+# @path: vCPU qom path.
+# @providers: per provider results.
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsResultsVCPUEntry',
+ 'data': { 'path': 'str',
+ 'providers': [ 'StatsResultsEntry' ] } }
+
+##
+# @StatsResults:
+#
+# Target specific results.
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsResults',
+ 'data': {
+ '*vcpus': [ 'StatsResultsVCPUEntry' ],
+ '*vm': [ 'StatsResultsEntry' ] } }
+
+##
+# @query-stats:
+#
+# data: @StatsFilter.
+# Returns: @StatsResults.
+#
+# Since: 7.0
+##
+{ 'command': 'query-stats',
+ 'data': 'StatsFilter',
+ 'boxed': true,
+ 'returns': 'StatsResults' }
+
+##
+# @StatsSchemaValue:
+#
+# Individual stat schema.
+# @name: stat name.
+# @type: @StatType.
+# @unit: @StatUnit.
+# @base: @StatBase.
+# @exponent: Used together with @base.
+# @bucket-size: Used with linear-hist to report bucket size
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsSchemaValue',
+ 'data': { 'name': 'str',
+ 'type': 'StatsType',
+ 'unit': 'StatsUnit',
+ 'base': 'StatsBase',
+ 'exponent': 'int16',
+ '*bucket-size': 'uint32' } }
+
+##
+# @StatsSchemaProvider:
+#
+# @provider: @StatsProvider.
+# @stats: list of stats.
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsSchemaProvider',
+ 'data': { 'provider': 'StatsProvider',
+ 'stats': [ 'StatsSchemaValue' ] } }
+
+##
+# @StatsSchemaResults:
+#
+# @vm: vm stats schemas.
+# @vcpu: vcpu stats schemas.
+#
+# Since: 7.0
+##
+{ 'struct': 'StatsSchemaResults',
+ 'data': { '*vm': [ 'StatsSchemaProvider' ],
+ '*vcpu': [ 'StatsSchemaProvider' ] } }
+
+##
+# @query-stats-schemas:
+#
+# Query Stats schemas.
+# Returns @StatsSchemaResult.
+#
+# Since: 7.0
+##
+{ 'command': 'query-stats-schemas',
+ 'data': { '*provider': 'StatsProvider' },
+ 'returns': 'StatsSchemaResults' }
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/3] hmp: Support for querying stats
2022-02-15 15:04 [PATCH v4 0/3] Support fd-based KVM stats Mark Kanda
2022-02-15 15:04 ` [PATCH v4 1/3] qmp: Support for querying stats Mark Kanda
@ 2022-02-15 15:04 ` Mark Kanda
2022-02-15 15:04 ` [PATCH v4 3/3] kvm: Support for querying fd-based stats Mark Kanda
2 siblings, 0 replies; 10+ messages in thread
From: Mark Kanda @ 2022-02-15 15:04 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, armbru
Leverage the QMP support for querying stats. The interface supports similar
arguments as the QMP interface. Wildcard char (*) is accepted for names and
stats target.
Examples (with fd-based KVM stats):
- Display all stats
(qemu) info stats
vm
provider: kvm
max_mmu_page_hash_collisions (peak): 0
max_mmu_rmap_size (peak): 0
...
vcpu (qom path: /machine/unattached/device[0])
provider: kvm
guest_mode (instant): 0
directed_yield_successful (cumulative): 0
...
(qemu) info stats-schemas
vm
provider: kvm
max_mmu_page_hash_collisions (peak)
max_mmu_rmap_size (peak)
...
vcpu
provider: kvm
guest_mode (instant)
directed_yield_successful (cumulative)
- Display 'halt_wait_ns' and 'exits' for vCPUs with qom paths
/machine/unattached/device[2] and /machine/unattached/device[4]:
(qemu) info stats exits,halt_wait_ns /machine/unattached/device[2],
/machine/unattached/device[4]
vcpu (qom path: /machine/unattached/device[2])
provider: kvm
exits (cumulative): 52369
halt_wait_ns (cumulative nanoseconds): 416092704390
vcpu (qom path: /machine/unattached/device[4])
provider: kvm
exits (cumulative): 52550
halt_wait_ns (cumulative nanoseconds): 419637402657
- Display all VM stats for provider KVM:
(qemu) info stats * vm kvm
vm
provider: kvm
max_mmu_page_hash_collisions (peak): 0
max_mmu_rmap_size (peak): 0
nx_lpage_splits (instant): 51
...
Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
hmp-commands-info.hx | 28 ++++
include/monitor/hmp.h | 2 +
monitor/hmp-cmds.c | 328 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 358 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index e90f20a107..c0ad3a3e2a 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -879,3 +879,31 @@ SRST
``info sgx``
Show intel SGX information.
ERST
+
+ {
+ .name = "stats",
+ .args_type = "names:s?,vcpus:s?,provider:s?",
+ .params = "[names] [vcpus] [provider]",
+ .help = "show statistics; optional comma separated names, "
+ "vcpu qom paths, and provider",
+ .cmd = hmp_info_stats,
+ },
+
+SRST
+ ``stats``
+ Show stats
+ERST
+
+ {
+ .name = "stats-schemas",
+ .args_type = "provider:s?",
+ .params = "[provider]",
+ .help = "show statistics schema for each provider",
+ .cmd = hmp_info_stats_schemas,
+ },
+
+SRST
+ ``stats-schemas``
+ Show stats per schema
+ERST
+
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 96d014826a..a748511105 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -133,5 +133,7 @@ void hmp_info_dirty_rate(Monitor *mon, const QDict *qdict);
void hmp_calc_dirty_rate(Monitor *mon, const QDict *qdict);
void hmp_human_readable_text_helper(Monitor *mon,
HumanReadableText *(*qmp_handler)(Error **));
+void hmp_info_stats(Monitor *mon, const QDict *qdict);
+void hmp_info_stats_schemas(Monitor *mon, const QDict *qdict);
#endif
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 8c384dc1b2..2a8f1432fb 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -40,6 +40,7 @@
#include "qapi/qapi-commands-pci.h"
#include "qapi/qapi-commands-rocker.h"
#include "qapi/qapi-commands-run-state.h"
+#include "qapi/qapi-commands-stats.h"
#include "qapi/qapi-commands-tpm.h"
#include "qapi/qapi-commands-ui.h"
#include "qapi/qapi-visit-net.h"
@@ -2178,3 +2179,330 @@ void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict)
}
hmp_handle_error(mon, err);
}
+
+static void print_stats_schema_value(Monitor *mon, StatsSchemaValue *value)
+{
+ monitor_printf(mon, " %s (%s", value->name, StatsType_str(value->type));
+
+ if (value->unit == STATS_UNIT_SECONDS &&
+ value->exponent >= -9 && value->exponent <= 0 &&
+ value->exponent % 3 == 0 && value->base == STATS_BASE_POW10) {
+
+ const char *si_prefix[] = { "", "milli", "micro", "nano" };
+ monitor_printf(mon, " %s", si_prefix[value->exponent / -3]);
+
+ } else if (value->unit == STATS_UNIT_BYTES &&
+ value->exponent >= 0 && value->exponent <= 40 &&
+ value->exponent % 10 == 0 && value->base == STATS_BASE_POW2) {
+
+ const char *si_prefix[] = {
+ "", "kilo", "mega", "giga", "tera" };
+ monitor_printf(mon, " %s", si_prefix[value->exponent / 10]);
+
+ } else if (value->exponent) {
+ /* Print the base and exponent as "x <base>^<exp>" */
+ monitor_printf(mon, " x %s^%d", StatsBase_str(value->base),
+ value->exponent);
+ }
+
+ /* Don't print "none" unit type */
+ monitor_printf(mon, "%s)", value->unit == STATS_UNIT_NONE ?
+ "" : StatsUnit_str(value->unit));
+
+ /* Print bucket size for linear histograms */
+ if ((value->type == STATS_TYPE_LINEAR_HIST) && value->has_bucket_size) {
+ monitor_printf(mon, " bucket-size=%d", value->bucket_size);
+ }
+}
+
+static StatsSchemaValueList *find_schema_value_list(
+ StatsSchemaProviderList *list, StatsProvider provider)
+{
+ StatsSchemaProviderList *schema_provider_list;
+
+ for (schema_provider_list = list;
+ schema_provider_list;
+ schema_provider_list = schema_provider_list->next) {
+ if (schema_provider_list->value->provider == provider) {
+ return schema_provider_list->value->stats;
+ }
+ }
+ return NULL;
+}
+
+static void print_stats_results(Monitor *mon, StatsTarget type,
+ StatsResultsEntryList *list,
+ StatsSchemaProviderList *schema)
+{
+ StatsResultsEntryList *results_entry_list;
+
+ for (results_entry_list = list;
+ results_entry_list;
+ results_entry_list = results_entry_list->next) {
+
+ StatsResultsEntry *results_entry = results_entry_list->value;
+ /* Find provider schema */
+ StatsSchemaValueList *schema_value_list =
+ find_schema_value_list(schema, results_entry->provider);
+ StatsList *stats_list;
+
+ if (!schema_value_list) {
+ monitor_printf(mon, "failed to find schema list for %s\n",
+ StatsProvider_str(results_entry->provider));
+ return;
+ }
+
+ monitor_printf(mon, " provider: %s\n",
+ StatsProvider_str(results_entry->provider));
+
+ for (stats_list = results_entry->stats;
+ stats_list;
+ stats_list = stats_list->next,
+ schema_value_list = schema_value_list->next) {
+
+ Stats *stats = stats_list->value;
+ StatsValue *stats_value = stats->value;
+ StatsSchemaValue *schema_value = schema_value_list->value;
+
+ /* Find schema entry */
+ while (!g_str_equal(stats->name, schema_value->name)) {
+ if (!schema_value_list->next) {
+ monitor_printf(mon, "failed to find schema entry for %s\n",
+ stats->name);
+ return;
+ }
+ schema_value_list = schema_value_list->next;
+ schema_value = schema_value_list->value;
+ }
+
+ print_stats_schema_value(mon, schema_value);
+
+ if (stats_value->type == QTYPE_QNUM) {
+ monitor_printf(mon, ": %" PRId64 "\n", stats_value->u.scalar);
+ } else if (stats_value->type == QTYPE_QDICT) {
+ uint64List *list;
+ int i;
+
+ monitor_printf(mon, ": ");
+ for (list = stats_value->u.list.values, i = 1;
+ list;
+ list = list->next, i++) {
+ monitor_printf(mon, "[%d]=%" PRId64 " ", i, list->value);
+ }
+ monitor_printf(mon, "\n");
+ }
+ }
+ }
+}
+
+static const char wildcard[] = "*";
+static StatsFilter *stats_filter(StatsTarget target, const char *names,
+ const char *vcpus, StatsProvider provider)
+{
+ StatsFilter *filter = g_malloc0(sizeof(*filter));
+ filter->target = target;
+
+ switch (target) {
+ case STATS_TARGET_VM:
+ {
+ StatsRequestList *request_list = NULL;
+ StatsRequest *request = g_malloc0(sizeof(*request));
+
+ if (names && !g_str_equal(names, wildcard)) {
+ request->fields = strList_from_comma_list(names);
+ request->has_fields = true;
+ }
+ if (provider != STATS_PROVIDER__MAX) {
+ request->provider = provider;
+ request->has_provider = true;
+ }
+
+ QAPI_LIST_PREPEND(request_list, request);
+ filter->u.vm.filters = request_list;
+ filter->u.vm.has_filters = true;
+ break;
+ }
+ case STATS_TARGET_VCPU:
+ {
+ StatsRequestList *request_list = NULL;
+ StatsRequest *request = g_malloc0(sizeof(*request));
+ if (names && !g_str_equal(names, wildcard)) {
+ request->fields = strList_from_comma_list(names);
+ request->has_fields = true;
+ }
+ if (provider != STATS_PROVIDER__MAX) {
+ request->provider = provider;
+ request->has_provider = true;
+ }
+
+ if (vcpus && !g_str_equal(vcpus, wildcard) &&
+ !g_str_equal(vcpus, StatsTarget_str(STATS_TARGET_VCPU))) {
+ filter->u.vcpu.vcpus = strList_from_comma_list(vcpus);
+ filter->u.vcpu.has_vcpus = true;
+ }
+
+ QAPI_LIST_PREPEND(request_list, request);
+ filter->u.vcpu.filters = request_list;
+ filter->u.vcpu.has_filters = true;
+ break;
+ }
+ default:
+ break;
+ }
+ return filter;
+}
+
+void hmp_info_stats(Monitor *mon, const QDict *qdict)
+{
+ const char *names = qdict_get_try_str(qdict, "names");
+ const char *vcpus = qdict_get_try_str(qdict, "vcpus");
+ const char *provider = qdict_get_try_str(qdict, "provider");
+
+ StatsProvider stats_provider = STATS_PROVIDER__MAX;
+ StatsTarget target;
+ Error *err = NULL;
+
+ if (provider) {
+ for (stats_provider = 0; stats_provider < STATS_PROVIDER__MAX;
+ stats_provider++) {
+ if (g_str_equal(StatsProvider_str(stats_provider), provider)) {
+ break;
+ }
+ }
+ if (stats_provider == STATS_PROVIDER__MAX) {
+ monitor_printf(mon, "invalid stats filter provider %s\n",
+ provider);
+ goto exit;
+ }
+ }
+
+ for (target = 0; target < STATS_TARGET__MAX; target++) {
+ StatsResults *stats_results = NULL;
+ StatsSchemaResults *schema_results = NULL;
+ StatsResultsVCPUEntryList *results_entry_list;
+
+ StatsFilter *filter = stats_filter(target, names, vcpus,
+ stats_provider);
+ switch (target) {
+ case STATS_TARGET_VM:
+ if (vcpus && !g_str_equal(vcpus, wildcard) &&
+ !g_str_equal(vcpus, StatsTarget_str(STATS_TARGET_VM))) {
+ break;
+ }
+ stats_results = qmp_query_stats(filter, &err);
+ if (err) {
+ goto exit;
+ }
+ schema_results =
+ qmp_query_stats_schemas(provider ? true : false,
+ stats_provider, &err);
+ if (err) {
+ goto exit;
+ }
+ if (!stats_results->has_vm) {
+ break;
+ }
+ monitor_printf(mon, "%s\n", StatsTarget_str(STATS_TARGET_VM));
+ print_stats_results(mon, STATS_TARGET_VM, stats_results->vm,
+ schema_results->vm);
+
+ break;
+ case STATS_TARGET_VCPU:
+ stats_results = qmp_query_stats(filter, &err);
+ if (err) {
+ goto exit;
+ }
+ schema_results =
+ qmp_query_stats_schemas(provider ? true : false,
+ stats_provider, &err);
+ if (err) {
+ goto exit;
+ }
+ for (results_entry_list = stats_results->vcpus;
+ results_entry_list;
+ results_entry_list = results_entry_list->next) {
+ monitor_printf(mon, "%s (qom path: %s)\n",
+ StatsTarget_str(STATS_TARGET_VCPU),
+ results_entry_list->value->path);
+
+ print_stats_results(mon, STATS_TARGET_VCPU,
+ results_entry_list->value->providers,
+ schema_results->vcpu);
+ }
+ break;
+ default:
+ break;
+ }
+ qapi_free_StatsFilter(filter);
+ qapi_free_StatsSchemaResults(schema_results);
+ qapi_free_StatsResults(stats_results);
+ }
+
+exit:
+ if (err) {
+ monitor_printf(mon, "%s\n", error_get_pretty(err));
+ error_free(err);
+ }
+}
+
+static void print_stats_schema_list(Monitor *mon, StatsSchemaProviderList *list)
+{
+ StatsSchemaProviderList *schema_provider_list;
+
+ for (schema_provider_list = list;
+ schema_provider_list;
+ schema_provider_list = schema_provider_list->next) {
+
+ StatsSchemaProvider *schema_provider =
+ schema_provider_list->value;
+ monitor_printf(mon, " provider: %s\n",
+ StatsProvider_str(schema_provider->provider));
+
+ StatsSchemaValueList *schema_value_list;
+ for (schema_value_list = schema_provider->stats;
+ schema_value_list;
+ schema_value_list = schema_value_list->next) {
+
+ StatsSchemaValue *schema_value = schema_value_list->value;
+ print_stats_schema_value(mon, schema_value);
+ monitor_printf(mon, "\n");
+ }
+ }
+}
+
+void hmp_info_stats_schemas(Monitor *mon, const QDict *qdict)
+{
+ const char *provider = qdict_get_try_str(qdict, "provider");
+ StatsProvider stats_provider = STATS_PROVIDER__MAX;
+ StatsSchemaResults *schema_result;
+ Error *err = NULL;
+
+ if (provider) {
+ for (stats_provider = 0; stats_provider < STATS_PROVIDER__MAX;
+ stats_provider++) {
+ if (g_str_equal(StatsProvider_str(stats_provider), provider)) {
+ break;
+ }
+ }
+ if (stats_provider == STATS_PROVIDER__MAX) {
+ monitor_printf(mon, "invalid stats filter provider %s\n", provider);
+ return;
+ }
+ }
+
+ schema_result =
+ qmp_query_stats_schemas(provider ? true : false, stats_provider, &err);
+
+ if (err) {
+ monitor_printf(mon, "%s\n", error_get_pretty(err));
+ error_free(err);
+ return;
+ }
+
+ monitor_printf(mon, "%s\n", StatsTarget_str(STATS_TARGET_VM));
+ print_stats_schema_list(mon, schema_result->vm);
+ monitor_printf(mon, "%s\n", StatsTarget_str(STATS_TARGET_VCPU));
+ print_stats_schema_list(mon, schema_result->vcpu);
+
+ qapi_free_StatsSchemaResults(schema_result);
+}
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] kvm: Support for querying fd-based stats
2022-02-15 15:04 [PATCH v4 0/3] Support fd-based KVM stats Mark Kanda
2022-02-15 15:04 ` [PATCH v4 1/3] qmp: Support for querying stats Mark Kanda
2022-02-15 15:04 ` [PATCH v4 2/3] hmp: " Mark Kanda
@ 2022-02-15 15:04 ` Mark Kanda
2 siblings, 0 replies; 10+ messages in thread
From: Mark Kanda @ 2022-02-15 15:04 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, berrange, armbru
Add support for querying fd-based KVM stats - as introduced by Linux kernel
commit:
cb082bfab59a ("KVM: stats: Add fd-based API to read binary stats data")
Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
accel/kvm/kvm-all.c | 393 ++++++++++++++++++++++++++++++++++++++++++++
qapi/stats.json | 2 +-
2 files changed, 394 insertions(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 0e66ebb497..7efd9b86c8 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -47,6 +47,7 @@
#include "kvm-cpus.h"
#include "hw/boards.h"
+#include "monitor/stats.h"
/* This check must be after config-host.h is included */
#ifdef CONFIG_EVENTFD
@@ -2309,6 +2310,9 @@ bool kvm_dirty_ring_enabled(void)
return kvm_state->kvm_dirty_ring_size ? true : false;
}
+static void query_stats_cb(StatsResults *, StatsFilter *, Error **);
+static void query_stats_schemas_cb(StatsSchemaResults *, Error **);
+
static int kvm_init(MachineState *ms)
{
MachineClass *mc = MACHINE_GET_CLASS(ms);
@@ -2637,6 +2641,11 @@ static int kvm_init(MachineState *ms)
}
}
+ if (kvm_check_extension(kvm_state, KVM_CAP_BINARY_STATS_FD)) {
+ add_stats_callbacks(STATS_PROVIDER_KVM, &query_stats_cb,
+ &query_stats_schemas_cb);
+ }
+
return 0;
err:
@@ -3696,3 +3705,387 @@ static void kvm_type_init(void)
}
type_init(kvm_type_init);
+
+typedef struct StatsArgs {
+ StatsFilter *filter;
+ bool is_schema;
+ union StatsResultsType {
+ StatsResults *stats;
+ StatsSchemaResults *schema;
+ } result;
+ Error **errp;
+} StatsArgs;
+
+static StatsList *add_kvmstat_entry(struct kvm_stats_desc *pdesc,
+ uint64_t *stats_data,
+ StatsList *stats_list,
+ Error **errp)
+{
+
+ StatsList *stats_entry;
+ Stats *stats;
+ uint64List *val_list = NULL;
+
+ switch (pdesc->flags & KVM_STATS_TYPE_MASK) {
+ case KVM_STATS_TYPE_CUMULATIVE:
+ case KVM_STATS_TYPE_INSTANT:
+ case KVM_STATS_TYPE_PEAK:
+ case KVM_STATS_TYPE_LINEAR_HIST:
+ case KVM_STATS_TYPE_LOG_HIST:
+ break;
+ default:
+ return stats_list;
+ }
+
+ switch (pdesc->flags & KVM_STATS_UNIT_MASK) {
+ case KVM_STATS_UNIT_NONE:
+ case KVM_STATS_UNIT_BYTES:
+ case KVM_STATS_UNIT_CYCLES:
+ case KVM_STATS_UNIT_SECONDS:
+ break;
+ default:
+ return stats_list;
+ }
+
+ switch (pdesc->flags & KVM_STATS_BASE_MASK) {
+ case KVM_STATS_BASE_POW10:
+ case KVM_STATS_BASE_POW2:
+ break;
+ default:
+ return stats_list;
+ }
+
+ /* Alloc and populate data list */
+ stats_entry = g_malloc0(sizeof(*stats_entry));
+ stats = g_malloc0(sizeof(*stats));
+ stats->name = g_strdup(pdesc->name);
+ stats->value = g_malloc0(sizeof(*stats->value));
+
+ if (pdesc->size == 1) {
+ stats->value->u.scalar = *stats_data;
+ stats->value->type = QTYPE_QNUM;
+ } else {
+ int i;
+ for (i = 0; i < pdesc->size; i++) {
+ uint64List *val_entry = g_malloc0(sizeof(*val_entry));
+ val_entry->value = stats_data[i];
+ val_entry->next = val_list;
+ val_list = val_entry;
+ }
+ stats->value->u.list.values = val_list;
+ stats->value->type = QTYPE_QDICT;
+ }
+
+ stats_entry->value = stats;
+ stats_entry->next = stats_list;
+
+ return stats_entry;
+}
+
+static StatsSchemaValueList *add_kvmschema_entry(struct kvm_stats_desc *pdesc,
+ StatsSchemaValueList *list,
+ Error **errp)
+{
+ StatsSchemaValueList *schema_entry = g_malloc0(sizeof(*schema_entry));
+ schema_entry->value = g_malloc0(sizeof(*schema_entry->value));
+
+ switch (pdesc->flags & KVM_STATS_TYPE_MASK) {
+ case KVM_STATS_TYPE_CUMULATIVE:
+ schema_entry->value->type = STATS_TYPE_CUMULATIVE;
+ break;
+ case KVM_STATS_TYPE_INSTANT:
+ schema_entry->value->type = STATS_TYPE_INSTANT;
+ break;
+ case KVM_STATS_TYPE_PEAK:
+ schema_entry->value->type = STATS_TYPE_PEAK;
+ break;
+ case KVM_STATS_TYPE_LINEAR_HIST:
+ schema_entry->value->type = STATS_TYPE_LINEAR_HIST;
+ schema_entry->value->bucket_size = pdesc->bucket_size;
+ schema_entry->value->has_bucket_size = true;
+ break;
+ case KVM_STATS_TYPE_LOG_HIST:
+ schema_entry->value->type = STATS_TYPE_LOG_HIST;
+ break;
+ default:
+ goto exit;
+ }
+
+ switch (pdesc->flags & KVM_STATS_UNIT_MASK) {
+ case KVM_STATS_UNIT_NONE:
+ schema_entry->value->unit = STATS_UNIT_NONE;
+ break;
+ case KVM_STATS_UNIT_BYTES:
+ schema_entry->value->unit = STATS_UNIT_BYTES;
+ break;
+ case KVM_STATS_UNIT_CYCLES:
+ schema_entry->value->unit = STATS_UNIT_CYCLES;
+ break;
+ case KVM_STATS_UNIT_SECONDS:
+ schema_entry->value->unit = STATS_UNIT_SECONDS;
+ break;
+ default:
+ goto exit;
+ }
+
+ switch (pdesc->flags & KVM_STATS_BASE_MASK) {
+ case KVM_STATS_BASE_POW10:
+ schema_entry->value->base = STATS_BASE_POW10;
+ break;
+ case KVM_STATS_BASE_POW2:
+ schema_entry->value->base = STATS_BASE_POW2;
+ break;
+ default:
+ goto exit;
+ }
+
+ schema_entry->value->name = g_strdup(pdesc->name);
+ schema_entry->value->exponent = pdesc->exponent;
+ schema_entry->next = list;
+ return schema_entry;
+exit:
+ g_free(schema_entry->value);
+ g_free(schema_entry);
+ return list;
+}
+
+/* Cached stats descriptors */
+typedef struct StatsDescriptors {
+ char *ident; /* 'vm' or vCPU qom path */
+ struct kvm_stats_desc *kvm_stats_desc;
+ struct kvm_stats_header *kvm_stats_header;
+ QTAILQ_ENTRY(StatsDescriptors) next;
+} StatsDescriptors;
+
+static QTAILQ_HEAD(, StatsDescriptors) stats_descriptors =
+ QTAILQ_HEAD_INITIALIZER(stats_descriptors);
+
+static StatsDescriptors *find_stats_descriptors(StatsFilter *filter)
+{
+ StatsDescriptors *entry;
+ QTAILQ_FOREACH(entry, &stats_descriptors, next) {
+ switch (filter->target) {
+ case STATS_TARGET_VM:
+ if (g_str_equal(entry->ident, StatsTarget_str(STATS_TARGET_VM))) {
+ return entry;
+ }
+ break;
+ case STATS_TARGET_VCPU:
+ if (g_str_equal(entry->ident,
+ current_cpu->parent_obj.canonical_path)) {
+ return entry;
+ }
+ break;
+ default:
+ break;
+ }
+ }
+ return NULL;
+}
+
+static void query_stats(union StatsResultsType result, StatsFilter *filter,
+ int stats_fd, bool is_schema, Error **errp)
+{
+ struct kvm_stats_desc *kvm_stats_desc;
+ struct kvm_stats_header *kvm_stats_header;
+ StatsDescriptors *descriptors;
+ g_autofree uint64_t *stats_data = NULL;
+ struct kvm_stats_desc *pdesc;
+ void *stats_list = NULL;
+ size_t size_desc, size_data = 0;
+ ssize_t ret;
+ int i;
+
+ descriptors = find_stats_descriptors(filter);
+ if (!descriptors) {
+ descriptors = g_malloc0(sizeof(*descriptors));
+
+ /* Read stats header */
+ kvm_stats_header = g_malloc(sizeof(*kvm_stats_header));
+ ret = read(stats_fd, kvm_stats_header, sizeof(*kvm_stats_header));
+ if (ret != sizeof(*kvm_stats_header)) {
+ error_setg(errp, "KVM stats: failed to read stats header: "
+ "expected %zu actual %zu",
+ sizeof(*kvm_stats_header), ret);
+ return;
+ }
+ size_desc = sizeof(*kvm_stats_desc) + kvm_stats_header->name_size;
+
+ /* Read stats descriptors */
+ kvm_stats_desc = g_malloc0(kvm_stats_header->num_desc * size_desc);
+ ret = pread(stats_fd, kvm_stats_desc,
+ size_desc * kvm_stats_header->num_desc,
+ kvm_stats_header->desc_offset);
+
+ if (ret != size_desc * kvm_stats_header->num_desc) {
+ error_setg(errp, "KVM stats: failed to read stats descriptors: "
+ "expected %zu actual %zu",
+ size_desc * kvm_stats_header->num_desc, ret);
+ return;
+ }
+ descriptors->kvm_stats_header = kvm_stats_header;
+ descriptors->kvm_stats_desc = kvm_stats_desc;
+ descriptors->ident = strdup(filter->target == STATS_TARGET_VM ?
+ StatsTarget_str(STATS_TARGET_VM) :
+ current_cpu->parent_obj.canonical_path);
+ QTAILQ_INSERT_TAIL(&stats_descriptors, descriptors, next);
+ } else {
+ kvm_stats_header = descriptors->kvm_stats_header;
+ kvm_stats_desc = descriptors->kvm_stats_desc;
+ size_desc = sizeof(*kvm_stats_desc) + kvm_stats_header->name_size;
+ }
+
+ /* Tally the total data size; read schema data */
+ for (i = 0; i < kvm_stats_header->num_desc; ++i) {
+ pdesc = (void *)kvm_stats_desc + i * size_desc;
+ size_data += pdesc->size * sizeof(*stats_data);
+ if (is_schema) {
+ stats_list = add_kvmschema_entry(pdesc, (StatsSchemaValueList *)
+ stats_list, errp);
+ }
+ }
+
+ /* If schema request, add the entries and return */
+ if (is_schema) {
+ switch (filter->target) {
+ case STATS_TARGET_VM:
+ add_vm_stats_schema((StatsSchemaValueList *)stats_list,
+ result.schema, STATS_PROVIDER_KVM);
+ break;
+ case STATS_TARGET_VCPU:
+ add_vcpu_stats_schema((StatsSchemaValueList *)stats_list,
+ result.schema, STATS_PROVIDER_KVM);
+ break;
+ default:
+ break;
+ }
+ return;
+ }
+
+ stats_data = g_malloc0(size_data);
+ ret = pread(stats_fd, stats_data, size_data, kvm_stats_header->data_offset);
+
+ if (ret != size_data) {
+ error_setg(errp, "KVM stats: failed to read data: "
+ "expected %zu actual %zu", size_data, ret);
+ return;
+ }
+
+ for (i = 0; i < kvm_stats_header->num_desc; ++i) {
+ uint64_t *stats;
+ pdesc = (void *)kvm_stats_desc + i * size_desc;
+
+ /* Add entry to the list */
+ stats = (void *)stats_data + pdesc->offset;
+ if (!stats_requested_name(pdesc->name, STATS_PROVIDER_KVM, filter)) {
+ continue;
+ }
+ stats_list =
+ add_kvmstat_entry(pdesc, stats, (StatsList *) stats_list, errp);
+ }
+
+ if (!stats_list) {
+ return;
+ }
+
+ switch (filter->target) {
+ case STATS_TARGET_VM:
+ add_vm_stats_entry((StatsList *)stats_list, result.stats,
+ STATS_PROVIDER_KVM);
+ break;
+ case STATS_TARGET_VCPU:
+ add_vcpu_stats_entry((StatsList *)stats_list, result.stats,
+ STATS_PROVIDER_KVM,
+ current_cpu->parent_obj.canonical_path);
+ break;
+ default:
+ break;
+ }
+}
+
+static void query_stats_vcpu(CPUState *cpu, run_on_cpu_data data)
+{
+ StatsArgs *kvm_stats_args = (StatsArgs *) data.host_ptr;
+ int stats_fd = kvm_vcpu_ioctl(cpu, KVM_GET_STATS_FD, NULL);
+ Error *local_err = NULL;
+
+ if (stats_fd == -1) {
+ error_setg(&local_err, "KVM stats: ioctl failed");
+ error_propagate(kvm_stats_args->errp, local_err);
+ return;
+ }
+ query_stats(kvm_stats_args->result, kvm_stats_args->filter, stats_fd,
+ kvm_stats_args->is_schema, kvm_stats_args->errp);
+ close(stats_fd);
+}
+
+static void query_stats_cb(StatsResults *stats_result, StatsFilter *filter,
+ Error **errp)
+{
+ KVMState *s = kvm_state;
+ CPUState *cpu;
+ int stats_fd;
+
+ switch (filter->target) {
+ case STATS_TARGET_VM:
+ {
+ union StatsResultsType result;
+ result.stats = stats_result;
+ stats_fd = kvm_vm_ioctl(s, KVM_GET_STATS_FD, NULL);
+ if (stats_fd == -1) {
+ error_setg(errp, "KVM stats: ioctl failed");
+ return;
+ }
+ query_stats(result, filter, stats_fd, false, errp);
+ break;
+ }
+ case STATS_TARGET_VCPU:
+ {
+ g_autofree StatsArgs *stats_args = g_malloc0(sizeof(*stats_args));
+ stats_args->filter = filter;
+ stats_args->errp = errp;
+ stats_args->result.stats = stats_result;
+ CPU_FOREACH(cpu) {
+ if (!stats_requested_vcpu(cpu->parent_obj.canonical_path,
+ STATS_PROVIDER_KVM, filter)) {
+ continue;
+ }
+ run_on_cpu(cpu, query_stats_vcpu, RUN_ON_CPU_HOST_PTR(stats_args));
+ }
+ break;
+ }
+ default:
+ break;
+ }
+}
+
+void query_stats_schemas_cb(StatsSchemaResults *stats_result, Error **errp)
+{
+ g_autofree StatsArgs *stats_args = g_malloc0(sizeof(*stats_args));
+ g_autofree StatsFilter *filter = g_malloc0(sizeof(*filter));
+ union StatsResultsType result;
+ KVMState *s = kvm_state;
+ int stats_fd;
+
+ stats_args->filter = filter;
+ stats_args->errp = errp;
+
+ result.schema = stats_result;
+
+ stats_fd = kvm_vm_ioctl(s, KVM_GET_STATS_FD, NULL);
+ if (stats_fd == -1) {
+ error_setg(errp, "KVM stats: ioctl failed");
+ return;
+ }
+
+ stats_args->result.schema = stats_result;
+ stats_args->is_schema = true;
+
+ /* Query VM */
+ stats_args->filter->target = STATS_TARGET_VM;
+ query_stats(result, filter, stats_fd, true, errp);
+
+ /* Query vCPU */
+ stats_args->filter->target = STATS_TARGET_VCPU;
+ run_on_cpu(first_cpu, query_stats_vcpu, RUN_ON_CPU_HOST_PTR(stats_args));
+}
diff --git a/qapi/stats.json b/qapi/stats.json
index ae5dc3ee2c..13708e19c1 100644
--- a/qapi/stats.json
+++ b/qapi/stats.json
@@ -61,7 +61,7 @@
# Since: 7.0
##
{ 'enum': 'StatsProvider',
- 'data': [ ] }
+ 'data': [ 'kvm' ] }
##
# @StatsTarget:
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] qmp: Support for querying stats
2022-02-15 15:04 ` [PATCH v4 1/3] qmp: Support for querying stats Mark Kanda
@ 2022-03-11 13:06 ` Markus Armbruster
2022-03-11 13:12 ` Daniel P. Berrangé
2022-03-14 17:28 ` Mark Kanda
0 siblings, 2 replies; 10+ messages in thread
From: Markus Armbruster @ 2022-03-11 13:06 UTC (permalink / raw)
To: Mark Kanda; +Cc: pbonzini, berrange, qemu-devel
Mark Kanda <mark.kanda@oracle.com> writes:
> Introduce QMP support for querying stats. Provide a framework for adding new
> stats and support for the following commands:
>
> - query-stats
> Returns a list of all stats per target type (only VM and vCPU to start), with
> additional options for specifying stat names, vCPU qom paths, and providers.
>
> - query-stats-schemas
> Returns a list of stats included in each target type, with an option for
> specifying the provider.
>
> The framework provides a method to register callbacks for these QMP commands.
>
> The first use-case will be for fd-based KVM stats (in an upcoming patch).
>
> Examples (with fd-based KVM stats):
>
> - Query all VM stats:
>
> { "execute": "query-stats", "arguments" : { "target": "vm" } }
>
> { "return": {
> "vm": [
> { "provider": "kvm",
> "stats": [
> { "name": "max_mmu_page_hash_collisions", "value": 0 },
> { "name": "max_mmu_rmap_size", "value": 0 },
> { "name": "nx_lpage_splits", "value": 148 },
> ...
> { "provider": "xyz",
> "stats": [ ...
> ...
> ] } }
>
> - Query all vCPU stats:
>
> { "execute": "query-stats", "arguments" : { "target": "vcpu" } }
>
> { "return": {
> "vcpus": [
> { "path": "/machine/unattached/device[0]"
> "providers": [
> { "provider": "kvm",
> "stats": [
> { "name": "guest_mode", "value": 0 },
> { "name": "directed_yield_successful", "value": 0 },
> { "name": "directed_yield_attempted", "value": 106 },
> ...
> { "provider": "xyz",
> "stats": [ ...
> ...
> { "path": "/machine/unattached/device[1]"
> "providers": [
> { "provider": "kvm",
> "stats": [...
> ...
> } ] } }
>
> - Query 'exits' and 'l1d_flush' KVM stats, and 'somestat' from provider 'xyz'
> for vCPUs '/machine/unattached/device[2]' and '/machine/unattached/device[4]':
>
> { "execute": "query-stats",
> "arguments": {
> "target": "vcpu",
> "vcpus": [ "/machine/unattached/device[2]",
> "/machine/unattached/device[4]" ],
> "filters": [
> { "provider": "kvm",
> "fields": [ "l1d_flush", "exits" ] },
> { "provider": "xyz",
> "fields": [ "somestat" ] } ] } }
Are the stats bulky enough to justfify the extra complexity of
filtering?
>
> { "return": {
> "vcpus": [
> { "path": "/machine/unattached/device[2]"
> "providers": [
> { "provider": "kvm",
> "stats": [ { "name": "l1d_flush", "value": 41213 },
> { "name": "exits", "value": 74291 } ] },
> { "provider": "xyz",
> "stats": [ ... ] } ] },
> { "path": "/machine/unattached/device[4]"
> "providers": [
> { "provider": "kvm",
> "stats": [ { "name": "l1d_flush", "value": 16132 },
> { "name": "exits", "value": 57922 } ] },
> { "provider": "xyz",
> "stats": [ ... ] } ] } ] } }
>
> - Query stats schemas:
>
> { "execute": "query-stats-schemas" }
>
> { "return": {
> "vcpu": [
> { "provider": "kvm",
> "stats": [
> { "name": "guest_mode",
> "unit": "none",
> "base": 10,
> "exponent": 0,
> "type": "instant" },
> { "name": "directed_yield_successful",
> "unit": "none",
> "base": 10,
> "exponent": 0,
> "type": "cumulative" },
> ...
> { "provider": "xyz",
> ...
> "vm": [
> { "provider": "kvm",
> "stats": [
> { "name": "max_mmu_page_hash_collisions",
> "unit": "none",
> "base": 10,
> "exponent": 0,
> "type": "peak" },
> { "provider": "xyz",
> ...
Can you give a use case for query-stats-schemas?
>
> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> ---
> include/monitor/stats.h | 51 ++++++++
> monitor/qmp-cmds.c | 219 +++++++++++++++++++++++++++++++++
> qapi/meson.build | 1 +
> qapi/qapi-schema.json | 1 +
> qapi/stats.json | 259 ++++++++++++++++++++++++++++++++++++++++
That's a lot of schema code.
How much of it is for query-stats, and how much for query-stats-schemas?
How much of the query-stats part is for filtering?
> 5 files changed, 531 insertions(+)
> create mode 100644 include/monitor/stats.h
> create mode 100644 qapi/stats.json
[...]
> diff --git a/qapi/stats.json b/qapi/stats.json
> new file mode 100644
> index 0000000000..ae5dc3ee2c
> --- /dev/null
> +++ b/qapi/stats.json
> @@ -0,0 +1,259 @@
> +# -*- Mode: Python -*-
> +# vim: filetype=python
> +#
> +# Copyright (c) 2022 Oracle and/or its affiliates.
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +##
> +# = Stats
> +##
> +
> +##
> +# @StatsType:
> +#
> +# Enumeration of stats types
We commonly put a blank line between overview and arguments.
> +# @cumulative: stat is cumulative; value can only increase.
> +# @instant: stat is instantaneous; value can increase or decrease.
> +# @peak: stat is the peak value; value can only increase.
> +# @linear-hist: stat is a linear histogram.
> +# @log-hist: stat is a logarithmic histogram.
> +#
> +# Since: 7.0
> +##
> +{ 'enum' : 'StatsType',
> + 'data' : [ 'cumulative', 'instant', 'peak', 'linear-hist', 'log-hist' ] }
> +
> +##
> +# @StatsUnit:
> +#
> +# Enumeration of stats units
> +# @bytes: stat reported in bytes.
> +# @seconds: stat reported in seconds.
> +# @cycles: stat reported in clock cycles.
> +# @none: no unit for this stat.
> +#
> +# Since: 7.0
> +##
> +{ 'enum' : 'StatsUnit',
> + 'data' : [ 'bytes', 'seconds', 'cycles', 'none' ] }
> +
> +##
> +# @StatsBase:
> +#
> +# Enumeration of stats base
> +# @pow10: scale is based on power of 10.
> +# @pow2: scale is based on power of 2.
> +#
> +# Since: 7.0
> +##
> +{ 'enum' : 'StatsBase',
> + 'data' : [ 'pow10', 'pow2' ] }
> +
> +##
> +# @StatsProvider:
> +#
> +# Enumeration of stats providers.
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'StatsProvider',
> + 'data': [ ] }
> +
> +##
> +# @StatsTarget:
> +#
> +# Enumeration of stats targets.
> +# @vm: stat is per vm.
> +# @vcpu: stat is per vcpu.
> +#
> +# Since: 7.0
> +##
> +{ 'enum': 'StatsTarget',
> + 'data': [ 'vm', 'vcpu' ] }
> +
> +##
> +# @StatsRequest:
> +#
> +# Stats filter element.
> +# @provider: stat provider.
> +# @fields: list of stat names.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsRequest',
> + 'data': { '*provider': 'StatsProvider',
> + '*fields': [ 'str' ] } }
> +
> +##
> +# @StatsRequestArray:
> +#
> +# @filters: filters for this request.
> +##
> +{ 'struct': 'StatsRequestArray',
> + 'data': { '*filters': [ 'StatsRequest' ] } }
> +
> +##
> +# @StatsVCPURequestArray:
> +#
> +# @vcpus: list of qom paths.
> +##
> +{ 'struct': 'StatsVCPURequestArray',
> + 'base': 'StatsRequestArray',
> + 'data': { '*vcpus': [ 'str' ] } }
> +
> +##
> +# @StatsFilter:
> +#
> +# Target specific filter.
> +#
> +# Since: 7.0
> +##
> +{ 'union': 'StatsFilter',
> + 'base': { 'target': 'StatsTarget' },
> + 'discriminator': 'target',
> + 'data': { 'vcpu': 'StatsVCPURequestArray',
> + 'vm': 'StatsRequestArray' } }
> +
> +##
> +# @StatsValueArray:
> +#
> +# @values: uint64 list.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsValueArray',
> + 'data': { 'values' : [ 'uint64' ] } }
> +
> +##
> +# @StatsValue:
> +#
> +# @scalar: single uint64.
> +# @list: list of uint64.
> +#
> +# Since: 7.0
> +##
> +{ 'alternate': 'StatsValue',
> + 'data': { 'scalar': 'uint64',
> + 'list': 'StatsValueArray' } }
Any particular reason for wrapping the array in a struct?
> +
> +##
> +# @Stats:
> +#
> +# @name: name of stat.
> +# @value: stat value.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'Stats',
> + 'data': { 'name': 'str',
> + 'value' : 'StatsValue' } }
> +
> +##
> +# @StatsResultsEntry:
> +#
> +# @provider: stat provider.
> +# @stats: list of stats.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsResultsEntry',
> + 'data': { 'provider': 'StatsProvider',
> + 'stats': [ 'Stats' ] } }
> +
> +##
> +# @StatsResultsVCPUEntry:
> +#
> +# @path: vCPU qom path.
> +# @providers: per provider results.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsResultsVCPUEntry',
> + 'data': { 'path': 'str',
> + 'providers': [ 'StatsResultsEntry' ] } }
> +
> +##
> +# @StatsResults:
> +#
> +# Target specific results.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsResults',
> + 'data': {
> + '*vcpus': [ 'StatsResultsVCPUEntry' ],
> + '*vm': [ 'StatsResultsEntry' ] } }
> +
> +##
> +# @query-stats:
> +#
> +# data: @StatsFilter.
> +# Returns: @StatsResults.
> +#
> +# Since: 7.0
> +##
> +{ 'command': 'query-stats',
> + 'data': 'StatsFilter',
> + 'boxed': true,
> + 'returns': 'StatsResults' }
> +
> +##
> +# @StatsSchemaValue:
> +#
> +# Individual stat schema.
> +# @name: stat name.
> +# @type: @StatType.
> +# @unit: @StatUnit.
> +# @base: @StatBase.
> +# @exponent: Used together with @base.
> +# @bucket-size: Used with linear-hist to report bucket size
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsSchemaValue',
> + 'data': { 'name': 'str',
> + 'type': 'StatsType',
> + 'unit': 'StatsUnit',
> + 'base': 'StatsBase',
> + 'exponent': 'int16',
> + '*bucket-size': 'uint32' } }
> +
> +##
> +# @StatsSchemaProvider:
> +#
> +# @provider: @StatsProvider.
> +# @stats: list of stats.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsSchemaProvider',
> + 'data': { 'provider': 'StatsProvider',
> + 'stats': [ 'StatsSchemaValue' ] } }
> +
> +##
> +# @StatsSchemaResults:
> +#
> +# @vm: vm stats schemas.
> +# @vcpu: vcpu stats schemas.
> +#
> +# Since: 7.0
> +##
> +{ 'struct': 'StatsSchemaResults',
> + 'data': { '*vm': [ 'StatsSchemaProvider' ],
> + '*vcpu': [ 'StatsSchemaProvider' ] } }
> +
> +##
> +# @query-stats-schemas:
> +#
> +# Query Stats schemas.
> +# Returns @StatsSchemaResult.
> +#
> +# Since: 7.0
> +##
> +{ 'command': 'query-stats-schemas',
> + 'data': { '*provider': 'StatsProvider' },
> + 'returns': 'StatsSchemaResults' }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] qmp: Support for querying stats
2022-03-11 13:06 ` Markus Armbruster
@ 2022-03-11 13:12 ` Daniel P. Berrangé
2022-03-14 17:28 ` Mark Kanda
1 sibling, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2022-03-11 13:12 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, qemu-devel
On Fri, Mar 11, 2022 at 02:06:46PM +0100, Markus Armbruster wrote:
> Mark Kanda <mark.kanda@oracle.com> writes:
>
> > Introduce QMP support for querying stats. Provide a framework for adding new
> > stats and support for the following commands:
> >
> > - query-stats
> > Returns a list of all stats per target type (only VM and vCPU to start), with
> > additional options for specifying stat names, vCPU qom paths, and providers.
> >
> > - query-stats-schemas
> > Returns a list of stats included in each target type, with an option for
> > specifying the provider.
> >
> > The framework provides a method to register callbacks for these QMP commands.
> >
> > The first use-case will be for fd-based KVM stats (in an upcoming patch).
> >
> > Examples (with fd-based KVM stats):
> >
> > - Query all VM stats:
> >
> > { "execute": "query-stats", "arguments" : { "target": "vm" } }
> >
> > { "return": {
> > "vm": [
> > { "provider": "kvm",
> > "stats": [
> > { "name": "max_mmu_page_hash_collisions", "value": 0 },
> > { "name": "max_mmu_rmap_size", "value": 0 },
> > { "name": "nx_lpage_splits", "value": 148 },
> > ...
> > { "provider": "xyz",
> > "stats": [ ...
> > ...
> > ] } }
> >
> > - Query all vCPU stats:
> >
> > { "execute": "query-stats", "arguments" : { "target": "vcpu" } }
> >
> > { "return": {
> > "vcpus": [
> > { "path": "/machine/unattached/device[0]"
> > "providers": [
> > { "provider": "kvm",
> > "stats": [
> > { "name": "guest_mode", "value": 0 },
> > { "name": "directed_yield_successful", "value": 0 },
> > { "name": "directed_yield_attempted", "value": 106 },
> > ...
> > { "provider": "xyz",
> > "stats": [ ...
> > ...
> > { "path": "/machine/unattached/device[1]"
> > "providers": [
> > { "provider": "kvm",
> > "stats": [...
> > ...
> > } ] } }
> >
> > - Query 'exits' and 'l1d_flush' KVM stats, and 'somestat' from provider 'xyz'
> > for vCPUs '/machine/unattached/device[2]' and '/machine/unattached/device[4]':
> >
> > { "execute": "query-stats",
> > "arguments": {
> > "target": "vcpu",
> > "vcpus": [ "/machine/unattached/device[2]",
> > "/machine/unattached/device[4]" ],
> > "filters": [
> > { "provider": "kvm",
> > "fields": [ "l1d_flush", "exits" ] },
> > { "provider": "xyz",
> > "fields": [ "somestat" ] } ] } }
>
> Are the stats bulky enough to justfify the extra complexity of
> filtering?
I viewed it more as a mechanism to cope with a scenario where
some stats are expensive to query. If the mgmt app only want
to get one specific cheap stat, we don't want to trigger code
that does expensive queries of other stats as a side effect.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] qmp: Support for querying stats
2022-03-11 13:06 ` Markus Armbruster
2022-03-11 13:12 ` Daniel P. Berrangé
@ 2022-03-14 17:28 ` Mark Kanda
2022-03-21 13:50 ` Markus Armbruster
1 sibling, 1 reply; 10+ messages in thread
From: Mark Kanda @ 2022-03-14 17:28 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, berrange, qemu-devel
Thank you Markus.
On 3/11/2022 7:06 AM, Markus Armbruster wrote:
> Mark Kanda <mark.kanda@oracle.com> writes:
>
>> Introduce QMP support for querying stats. Provide a framework for adding new
>> stats and support for the following commands:
>>
>> - query-stats
>> Returns a list of all stats per target type (only VM and vCPU to start), with
>> additional options for specifying stat names, vCPU qom paths, and providers.
>>
>> - query-stats-schemas
>> Returns a list of stats included in each target type, with an option for
>> specifying the provider.
>>
>> The framework provides a method to register callbacks for these QMP commands.
>>
>> The first use-case will be for fd-based KVM stats (in an upcoming patch).
>>
>> Examples (with fd-based KVM stats):
>>
>> - Query all VM stats:
>>
>> { "execute": "query-stats", "arguments" : { "target": "vm" } }
>>
>> { "return": {
>> "vm": [
>> { "provider": "kvm",
>> "stats": [
>> { "name": "max_mmu_page_hash_collisions", "value": 0 },
>> { "name": "max_mmu_rmap_size", "value": 0 },
>> { "name": "nx_lpage_splits", "value": 148 },
>> ...
>> { "provider": "xyz",
>> "stats": [ ...
>> ...
>> ] } }
>>
>> - Query all vCPU stats:
>>
>> { "execute": "query-stats", "arguments" : { "target": "vcpu" } }
>>
>> { "return": {
>> "vcpus": [
>> { "path": "/machine/unattached/device[0]"
>> "providers": [
>> { "provider": "kvm",
>> "stats": [
>> { "name": "guest_mode", "value": 0 },
>> { "name": "directed_yield_successful", "value": 0 },
>> { "name": "directed_yield_attempted", "value": 106 },
>> ...
>> { "provider": "xyz",
>> "stats": [ ...
>> ...
>> { "path": "/machine/unattached/device[1]"
>> "providers": [
>> { "provider": "kvm",
>> "stats": [...
>> ...
>> } ] } }
>>
>> - Query 'exits' and 'l1d_flush' KVM stats, and 'somestat' from provider 'xyz'
>> for vCPUs '/machine/unattached/device[2]' and '/machine/unattached/device[4]':
>>
>> { "execute": "query-stats",
>> "arguments": {
>> "target": "vcpu",
>> "vcpus": [ "/machine/unattached/device[2]",
>> "/machine/unattached/device[4]" ],
>> "filters": [
>> { "provider": "kvm",
>> "fields": [ "l1d_flush", "exits" ] },
>> { "provider": "xyz",
>> "fields": [ "somestat" ] } ] } }
> Are the stats bulky enough to justfify the extra complexity of
> filtering?
If this was only for KVM, the complexity probably isn't worth it. However, the
framework is intended to support future stats with new providers and targets
(there has also been mention of moving existing stats to this framework).
Without some sort of filtering, I think the payload could become unmanageable.
>
>> { "return": {
>> "vcpus": [
>> { "path": "/machine/unattached/device[2]"
>> "providers": [
>> { "provider": "kvm",
>> "stats": [ { "name": "l1d_flush", "value": 41213 },
>> { "name": "exits", "value": 74291 } ] },
>> { "provider": "xyz",
>> "stats": [ ... ] } ] },
>> { "path": "/machine/unattached/device[4]"
>> "providers": [
>> { "provider": "kvm",
>> "stats": [ { "name": "l1d_flush", "value": 16132 },
>> { "name": "exits", "value": 57922 } ] },
>> { "provider": "xyz",
>> "stats": [ ... ] } ] } ] } }
>>
>> - Query stats schemas:
>>
>> { "execute": "query-stats-schemas" }
>>
>> { "return": {
>> "vcpu": [
>> { "provider": "kvm",
>> "stats": [
>> { "name": "guest_mode",
>> "unit": "none",
>> "base": 10,
>> "exponent": 0,
>> "type": "instant" },
>> { "name": "directed_yield_successful",
>> "unit": "none",
>> "base": 10,
>> "exponent": 0,
>> "type": "cumulative" },
>> ...
>> { "provider": "xyz",
>> ...
>> "vm": [
>> { "provider": "kvm",
>> "stats": [
>> { "name": "max_mmu_page_hash_collisions",
>> "unit": "none",
>> "base": 10,
>> "exponent": 0,
>> "type": "peak" },
>> { "provider": "xyz",
>> ...
> Can you give a use case for query-stats-schemas?
'query-stats-schemas' provide the the type details about each stat; such as the
unit, base, etc. These details are not reported by 'query-stats' (only the stat
name and raw values are returned).
>
>> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
>> ---
>> include/monitor/stats.h | 51 ++++++++
>> monitor/qmp-cmds.c | 219 +++++++++++++++++++++++++++++++++
>> qapi/meson.build | 1 +
>> qapi/qapi-schema.json | 1 +
>> qapi/stats.json | 259 ++++++++++++++++++++++++++++++++++++++++
> That's a lot of schema code.
>
> How much of it is for query-stats, and how much for query-stats-schemas?
It's roughly 60% query-stats, 40% query-stats-schemas.
> How much of the query-stats part is for filtering?
I think filtering is about 40% of query-stats.
>
>> 5 files changed, 531 insertions(+)
>> create mode 100644 include/monitor/stats.h
>> create mode 100644 qapi/stats.json
> [...]
>
>> diff --git a/qapi/stats.json b/qapi/stats.json
>> new file mode 100644
>> index 0000000000..ae5dc3ee2c
>> --- /dev/null
>> +++ b/qapi/stats.json
>> @@ -0,0 +1,259 @@
>> +# -*- Mode: Python -*-
>> +# vim: filetype=python
>> +#
>> +# Copyright (c) 2022 Oracle and/or its affiliates.
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
>> +# See the COPYING file in the top-level directory.
>> +#
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +##
>> +# = Stats
>> +##
>> +
>> +##
>> +# @StatsType:
>> +#
>> +# Enumeration of stats types
> We commonly put a blank line between overview and arguments.
Will fix.
>> +# @cumulative: stat is cumulative; value can only increase.
>> +# @instant: stat is instantaneous; value can increase or decrease.
>> +# @peak: stat is the peak value; value can only increase.
>> +# @linear-hist: stat is a linear histogram.
>> +# @log-hist: stat is a logarithmic histogram.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'enum' : 'StatsType',
>> + 'data' : [ 'cumulative', 'instant', 'peak', 'linear-hist', 'log-hist' ] }
>> +
>> +##
>> +# @StatsUnit:
>> +#
>> +# Enumeration of stats units
>> +# @bytes: stat reported in bytes.
>> +# @seconds: stat reported in seconds.
>> +# @cycles: stat reported in clock cycles.
>> +# @none: no unit for this stat.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'enum' : 'StatsUnit',
>> + 'data' : [ 'bytes', 'seconds', 'cycles', 'none' ] }
>> +
>> +##
>> +# @StatsBase:
>> +#
>> +# Enumeration of stats base
>> +# @pow10: scale is based on power of 10.
>> +# @pow2: scale is based on power of 2.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'enum' : 'StatsBase',
>> + 'data' : [ 'pow10', 'pow2' ] }
>> +
>> +##
>> +# @StatsProvider:
>> +#
>> +# Enumeration of stats providers.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'enum': 'StatsProvider',
>> + 'data': [ ] }
>> +
>> +##
>> +# @StatsTarget:
>> +#
>> +# Enumeration of stats targets.
>> +# @vm: stat is per vm.
>> +# @vcpu: stat is per vcpu.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'enum': 'StatsTarget',
>> + 'data': [ 'vm', 'vcpu' ] }
>> +
>> +##
>> +# @StatsRequest:
>> +#
>> +# Stats filter element.
>> +# @provider: stat provider.
>> +# @fields: list of stat names.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsRequest',
>> + 'data': { '*provider': 'StatsProvider',
>> + '*fields': [ 'str' ] } }
>> +
>> +##
>> +# @StatsRequestArray:
>> +#
>> +# @filters: filters for this request.
>> +##
>> +{ 'struct': 'StatsRequestArray',
>> + 'data': { '*filters': [ 'StatsRequest' ] } }
>> +
>> +##
>> +# @StatsVCPURequestArray:
>> +#
>> +# @vcpus: list of qom paths.
>> +##
>> +{ 'struct': 'StatsVCPURequestArray',
>> + 'base': 'StatsRequestArray',
>> + 'data': { '*vcpus': [ 'str' ] } }
>> +
>> +##
>> +# @StatsFilter:
>> +#
>> +# Target specific filter.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'union': 'StatsFilter',
>> + 'base': { 'target': 'StatsTarget' },
>> + 'discriminator': 'target',
>> + 'data': { 'vcpu': 'StatsVCPURequestArray',
>> + 'vm': 'StatsRequestArray' } }
>> +
>> +##
>> +# @StatsValueArray:
>> +#
>> +# @values: uint64 list.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsValueArray',
>> + 'data': { 'values' : [ 'uint64' ] } }
>> +
>> +##
>> +# @StatsValue:
>> +#
>> +# @scalar: single uint64.
>> +# @list: list of uint64.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'alternate': 'StatsValue',
>> + 'data': { 'scalar': 'uint64',
>> + 'list': 'StatsValueArray' } }
> Any particular reason for wrapping the array in a struct?
Due to the limitation in the QAPI framework, I hit:
../qapi/stats.json:139: 'data' member 'list' cannot be an array
I can look at adding support...
Thanks/regards,
-Mark
>> +
>> +##
>> +# @Stats:
>> +#
>> +# @name: name of stat.
>> +# @value: stat value.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'Stats',
>> + 'data': { 'name': 'str',
>> + 'value' : 'StatsValue' } }
>> +
>> +##
>> +# @StatsResultsEntry:
>> +#
>> +# @provider: stat provider.
>> +# @stats: list of stats.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsResultsEntry',
>> + 'data': { 'provider': 'StatsProvider',
>> + 'stats': [ 'Stats' ] } }
>> +
>> +##
>> +# @StatsResultsVCPUEntry:
>> +#
>> +# @path: vCPU qom path.
>> +# @providers: per provider results.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsResultsVCPUEntry',
>> + 'data': { 'path': 'str',
>> + 'providers': [ 'StatsResultsEntry' ] } }
>> +
>> +##
>> +# @StatsResults:
>> +#
>> +# Target specific results.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsResults',
>> + 'data': {
>> + '*vcpus': [ 'StatsResultsVCPUEntry' ],
>> + '*vm': [ 'StatsResultsEntry' ] } }
>> +
>> +##
>> +# @query-stats:
>> +#
>> +# data: @StatsFilter.
>> +# Returns: @StatsResults.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'command': 'query-stats',
>> + 'data': 'StatsFilter',
>> + 'boxed': true,
>> + 'returns': 'StatsResults' }
>> +
>> +##
>> +# @StatsSchemaValue:
>> +#
>> +# Individual stat schema.
>> +# @name: stat name.
>> +# @type: @StatType.
>> +# @unit: @StatUnit.
>> +# @base: @StatBase.
>> +# @exponent: Used together with @base.
>> +# @bucket-size: Used with linear-hist to report bucket size
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsSchemaValue',
>> + 'data': { 'name': 'str',
>> + 'type': 'StatsType',
>> + 'unit': 'StatsUnit',
>> + 'base': 'StatsBase',
>> + 'exponent': 'int16',
>> + '*bucket-size': 'uint32' } }
>> +
>> +##
>> +# @StatsSchemaProvider:
>> +#
>> +# @provider: @StatsProvider.
>> +# @stats: list of stats.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsSchemaProvider',
>> + 'data': { 'provider': 'StatsProvider',
>> + 'stats': [ 'StatsSchemaValue' ] } }
>> +
>> +##
>> +# @StatsSchemaResults:
>> +#
>> +# @vm: vm stats schemas.
>> +# @vcpu: vcpu stats schemas.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'struct': 'StatsSchemaResults',
>> + 'data': { '*vm': [ 'StatsSchemaProvider' ],
>> + '*vcpu': [ 'StatsSchemaProvider' ] } }
>> +
>> +##
>> +# @query-stats-schemas:
>> +#
>> +# Query Stats schemas.
>> +# Returns @StatsSchemaResult.
>> +#
>> +# Since: 7.0
>> +##
>> +{ 'command': 'query-stats-schemas',
>> + 'data': { '*provider': 'StatsProvider' },
>> + 'returns': 'StatsSchemaResults' }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] qmp: Support for querying stats
2022-03-14 17:28 ` Mark Kanda
@ 2022-03-21 13:50 ` Markus Armbruster
2022-03-21 14:55 ` Paolo Bonzini
0 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2022-03-21 13:50 UTC (permalink / raw)
To: Mark Kanda; +Cc: pbonzini, berrange, qemu-devel
First: sorry for my slow response.
Mark Kanda <mark.kanda@oracle.com> writes:
> Thank you Markus.
>
> On 3/11/2022 7:06 AM, Markus Armbruster wrote:
>> Mark Kanda <mark.kanda@oracle.com> writes:
>>
>>> Introduce QMP support for querying stats. Provide a framework for adding new
>>> stats and support for the following commands:
>>>
>>> - query-stats
>>> Returns a list of all stats per target type (only VM and vCPU to start), with
>>> additional options for specifying stat names, vCPU qom paths, and providers.
>>>
>>> - query-stats-schemas
>>> Returns a list of stats included in each target type, with an option for
>>> specifying the provider.
>>>
>>> The framework provides a method to register callbacks for these QMP commands.
>>>
>>> The first use-case will be for fd-based KVM stats (in an upcoming patch).
>>>
>>> Examples (with fd-based KVM stats):
>>>
>>> - Query all VM stats:
>>>
>>> { "execute": "query-stats", "arguments" : { "target": "vm" } }
>>>
>>> { "return": {
>>> "vm": [
>>> { "provider": "kvm",
>>> "stats": [
>>> { "name": "max_mmu_page_hash_collisions", "value": 0 },
>>> { "name": "max_mmu_rmap_size", "value": 0 },
>>> { "name": "nx_lpage_splits", "value": 148 },
>>> ...
>>> { "provider": "xyz",
>>> "stats": [ ...
>>> ...
>>> ] } }
>>>
>>> - Query all vCPU stats:
>>>
>>> { "execute": "query-stats", "arguments" : { "target": "vcpu" } }
>>>
>>> { "return": {
>>> "vcpus": [
>>> { "path": "/machine/unattached/device[0]"
>>> "providers": [
>>> { "provider": "kvm",
>>> "stats": [
>>> { "name": "guest_mode", "value": 0 },
>>> { "name": "directed_yield_successful", "value": 0 },
>>> { "name": "directed_yield_attempted", "value": 106 },
>>> ...
>>> { "provider": "xyz",
>>> "stats": [ ...
>>> ...
>>> { "path": "/machine/unattached/device[1]"
>>> "providers": [
>>> { "provider": "kvm",
>>> "stats": [...
>>> ...
>>> } ] } }
>>>
>>> - Query 'exits' and 'l1d_flush' KVM stats, and 'somestat' from provider 'xyz'
>>> for vCPUs '/machine/unattached/device[2]' and '/machine/unattached/device[4]':
>>>
>>> { "execute": "query-stats",
>>> "arguments": {
>>> "target": "vcpu",
>>> "vcpus": [ "/machine/unattached/device[2]",
>>> "/machine/unattached/device[4]" ],
>>> "filters": [
>>> { "provider": "kvm",
>>> "fields": [ "l1d_flush", "exits" ] },
>>> { "provider": "xyz",
>>> "fields": [ "somestat" ] } ] } }
>> Are the stats bulky enough to justfify the extra complexity of
>> filtering?
>
> If this was only for KVM, the complexity probably isn't worth it. However, the
> framework is intended to support future stats with new providers and targets
> (there has also been mention of moving existing stats to this framework).
> Without some sort of filtering, I think the payload could become unmanageable.
I'm deeply wary of "may need $complexity in the future" when $complexity
could be added when we actually need it :)
>>> { "return": {
>>> "vcpus": [
>>> { "path": "/machine/unattached/device[2]"
>>> "providers": [
>>> { "provider": "kvm",
>>> "stats": [ { "name": "l1d_flush", "value": 41213 },
>>> { "name": "exits", "value": 74291 } ] },
>>> { "provider": "xyz",
>>> "stats": [ ... ] } ] },
>>> { "path": "/machine/unattached/device[4]"
>>> "providers": [
>>> { "provider": "kvm",
>>> "stats": [ { "name": "l1d_flush", "value": 16132 },
>>> { "name": "exits", "value": 57922 } ] },
>>> { "provider": "xyz",
>>> "stats": [ ... ] } ] } ] } }
>>>
>>> - Query stats schemas:
>>>
>>> { "execute": "query-stats-schemas" }
>>>
>>> { "return": {
>>> "vcpu": [
>>> { "provider": "kvm",
>>> "stats": [
>>> { "name": "guest_mode",
>>> "unit": "none",
>>> "base": 10,
>>> "exponent": 0,
>>> "type": "instant" },
>>> { "name": "directed_yield_successful",
>>> "unit": "none",
>>> "base": 10,
>>> "exponent": 0,
>>> "type": "cumulative" },
>>> ...
>>> { "provider": "xyz",
>>> ...
>>> "vm": [
>>> { "provider": "kvm",
>>> "stats": [
>>> { "name": "max_mmu_page_hash_collisions",
>>> "unit": "none",
>>> "base": 10,
>>> "exponent": 0,
>>> "type": "peak" },
>>> { "provider": "xyz",
>>> ...
>> Can you give a use case for query-stats-schemas?
>
> 'query-stats-schemas' provide the the type details about each stat; such as the
> unit, base, etc. These details are not reported by 'query-stats' (only the stat
> name and raw values are returned).
Yes, but what is going to use these type details, and for what purpose?
>>> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
>>> ---
>>> include/monitor/stats.h | 51 ++++++++
>>> monitor/qmp-cmds.c | 219 +++++++++++++++++++++++++++++++++
>>> qapi/meson.build | 1 +
>>> qapi/qapi-schema.json | 1 +
>>> qapi/stats.json | 259 ++++++++++++++++++++++++++++++++++++++++
>>
>> That's a lot of schema code.
>>
>> How much of it is for query-stats, and how much for query-stats-schemas?
>
> It's roughly 60% query-stats, 40% query-stats-schemas.
>
>> How much of the query-stats part is for filtering?
>
> I think filtering is about 40% of query-stats.
Have you considered splitting this up into three parts: unfiltered
query-stats, filtering, and query-stats-schemas?
[...]
>>> 5 files changed, 531 insertions(+)
>>> create mode 100644 include/monitor/stats.h
>>> create mode 100644 qapi/stats.json
>> [...]
>>
>>> diff --git a/qapi/stats.json b/qapi/stats.json
>>> new file mode 100644
>>> index 0000000000..ae5dc3ee2c
>>> --- /dev/null
>>> +++ b/qapi/stats.json
[...]
>>> +##
>>> +# @StatsValue:
>>> +#
>>> +# @scalar: single uint64.
>>> +# @list: list of uint64.
>>> +#
>>> +# Since: 7.0
>>> +##
>>> +{ 'alternate': 'StatsValue',
>>> + 'data': { 'scalar': 'uint64',
>>> + 'list': 'StatsValueArray' } }
>>
>> Any particular reason for wrapping the array in a struct?
>
> Due to the limitation in the QAPI framework, I hit:
> ../qapi/stats.json:139: 'data' member 'list' cannot be an array
>
> I can look at adding support...
That would be nice. Could you use help to get started with it?
We could perhaps merge with the current schema, then clean it up on top,
both in 7.1, if that's easier for you.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] qmp: Support for querying stats
2022-03-21 13:50 ` Markus Armbruster
@ 2022-03-21 14:55 ` Paolo Bonzini
2022-03-21 15:18 ` Mark Kanda
0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2022-03-21 14:55 UTC (permalink / raw)
To: Markus Armbruster, Mark Kanda; +Cc: berrange, qemu-devel
On 3/21/22 14:50, Markus Armbruster wrote:
> Mark Kanda <mark.kanda@oracle.com> writes:
>> Thank you Markus.
>> On 3/11/2022 7:06 AM, Markus Armbruster wrote:
>>> Are the stats bulky enough to justfify the extra complexity of
>>> filtering?
>>
>> If this was only for KVM, the complexity probably isn't worth it. However, the
>> framework is intended to support future stats with new providers and targets
>> (there has also been mention of moving existing stats to this framework).
>> Without some sort of filtering, I think the payload could become unmanageable.
>
> I'm deeply wary of "may need $complexity in the future" when $complexity
> could be added when we actually need it :)
I think it's better to have the filtering already. There are several
uses for it.
Regarding filtering by provider, consider that a command like "info jit"
should be a wrapper over
{ "execute": "query-stats", "arguments" : { "target": "vm",
"filters": [ { "provider": "tcg" } ] } }
So we have an example of the intended use already within QEMU. Yes, the
usefulness depends on actually having >1 provider but I think it's
pretty central to the idea of having a statistics *subsystem*.
Regarding filtering by name, query-stats mostly has two usecases. The
first is retrieving all stats and publishing them up to the user, for
example once per minute per VM. The second is monitoring a small number
and building a relatively continuous plot (e.g. 1-10 times per second
per vCPU). For the latter, not having to return hundreds of values
unnecessarily (KVM has almost 60 stats, multiply by the number of vCPUs
and the frequency) is worth having even just with the KVM provider.
>>> Can you give a use case for query-stats-schemas?
>>
>> 'query-stats-schemas' provide the the type details about each stat; such as the
>> unit, base, etc. These details are not reported by 'query-stats' (only the stat
>> name and raw values are returned).
>
> Yes, but what is going to use these type details, and for what purpose?
QEMU does not know in advance which stats are provided. The types, etc.
are provided by the kernel and can change by architecture and kernel
version. In the case of KVM, introspection is done through a file
descriptor. QEMU passes these up as QMP and in the future it
could/should extend this to other providers (such as TCG) and devices
(such as block devices).
See the "info stats" implementation for how it uses the schema:
vcpu (qom path: /machine/unattached/device[2])
provider: kvm
exits (cumulative): 52369
halt_wait_ns (cumulative nanoseconds): 416092704390
Information such as "cumulative nanoseconds" is provided by the schema.
> Have you considered splitting this up into three parts: unfiltered
> query-stats, filtering, and query-stats-schemas?
Splitting could be an idea, but I think only filtering would be a
separate step. The stats are not really usable without a schema that
tells you the units, or whether a number can go down or only up. (Well,
a human export could use them through its intuition, but a HMP-level
command could not be provided).
> We could perhaps merge with the current schema, then clean it up on top,
> both in 7.1, if that's easier for you.
The serialized JSON would change, so that would be a bit worrisome (but
it makes me feel a little less bad about this missing 7.0). It seems to
be as easy as this, as far as alternates go:
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 3cb389e875..48578e1698 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -554,7 +554,7 @@ def check_alternate(expr: _JSONObject, info:
QAPISourceInfo) -> None:
check_name_lower(key, info, source)
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
- check_type(value['type'], info, source)
+ check_type(value['type'], info, source, allow_array=True)
def check_command(expr: _JSONObject, info: QAPISourceInfo) -> None:
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index b7b3fc0ce4..3728340c37 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -243,6 +243,7 @@ def alternate_qtype(self):
'number': 'QTYPE_QNUM',
'int': 'QTYPE_QNUM',
'boolean': 'QTYPE_QBOOL',
+ 'array': 'QTYPE_QLIST',
'object': 'QTYPE_QDICT'
}
return json2qtype.get(self.json_type())
@@ -1069,6 +1070,9 @@ def _def_struct_type(self, expr, info, doc):
None))
def _make_variant(self, case, typ, ifcond, info):
+ if isinstance(typ, list):
+ assert len(typ) == 1
+ typ = self._make_array_type(typ[0], info)
return QAPISchemaVariant(case, info, typ, ifcond)
def _def_union_type(self, expr, info, doc):
I'll try to write some testcases and also cover other uses of
_make_variant, which will undoubtedly find some issue.
Paolo
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/3] qmp: Support for querying stats
2022-03-21 14:55 ` Paolo Bonzini
@ 2022-03-21 15:18 ` Mark Kanda
0 siblings, 0 replies; 10+ messages in thread
From: Mark Kanda @ 2022-03-21 15:18 UTC (permalink / raw)
To: Paolo Bonzini, Markus Armbruster; +Cc: berrange, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5473 bytes --]
On 3/21/2022 9:55 AM, Paolo Bonzini wrote:
> On 3/21/22 14:50, Markus Armbruster wrote:
>> Mark Kanda <mark.kanda@oracle.com> writes:
>>> Thank you Markus.
>>> On 3/11/2022 7:06 AM, Markus Armbruster wrote:
>>>> Are the stats bulky enough to justfify the extra complexity of
>>>> filtering?
>>>
>>> If this was only for KVM, the complexity probably isn't worth it. However, the
>>> framework is intended to support future stats with new providers and targets
>>> (there has also been mention of moving existing stats to this framework).
>>> Without some sort of filtering, I think the payload could become unmanageable.
>>
>> I'm deeply wary of "may need $complexity in the future" when $complexity
>> could be added when we actually need it :)
>
> I think it's better to have the filtering already. There are several uses for
> it.
>
> Regarding filtering by provider, consider that a command like "info jit"
> should be a wrapper over
>
> { "execute": "query-stats", "arguments" : { "target": "vm",
> "filters": [ { "provider": "tcg" } ] } }
>
> So we have an example of the intended use already within QEMU. Yes, the
> usefulness depends on actually having >1 provider but I think it's pretty
> central to the idea of having a statistics *subsystem*.
>
> Regarding filtering by name, query-stats mostly has two usecases. The first is
> retrieving all stats and publishing them up to the user, for example once per
> minute per VM. The second is monitoring a small number and building a
> relatively continuous plot (e.g. 1-10 times per second per vCPU). For the
> latter, not having to return hundreds of values unnecessarily (KVM has almost
> 60 stats, multiply by the number of vCPUs and the frequency) is worth having
> even just with the KVM provider.
>
>>>> Can you give a use case for query-stats-schemas?
>>>
>>> 'query-stats-schemas' provide the the type details about each stat; such as the
>>> unit, base, etc. These details are not reported by 'query-stats' (only the stat
>>> name and raw values are returned).
>>
>> Yes, but what is going to use these type details, and for what purpose?
>
> QEMU does not know in advance which stats are provided. The types, etc. are
> provided by the kernel and can change by architecture and kernel version. In
> the case of KVM, introspection is done through a file descriptor. QEMU passes
> these up as QMP and in the future it could/should extend this to other
> providers (such as TCG) and devices (such as block devices).
>
> See the "info stats" implementation for how it uses the schema:
>
> vcpu (qom path: /machine/unattached/device[2])
> provider: kvm
> exits (cumulative): 52369
> halt_wait_ns (cumulative nanoseconds): 416092704390
>
> Information such as "cumulative nanoseconds" is provided by the schema.
>
>> Have you considered splitting this up into three parts: unfiltered
>> query-stats, filtering, and query-stats-schemas?
>
> Splitting could be an idea, but I think only filtering would be a separate
> step. The stats are not really usable without a schema that tells you the
> units, or whether a number can go down or only up. (Well, a human export
> could use them through its intuition, but a HMP-level command could not be
> provided).
>
>> We could perhaps merge with the current schema, then clean it up on top,
>> both in 7.1, if that's easier for you.
>
> The serialized JSON would change, so that would be a bit worrisome (but it
> makes me feel a little less bad about this missing 7.0). It seems to be as
> easy as this, as far as alternates go:
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index 3cb389e875..48578e1698 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -554,7 +554,7 @@ def check_alternate(expr: _JSONObject, info:
> QAPISourceInfo) -> None:
> check_name_lower(key, info, source)
> check_keys(value, info, source, ['type'], ['if'])
> check_if(value, info, source)
> - check_type(value['type'], info, source)
> + check_type(value['type'], info, source, allow_array=True)
>
>
> def check_command(expr: _JSONObject, info: QAPISourceInfo) -> None:
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index b7b3fc0ce4..3728340c37 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -243,6 +243,7 @@ def alternate_qtype(self):
> 'number': 'QTYPE_QNUM',
> 'int': 'QTYPE_QNUM',
> 'boolean': 'QTYPE_QBOOL',
> + 'array': 'QTYPE_QLIST',
> 'object': 'QTYPE_QDICT'
> }
> return json2qtype.get(self.json_type())
> @@ -1069,6 +1070,9 @@ def _def_struct_type(self, expr, info, doc):
> None))
>
> def _make_variant(self, case, typ, ifcond, info):
> + if isinstance(typ, list):
> + assert len(typ) == 1
> + typ = self._make_array_type(typ[0], info)
> return QAPISchemaVariant(case, info, typ, ifcond)
>
> def _def_union_type(self, expr, info, doc):
>
>
> I'll try to write some testcases and also cover other uses of
> _make_variant, which will undoubtedly find some issue.
>
Hi Paolo,
FWIW, the attached patch adjusts some tests for alternates with arrays..
Thanks/regards,
-Mark
[-- Attachment #2: 0001-qapi-Add-support-for-alternates-with-arrays.patch --]
[-- Type: text/plain, Size: 4993 bytes --]
From 8d02b1b3cbb0dcc08875e307199d06c3995b3cf2 Mon Sep 17 00:00:00 2001
From: Mark Kanda <mark.kanda@oracle.com>
Date: Tue, 15 Mar 2022 20:42:05 -0500
Subject: [PATCH] qapi: Add support for alternates with arrays
Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
---
scripts/qapi/expr.py | 2 +-
scripts/qapi/schema.py | 6 +++++-
tests/qapi-schema/alternate-array.err | 2 --
tests/qapi-schema/alternate-array.json | 7 -------
tests/qapi-schema/alternate-array.out | 0
tests/qapi-schema/meson.build | 1 -
tests/qapi-schema/qapi-schema-test.json | 6 ++++++
tests/qapi-schema/qapi-schema-test.out | 8 ++++++++
8 files changed, 20 insertions(+), 12 deletions(-)
delete mode 100644 tests/qapi-schema/alternate-array.err
delete mode 100644 tests/qapi-schema/alternate-array.json
delete mode 100644 tests/qapi-schema/alternate-array.out
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 3cb389e875..48578e1698 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -554,7 +554,7 @@ def check_alternate(expr: _JSONObject, info: QAPISourceInfo) -> None:
check_name_lower(key, info, source)
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
- check_type(value['type'], info, source)
+ check_type(value['type'], info, source, allow_array=True)
def check_command(expr: _JSONObject, info: QAPISourceInfo) -> None:
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index b7b3fc0ce4..7eedfa6cc2 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -243,7 +243,8 @@ def alternate_qtype(self):
'number': 'QTYPE_QNUM',
'int': 'QTYPE_QNUM',
'boolean': 'QTYPE_QBOOL',
- 'object': 'QTYPE_QDICT'
+ 'object': 'QTYPE_QDICT',
+ 'array': 'QTYPE_QLIST'
}
return json2qtype.get(self.json_type())
@@ -1069,6 +1070,9 @@ def _def_struct_type(self, expr, info, doc):
None))
def _make_variant(self, case, typ, ifcond, info):
+ if isinstance(typ, list):
+ assert len(typ) == 1
+ typ = self._make_array_type(typ[0], info)
return QAPISchemaVariant(case, info, typ, ifcond)
def _def_union_type(self, expr, info, doc):
diff --git a/tests/qapi-schema/alternate-array.err b/tests/qapi-schema/alternate-array.err
deleted file mode 100644
index b1aa1f4e8d..0000000000
--- a/tests/qapi-schema/alternate-array.err
+++ /dev/null
@@ -1,2 +0,0 @@
-alternate-array.json: In alternate 'Alt':
-alternate-array.json:5: 'data' member 'two' cannot be an array
diff --git a/tests/qapi-schema/alternate-array.json b/tests/qapi-schema/alternate-array.json
deleted file mode 100644
index f241aac122..0000000000
--- a/tests/qapi-schema/alternate-array.json
+++ /dev/null
@@ -1,7 +0,0 @@
-# we do not allow array branches in alternates
-# TODO: should we support this?
-{ 'struct': 'One',
- 'data': { 'name': 'str' } }
-{ 'alternate': 'Alt',
- 'data': { 'one': 'One',
- 'two': [ 'int' ] } }
diff --git a/tests/qapi-schema/alternate-array.out b/tests/qapi-schema/alternate-array.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index caf0791ba8..3dbd5f8bfb 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -4,7 +4,6 @@ test_env.set('PYTHONIOENCODING', 'utf-8')
schemas = [
'alternate-any.json',
- 'alternate-array.json',
'alternate-base.json',
'alternate-branch-if-invalid.json',
'alternate-clash.json',
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 43b8697002..64dcbbbe2a 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -123,6 +123,12 @@
# for testing use of 'str' within alternates
{ 'alternate': 'AltStrObj', 'data': { 's': 'str', 'o': 'TestStruct' } }
+# for testing use of an 'int' array within alternates
+{ 'alternate': 'AltIntArray', 'data': { 'a': [ 'int' ], 'o': 'TestStruct' } }
+
+# for testing use of an 'TestStruct' array within alternates
+{ 'alternate': 'AltStructArray', 'data': { 'a': [ 'TestStruct' ], 'o': 'TestStruct' } }
+
{ 'struct': 'ArrayStruct',
'data': { 'integer': ['int'],
's8': ['int8'],
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 1f9585fa9b..b899c30158 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -125,6 +125,14 @@ alternate AltStrObj
tag type
case s: str
case o: TestStruct
+alternate AltIntArray
+ tag type
+ case a: intList
+ case o: TestStruct
+alternate AltStructArray
+ tag type
+ case a: TestStructList
+ case o: TestStruct
object ArrayStruct
member integer: intList optional=False
member s8: int8List optional=False
--
2.27.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-03-21 15:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-15 15:04 [PATCH v4 0/3] Support fd-based KVM stats Mark Kanda
2022-02-15 15:04 ` [PATCH v4 1/3] qmp: Support for querying stats Mark Kanda
2022-03-11 13:06 ` Markus Armbruster
2022-03-11 13:12 ` Daniel P. Berrangé
2022-03-14 17:28 ` Mark Kanda
2022-03-21 13:50 ` Markus Armbruster
2022-03-21 14:55 ` Paolo Bonzini
2022-03-21 15:18 ` Mark Kanda
2022-02-15 15:04 ` [PATCH v4 2/3] hmp: " Mark Kanda
2022-02-15 15:04 ` [PATCH v4 3/3] kvm: Support for querying fd-based stats Mark Kanda
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.