On Tue, Aug 30, 2016 at 10:27:44 -0400, John Ferlan wrote: ...
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8f55fcc..97dc877 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -374,6 +374,7 @@ struct _virQEMUCaps { virArch arch;
virDomainCapsCPUModelsPtr cpuDefinitions; + virCPUDefPtr cpuModel;
hostCpuModel ?
IOW: Some way to make it more clear to a casual reader and perhaps easier to find via cscope
Also I note that this isn't being written out to the cache (e.g. no change to virQEMUCapsFormatCache), so probably need to "note" that some how. Furthermore, perhaps this should be moved after the gic stuff and the note made that anything "after" this point isn't written out, rather it's refetched during Load
Makes sense, all of it. Fixed. ...
+void +virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, + virCapsHostPtr host) +{ + virCPUDefPtr cpu = NULL; + + if (!virQEMUCapsGuestIsNative(host->arch, qemuCaps->arch)) + goto error; + + if (host->cpu && host->cpu->model) { + if (VIR_ALLOC(cpu) < 0) + goto error; + + cpu->sockets = cpu->cores = cpu->threads = 0; + cpu->type = VIR_CPU_TYPE_GUEST; + cpu->mode = VIR_CPU_MODE_CUSTOM; + cpu->match = VIR_CPU_MATCH_EXACT; + + if (virCPUDefCopyModel(cpu, host->cpu, true) < 0) + goto error; + } + + qemuCaps->cpuModel = cpu;
This is leaked - eg. no virCPUDefFree in virQEMUCapsDispose
Oops. Fixed.
Since this is a void function it is possible to have a failure (above) thus having qemuCaps->cpuModel be NULL... Store that knowledge away as it becomes important later in patch 40 as processing will call virQEMUCapsGetHostModel which returns this field.
Not only that. The host->cpu->model is not guaranteed to be set (e.g., it's not set on AArch64) and thus qemuCaps->hostCPUModel can be NULL even if there's no failure.
+ return; + + error: + virCPUDefFree(cpu); + qemuCaps->cpuModel = NULL;
Should we virResetLastError() since we don't care about the failure in the two callers?
I guess so. Fixed. Jirka