Re: [qemu-s390x] [PATCHv3 2/4] qmp: add query-cpus-fast

[Eric Blake]

On 02/15/2018 04:20 AM, Viktor Mihajlovski wrote:
> From: Luiz Capitulino <address@hidden>
>
> The query-cpus command has an extremely serious side effect:
> it always interrupts all running vCPUs so that they can run
> ioctl calls. This can cause a huge performance degradation for
> some workloads. And most of the information retrieved by the
> ioctl calls are not even used by query-cpus.
>
> This commit introduces a replacement for query-cpus called
> query-cpus-fast, which has the following features:
>
>   o Never interrupt vCPUs threads. query-cpus-fast only returns
>     vCPU information maintained by QEMU itself, which should be
>     sufficient for most management software needs
>
>   o Drop "halted" field as it can not retrieved in a fast

s/can not retrieved/cannot be retrieved/

>     way on most architectures
>
>   o Drop irrelevant fields such as "current", "pc" and "arch"
>
>   o Rename some fields for better clarification & proper naming
>     standard
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> Signed-off-by: Viktor Mihajlovski <address@hidden>
> Reviewed-by: Cornelia Huck <address@hidden>
> Acked-by: Dr. David Alan Gilbert <address@hidden>
> Acked-by: Eric Blake <address@hidden>
> ---

> +++ b/hmp-commands-info.hx
> @@ -160,6 +160,20 @@ Show infos for each CPU.

Pre-existing, but while we are here, it wouldn't hurt to do 
s/infos/information/

>   ETEXI
>   
>       {
> +        .name       = "cpus_fast",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show information for each CPU without interrupting 
> them",
> +        .cmd        = hmp_info_cpus_fast,
> +    },
> +
> +STEXI
> address@hidden info cpus_fast
> address@hidden info cpus_fast
> +Show infos for each CPU without performance penalty.

and again here, where you copied and pasted.

You know, we have no back-compat guarantees on HMP.  We could make 'info 
cpu' just ALWAYS call query-cpus-fast, with no HMP counterpart for the 
slower query-cpus, and without needing a deprecation period.  But I'll 
leave that up to David if that makes more sense.

> +++ b/hmp.c
> @@ -410,6 +410,20 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
>       qapi_free_CpuInfoList(cpu_list);
>   }
>   
> +void hmp_info_cpus_fast(Monitor *mon, const QDict *qdict)
> +{
> +    CpuInfoFastList *head, *cpu;
> +
> +    head = qmp_query_cpus_fast(NULL);
> +
> +    for (cpu = head; cpu; cpu = cpu->next) {
> +        monitor_printf(mon, "  CPU #%" PRId64 ":", cpu->value->cpu_index);
> +        monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id);
> +    }

This is particularly true since your HMP implementation is not even 
printing all the information returned by query-cpus-fast!

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org