[libvirt] [PATCH 30/41] qemu: Introduce virQEMUCapsIsCPUModeSupported

Jiri Denemark jdenemar at redhat.com
Thu Sep 15 11:30:43 UTC 2016


On Tue, Aug 30, 2016 at 11:08:44 -0400, John Ferlan wrote:
> 
> 
> On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> > Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> > ---
> >  src/qemu/qemu_capabilities.c | 55 ++++++++++++++++++++++++++++++++++----------
> >  src/qemu/qemu_capabilities.h |  4 ++++
> >  2 files changed, 47 insertions(+), 12 deletions(-)
> > 
> 
> So another 10 down almost 75% done!  Let's consider ACK's again...
> 
> Still not sure about patch 20 w/r/t to the need for "unknown" printing,
> the sorting by yes, no, and unknown, and whether LoadCache should "read"
> what's been printed and validate against the current (if that matters).
> 
> As for 21-30, if there's no reply then an ACK can be implied.  A couple
> of replies (23, 24) make suggestions for function name changes - your
> choice on those. Two (25, 27) I've just left some thoughts - they don't
> really require changes.
> 
> Patch 22 will need an adjustment for an ACK, but the build would fail
> anyway, so it's obvious.
> 
> Patch 26 needs an adjust to fix a leak for an ACK, your choice on
> renaming cpuModel, and I think the virResetLastError should be called.
> I've seen "future" patches where returning NULL may come into play...
> 
> Patch 28 - your call on the domain page reference
> 
> That just leaves this one with one innocuous adjustment shown below. Not
> required for an ACK

Heh, this summary doesn't really clarify the situation and it's not
entirely obvious which patches are ACKed which patches are ACKed once I
make the suggested changes, and which patches need additional review of
the changes. We'll see if it changes after I process all review
comments.

...
> > +    if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
> > +                                      VIR_CPU_MODE_CUSTOM)) {
> > +        virDomainCapsCPUModelsPtr filtered = NULL;
> > +        char **models = NULL;
> > +
> > +        if (qemuCaps->cpuDefinitions &&
> 
> Redundant check for MODE_CUSTOM when ModeSupported is true

Right. Fixed.

Jirka




More information about the libvir-list mailing list