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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 5/13/22 15:10, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>> On 5/5/22 15:28, Markus Armbruster wrote:
>>>> This means we'll acquire yet another introspection system, unrelated to
>>>> the introspection systems we already have in QEMU.
>> 
>> ... ^^^ needs justification.  Explain why passing the kernel's
>> existing interface through QEMU is useful, and to whom.
>
> There are two justifications.
>
> The first is the contents of the schemas: the new introspection data 
> provides different information than the QAPI data, namely unit of 
> measurement, how the numbers are gathered and change 
> (peak/instant/cumulative/histogram), and histogram bucket sizes.  Unless 
> you think those should be added to the QAPI introspection (and IMO there 
> might be a case only for the unit of measure---and even then it's only a 
> very weak case), the separate introspection data justifies itself.
>
> So the existence of query-stats-schemas in my opinion is justified even 
> if don't consider the usecase of passing through the kernel's descriptions.
>
> The second justification however is indeed about the dynamicity of the 
> schema.  The QAPI introspection data is very much static; and while QOM 
> is somewhat more dynamic, generally we consider that to be a bug rather 
> than a feature these days.  On the other hand, running old QEMU with new 
> kernel is a supported usecase; if old QEMU cannot expose statistics from 
> a new kernel, or if a kernel developer needs to change QEMU before 
> gathering new info from the new kernel, then that is a poor user interface.
>
> Gathering statistics is important for development, for monitoring and 
> for performance measurement.  There are tools such as kvm_stat that do 
> this and they rely on the _user_ knowing the interesting data points 
> rather than the tool (which can treat them as opaque).  The goal here is 
> to take the capabilities of these tools and making them available 
> throughout the whole virtualization stack, so that one can observe, 
> monitor and measure virtual machines without having shell access + root 
> on the host that runs them.

Work this into one of the commit messages, please.

>> How strong do we feel about the stability of the stats exposed by this
>> command?  Separate answers for *structure* of the stats and concrete
>> stats.
>
> I'll try to answer this from the point of view of the kernel:
>
> - will "some" statistics ever be available for all targets that are 
> currently supported?  Yes, resoundingly.
>
> - are the names of statistics stable?  Mostly, but not 100%.  If 
> somebody notices the same value is being tracked with different names in 
> two different architectures, one of them might be renamed.  If the 
> statistic tracks a variable that does not exist anymore as the code 
> changes, the statistic will go away.  If KVM grows two different ways to 
> do the same thing and the default changes, some statistics that were 
> previously useful could now be stuck at 0.  All of these events are 
> expected to be rare, but 100% stability is neither a goal nor attainable 
> in my opinion.
>
> - is the schema format stable?  Yes, it is designed to be extensible but 
> it will be backwards compatible.  Don't break userspace and all that.
>
> And for QEMU:
>
> - will "some" statistics ever be available for all targets that are 
> currently supported?  Yes, and this will be true even if other 
> QEMU-specific targets are added, e.g. block devices.
>
> - will other providers have the same guarantees of stability?  It 
> depends.  Statistics based on the current "query-blockstats" output will 
> probably be even more stable than KVM stats.  TCG stats might be of 
> variable stability.  We can add "x-" in front of providers if we decide 
> that such a convention is useful.
>
> - is the QEMU schema format stable?  Yes.  What we have is more or less 
> a 1:1 conversion of the KVM schema format, which is pretty 
> comprehensive. But if an addition to the schema proves itself worthwhile 
> it can be added with the usual care to QAPI backwards compatibility.
>
>> If we're confident neither structure nor concrete stats will change
>> incompatibly, the commands are stable without reservations.
>> 
>> If we're confident the structure is stable, but unable or unwilling to
>> commit to the concrete stats, we should explain this in documentation.
>
> Based on the above text do you have a suggested wording and, especially, 

Friday afternoon, worst time for word-smithing...  Feel free to ask
again on Monday :)

> a suggested place?  For example, do you think it would fit better in the 
> query-stats or query-stats-schemas documentation?

No obvious best choice.  I'd lean towards query-stats-schema.  Or
perhaps neither; write a separate introduction instead, like this:

    ##
    # = Statistics
    #
    # Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do
    # eiusmod tempor incididunt ut labore et dolore magna aliqua.  Ut enim
    # ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
    # aliquip ex ea commodo consequat.  Duis aute irure dolor in
    # reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla
    # pariatur.  Excepteur sint occaecat cupidatat non proident, sunt in
    # culpa qui officia deserunt mollit anim id est laborum.
    ##

Comes out in HTML as you'd expect, except it gets also included in the
table of contents, which is a bug.

>> If we're unsure about both, the commands should be marked unstable.  We
>> can always upgrade stability later.