Re: [PATCH v4 1/3] qmp: Support for querying stats

2022-03-21 Thread Mark Kanda

On 3/21/2022 9:55 AM, Paolo Bonzini wrote:

On 3/21/22 14:50, Markus Armbruster wrote:

Mark Kanda  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 

Re: [PATCH v4 1/3] qmp: Support for querying stats

2022-03-21 Thread Paolo Bonzini

On 3/21/22 14:50, Markus Armbruster wrote:

Mark Kanda  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



Re: [PATCH v4 1/3] qmp: Support for querying stats

2022-03-21 Thread Markus Armbruster
First: sorry for my slow response.

Mark Kanda  writes:

> Thank you Markus.
>
> On 3/11/2022 7:06 AM, Markus Armbruster wrote:
>> Mark Kanda  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 

Re: [PATCH v4 1/3] qmp: Support for querying stats

2022-03-14 Thread Mark Kanda

Thank you Markus.

On 3/11/2022 7:06 AM, Markus Armbruster wrote:

Mark Kanda  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 
---
  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 

Re: [PATCH v4 1/3] qmp: Support for querying stats

2022-03-11 Thread Daniel P . Berrangé
On Fri, Mar 11, 2022 at 02:06:46PM +0100, Markus Armbruster wrote:
> Mark Kanda  writes:
> 
> > Introduce QMP support for querying stats. Provide a framework for adding new
> > stats and support for the following commands:
> >
> > - query-stats
> > Returns a list of all stats per target type (only VM and vCPU to start), 
> > with
> > additional options for specifying stat names, vCPU qom paths, and providers.
> >
> > - query-stats-schemas
> > Returns a list of stats included in each target type, with an option for
> > specifying the provider.
> >
> > The framework provides a method to register callbacks for these QMP 
> > commands.
> >
> > The first use-case will be for fd-based KVM stats (in an upcoming patch).
> >
> > Examples (with fd-based KVM stats):
> >
> > - Query all VM stats:
> >
> > { "execute": "query-stats", "arguments" : { "target": "vm" } }
> >
> > { "return": {
> >   "vm": [
> >  { "provider": "kvm",
> >"stats": [
> >   { "name": "max_mmu_page_hash_collisions", "value": 0 },
> >   { "name": "max_mmu_rmap_size", "value": 0 },
> >   { "name": "nx_lpage_splits", "value": 148 },
> >   ...
> >  { "provider": "xyz",
> >"stats": [ ...
> >  ...
> > ] } }
> >
> > - Query all vCPU stats:
> >
> > { "execute": "query-stats", "arguments" : { "target": "vcpu" } }
> >
> > { "return": {
> > "vcpus": [
> >   { "path": "/machine/unattached/device[0]"
> > "providers": [
> >   { "provider": "kvm",
> > "stats": [
> >   { "name": "guest_mode", "value": 0 },
> >   { "name": "directed_yield_successful", "value": 0 },
> >   { "name": "directed_yield_attempted", "value": 106 },
> >   ...
> >   { "provider": "xyz",
> > "stats": [ ...
> >...
> >   { "path": "/machine/unattached/device[1]"
> > "providers": [
> >   { "provider": "kvm",
> > "stats": [...
> >   ...
> > } ] } }
> >
> > - Query 'exits' and 'l1d_flush' KVM stats, and 'somestat' from provider 
> > 'xyz'
> > for vCPUs '/machine/unattached/device[2]' and 
> > '/machine/unattached/device[4]':
> >
> > { "execute": "query-stats",
> >   "arguments": {
> > "target": "vcpu",
> > "vcpus": [ "/machine/unattached/device[2]",
> >"/machine/unattached/device[4]" ],
> > "filters": [
> >   { "provider": "kvm",
> > "fields": [ "l1d_flush", "exits" ] },
> >   { "provider": "xyz",
> > "fields": [ "somestat" ] } ] } }
> 
> Are the stats bulky enough to justfify the extra complexity of
> filtering?

I viewed it more as a mechanism to cope with a scenario where
some stats are expensive to query. If the mgmt app only want
to get one specific cheap stat, we don't want to trigger code
that does expensive queries of other stats as a side effect.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




Re: [PATCH v4 1/3] qmp: Support for querying stats

2022-03-11 Thread Markus Armbruster
Mark Kanda  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 
> ---
>  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 00..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
> +
> +##
> +# 

[PATCH v4 1/3] qmp: Support for querying stats

2022-02-15 Thread Mark Kanda
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 
---
 include/monitor/stats.h |  51 
 monitor/qmp-cmds.c  | 219 +
 qapi/meson.build|   1 +
 qapi/qapi-schema.json   |   1 +
 qapi/stats.json | 259 
 5 files changed, 531 insertions(+)
 create mode 100644 include/monitor/stats.h
 create mode 100644 qapi/stats.json

diff --git a/include/monitor/stats.h b/include/monitor/stats.h
new file mode 100644
index 00..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