On 12/7/21 19:42, Daniel P. Berrangé wrote:
Now onto the values being reported. AFAICT from the kernel
docs, for all the types of data it currently reports
(cumulative, instant, peak, none), there is only ever going
to be a single value. I assume the ability to report multiple
values is future proofing for a later requirement.
Yes, in fact histogram values have since been added.
Second, for a given named statistic, AFAICT, the data type,
unit, base and exponent are all fixed. I don't see a reason
for us to be reporting that information every time we call
'query-stats'. Just report the name + value(s). Apps that
want a specific named stat won't care about the dimensions,
because they'll already know what the value means.
I agree on this.
The 'name' and 'type' are used for filtering I presume. Only allowing
a single value for each feels pretty inflexible. I'd say we want to
allow mutliple requests at a time for efficiency.
Bearing in mind my other suggestions above, I'd think we should have
something more like
{ 'enum': 'StatsProvider',
'data': ["kvm", "qemu", "tcg", ....],
}
{ 'struct': 'StatsRequest',
'data': {
'provider': 'StatsProvider',
// If omitted, report everything for this provider
'*fields': [ 'str' ]
I think provider should be optional as well. See below.
}
}
{ 'struct': 'StatsVCPURequest',
'base': 'StatsRequest',
'data': {
// To request subset of vCPUs e.g.
// "cpu_set": "0-3"
// Could use ['int'] instead if desired
'*cpu_set': str,
Yes, ['int'] is preferrable.
},
}
{ 'struct': 'StatsFilter',
'data': {
// If omitted means don't report that group of data
'*vcpu': 'StatsVCPURequest',
'*vm': 'StatsRequest',
},
}
I agree except that I think this and StatsResults should be unions, even if it
means running multiple query-stats commands.
There should also be an enum ['vcpu', 'vm'] that acts as the discriminator for
both StatsFilter and StatsResults:
{ 'enum': 'StatsTarget',
'data': [ 'vcpu', 'vm' ] }
{ 'union': 'StatsFilter',
'base': { 'target': 'StatsTarget', '*providers': ['StatsProvider'] },
'discriminator': 'target',
'data': { 'vcpu': ['*cpu-set': ['int']] }
}
{ 'union': 'StatsResults',
'base': { 'target': 'StatsTarget', stats: ['StatsResultsEntry'] },
'discriminator': 'target',
'data': { 'vcpu': ['id': 'int'] }
}
Alternatively, the providers should simply be keys in the dictionary
Paolo
{ 'alternate': 'StatsValue',
'data': { 'scalar': 'int64',
'list': [ 'int64 ' ] }
}
{ 'struct': 'StatsResultsEntry',
'data': {
'provider': 'StatsProvider',
'stats': [ 'StatsValue' ]
}
}
{ 'struct': 'StatsResults':
'data': {
'*vcpu': [ [ 'StatsResultsEntry' ] ],
'*vm': [ 'StatsResultsEntry' ]
}
}
{ 'command': 'query-stats',
'data': { 'filter': '*StatsFilter' },
'returns': 'StatsResults' }