[libvirt] [PATCH 5/7] qemu: Store migratable host CPU model in qemuCaps

[John Ferlan]

On 03/31/2017 01:54 PM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 91 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 71 insertions(+), 20 deletions(-)
> 

There's three patches in one here...  Adding virQEMUCapsCPUModel, adding
virQEMUCapsNewHostCPUModel, and creating the migratable CPUs defs and code.

Once I got to this patch, I went back to patch 4 with my thoughts around
nested structs and a flatter capsCPUModel instead...

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index c75aa570c..b426a5abc 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -373,6 +373,14 @@ struct virQEMUCapsMachineType {
>      unsigned int maxCpus;
>      bool hotplugCpus;
>  };
> +
> +typedef struct _virQEMUCapsCPUModel virQEMUCapsCPUModel;
> +typedef virQEMUCapsCPUModel *virQEMUCapsCPUModelPtr;
> +struct _virQEMUCapsCPUModel {
> +    virCPUDefPtr full;
> +    virCPUDefPtr migratable;
> +};
> +
>  /*
>   * Update the XML parser/formatter when adding more
>   * information to this struct so that it gets cached
> @@ -415,8 +423,8 @@ struct _virQEMUCaps {
>       * time we probe QEMU or load the results from the cache.
>       */
>      struct {
> -        virCPUDefPtr kvm;
> -        virCPUDefPtr tcg;
> +        virQEMUCapsCPUModel kvm;
> +        virQEMUCapsCPUModel tcg;
>      } hostCPU;
>  };
>  
> @@ -2097,12 +2105,20 @@ virQEMUCapsCopyHostCPUData(virQEMUCapsPtr dst,
>          !(dst->tcgCPUModelInfo = qemuMonitorCPUModelInfoCopy(src->tcgCPUModelInfo)))
>          return -1;
>  
> -    if (src->hostCPU.kvm &&
> -        !(dst->hostCPU.kvm = virCPUDefCopy(src->hostCPU.kvm)))
> +    if (src->hostCPU.kvm.full &&
> +        !(dst->hostCPU.kvm.full = virCPUDefCopy(src->hostCPU.kvm.full)))
>          return -1;
>  
> -    if (src->hostCPU.tcg &&
> -        !(dst->hostCPU.tcg = virCPUDefCopy(src->hostCPU.tcg)))
> +    if (src->hostCPU.kvm.migratable &&
> +        !(dst->hostCPU.kvm.migratable = virCPUDefCopy(src->hostCPU.kvm.migratable)))
> +        return -1;
> +
> +    if (src->hostCPU.tcg.full &&
> +        !(dst->hostCPU.tcg.full = virCPUDefCopy(src->hostCPU.tcg.full)))
> +        return -1;
> +
> +    if (src->hostCPU.tcg.migratable &&
> +        !(dst->hostCPU.tcg.migratable = virCPUDefCopy(src->hostCPU.tcg.migratable)))
>          return -1;

Lots of copy'n'paste, but if you went with a flatter capsCPUModel, then
there'd be less churn in name changes and only new settings for
kvmMigrate and tcgMigrate.

>  
>      return 0;
> @@ -2197,8 +2213,10 @@ void virQEMUCapsDispose(void *obj)
>  
>      qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
>      qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
> -    virCPUDefFree(qemuCaps->hostCPU.kvm);
> -    virCPUDefFree(qemuCaps->hostCPU.tcg);
> +    virCPUDefFree(qemuCaps->hostCPU.kvm.full);
> +    virCPUDefFree(qemuCaps->hostCPU.kvm.migratable);
> +    virCPUDefFree(qemuCaps->hostCPU.tcg.full);
> +    virCPUDefFree(qemuCaps->hostCPU.tcg.migratable);

One helper to free...

