[libvirt] [PATCH v2 05/33] qemu: Refactor virQEMUCapsInitHostCPUModel
Jiri Denemark
jdenemar at redhat.com
Wed Feb 22 12:46:52 UTC 2017
On Tue, Feb 21, 2017 at 09:24:13 -0500, John Ferlan wrote:
>
>
> On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >
> > Notes:
> > Version 2:
> > - no change
> >
> > src/qemu/qemu_capabilities.c | 109 ++++++++++++++++++++++---------------------
> > 1 file changed, 55 insertions(+), 54 deletions(-)
> >
>
> This is "visually" more than a refactor since you've specified an
> initialization of cpu->fallback... That initialization gets essentially
> the same value of 0 (ALLOW == 0) as would any VIR_ALLOC field, so it's
> not a problem per se.
It's just to make the value more visible. We use VIR_CPU_FALLBACK_ALLOW
by default and change it to VIR_CPU_FALLBACK_FORBID when we get the CPU
model from QEMU (rather than by querying the host CPU ourselves).
> Still makes me wonder if there should have been an "UNDEFINED"
> category...
No. Originally there was no fallback attribute and the functionality was
equivalent to fallback="allow". That is the attribute was added just to
be able to turn fallback off.
> My only other comment here is whether there is a concern that your error
> path doesn't clear the qemuCaps->hostCPUModel. It wasn't clear to me
> whether this path can be called after libvirtd restart and if failure
> would mean anything or not (perhaps the one reason I could think of
> setting NULL previously).
>
> ACK in principal - might be nice to mention why clearing hostCPUModel on
> failure isn't required anymore.
It didn't make sense even in the original code (if it did, setting it to
NULL would be a clear memory leak). The initial value of
qemuCaps->hostCPUModel is NULL and the code doesn't touch it until we
know everything is OK.
Jirka
More information about the libvir-list
mailing list