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