On 07/09/2018 11:56 PM, Chris Venteicher wrote: > Bi-directional conversion functions. > Converts model / name and features / properties between the two structures. > > Created reusable functions to bridge the internal (virCpuDef) and > QEMU/QMP specific structures for describing CPU Models. > --- > src/qemu/qemu_capabilities.c | 146 +++++++++++++++++++++++++++++------ > src/qemu/qemu_capabilities.h | 3 + > 2 files changed, 127 insertions(+), 22 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 72ab012a78..d3f2317a1d 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -2758,7 +2758,8 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps, > virCPUDefPtr cpu, > bool migratable) > { > - size_t i; > + virCPUDefPtr tmp = NULL; > + int ret = -1; > > if (!modelInfo) { > if (type == VIR_DOMAIN_VIRT_KVM) { > @@ -2771,32 +2772,21 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps, > return 2; > } > > - if (VIR_STRDUP(cpu->model, modelInfo->name) < 0 || > - VIR_ALLOC_N(cpu->features, modelInfo->nprops) < 0) > - return -1; > + if (!(tmp = virQEMUCapsCPUModelInfoToCPUDef(migratable, modelInfo))) > + goto cleanup; > > - cpu->nfeatures_max = modelInfo->nprops; > - cpu->nfeatures = 0; > + /* Free then copy over model, vendor, vendor_id and features */ > + virCPUDefFreeModel(cpu); > > - for (i = 0; i < modelInfo->nprops; i++) { > - virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures; > - qemuMonitorCPUPropertyPtr prop = modelInfo->props + i; > - > - if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) > - continue; > + if (virCPUDefCopyModel(cpu, tmp, true) < 0) > + goto cleanup; > > - if (VIR_STRDUP(feature->name, prop->name) < 0) > - return -1; > + ret = 0; > > - if (!prop->value.boolean || > - (migratable && prop->migratable == VIR_TRISTATE_BOOL_NO)) > - feature->policy = VIR_CPU_FEATURE_DISABLE; > - else > - feature->policy = VIR_CPU_FEATURE_REQUIRE; > - cpu->nfeatures++; > - } > + cleanup: > + virCPUDefFree(tmp); > > - return 0; > + return ret; > } > > > @@ -3547,6 +3537,118 @@ virQEMUCapsLoadCache(virArch hostArch, > return ret; > } > It might make sense to rename these to|from "CPUDefS390" or something, since s390x discards the migratability fields and only considers CPU policies enabled and disabled. Or maybe someone with a stronger understanding of other libvirt CPU defs can help make these functions more agnostic. > +/* virCPUDef model => qemuMonitorCPUModelInfo name > + * virCPUDef features => qemuMonitorCPUModelInfo boolean properties */ > +qemuMonitorCPUModelInfoPtr > +virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef) > +{ > + size_t i; > + qemuMonitorCPUModelInfoPtr cpuModel = NULL; > + qemuMonitorCPUModelInfoPtr ret = NULL; > + > + if (!cpuDef || (VIR_ALLOC(cpuModel) < 0)) > + goto cleanup; > + > + VIR_DEBUG("cpuDef->model = %s", NULLSTR(cpuDef->model)); > + > + if (VIR_STRDUP(cpuModel->name, cpuDef->model) < 0 || > + VIR_ALLOC_N(cpuModel->props, cpuDef->nfeatures) < 0) > + goto cleanup; > + > + /* no visibility on migratability of properties / features */ > + cpuModel->props_migratable_valid = false; I think this should be set to VIR_TRISTATE_BOOL_ABSENT (which evaluates to the same thing anyway) > + > + cpuModel->nprops = 0; > + > + for (i = 0; i < cpuDef->nfeatures; i++) { > + qemuMonitorCPUPropertyPtr prop = &(cpuModel->props[cpuModel->nprops]); > + virCPUFeatureDefPtr feature = &(cpuDef->features[i]); > + > + if (!feature || !(feature->name)) > + continue; > + > + if (VIR_STRDUP(prop->name, feature->name) < 0) > + goto cleanup; > + > + prop->migratable = VIR_TRISTATE_BOOL_ABSENT; Set this to false. > + prop->type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; > + > + switch (feature->policy) { > + case -1: /* policy undefined */ > + case VIR_CPU_FEATURE_FORCE: > + case VIR_CPU_FEATURE_REQUIRE: > + prop->value.boolean = true; > + break; > + > + case VIR_CPU_FEATURE_FORBID: > + case VIR_CPU_FEATURE_DISABLE: > + case VIR_CPU_FEATURE_OPTIONAL: > + case VIR_CPU_FEATURE_LAST: > + prop->value.boolean = false; > + break; > + } > + > + cpuModel->nprops += 1; > + } > + > + VIR_STEAL_PTR(ret, cpuModel); > + > + cleanup: > + qemuMonitorCPUModelInfoFree(cpuModel); > + return ret; > +} > + > + > +/* qemuMonitorCPUModelInfo name => virCPUDef model > + * qemuMonitorCPUModelInfo boolean properties => virCPUDef features > + * > + * migratable_only true: mark non-migratable features as disabled > + * false: allow all features as required > + */ > +virCPUDefPtr > +virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model) > +{ > + virCPUDefPtr cpu = NULL; > + virCPUDefPtr ret = NULL; > + size_t i; > + > + if (!model || (VIR_ALLOC(cpu) < 0)) > + goto cleanup; > + > + VIR_DEBUG("model->name= %s", NULLSTR(model->name)); > + > + if (VIR_STRDUP(cpu->model, model->name) < 0 || > + VIR_ALLOC_N(cpu->features, model->nprops) < 0) > + goto cleanup; > + > + cpu->nfeatures_max = model->nprops; > + cpu->nfeatures = 0; > + > + for (i = 0; i < model->nprops; i++) { > + virCPUFeatureDefPtr feature = cpu->features + cpu->nfeatures; > + qemuMonitorCPUPropertyPtr prop = model->props + i; > + > + if (prop->type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) > + continue; > + > + if (VIR_STRDUP(feature->name, prop->name) < 0) > + goto cleanup; > + > + if (!prop->value.boolean || > + (migratable_only && prop->migratable == VIR_TRISTATE_BOOL_NO)) > + feature->policy = VIR_CPU_FEATURE_DISABLE; > + else > + feature->policy = VIR_CPU_FEATURE_REQUIRE; > + > + cpu->nfeatures++; > + } > + > + VIR_STEAL_PTR(ret, cpu); > + > + cleanup: > + virCPUDefFree(cpu); > + return ret; > +} > > static void > virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps, > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index a048a1cf02..d5cd486295 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -564,6 +564,9 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, > void virQEMUCapsFilterByMachineType(virQEMUCapsPtr qemuCaps, > const char *machineType); > > +qemuMonitorCPUModelInfoPtr virQEMUCapsCPUModelInfoFromCPUDef(const virCPUDef *cpuDef); > +virCPUDefPtr virQEMUCapsCPUModelInfoToCPUDef(bool migratable_only, qemuMonitorCPUModelInfoPtr model); > + > virFileCachePtr virQEMUCapsCacheNew(const char *libDir, > const char *cacheDir, > uid_t uid, > -- Respectfully, - Collin Walling -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list