Re: [PATCH v5 02/10] kvm: Support for querying fd-based stats

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> On 6/8/22 16:52, Dr. David Alan Gilbert wrote:
> > > If you mean why not some other source, each source has a different file
> > > descriptor:
> > > 
> > > +    int stats_fd = kvm_vcpu_ioctl(cpu, KVM_GET_STATS_FD, NULL);
> > > 
> > > but the descriptors are consistent every time KVM_GET_STATS_FD is called, so
> > > basically "ident" can be used as a cache key.
> > 
> > Ah OK, this is what I was after; it's a little weird that the caller
> > does the ioctl to get the stats-fd, but it does the lookup internally
> > with current_cpu for the ident.
> 
> Oh yeah that's weird.
> 
> Let me squash in this:

Yeh that's nicer; a comment something like:

'Find descriptors for 'target', either that have already been read or
 query 'stats_fd' to read them from kvm'

?

Dave

> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 023bf4ea79..71896ad173 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3871,17 +3871,7 @@ static StatsDescriptors *find_stats_descriptors(StatsTarget target, int stats_fd
>      size_t size_desc;
>      ssize_t ret;
> -    switch (target) {
> -    case STATS_TARGET_VM:
> -        ident = StatsTarget_str(STATS_TARGET_VM);
> -        break;
> -    case STATS_TARGET_VCPU:
> -        ident = current_cpu->parent_obj.canonical_path;
> -        break;
> -    default:
> -        abort();
> -    }
> -
> +    ident = StatsTarget_str(target);
>      QTAILQ_FOREACH(descriptors, &stats_descriptors, next) {
>          if (g_str_equal(descriptors->ident, ident)) {
>              return descriptors;
> @@ -3917,7 +3907,7 @@ static StatsDescriptors *find_stats_descriptors(StatsTarget target, int stats_fd
>      }
>      descriptors->kvm_stats_header = kvm_stats_header;
>      descriptors->kvm_stats_desc = kvm_stats_desc;
> -    descriptors->ident = g_strdup(ident);
> +    descriptors->ident = ident;
>      QTAILQ_INSERT_TAIL(&stats_descriptors, descriptors, next);
>      return descriptors;
>  }
> 
> (once I test it).
> 
> Paolo
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK