[libvirt] [PATCHv2 07/11] qemu_capabilities: Introduce virCPUDef / qemuMonitorCPUModelInfo conversions

[Collin Walling]

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