On Tue, 13 Feb 2018 17:12:50 +0100 Viktor Mihajlovski <mihaj...@linux.vnet.ibm.com> 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. > > >