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 <bhar...@linux.vnet.ibm.com>
> ---
> 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
