Re: [PATCH v4 13/15] target-s390x: Extend QMP command query-cpu-definitions

From: Eduardo Habkost
Date: Mon Mar 30 2015 - 15:54:46 EST


On Mon, Mar 30, 2015 at 04:28:26PM +0200, Michael Mueller wrote:
[...]
> CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
> const char *machine,
> bool has_accel,
> AccelId accel,
> Error **errp)
> {
> - CpuDefinitionInfoList *entry;
> - CpuDefinitionInfo *info;
> + S390MachineProps mach;
> + GSList *classes;
> + uint64_t *mask;
> + CpuDefParm parm = {
> + .list = NULL,
> + .showFullEntry = false,
> + };
>
> - info = g_malloc0(sizeof(*info));
> - info->name = g_strdup("host");
> + if (!s390_cpu_models_used()) {
> + return qmp_query_cpu_definition_host();
> + }

Why exactly are you returning different results depending on a global
set by machine initialization? I expect query-cpu-definitions to always
return the same data, no matter what were the QEMU command-line options
used to initialize the machine.

>
> - entry = g_malloc0(sizeof(*entry));
> - entry->value = info;
> + if (!has_accel || !has_machine) {
> + goto out;
> + }
> +
> + switch (accel) {
> + case ACCEL_ID_KVM:
> + if (kvm_s390_get_machine_props(NULL, &mach) < 0) {
> + return qmp_query_cpu_definition_host();
> + }
> + break;
> + default:
> + goto out_empty;
> + }
> +
> + mask = s390_fac_list_mask_by_machine(machine);
> + if (!mask) {
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "machine",
> + "a valid machine type");
> + goto out_empty;
> + }
>
> - return entry;
> + s390_setup_cpu_classes(ACCEL_TEMP, &mach, mask);
> + parm.showFullEntry = true;
> +out:
> + classes = object_class_get_list(TYPE_S390_CPU, false);
> + classes = g_slist_sort(classes, s390_cpu_class_asc_order_compare);
> + g_slist_foreach(classes, qmp_query_cpu_definition_entry, &parm);
> + g_slist_free(classes);
> +out_empty:
> + return parm.list;
> }
>

--
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/