Re: [Qemu-devel] [PATCH v1 2/2] cpus.c: change qmp_query_cpus 'value->current' logic

[Daniel Henrique Barboza]

On 12/13/2017 04:15 PM, Daniel Henrique Barboza wrote:
> qmp_query_cpus always return the same information, ignoring the
> monitor state. This means that information such as cpu->value->current
> will always return the same value ((cpu == first_cpu) at this moment).
> This function is used by hmp_info_cpus that does its own logic based on
> monitor_get_cpu_index() to print the current/active CPU information,
> ignoring cpu->value->current. This value is used as is in QMP
> query-cpus though.
>
> This wasn't a problem because QMP couldn't set its current CPU via
> qmp_cpu anyway, so query-cpus returning the same state all the time
> (regarding current CPU for the monitor) wasn't a problem. But now
> we can use qmp_cpu to set the current monitor CPU, making the return
> from query-cpus wrong.
>
> This patch makes a simple change in qmp_query_cpus to change the
> cpu->value->current logic to consider the current monitor state,
> like it was being done in hmp_info_cpus. Now, cpu->value->current
> will return an accurate value for both QMP and HMP calls. hmp_info_cpus
> can no use this value directly instead of doing its own logic.
>
> Signed-off-by: Daniel Henrique Barboza <address@hidden>
> ---
The purpose of this patch watered away with qmp_cpu doing nothing.

Although there is a point to be made that this might be "nicer" that
what we have, a sort of cleanup, I am not sure if it's worth it. Apply
it or leave it alone, either way works for me.


Daniel


>   cpus.c | 2 +-
>   hmp.c  | 6 +-----
>   2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 114c29b6a0..d3b79d1c02 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1919,7 +1919,7 @@ CpuInfoList *qmp_query_cpus(Error **errp)
>           info = g_malloc0(sizeof(*info));
>           info->value = g_malloc0(sizeof(*info->value));
>           info->value->CPU = cpu->cpu_index;
> -        info->value->current = (cpu == first_cpu);
> +        info->value->current = (cpu->cpu_index == monitor_get_cpu_index());
>           info->value->halted = cpu->halted;
>           info->value->qom_path = object_get_canonical_path(OBJECT(cpu));
>           info->value->thread_id = cpu->thread_id;
> diff --git a/hmp.c b/hmp.c
> index 7506f105a0..1259cafc1f 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -363,11 +363,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
>       cpu_list = qmp_query_cpus(NULL);
>
>       for (cpu = cpu_list; cpu; cpu = cpu->next) {
> -        int active = ' ';
> -
> -        if (cpu->value->CPU == monitor_get_cpu_index()) {
> -            active = '*';
> -        }
> +        int active = cpu->value->current ? '*' : ' ';
>
>           monitor_printf(mon, "%c CPU #%" PRId64 ":", active, cpu->value->CPU);