On 03/31/2017 01:54 PM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> 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));
> }
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
