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. 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 :|