| Message ID | 20220131194312.1192626-2-mark.kanda@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support fd-based KVM stats | expand |
On 1/31/22 20:43, Mark Kanda wrote: > > { "execute": "query-stats", "arguments" : { "target": "vm" } } > { "return": { > "list": [ > { "provider": "kvm", > "stats": [ > { "name": "max_mmu_page_hash_collisions", "value": 0 }, > { "name": "max_mmu_rmap_size", "value": 0 }, > { "name": "nx_lpage_splits", "value": 131 }, > ... > ] } > { "provider": "provider XYZ", > ... > ], > "target": "vm" > } > } Perhaps it's better to have a better name than "list" for clarity, like you already did with 'stats': { 'struct': 'VCPUResultsEntry', 'data': { 'path': 'str', 'providers': [ 'StatsResultsEntry' ] } } { 'struct': 'VCPUStatsResults', 'data': { 'objects': [ 'VCPUResultsEntry' ] } } { 'struct': 'VMStatsResults', 'data': { 'providers' : [ 'StatsResultsEntry' ] } } Also, here: > +{ 'alternate': 'StatsValue', > + 'data': { 'scalar': 'uint64', > + 'list': 'StatsValueArray' } } is it possible to just do { 'alternate': 'StatsValue', 'data': { 'scalar': 'uint64', 'list': ['uint64'] } } Thanks, Paolo
On Tue, Feb 01, 2022 at 11:51:26AM +0100, Paolo Bonzini wrote: > On 1/31/22 20:43, Mark Kanda wrote: > > > > { "execute": "query-stats", "arguments" : { "target": "vm" } } > > { "return": { > > "list": [ > > { "provider": "kvm", > > "stats": [ > > { "name": "max_mmu_page_hash_collisions", "value": 0 }, > > { "name": "max_mmu_rmap_size", "value": 0 }, > > { "name": "nx_lpage_splits", "value": 131 }, > > ... > > ] } > > { "provider": "provider XYZ", > > ... > > ], > > "target": "vm" > > } > > } > > Perhaps it's better to have a better name than "list" for clarity, like you > already did with 'stats': > > { 'struct': 'VCPUResultsEntry', > 'data': { 'path': 'str', > 'providers': [ 'StatsResultsEntry' ] } } > > { 'struct': 'VCPUStatsResults', > 'data': { 'objects': [ 'VCPUResultsEntry' ] } } > > > { 'struct': 'VMStatsResults', > 'data': { 'providers' : [ 'StatsResultsEntry' ] } } > > Also, here: > > > +{ 'alternate': 'StatsValue', > > + 'data': { 'scalar': 'uint64', > > + 'list': 'StatsValueArray' } } > > is it possible to just do > > { 'alternate': 'StatsValue', > 'data': { 'scalar': 'uint64', > 'list': ['uint64'] } } No, the QAPI generator throws its toys out of the pram. It claims you can have any set of data types which have a distinct representation on the wire, so this is valid from that POV. Something about the parser/code generator can't cope with this inline array though - it wants a named type which means a built-in scalar, or a compound type, but not an array :-( Regards, Daniel
On Mon, Jan 31, 2022 at 01:43:10PM -0600, Mark Kanda wrote: > 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 for now), with > additional options for specifying stat names, VCPU qom paths, and stat provider. > > - query-stats-schemas > Returns a list of stats included in each schema type, with an option for > specifying the stat provider. > > The framework provides a method to register callbacks for these QMP commands. > > The first usecase will be for fd-based KVM stats (in an upcoming patch). > > Examples (with fd-based KVM stats): > > - Display all VM stats: > > { "execute": "query-stats", "arguments" : { "target": "vm" } } > { "return": { > "list": [ > { "provider": "kvm", > "stats": [ > { "name": "max_mmu_page_hash_collisions", "value": 0 }, > { "name": "max_mmu_rmap_size", "value": 0 }, > { "name": "nx_lpage_splits", "value": 131 }, > ... > ] } > { "provider": "provider XYZ", > ... > ], > "target": "vm" > } > } I still feel like this is rather verbose, and should be simplified down to. { "return": { "vm": { "kvm": [ ... ] "provider-XYZ": [ ... ], ... } } While vCPU would need one extra nesting layer { "return": { "vcpus": [ { "path": "/vcpu0/path" "kvm": [ ... ] "provider-XYZ": [ ... ], ... }, { "path": "/vcpu1/path" "kvm": [ ... ] "provider-XYZ": [ ... ], ... }, ... ], } The notable difference here is that we'd be adding new keys to the StatsResultEntry struct, every time we gain a new provider, so your QMP code couldn't be entirely metadata driven - you'd need new code to wire up each stats provider. > - Display 'exits' and 'l1d_flush' KVM stats for VCPUs at '/machine/unattached/device[2]' > and '/machine/unattached/device[4]': Shows the value of giving CPUs proper paths https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg06744.html > > { "execute": "query-stats", > "arguments" : { "target": "vcpu", > "fields": [ "exits", "l1d_flush" ], > "paths": [ "/machine/unattached/device[2]", > "/machine/unattached/device[4]" ] > "provider": "kvm" } } This design requires multiple query-stats calls to get data from multiple providers which I think is very undesirable from a performance POV. I'd like to see us able to query fields from many providers at once ie so we have something that looks like { "execute": "query-stats", "arguments" : { "target": "vcpu", "vcpus": [ "/machine/unattached/device[2]", "/machine/unattached/device[4]" ] "kvm": [ "exits", "l1d_flush" ], "someprovider: [ "somefield" ] } } > > { "return": { > "list": [ > { "list": [ > { "provider": "kvm", > "stats": [ > { "name": "l1d_flush", "value": 14690 }, > { "name": "exits", "value": 50898 } > ] } > ], > "path": "/machine/unattached/device[2]" > }, > { "list": [ > { "provider": "kvm", > "stats": [ > { "name": "l1d_flush", "value": 24902 }, > { "name": "exits", "value": 74374 } > ] } > ], > "path": "/machine/unattached/device[4]" > } > ], > "target": "vcpu" > } > } > > - 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": "provider XYZ", > ... > "vm": [ > { "provider": "kvm", > "stats": [ > { "name": "max_mmu_page_hash_collisions", > "unit": "none", > "base": 10, > "exponent": 0, > "type": "peak" }, > "provider": "provider XYZ", > ... > > Signed-off-by: Mark Kanda <mark.kanda@oracle.com> > --- > include/monitor/stats.h | 36 ++++++ > monitor/qmp-cmds.c | 183 +++++++++++++++++++++++++++++ > qapi/misc.json | 253 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 472 insertions(+) > create mode 100644 include/monitor/stats.h > > diff --git a/qapi/misc.json b/qapi/misc.json > index e8054f415b..8d326464f0 100644 > --- a/qapi/misc.json > +++ b/qapi/misc.json I'd suggest we introduce a 'stats.json' file just for this. We have quite a few data types introduced, and its good to avoid 'misc.json' becoming a dumping ground for ranom unrelated stuff. > @@ -527,3 +527,256 @@ > 'data': { '*option': 'str' }, > 'returns': ['CommandLineOptionInfo'], > 'allow-preconfig': true } > + > +## > +# @StatType: There's inconsistency with naming through these changes. Can we ensure that everything uses 'Stats' (plural) as the prefix for every type. > +# > +# Enumeration of stat 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. Not documenting all members > +# > +# Since: 7.0 > +## > +{ 'enum' : 'StatType', > + 'data' : [ 'cumulative', 'instant', 'peak', > + 'linear-hist', 'log-hist', 'unknown' ] } IMHO 'unknown' shouldn't exist at all. > +## > +# @StatsVCPURequest: > +# > +# vcpu specific filter element. > +# @paths: list of qom paths. > +# > +# Since: 7.0 > +## > +{ 'struct': 'StatsVCPURequest', > + 'base': 'StatsRequest', > + 'data': { '*paths': [ 'str' ] } } Call the field 'vcpus' instead of 'paths' to make it clear that we're listing VCPU paths here. > +## > +# @StatsRequest: > +# > +# Stats filter element. > +# @provider: stat provider. > +# @fields: list of stat names. > +# > +# Since: 7.0 > +## > +{ 'struct': 'StatsRequest', > + 'data': { '*provider': 'StatsProvider', > + '*fields': [ 'str' ] } } As mentioned earlier I think we need to have aility to query from many providers at once. It'd be better to have provider name as the field name, eg { 'struct': 'StatsRequest', 'data': { '*kvm': ['str'], '*someprovider': [ 'str' ] } } > + > +## > +# @StatsFilter: > +# > +# Target specific filter. > +# > +# Since: 7.0 > +## > +{ 'union': 'StatsFilter', > + 'base': { 'target': 'StatsTarget' }, > + 'discriminator': 'target', > + 'data': { 'vcpu': 'StatsVCPURequest', > + 'vm': 'StatsRequest' } } > +## > +# @StatsValueArray: > +# > +# uint64 list for StatsValue. > +# > +# Since: 7.0 > +## > +{ 'struct': 'StatsValueArray', > + 'data': { 'list' : [ 'uint64' ] } } 'values' or 'elements' rather than repeating 'list' > + > +## > +# @StatsValue: > +# > +# @scalar: stat is single uint64. > +# @list: stat is a 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' ] } } > + > +## > +# @VCPUResultsEntry: > +# > +# @path: qom path. > +# @list: per provider @StatsResultEntry list. > +# > +# Since: 7.0 > +## > +{ 'struct': 'VCPUResultsEntry', > + 'data': { 'path': 'str', > + 'list': [ 'StatsResultsEntry' ] } } > + > +## > +# @VMStatsResults: > +# > +# Since: 7.0 > +## > +{ 'struct': 'VMStatsResults', > + 'data': { 'list' : [ 'StatsResultsEntry' ] } } > + > +## > +# @VCPUStatsResults: > +# > +# Since: 7.0 > +## > +{ 'struct': 'VCPUStatsResults', > + 'data': { 'list': [ 'VCPUResultsEntry' ] } } > + > +## > +# @StatsResults: > +# > +# Target specific results. > +# > +# Since: 7.0 > +## > +{ 'union': 'StatsResults', > + 'base': { 'target': 'StatsTarget' }, > + 'discriminator': 'target', > + 'data': { 'vcpu': 'VCPUStatsResults', > + 'vm': 'VMStatsResults' } } I feel we can simplify this all down somewhat, eliminating levels of redundant nesting { 'struct': 'StatsResultsEntry', 'data': { '*kvm': [ 'Stats' ] } } { 'struct': 'StatsResultsVCPUEntry', 'base': 'StatsResultsEntry', 'data': 'path': 'str' } } { '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' } Regards, Daniel
Thanks Daniel, On 2/1/2022 6:08 AM, Daniel P. Berrangé wrote: >> +# >> +# Since: 7.0 >> +## >> +{ 'enum' : 'StatType', >> + 'data' : [ 'cumulative', 'instant', 'peak', >> + 'linear-hist', 'log-hist', 'unknown' ] } > IMHO 'unknown' shouldn't exist at all. > I added the 'unknown' member here (and in other enums) to handle situations where QEMU is behind KVM in terms of enumerating the various stat types, units, etc. I feel this will be a semi-common scenario (old QEMU on a new kernel) and with 'unknown', QEMU can at least display the raw data. That said, I happy skip 'unknown' entries if you think that's better. Thanks/regards, -Mark
On Thu, Feb 03, 2022 at 12:12:57PM -0600, Mark Kanda wrote: > Thanks Daniel, > > On 2/1/2022 6:08 AM, Daniel P. Berrangé wrote: > > > +# > > > +# Since: 7.0 > > > +## > > > +{ 'enum' : 'StatType', > > > + 'data' : [ 'cumulative', 'instant', 'peak', > > > + 'linear-hist', 'log-hist', 'unknown' ] } > > IMHO 'unknown' shouldn't exist at all. > > > > I added the 'unknown' member here (and in other enums) to handle situations > where QEMU is behind KVM in terms of enumerating the various stat types, > units, etc. I feel this will be a semi-common scenario (old QEMU on a new > kernel) and with 'unknown', QEMU can at least display the raw data. > > That said, I happy skip 'unknown' entries if you think that's better. Yep, I don't think we should be including 'unknown' stuff. An application could use this, and then we add support for the new type and the application will now break with new QEMU because it is presented in QMP in a different way. The same for the 'unknown' base and unit too for that matter. Regards, Daniel
On 2/3/2022 12:30 PM, Daniel P. Berrangé wrote: > On Thu, Feb 03, 2022 at 12:12:57PM -0600, Mark Kanda wrote: >> Thanks Daniel, >> >> On 2/1/2022 6:08 AM, Daniel P. Berrangé wrote: >>>> +# >>>> +# Since: 7.0 >>>> +## >>>> +{ 'enum' : 'StatType', >>>> + 'data' : [ 'cumulative', 'instant', 'peak', >>>> + 'linear-hist', 'log-hist', 'unknown' ] } >>> IMHO 'unknown' shouldn't exist at all. >>> >> I added the 'unknown' member here (and in other enums) to handle situations >> where QEMU is behind KVM in terms of enumerating the various stat types, >> units, etc. I feel this will be a semi-common scenario (old QEMU on a new >> kernel) and with 'unknown', QEMU can at least display the raw data. >> >> That said, I happy skip 'unknown' entries if you think that's better. > Yep, I don't think we should be including 'unknown' stuff. > > An application could use this, and then we add support for the > new type and the application will now break with new QEMU because > it is presented in QMP in a different way. > > The same for the 'unknown' base and unit too for that matter. > Sounds good. I'll implement the other changes you suggested in v4. Thanks/regards, -Mark
On 2/1/22 13:08, Daniel P. Berrangé wrote: > I still feel like this is rather verbose, and should be simplified > down to. > > { "return": { > "vm": { > "kvm": [ ... ] > "provider-XYZ": [ ... ], > ... > } > } My main qualm with this is that not just QEMU, but every layer above then needs to either treat stats as a "dynamic" type unless they want to only handle providers that they know. The main reason why I asked Mark to do all this work, was so that new stats and stats providers could be added transparently, and only new *targets* would need work all over the stack (but those are fewer, for example blockdev/netdev/iothread). Paolo
On 2/3/22 19:12, Mark Kanda wrote: > > I added the 'unknown' member here (and in other enums) to handle > situations where QEMU is behind KVM in terms of enumerating the various > stat types, units, etc. I feel this will be a semi-common scenario (old > QEMU on a new kernel) and with 'unknown', QEMU can at least display the > raw data. I think you can skip them, there aren't really plans for new types (the "metaschema" is based on one that has been in use for quite some time at Google). Paolo
On 2/1/2022 6:08 AM, Daniel P. Berrangé wrote: >> +## >> +# @StatsResults: >> +# >> +# Target specific results. >> +# >> +# Since: 7.0 >> +## >> +{ 'union': 'StatsResults', >> + 'base': { 'target': 'StatsTarget' }, >> + 'discriminator': 'target', >> + 'data': { 'vcpu': 'VCPUStatsResults', >> + 'vm': 'VMStatsResults' } } > I feel we can simplify this all down somewhat, eliminating levels > of redundant nesting > > { 'struct': 'StatsResultsEntry', > 'data': { '*kvm': [ 'Stats' ] } } > > { 'struct': 'StatsResultsVCPUEntry', > 'base': 'StatsResultsEntry', > 'data': 'path': 'str' } } > > { 'struct': 'StatsResults', > 'data': { > '*vcpus': ['StatsResultsVCPUEntry'], > '*vm': 'StatsResultsEntry' > } > } > > I'm happy to make this change, but I would like Paolo to comment as he had suggested the StatsResults layout [1]. Thanks Daniel/Paolo, -Mark [1] https://lore.kernel.org/all/ee0d6990-06f3-9a1b-f7d5-7c379f0e9773@redhat.com/
On Thu, Feb 03, 2022 at 07:38:08PM +0100, Paolo Bonzini wrote: > On 2/1/22 13:08, Daniel P. Berrangé wrote: > > I still feel like this is rather verbose, and should be simplified > > down to. > > > > { "return": { > > "vm": { > > "kvm": [ ... ] > > "provider-XYZ": [ ... ], > > ... > > } > > } > > My main qualm with this is that not just QEMU, but every layer above then > needs to either treat stats as a "dynamic" type unless they want to only > handle providers that they know. > > The main reason why I asked Mark to do all this work, was so that new stats > and stats providers could be added transparently, and only new *targets* > would need work all over the stack (but those are fewer, for example > blockdev/netdev/iothread). Hmm, yes, i see what you mean. Regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Tue, Feb 01, 2022 at 11:51:26AM +0100, Paolo Bonzini wrote: >> On 1/31/22 20:43, Mark Kanda wrote: >> > >> > { "execute": "query-stats", "arguments" : { "target": "vm" } } >> > { "return": { >> > "list": [ >> > { "provider": "kvm", >> > "stats": [ >> > { "name": "max_mmu_page_hash_collisions", "value": 0 }, >> > { "name": "max_mmu_rmap_size", "value": 0 }, >> > { "name": "nx_lpage_splits", "value": 131 }, >> > ... >> > ] } >> > { "provider": "provider XYZ", >> > ... >> > ], >> > "target": "vm" >> > } >> > } >> >> Perhaps it's better to have a better name than "list" for clarity, like you >> already did with 'stats': >> >> { 'struct': 'VCPUResultsEntry', >> 'data': { 'path': 'str', >> 'providers': [ 'StatsResultsEntry' ] } } >> >> { 'struct': 'VCPUStatsResults', >> 'data': { 'objects': [ 'VCPUResultsEntry' ] } } >> >> >> { 'struct': 'VMStatsResults', >> 'data': { 'providers' : [ 'StatsResultsEntry' ] } } >> >> Also, here: >> >> > +{ 'alternate': 'StatsValue', >> > + 'data': { 'scalar': 'uint64', >> > + 'list': 'StatsValueArray' } } >> >> is it possible to just do >> >> { 'alternate': 'StatsValue', >> 'data': { 'scalar': 'uint64', >> 'list': ['uint64'] } } > > No, the QAPI generator throws its toys out of the pram. > > It claims you can have any set of data types which have a > distinct representation on the wire, so this is valid from > that POV. Something about the parser/code generator can't > cope with this inline array though - it wants a named type > which means a built-in scalar, or a compound type, but not > an array :-( Array is not implemented, simply because we haven't had a use for it. Should not make you settle for an inferior schema design! Implementing array alternates shouldn't be hard.
diff --git a/include/monitor/stats.h b/include/monitor/stats.h new file mode 100644 index 0000000000..d4b57322eb --- /dev/null +++ b/include/monitor/stats.h @@ -0,0 +1,36 @@ +/* + * 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 + +/* + * 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)(StatsSchemaResult *, Error **)); + +/* Stats helpers routines */ +StatsResultsEntry *add_vm_stats_entry(StatsResults *, StatsProvider); +StatsResultsEntry *add_vcpu_stats_entry(StatsResults *, StatsProvider, char *); +StatsSchemaProvider *add_vm_stats_schema(StatsSchemaResult *, StatsProvider); +StatsSchemaProvider *add_vcpu_stats_schema(StatsSchemaResult *, StatsProvider); + +bool stat_name_filter(StatsFilter *, StatsTarget, char *); +bool stat_cpu_filter(StatsFilter *, char *); + +#endif /* STATS_H */ diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index db4d186448..bd562edfb8 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -18,6 +18,7 @@ #include "qemu/cutils.h" #include "qemu/option.h" #include "monitor/monitor.h" +#include "monitor/stats.h" #include "sysemu/sysemu.h" #include "qemu/config-file.h" #include "qemu/uuid.h" @@ -448,3 +449,185 @@ 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)(StatsSchemaResult *, 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)(StatsSchemaResult *, 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); +} + +StatsResults *qmp_query_stats(StatsFilter *filter, Error **errp) +{ + StatsResults *stats_results = g_malloc0(sizeof(*stats_results)); + StatsCallbacks *entry; + StatsProvider provider = STATS_PROVIDER__MAX; + + if (filter->target == STATS_TARGET_VM && + filter->u.vm.has_provider) { + provider = filter->u.vm.provider; + } else if (filter->target == STATS_TARGET_VCPU && + filter->u.vcpu.has_provider) { + provider = filter->u.vcpu.provider; + } + + QTAILQ_FOREACH(entry, &stats_callbacks, next) { + if (provider != STATS_PROVIDER__MAX && (provider != entry->provider)) { + continue; + } + entry->stats_cb(stats_results, filter, errp); + } + + return stats_results; +} + +StatsSchemaResult *qmp_query_stats_schemas(bool has_provider, + StatsProvider provider, + Error **errp) +{ + StatsSchemaResult *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; +} + +StatsResultsEntry *add_vm_stats_entry(StatsResults *stats_results, + StatsProvider provider) +{ + StatsResultsEntry *entry = g_malloc0(sizeof(*entry)); + stats_results->target = STATS_TARGET_VM; + entry->provider = provider; + QAPI_LIST_PREPEND(stats_results->u.vm.list, entry); + + return entry; +} + +StatsResultsEntry *add_vcpu_stats_entry(StatsResults *stats_results, + StatsProvider provider, + char *path) +{ + StatsResultsEntry *entry = g_malloc0(sizeof(*entry)); + stats_results->target = STATS_TARGET_VCPU; + entry->provider = provider; + + /* + * Find the corresponding vCPU entry and add to its list. Else, create it. + */ + VCPUResultsEntryList **tailp = &stats_results->u.vcpu.list; + VCPUResultsEntryList *tail; + + 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->list, entry); + goto done; + } + tailp = &tail->next; + } + /* Create and populate a new entry for the vCPU */ + VCPUResultsEntry *value = g_malloc0(sizeof(*value)); + value->path = g_strdup(path); + value->list = g_malloc0(sizeof(*value->list)); + value->list->value = entry; + QAPI_LIST_APPEND(tailp, value); + +done: + return entry; +} + +StatsSchemaProvider *add_vm_stats_schema(StatsSchemaResult *schema_results, + StatsProvider provider) +{ + StatsSchemaProvider *entry = g_malloc0(sizeof(*entry)); + schema_results->has_vm = true; + entry->provider = provider; + QAPI_LIST_PREPEND(schema_results->vm, entry); + + return entry; +} + +StatsSchemaProvider *add_vcpu_stats_schema(StatsSchemaResult *schema_results, + StatsProvider provider) +{ + StatsSchemaProvider *entry = g_malloc0(sizeof(*entry)); + schema_results->has_vcpu = true; + entry->provider = provider; + QAPI_LIST_PREPEND(schema_results->vcpu, entry); + + return entry; +} + +/* True if stat doesn't match a requested name */ +bool stat_name_filter(StatsFilter *filter, StatsTarget type, char *name) +{ + strList *str_list = NULL; + + if (type == STATS_TARGET_VM) { + if (filter->target != STATS_TARGET_VM) { + return false; + } + if (!filter->u.vm.has_fields) { + return false; + } + str_list = filter->u.vm.fields; + } else if (type == STATS_TARGET_VCPU) { + if (filter->target != STATS_TARGET_VCPU) { + return false; + } + if (!filter->u.vcpu.has_fields) { + return false; + } + str_list = filter->u.vcpu.fields; + } + + for (; str_list; str_list = str_list->next) { + if (g_str_equal(name, str_list->value)) { + return false; + } + } + return true; +} + +/* True if cpu qom path doesn't match a requested path */ +bool stat_cpu_filter(StatsFilter *filter, char *path) +{ + strList *list; + + if (filter->target != STATS_TARGET_VCPU) { + return false; + } + + if (!filter->u.vcpu.has_paths) { + return false; + } + + for (list = filter->u.vcpu.paths; list; list = list->next) { + if (g_str_equal(list->value, path)) { + return false; + } + } + return true; +} diff --git a/qapi/misc.json b/qapi/misc.json index e8054f415b..8d326464f0 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -527,3 +527,256 @@ 'data': { '*option': 'str' }, 'returns': ['CommandLineOptionInfo'], 'allow-preconfig': true } + +## +# @StatType: +# +# Enumeration of stat 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. +# +# Since: 7.0 +## +{ 'enum' : 'StatType', + 'data' : [ 'cumulative', 'instant', 'peak', + 'linear-hist', 'log-hist', 'unknown' ] } + +## +# @StatUnit: +# +# Enumeration of stat 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' : 'StatUnit', + 'data' : [ 'none', 'bytes', 'seconds', 'cycles', 'unknown' ] } + +## +# @StatBase: +# +# Enumeration of stat base +# +# Since: 7.0 +## +{ 'enum' : 'StatBase', + 'data' : [ 'pow10', 'pow2', 'unknown' ] } + +## +# @StatsProvider: +# +# Enumeration of stat providers. +# +# Since: 7.0 +## +{ 'enum': 'StatsProvider', + 'data': [ ] } + +## +# @StatsTarget: +# +# Enumeration of stat targets. +# @vm: stat is per vm. +# @vcpu: stat is per vcpu. +# +# Since: 7.0 +## +{ 'enum': 'StatsTarget', + 'data': [ 'vm', 'vcpu' ] } + +## +# @StatsVCPURequest: +# +# vcpu specific filter element. +# @paths: list of qom paths. +# +# Since: 7.0 +## +{ 'struct': 'StatsVCPURequest', + 'base': 'StatsRequest', + 'data': { '*paths': [ 'str' ] } } + +## +# @StatsRequest: +# +# Stats filter element. +# @provider: stat provider. +# @fields: list of stat names. +# +# Since: 7.0 +## +{ 'struct': 'StatsRequest', + 'data': { '*provider': 'StatsProvider', + '*fields': [ 'str' ] } } + +## +# @StatsFilter: +# +# Target specific filter. +# +# Since: 7.0 +## +{ 'union': 'StatsFilter', + 'base': { 'target': 'StatsTarget' }, + 'discriminator': 'target', + 'data': { 'vcpu': 'StatsVCPURequest', + 'vm': 'StatsRequest' } } + +## +# @StatsValueArray: +# +# uint64 list for StatsValue. +# +# Since: 7.0 +## +{ 'struct': 'StatsValueArray', + 'data': { 'list' : [ 'uint64' ] } } + +## +# @StatsValue: +# +# @scalar: stat is single uint64. +# @list: stat is a 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' ] } } + +## +# @VCPUResultsEntry: +# +# @path: qom path. +# @list: per provider @StatsResultEntry list. +# +# Since: 7.0 +## +{ 'struct': 'VCPUResultsEntry', + 'data': { 'path': 'str', + 'list': [ 'StatsResultsEntry' ] } } + +## +# @VMStatsResults: +# +# Since: 7.0 +## +{ 'struct': 'VMStatsResults', + 'data': { 'list' : [ 'StatsResultsEntry' ] } } + +## +# @VCPUStatsResults: +# +# Since: 7.0 +## +{ 'struct': 'VCPUStatsResults', + 'data': { 'list': [ 'VCPUResultsEntry' ] } } + +## +# @StatsResults: +# +# Target specific results. +# +# Since: 7.0 +## +{ 'union': 'StatsResults', + 'base': { 'target': 'StatsTarget' }, + 'discriminator': 'target', + 'data': { 'vcpu': 'VCPUStatsResults', + 'vm': 'VMStatsResults' } } + +## +# @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. +# +# Since: 7.0 +## +{ 'struct': 'StatsSchemaValue', + 'data': { 'name': 'str', + 'type': 'StatType', + 'unit': 'StatUnit', + 'base': 'StatBase', + 'exponent': 'int16' } } + +## +# @StatsSchemaProvider: +# +# @provider: @StatsProvider. +# @stats: list of stats. +# +# Since: 7.0 +## +{ 'struct': 'StatsSchemaProvider', + 'data': { 'provider': 'StatsProvider', + 'stats': [ 'StatsSchemaValue' ] } } + +## +# @StatsSchemaResult: +# +# @vm: vm stats schemas. +# @vcpu: vcpu stats schemas. +# +# Since: 7.0 +## +{ 'struct': 'StatsSchemaResult', + 'data': { '*vm': [ 'StatsSchemaProvider' ], + '*vcpu': [ 'StatsSchemaProvider' ] } } + +## +# @query-stats-schemas: +# +# Query Stats schemas. +# Returns @StatsSchemaResult +# +# Since: 7.0 +## +{ 'command': 'query-stats-schemas', + 'data': { '*provider': 'StatsProvider' }, + 'returns': 'StatsSchemaResult' }
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 for now), with additional options for specifying stat names, VCPU qom paths, and stat provider. - query-stats-schemas Returns a list of stats included in each schema type, with an option for specifying the stat provider. The framework provides a method to register callbacks for these QMP commands. The first usecase will be for fd-based KVM stats (in an upcoming patch). Examples (with fd-based KVM stats): - Display all VM stats: { "execute": "query-stats", "arguments" : { "target": "vm" } } { "return": { "list": [ { "provider": "kvm", "stats": [ { "name": "max_mmu_page_hash_collisions", "value": 0 }, { "name": "max_mmu_rmap_size", "value": 0 }, { "name": "nx_lpage_splits", "value": 131 }, ... ] } { "provider": "provider XYZ", ... ], "target": "vm" } } - Display all VCPU stats: { "execute": "query-stats", "arguments" : { "target": "vcpu" } } { "return": { "list": [ { "list": [ { "provider": "kvm", "stats": [ { "name": "guest_mode", "value": 0 }, { "name": "directed_yield_successful", "value": 0 }, { "name": "directed_yield_attempted", "value": 76 }, ... ] } { "provider": "provider XYZ", ... ], "path": "/machine/unattached/device[0]" }, { "list": [ { "provider": "kvm", "stats": [ { "name": "guest_mode", "value": 0 }, { "name": "directed_yield_successful", "value": 0 }, { "name": "directed_yield_attempted", "value": 51 }, ... } ], "target": "vcpu" } } - Display 'exits' and 'l1d_flush' KVM stats for VCPUs at '/machine/unattached/device[2]' and '/machine/unattached/device[4]': { "execute": "query-stats", "arguments" : { "target": "vcpu", "fields": [ "exits", "l1d_flush" ], "paths": [ "/machine/unattached/device[2]", "/machine/unattached/device[4]" ] "provider": "kvm" } } { "return": { "list": [ { "list": [ { "provider": "kvm", "stats": [ { "name": "l1d_flush", "value": 14690 }, { "name": "exits", "value": 50898 } ] } ], "path": "/machine/unattached/device[2]" }, { "list": [ { "provider": "kvm", "stats": [ { "name": "l1d_flush", "value": 24902 }, { "name": "exits", "value": 74374 } ] } ], "path": "/machine/unattached/device[4]" } ], "target": "vcpu" } } - 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": "provider XYZ", ... "vm": [ { "provider": "kvm", "stats": [ { "name": "max_mmu_page_hash_collisions", "unit": "none", "base": 10, "exponent": 0, "type": "peak" }, "provider": "provider XYZ", ... Signed-off-by: Mark Kanda <mark.kanda@oracle.com> --- include/monitor/stats.h | 36 ++++++ monitor/qmp-cmds.c | 183 +++++++++++++++++++++++++++++ qapi/misc.json | 253 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 472 insertions(+) create mode 100644 include/monitor/stats.h