>  }
>  
>  void
> @@ -2427,9 +2445,9 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
>                          virDomainVirtType type)
>  {
>      if (type == VIR_DOMAIN_VIRT_KVM)
> -        return qemuCaps->hostCPU.kvm;
> +        return qemuCaps->hostCPU.kvm.full;
>      else
> -        return qemuCaps->hostCPU.tcg;
> +        return qemuCaps->hostCPU.tcg.full;

This wouldn't need a change with a flat capsCPUModel, just leaving the
subsequent patch to handle being able to return the migratable version...


>  }
>  
>  
> @@ -3276,26 +3294,40 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
>  }
>  
>  
> +static virCPUDefPtr
> +virQEMUCapsNewHostCPUModel(void)
> +{
> +    virCPUDefPtr cpu;
> +
> +    if (VIR_ALLOC(cpu) < 0)
> +        return NULL;
> +
> +    cpu->type = VIR_CPU_TYPE_GUEST;
> +    cpu->mode = VIR_CPU_MODE_CUSTOM;
> +    cpu->match = VIR_CPU_MATCH_EXACT;
> +    cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
> +
> +    return cpu;
> +}
> +
> +
>  void
>  virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>                              virCapsPtr caps,
>                              virDomainVirtType type)
>  {
>      virCPUDefPtr cpu = NULL;
> +    virCPUDefPtr migCPU = NULL;
>      virCPUDefPtr hostCPU = NULL;
> +    virQEMUCapsCPUModelPtr model;
>      int rc;
>  
>      if (!caps || !virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch))
>          return;
>  
> -    if (VIR_ALLOC(cpu) < 0)
> +    if (!(cpu = virQEMUCapsNewHostCPUModel()))
>          goto error;
>  
> -    cpu->type = VIR_CPU_TYPE_GUEST;
> -    cpu->mode = VIR_CPU_MODE_CUSTOM;
> -    cpu->match = VIR_CPU_MATCH_EXACT;
> -    cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
> -
>      if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu, false)) < 0) {
>          goto error;
>      } else if (rc == 1) {
> @@ -3309,10 +3341,26 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>              goto error;
>      }
>  
> +    if (!(migCPU = virQEMUCapsNewHostCPUModel()))
> +        goto error;
> +
> +    if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, migCPU, true)) < 0) {
> +        goto error;
> +    } else if (rc == 1) {
> +        VIR_DEBUG("CPU migratibility not provided by QEMU");

migratability (although the dictionary for my email client doesn't like
either word).

> +
> +        virCPUDefFree(migCPU);
> +        if (!(migCPU = virCPUCopyMigratable(qemuCaps->arch, cpu)))
> +            goto error;
> +    }
> +
>      if (type == VIR_DOMAIN_VIRT_KVM)
> -        qemuCaps->hostCPU.kvm = cpu;
> +        model = &qemuCaps->hostCPU.kvm;
>      else
> -        qemuCaps->hostCPU.tcg = cpu;
> +        model = &qemuCaps->hostCPU.tcg;
> +
> +    model->full = cpu;
> +    model->migratable = migCPU;

If we had a virQEMUCapsSetHostModel this would be included there too
hiding from this code...


>  
>   cleanup:
>      virCPUDefFree(hostCPU);
> @@ -3320,6 +3368,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>  
>   error:
>      virCPUDefFree(cpu);
> +    virCPUDefFree(migCPU);
>      virResetLastError();
>      goto cleanup;
>  }
> @@ -4067,8 +4116,10 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
>      qemuCaps->kvmCPUModelInfo = NULL;
>      qemuCaps->tcgCPUModelInfo = NULL;
>  
> -    virCPUDefFree(qemuCaps->hostCPU.kvm);
> -    virCPUDefFree(qemuCaps->hostCPU.tcg);
> +    virCPUDefFree(qemuCaps->hostCPU.kvm.full);
> +    virCPUDefFree(qemuCaps->hostCPU.kvm.migratable);
> +    virCPUDefFree(qemuCaps->hostCPU.tcg.full);
> +    virCPUDefFree(qemuCaps->hostCPU.tcg.migratable);

This is why I suggested a single API in patch 4...

John

>      memset(&qemuCaps->hostCPU, 0, sizeof(qemuCaps->hostCPU));
>  }
>  
>