[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 09/10] qmp: add filtering of statistics by name
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH v5 09/10] qmp: add filtering of statistics by name |
Date: |
Wed, 8 Jun 2022 14:36:16 +0100 |
User-agent: |
Mutt/2.2.1 (2022-02-19) |
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> Allow retrieving only a subset of statistics. This can be useful
> for example in order to plot a subset of the statistics many times
> a second.
>
> KVM publishes ~40 statistics for each vCPU on x86; retrieving and
> serializing all of them would be useless
>
> Another use will be in HMP in the following patch; implementing the
> filter in the backend is easy enough that it was deemed okay to make
> this a public interface.
>
> Example:
>
> { "execute": "query-stats",
> "arguments": {
> "target": "vcpu",
> "vcpus": [ "/machine/unattached/device[2]",
> "/machine/unattached/device[4]" ],
> "providers": [
> { "provider": "kvm",
> "names": [ "l1d_flush", "exits" ] } } }
>
> { "return": {
> "vcpus": [
> { "path": "/machine/unattached/device[2]"
> "providers": [
> { "provider": "kvm",
> "stats": [ { "name": "l1d_flush", "value": 41213 },
> { "name": "exits", "value": 74291 } ] } ] },
> { "path": "/machine/unattached/device[4]"
> "providers": [
> { "provider": "kvm",
> "stats": [ { "name": "l1d_flush", "value": 16132 },
> { "name": "exits", "value": 57922 } ] } ] } ] } }
>
> Extracted from a patch by Mark Kanda.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> accel/kvm/kvm-all.c | 17 +++++++++++------
> include/monitor/stats.h | 2 +-
> monitor/qmp-cmds.c | 7 ++++++-
> qapi/stats.json | 6 +++++-
> 4 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 66c4ac1ac6..f90f0602bc 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -2312,7 +2312,7 @@ bool kvm_dirty_ring_enabled(void)
> }
>
> static void query_stats_cb(StatsResultList **result, StatsTarget target,
> - strList *targets, Error **errp);
> + strList *names, strList *targets, Error **errp);
> static void query_stats_schemas_cb(StatsSchemaList **result, Error **errp);
>
> static int kvm_init(MachineState *ms)
> @@ -3713,6 +3713,7 @@ typedef struct StatsArgs {
> StatsResultList **stats;
> StatsSchemaList **schema;
> } result;
> + strList *names;
> Error **errp;
> } StatsArgs;
>
> @@ -3921,7 +3922,7 @@ static StatsDescriptors
> *find_stats_descriptors(StatsTarget target, int stats_fd
> }
>
> static void query_stats(StatsResultList **result, StatsTarget target,
> - int stats_fd, Error **errp)
> + strList *names, int stats_fd, Error **errp)
> {
> struct kvm_stats_desc *kvm_stats_desc;
> struct kvm_stats_header *kvm_stats_header;
> @@ -3963,6 +3964,9 @@ static void query_stats(StatsResultList **result,
> StatsTarget target,
>
> /* Add entry to the list */
> stats = (void *)stats_data + pdesc->offset;
> + if (!apply_str_list_filter(pdesc->name, names)) {
> + continue;
> + }
> stats_list = add_kvmstat_entry(pdesc, stats, stats_list, errp);
> }
>
> @@ -4024,8 +4028,8 @@ static void query_stats_vcpu(CPUState *cpu,
> run_on_cpu_data data)
> error_propagate(kvm_stats_args->errp, local_err);
> return;
> }
> - query_stats(kvm_stats_args->result.stats, STATS_TARGET_VCPU, stats_fd,
> - kvm_stats_args->errp);
> + query_stats(kvm_stats_args->result.stats, STATS_TARGET_VCPU,
> + kvm_stats_args->names, stats_fd, kvm_stats_args->errp);
> close(stats_fd);
> }
>
> @@ -4046,7 +4050,7 @@ static void query_stats_schema_vcpu(CPUState *cpu,
> run_on_cpu_data data)
> }
>
> static void query_stats_cb(StatsResultList **result, StatsTarget target,
> - strList *targets, Error **errp)
> + strList *names, strList *targets, Error **errp)
> {
> KVMState *s = kvm_state;
> CPUState *cpu;
> @@ -4060,7 +4064,7 @@ static void query_stats_cb(StatsResultList **result,
> StatsTarget target,
> error_setg_errno(errp, errno, "KVM errno, stats: ioctl failed");
> return;
> }
> - query_stats(result, target, stats_fd, errp);
> + query_stats(result, target, names, stats_fd, errp);
> close(stats_fd);
> break;
> }
> @@ -4068,6 +4072,7 @@ static void query_stats_cb(StatsResultList **result,
> StatsTarget target,
> {
> StatsArgs stats_args;
> stats_args.result.stats = result;
> + stats_args.names = names;
> stats_args.errp = errp;
> CPU_FOREACH(cpu) {
> if (!apply_str_list_filter(cpu->parent_obj.canonical_path,
> targets)) {
> diff --git a/include/monitor/stats.h b/include/monitor/stats.h
> index 80a523dd29..fcf0983154 100644
> --- a/include/monitor/stats.h
> +++ b/include/monitor/stats.h
> @@ -11,7 +11,7 @@
> #include "qapi/qapi-types-stats.h"
>
> typedef void StatRetrieveFunc(StatsResultList **result, StatsTarget target,
> - strList *targets, Error **errp);
> + strList *names, strList *targets, Error
> **errp);
> typedef void SchemaRetrieveFunc(StatsSchemaList **result, Error **errp);
>
> /*
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index e49ab345d7..7314cd813d 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -472,12 +472,17 @@ static bool invoke_stats_cb(StatsCallbacks *entry,
> Error **errp)
> {
> strList *targets = NULL;
> + strList *names = NULL;
> ERRP_GUARD();
>
> if (request) {
> if (request->provider != entry->provider) {
> return true;
> }
> + if (request->has_names && !request->names) {
> + return true;
> + }
> + names = request->has_names ? request->names : NULL;
> }
>
> switch (filter->target) {
> @@ -496,7 +501,7 @@ static bool invoke_stats_cb(StatsCallbacks *entry,
> abort();
> }
>
> - entry->stats_cb(stats_results, filter->target, targets, errp);
> + entry->stats_cb(stats_results, filter->target, names, targets, errp);
> if (*errp) {
> qapi_free_StatsResultList(*stats_results);
> *stats_results = NULL;
> diff --git a/qapi/stats.json b/qapi/stats.json
> index 503918ea4c..2f8bfe8fdb 100644
> --- a/qapi/stats.json
> +++ b/qapi/stats.json
> @@ -75,11 +75,14 @@
> # Indicates a set of statistics that should be returned by query-stats.
> #
> # @provider: provider for which to return statistics.
> +
> +# @names: statistics to be returned (all if omitted).
> #
> # Since: 7.1
> ##
> { 'struct': 'StatsRequest',
> - 'data': { 'provider': 'StatsProvider' } }
> + 'data': { 'provider': 'StatsProvider',
> + '*names': [ 'str' ] } }
>
> ##
> # @StatsVCPUFilter:
> @@ -99,6 +102,7 @@
> # that target:
> # - which vCPUs to request statistics for
> # - which providers to request statistics from
> +# - which named values to return within each provider
> #
> # Since: 7.1
> ##
> --
> 2.36.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v5 09/10] qmp: add filtering of statistics by name,
Dr. David Alan Gilbert <=