Drop "(baseline using QEMU)" from Subject to make it shorter. On Mon, Jul 09, 2018 at 22:56:54 -0500, 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(+) This code has nothing to do with QEMU capabilities, it should be implemented in qemu_driver.c. > > 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"; > + * 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; One space between the type and the variable name would be enough. > + 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; > + } I guess you're going to special case the single CPU use case in the caller... > + > + for (i = 0; !cpus[i]; i++) { /* last element in cpus == NULL */ As already mentioned by Collin, this can't really work unless you remove '!'. > + 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")); This would override any error reported by virQEMUCapsCPUModelInfoFromCPUDef. > + goto cleanup; > + } > + > + if (i == 0) { > + model_baseline = next_model; > + continue; > + } Alternatively you could just call virQEMUCapsCPUModelInfoFromCPUDef once before the for loop to set model_baseline and start the for loop at index 1. > + > + if (qemuMonitorGetCPUModelBaseline(cmd->mon, model_baseline, > + next_model, &new_model_baseline) < 0) > + goto cleanup; > + > + 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; This doesn't make a lot of sense. It's a real error so why you'd return 0? And the error message should be reported by qemuMonitorGetCPUModelBaseline. > + 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)); How could the model name be NULL at this point? > + > + 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, Jirka -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list