Re: [Qemu-devel] [RFC PATCH v0 5/6] qmp, spapr: Show hot-plugged/pluggable CPU slots in the Machine

[Thomas Huth]

On 25.02.2016 17:22, Bharata B Rao wrote:
> Implement query cpu-slots that provides information about hot-plugged
> as well as hot-pluggable CPU slots that the machine supports.
> 
> TODO: As Eric suggested use enum for type instead of str.
> TODO: @hotplug-granularity probably isn't required.
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/core/machine.c   |  19 +++++++++
>  hw/ppc/spapr.c      | 112 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/boards.h |   4 ++
>  qapi-schema.json    |  85 +++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx     |  47 ++++++++++++++++++++++
>  5 files changed, 267 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 6d1a0d8..3055ef8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -17,6 +17,25 @@
>  #include "hw/sysbus.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
> +#include "qmp-commands.h"
> +
> +/*
> + * QMP: query-cpu-slots
> + *
> + * TODO: Ascertain if this is the right place to for this arch-neutral 
> routine.
> + */
> +CPUSlotInfoList *qmp_query_cpu_slots(Error **errp)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> +    if (!mc->cpu_slots) {
> +        error_setg(errp, QERR_UNSUPPORTED);
> +        return NULL;
> +    }
> +
> +    return mc->cpu_slots(ms);
> +}
>  
>  static char *machine_get_accel(Object *obj, Error **errp)
>  {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 780cd00..b76ed85 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2453,6 +2453,117 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned 
> cpu_index)
>      return cpu_index / smp_threads / smp_cores;
>  }
>  
> +static int spapr_cpuinfo_list(Object *obj, void *opaque)
> +{
> +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> +    CPUInfoList ***prev = opaque;
> +
> +    if (object_dynamic_cast(obj, TYPE_CPU)) {
> +        CPUInfoList *elem = g_new0(CPUInfoList, 1);
> +        CPUInfo *s = g_new0(CPUInfo, 1);
> +        CPUState *cpu = CPU(obj);
> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
> +
> +        s->arch_id = ppc_get_vcpu_dt_id(pcpu);
> +        s->type = g_strdup(object_get_typename(obj));
> +        s->thread = cpu->cpu_index;
> +        s->has_thread = true;
> +        s->core = cpu->cpu_index / smp_threads;
> +        s->has_core = true;
> +        if (mc->cpu_index_to_socket_id) {
> +            s->socket = mc->cpu_index_to_socket_id(cpu->cpu_index);
> +        } else {
> +            s->socket = cpu->cpu_index / smp_threads / smp_cores;
> +        }
> +        s->has_socket = true;
> +        s->node = cpu->numa_node;
> +        s->has_node = true;
> +        s->qom_path = object_get_canonical_path(obj);
> +        s->has_qom_path = true;
> +
> +        elem->value = s;
> +        elem->next = NULL;
> +        **prev = elem;
> +        *prev = &elem->next;
> +    }
> +    object_child_foreach(obj, spapr_cpuinfo_list, opaque);
> +    return 0;
> +}
> +
> +static CPUSlotInfoList *spapr_cpu_slots(MachineState *machine)
> +{
> +    CPUSlotInfoList *head = NULL;
> +    CPUSlotInfoList **prev = &head;
> +    Object *root_container;
> +    ObjectProperty *prop;
> +    ObjectPropertyIterator iter;
> +
> +    /*
> +     * TODO: There surely must be a better/easier way to walk all
> +     * the link properties of an object ?
> +     */
> +    root_container = container_get(object_get_root(), "/machine");
> +    object_property_iter_init(&iter, root_container);
> +
> +    while ((prop = object_property_iter_next(&iter))) {
> +        Object *obj;
> +        DeviceState *dev;
> +        CPUSlotInfoList *elem;
> +        CPUSlotInfo *s;
> +        CPUInfoList *cpu_head = NULL;
> +        CPUInfoList **cpu_prev = &cpu_head;
> +        sPAPRCPUCore *core;
> +
> +        if (!strstart(prop->type, "link<", NULL)) {
> +            continue;
> +        }
> +
> +        if (!strstart(prop->name, SPAPR_MACHINE_CPU_CORE_PROP, NULL)) {
> +            continue;
> +        }
> +
> +        elem = g_new0(CPUSlotInfoList, 1);
> +        s = g_new0(CPUSlotInfo, 1);
> +
> +        obj = object_property_get_link(root_container, prop->name, NULL);
> +        if (obj) {
> +            /* Slot populated */
> +            dev = DEVICE(obj);
> +            core = SPAPR_CPU_CORE(obj);
> +
> +            if (dev->id) {
> +                s->has_id = true;
> +                s->id = g_strdup(dev->id);
> +            }
> +            s->realized = object_property_get_bool(obj, "realized", NULL);
> +            s->nr_cpus = core->nr_threads;
> +            s->has_nr_cpus = true;
> +            s->qom_path = object_get_canonical_path(obj);
> +            s->has_qom_path = true;
> +            if (s->realized) {
> +                spapr_cpuinfo_list(obj, &cpu_prev);
> +            }
> +            s->has_cpus = true;
> +        } else {
> +            /* Slot empty */
> +            s->has_id = false;
> +            s->has_nr_cpus = false;
> +            s->has_qom_path = false;
> +            s->has_cpus = false;
> +            s->realized = false;
> +        }

I think you could drop that whole else-path since you've already used
g_new0 to make sure that the memory of the new CPUSlotInfo is cleared.

> +        s->type = g_strdup(TYPE_SPAPR_CPU_CORE);
> +        s->hotplug_granularity = g_strdup(SPAPR_MACHINE_CPU_CORE_PROP);
> +        s->slot_id = g_strdup(prop->name);
> +        s->cpus = cpu_head;
> +        elem->value = s;
> +        elem->next = NULL;
> +        *prev = elem;
> +        prev = &elem->next;
> +    }
> +    return head;
> +}

 Thomas