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

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 18 Jan 2022 12:52:10 +0000
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Tue, Jan 18, 2022 at 01:26:32PM +0100, Paolo Bonzini wrote:
> > On 1/17/22 16:17, Mark Kanda wrote:  
> > > > 
> > > > I agree except that I think this and StatsResults should be unions,
> > > > even if it means running multiple query-stats commands.  
> > > 
> > > IIUC, making StatsResults a union implies the filter is a required
> > > argument (currently it is optional - omitting it dumps all VM and VCPU
> > > stats). Just to confirm - we want the filter to be required?  
> > 
> > Yeah, I think at least the "kind" (vcpu, vm, perhaps in the future block or
> > net) should be mandatory.  If the caller doesn't know of a "kind", chances
> > are it won't be able to understand what object the stats refer to, for
> > example the vcpu "id" here:
> > 
> > { 'union': 'StatsResults',
> >    'base': { 'target': 'StatsTarget', stats: ['StatsResultsEntry'] },
> >    'discriminator': 'target',
> >    'data': { 'vcpu': ['id': 'int'] }
> > }
> > 
> > (which is another different between Daniel's proposal and mine; his just
> > placed all vcpus into an array with no explicit id, if I understand
> > correctly).  
> 
> An explicit ID isn't strictly required, since the caller can assume
> the results are ordered on CPU ID, so even if they gave a request
> for a sparse subset of CPUs, the results can be interpreted.  None
> the less having a vCPU id included is more friendly, so worth
> having.

and what exactly this CPU ID is,
may QOM path pointing to VCPU instance would be better?

> 
> Regards,
> Daniel