| Message ID | 20220215150433.2310711-2-mark.kanda@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support fd-based KVM stats | expand |
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' }
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
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' }
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.
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
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 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
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' }
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