Re: [Qemu-devel] [PATCHv3 2/4] qmp: add query-cpus-fast

[Viktor Mihajlovski]

On 15.02.2018 15:19, Eric Blake wrote:
> 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.
Ditching info cpus_fast would make me happy as well, because it would
cause less headache on the libvirt side of things.
> 
>> +++ 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!
> 


-- 
Regards,
 Viktor Mihajlovski