On Mon, 12 Feb 2018 13:14:32 +0100 Viktor Mihajlovski <mihaj...@linux.vnet.ibm.com> wrote:
> The s390 CPU state can be retrieved without interrupting the > VM execution. Extendend the CpuInfoFast union with architecture > specific data and an implementation for s390. > > Return data looks like this: > [ > {"thread-id":64301,"props":{"core-id":0}, > "arch":"s390","cpu-state":"operating", > "qom-path":"/machine/unattached/device[0]","cpu-index":0}, > {"thread-id":64302,"props":{"core-id":1}, > "arch":"s390","cpu-state":"operating", > "qom-path":"/machine/unattached/device[1]","cpu-index":1} > ] > > Currently there's a certain amount of duplication between > the definitions of CpuInfo and CpuInfoFast, both in the > base and variable areas, since there are data fields common > to the slow and fast variants. > > A suggestion was made on the mailing list to enhance the QAPI > code generation to support two layers of unions. This would > allow to specify the common fields once and avoid the duplication > in the leaf unions. > > On the other hand, the slow query-cpus should be deprecated > along with the slow CpuInfo type and eventually be removed. > Assuming that new architectures will not be added at high > rates, we could live with the duplication for the time being. > > Signed-off-by: Viktor Mihajlovski <mihaj...@linux.vnet.ibm.com> > --- > cpus.c | 10 ++++++++++ > hmp.c | 8 ++++++++ > qapi-schema.json | 35 +++++++++++++++++++++++++++++------ > 3 files changed, 47 insertions(+), 6 deletions(-) > > 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; > + 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.