Re: [Qemu-devel] [PATCHv5 5/5] hmp: change hmp_info_cpus to use query-cpus-fast

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

* Viktor Mihajlovski (mihajlov@linux.vnet.ibm.com) wrote:
> Changing the implementation of hmp_info_cpus() to call
> qmp_query_cpus_fast() instead of qmp_query_cpus. This has the
> following consequences:
> 
>   o No further code change required for qmp_query_cpus deprecation
> 
>   o HMP profits from the less disruptive cpu information retrieval
> 
>   o HMP 'info cpus' won't display architecture specific data anymore,
>     which should be tolerable in the light of the deprecation of
>     query-cpus.
> 
> In order to allow 'info cpus' to be executed completely on the
> fast path, monitor_get_cpu_index() has been adapted to not synchronize
> the cpu state.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

Yep, that's fine we let HMP change over time.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  hmp.c     | 41 +++++++----------------------------------
>  monitor.c | 13 ++++++++++---
>  2 files changed, 17 insertions(+), 37 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 7870d6a..ae86bfb 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -360,50 +360,23 @@ void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict)
>  
>  void hmp_info_cpus(Monitor *mon, const QDict *qdict)
>  {
> -    CpuInfoList *cpu_list, *cpu;
> +    CpuInfoFastList *cpu_list, *cpu;
>  
> -    cpu_list = qmp_query_cpus(NULL);
> +    cpu_list = qmp_query_cpus_fast(NULL);
>  
>      for (cpu = cpu_list; cpu; cpu = cpu->next) {
>          int active = ' ';
>  
> -        if (cpu->value->CPU == monitor_get_cpu_index()) {
> +        if (cpu->value->cpu_index == monitor_get_cpu_index()) {
>              active = '*';
>          }
>  
> -        monitor_printf(mon, "%c CPU #%" PRId64 ":", active, cpu->value->CPU);
> -
> -        switch (cpu->value->arch) {
> -        case CPU_INFO_ARCH_X86:
> -            monitor_printf(mon, " pc=0x%016" PRIx64, cpu->value->u.x86.pc);
> -            break;
> -        case CPU_INFO_ARCH_PPC:
> -            monitor_printf(mon, " nip=0x%016" PRIx64, cpu->value->u.ppc.nip);
> -            break;
> -        case CPU_INFO_ARCH_SPARC:
> -            monitor_printf(mon, " pc=0x%016" PRIx64,
> -                           cpu->value->u.q_sparc.pc);
> -            monitor_printf(mon, " npc=0x%016" PRIx64,
> -                           cpu->value->u.q_sparc.npc);
> -            break;
> -        case CPU_INFO_ARCH_MIPS:
> -            monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.q_mips.PC);
> -            break;
> -        case CPU_INFO_ARCH_TRICORE:
> -            monitor_printf(mon, " PC=0x%016" PRIx64, cpu->value->u.tricore.PC);
> -            break;
> -        default:
> -            break;
> -        }
> -
> -        if (cpu->value->halted) {
> -            monitor_printf(mon, " (halted)");
> -        }
> -
> -        monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
> +        monitor_printf(mon, "%c CPU #%" PRId64 ":", active,
> +                       cpu->value->cpu_index);
> +        monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id);
>      }
>  
> -    qapi_free_CpuInfoList(cpu_list);
> +    qapi_free_CpuInfoFastList(cpu_list);
>  }
>  
>  static void print_block_info(Monitor *mon, BlockInfo *info,
> diff --git a/monitor.c b/monitor.c
> index f499250..db57241 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1055,7 +1055,7 @@ int monitor_set_cpu(int cpu_index)
>      return 0;
>  }
>  
> -CPUState *mon_get_cpu(void)
> +static CPUState *mon_get_cpu_sync(bool synchronize)
>  {
>      CPUState *cpu;
>  
> @@ -1074,10 +1074,17 @@ CPUState *mon_get_cpu(void)
>          monitor_set_cpu(first_cpu->cpu_index);
>          cpu = first_cpu;
>      }
> -    cpu_synchronize_state(cpu);
> +    if (synchronize) {
> +        cpu_synchronize_state(cpu);
> +    }
>      return cpu;
>  }
>  
> +CPUState *mon_get_cpu(void)
> +{
> +    return mon_get_cpu_sync(true);
> +}
> +
>  CPUArchState *mon_get_cpu_env(void)
>  {
>      CPUState *cs = mon_get_cpu();
> @@ -1087,7 +1094,7 @@ CPUArchState *mon_get_cpu_env(void)
>  
>  int monitor_get_cpu_index(void)
>  {
> -    CPUState *cs = mon_get_cpu();
> +    CPUState *cs = mon_get_cpu_sync(false);
>  
>      return cs ? cs->cpu_index : UNASSIGNED_CPU_INDEX;
>  }
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK