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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list