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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 4 maggio 2022 15:22:27 CEST, Markus Armbruster <armbru@redhat.com> ha scritto:
>Can you point to existing uses of KVM binary stats introspection data?

There's none, but Google is using it in house. The same data was available before in debugfs and available via the kvm_stat script, so you could also refer to Christian Borntraeger's KVM Forum 2019 talk. The problems with debugfs are basically that it's only available to root and is disabled by secure boot (both issues are not fixable on general because they are Linux policy).

>> index 4912b9744e..92d7ecc52c 100644
>> --- a/qapi/qapi-schema.json
>> +++ b/qapi/qapi-schema.json
>> @@ -93,3 +93,4 @@
>>  { 'include': 'audio.json' }
>>  { 'include': 'acpi.json' }
>>  { 'include': 'pci.json' }
>> +{ 'include': 'stats.json' }
>> diff --git a/qapi/stats.json b/qapi/stats.json
>> new file mode 100644
>> index 0000000000..7454dd7daa
>> --- /dev/null
>> +++ b/qapi/stats.json
>> @@ -0,0 +1,192 @@
>> +# -*- Mode: Python -*-
>> +# vim: filetype=python
>> +#
>> +# Copyright (c) 2022 Oracle and/or its affiliates.
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
>> +# See the COPYING file in the top-level directory.
>> +#
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +##
>> +# = Statistics
>> +##
>> +
>> +##
>> +# @StatsType:
>> +#
>> +# Enumeration of statistics types
>> +#
>> +# @cumulative: stat is cumulative; value can only increase.
>> +# @instant: stat is instantaneous; value can increase or decrease.
>> +# @peak: stat is the peak value; value can only increase.
>> +# @linear-hist: stat is a linear histogram.
>> +# @log-hist: stat is a logarithmic histogram.
>
>For better or worse, we tend to eschew abbreviations in schema
>identifiers.  Would you mind @linear-histogram and @log-histogram?

Sure.


>> +# Since: 7.1
>> +##
>> +{ 'enum': 'StatsTarget',
>> +  'data': [ 'vm', 'vcpu' ] }
>
>Do VM stats include vCPU stats?  "Entire virtual machine" suggests they
>do...

No, they don't. They are a different sets of data that is gathered on resources shared by the whole VM. Stuff such as "# of pages taken by the KVM page tables" goes there because VCPUs share a single copy of the page tables, as opposed to "# of page faults" which is a VCPU stat.

>> +# The arguments to the query-stats command; specifies a target for which to
>> +# request statistics, and which statistics are requested from each provider.
>> +#
>> +# Since: 7.1
>> +##
>> +{ 'struct': 'StatsFilter',
>> +  'data': { 'target': 'StatsTarget' } }
>
>The "and which statistics" part will be implemented later in this
>series?

Oh, indeed it is. Thanks for noticing.

>> +{ 'struct': 'StatsResult',
>> +  'data': { 'provider': 'StatsProvider',
>> +            '*qom-path': 'str',
>
>When exactly will @qom-path be present?

Only if the target is vcpus, for the current set of targets. Because the target is in the command I am not repeating it here with another discriminated record.

>> +# @type: kind of statistic, a @StatType.
>
>Generated documentation looks like
>
>       type: StatsType
>              kind of statistic, a StatType.
>
>I think ", a @StatType" should be dropped.
>
>If we decide to keep it: @StatsType.

Gotcha.

>
>> +#
>> +# @unit: base unit of measurement for the statistics @StatUnit.
>
>"@StatUnit", too.
>
>If we decide to keep it: @StatsUnit.
>
>@unit is optional.  What's the default?

The stat is an adimensional number: a count of events such a page faults, or the maximum length of a bucket in a hash table,  etc. It's actually the common case.

>> +# @base: base for the multiple of @unit that the statistic uses, either 2 or 10.
>> +#        Only present if @exponent is non-zero.
>> +#
>> +# @exponent: exponent for the multiple of @unit that the statistic uses
>
>Alright, given a stat value 42, what does it mean for the possible
>combinations of @base and @exponent?

Base and exponent are used to represent units like KiB, nanoseconds, etc.

>> +# @bucket-size: Used with linear-hist to report the width of each bucket
>> +#               of the histogram.
>
>Feels too terse.  Example, perhaps?
>
>I assume @bucket-size is present exactly when @type is @linear-hist.
>Correct?

Yep, will expand.

>> +##
>> +# @StatsSchema:
>> +#
>> +# Schema for all available statistics for a provider and target.
>> +#
>> +# @provider: provider for this set of statistics.
>> +#
>> +# @target: kind of object that can be queried through this provider.
>> +#
>> +# @stats: list of statistics.
>> +#
>> +# Since: 7.1
>> +##
>> +{ 'struct': 'StatsSchema',
>> +  'data': { 'provider': 'StatsProvider',
>> +            'target': 'StatsTarget',
>> +            'stats': [ 'StatsSchemaValue' ] } }
>
>How am I to connect each element of the result of query-stats to an
>element of the result of query-stats-schema?

You gave the target to query-stats and the result of query-stats has the provider and name. Target+provider+name uniquely identify a StatsSchemaValue in the result of query-stats-schemas.

Paolo

>
>> +
>> +##
>> +# @query-stats-schemas:
>> +#
>> +# Return the schema for all available runtime-collected statistics.
>> +#
>> +# Since: 7.1
>> +##
>> +{ 'command': 'query-stats-schemas',
>> +  'data': { },
>> +  'returns': [ 'StatsSchema' ] }
>