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

[Viktor Mihajlovski]

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.
>> +        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.
> 

-- 
Regards,
 Viktor Mihajlovski