On 19/10/21 22:29, Mark Kanda wrote:
> This patchset adds QEMU support for querying fd-based KVM stats. The kernel
> support is provided by:
>
> cb082bfab59a ("KVM: stats: Add fd-based API to read binary stats data")
>
> Patch 1 adds QMP support; patch 2 adds HMP support.
Hi Mark,
that's awesome, thanks!
Just a couple remarks on the implementation:
1) for QMP, it would be nice to have separate commands for the schema
and the value. This is because in the future we could consider passing
the file descriptor directly to the QMP client; in which case, it would
still be nicer to have QEMU turn the schema information into something
more easily consumable.
2) for HMP, it would be nice to have the exponent converted to e.g.
"nanoseconds" if the KvmStatUnit is seconds. It's not super important,
but it would be nice. Something as simple as
if (kvm_stat->unit == KVM_STAT_UNIT_SECONDS &&
kvm_stat->exponent >= -9 && kvm_stat->exponent <= 0 &&
kvm_stat->exponent % 3 == 0 && kvm_stat->base == 10) {
const char *si_prefix[] = { "", "milli", "micro", "nano" };
monitor_printf(" %lu %sseconds", val->value,
si_prefix[kvm_stat->exponent / -3]);
} else ...
(Yes, this is nitpicking; but in practice nanoseconds are the only case
where currently the exponent is not 0, so...).
3) more recent versions of Linux also support histograms, for which you
have to include the bucket size and the kind (logarithmic/linear) in
QMP. For HMP it would be nice to have it printed as
halt_wait_hist: [1 nanosecond]=200 [2]=30 [4]=50 [8]=120
but, for the first version, it's also okay if the QMP commands just
skips unknown KVM_STATS_TYPE values altogether.
Paolo
> Mark Kanda (2):
> qmp: Support fd-based KVM stats query
> hmp: Support fd-based KVM stats query
>
> accel/kvm/kvm-all.c | 246 ++++++++++++++++++++++++++++++++++++++++++
> hmp-commands-info.hx | 13 +++
> include/monitor/hmp.h | 1 +
> monitor/hmp-cmds.c | 52 +++++++++
> qapi/misc.json | 73 +++++++++++++
> 5 files changed, 385 insertions(+)
>