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