[libvirt] [PATCHv2 10/11] qemu_capabilities: Introduce virQEMUCapsQMPBaselineCPUModel (baseline using QEMU)

[Collin Walling]

On 07/09/2018 11:56 PM, Chris Venteicher wrote:
> Baseline cpu model using QEMU/QMP query-cpu-model-baseline
> 
> query-cpu-model-baseline only compares two CPUModels so multiple
> exchanges are needed to evaluate more than two CPUModels.
> ---
>  src/qemu/qemu_capabilities.c | 85 ++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_capabilities.h |  4 ++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 6f8983384a..e0bf78fbba 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5424,3 +5424,88 @@ virQEMUCapsStripMachineAliases(virQEMUCapsPtr qemuCaps)
>      for (i = 0; i < qemuCaps->nmachineTypes; i++)
>          VIR_FREE(qemuCaps->machineTypes[i].alias);
>  }
> +
> +
> +/* in:
> + *  cpus[0]->model = "z13-base";
> + *  cpus[0]->features[0].name = "xxx";
> + *  cpus[0]->features[1].name = "yyy";
> + *  ***
> + *  cpus[n]->model = "s390x";

"s390x" is not a valid CPU model.

> + *  cpus[n]->features[0].name = "xxx";
> + *  cpus[n]->features[1].name = "yyy";
> + *
> + * out:
> + *  *baseline->model = "s390x";
> + *  *baseline->features[0].name = "yyy";
> + *
> + * (ret==0) && (*baseline==NULL) if a QEMU rejects model name or baseline command
> + */
> +int
> +virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd,
> +                               virCPUDefPtr *cpus,
> +                               virCPUDefPtr *baseline)
> +{
> +    qemuMonitorCPUModelInfoPtr  model_baseline = NULL;
> +    qemuMonitorCPUModelInfoPtr  new_model_baseline = NULL;
> +    qemuMonitorCPUModelInfoPtr  next_model = NULL;
> +    bool migratable_only = true;
> +    int ret = -1;
> +    size_t i;
> +
> +    *baseline = NULL;
> +
> +    if (!cpus || !cpus[0] || !cpus[1]) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s", _("less than 2 cpus"));
> +        goto cleanup;
> +    }
> +
> +    for (i = 0; !cpus[i]; i++) {      /* last element in cpus == NULL */

Remove the ! from the condition here.

> +        virCPUDefPtr cpu = cpus[i];
> +
> +        VIR_DEBUG("cpu[%lu]->model = %s", i, NULLSTR(cpu->model));
> +
> +        if (!(next_model = virQEMUCapsCPUModelInfoFromCPUDef(cpu))) {
> +            virReportError(VIR_ERR_INVALID_ARG, "%s", _("cpu without content"));
> +            goto cleanup;
> +        }
> +
> +        if (i == 0) {
> +            model_baseline = next_model;
> +            continue;
> +        }
> +
> +        if (qemuMonitorGetCPUModelBaseline(cmd->mon, model_baseline,
> +                                           next_model, &new_model_baseline) < 0)
> +            goto cleanup;

Hmmm... since we're fencing who can use baseline based on the architecture, I wonder if it would
help to clean things up and have qemuMonitorJSONGetCPUModelBaseline return nonzero if "GenericError"
is reported? That would allow you to remove the condition block below, or at the very least clean up 
the error message to say something like "Either model %s or %s is not supported."

> +
> +        if (!new_model_baseline) {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("QEMU doesn't support baseline or recognize model %s or %s"),
> +                           model_baseline->name,
> +                           next_model->name);
> +            ret = 0;
> +            goto cleanup;
> +        }
> +
> +        qemuMonitorCPUModelInfoFree(model_baseline);
> +        qemuMonitorCPUModelInfoFree(next_model);
> +
> +        next_model = NULL;
> +
> +        model_baseline = new_model_baseline;
> +    }
> +
> +    if (!(*baseline = virQEMUCapsCPUModelInfoToCPUDef(migratable_only, model_baseline)))
> +        goto cleanup;
> +
> +    VIR_DEBUG("baseline->model = %s", NULLSTR((*baseline)->model));
> +
> +    ret = 0;
> +
> + cleanup:
> +    qemuMonitorCPUModelInfoFree(model_baseline);
> +    qemuMonitorCPUModelInfoFree(next_model);
> +
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 7be636d739..d49c8b32ec 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -593,6 +593,10 @@ void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps,
>  qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef);
>  virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model);
>  
> +int virQEMUCapsQMPBaselineCPUModel(virQEMUCapsInitQMPCommandPtr cmd,
> +                                   virCPUDefPtr *cpus,
> +                                   virCPUDefPtr *baseline);
> +
>  virFileCachePtr virQEMUCapsCacheNew(const char *libDir,
>                                      const char *cacheDir,
>                                      uid_t uid,
> 


-- 
Respectfully,
- Collin Walling