Re: [qemu-s390x] [Qemu-devel] [PATCH 3/3] qmp: add architecture specific cpu data for query-cpus-fast

[Cornelia Huck]

On Tue, 13 Feb 2018 17:12:50 +0100
Viktor Mihajlovski <address@hidden> wrote:

> On 12.02.2018 17:23, Cornelia Huck wrote:
> [...]
> >> diff --git a/cpus.c b/cpus.c
> >> index 6df6660..af67826 100644
> >> --- a/cpus.c
> >> +++ b/cpus.c
> >> @@ -2166,6 +2166,10 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
> >>      MachineClass *mc = MACHINE_GET_CLASS(ms);
> >>      CpuInfoFastList *head = NULL, *cur_item = NULL;
> >>      CPUState *cpu;
> >> +#if defined(TARGET_S390X)
> >> +    S390CPU *s390_cpu;
> >> +    CPUS390XState *env;
> >> +#endif
> >>  
> >>      CPU_FOREACH(cpu) {
> >>          CpuInfoFastList *info = g_malloc0(sizeof(*info));
> >> @@ -2183,6 +2187,12 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
> >>              info->value->props = props;
> >>          }
> >>  
> >> +#if defined(TARGET_S390X)
> >> +        s390_cpu = S390_CPU(cpu);
> >> +        env = &s390_cpu->env;  
> > 
> > You should be able to omit the s390_cpu variable by using
> > 
> >            env = &S390_CPU(cpu)->env;
> >   
> True, but I wanted to stay in style with the code in qmp_query_cpus.


TBH, I don't care too much one way or the other :)

> >> +        info->value->arch = CPU_INFO_ARCH_S390;
> >> +        info->value->u.s390.cpu_state = env->cpu_state;
> >> +#endif
> >>          if (!cur_item) {
> >>              head = cur_item = info;
> >>          } else {  
> > 
> > As you mentioned in the patch description, the duplication is a bit
> > awkward. I'll let the QAPI experts judge that; otherwise, this looks
> > fine to me.
> >   
>