[libvirt] [PATCH v2 03/15] qemu: Move skip for implicit PHB of pSeries guests

[Andrea Bolognani]

On Mon, 2018-02-19 at 15:58 +0100, Peter Krempa wrote:
> On Fri, Feb 16, 2018 at 17:28:00 +0100, Andrea Bolognani wrote:
> > Performing the skip earlier will help us making the function
> > nicer later on. We also make the condition for the skip a bit
> > more precise, though that'a more for self-documenting purposes
> > and doesn't change anything in practice.
> > 
> > Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> > ---
> >  src/qemu/qemu_command.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 1ab5b0818..5e4dfcf75 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -2732,6 +2732,14 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
> >              def->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST)
> >              modelName = virDomainControllerPCIModelNameTypeToString(pciopts->modelName);
> >  
> > +        /* Skip the implicit PHB for pSeries guests */
> > +        if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
> > +            pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
> > +            pciopts->targetIndex == 0 &&
> > +            qemuDomainIsPSeries(domainDef)) {
> 
> I'm not sure about the last line. Shouldn't that alredy be validated? At
> least in the case when the PHB model should not be present on
> non-pseries qemus?
> 
> ACK if you agree and drop the last line.

Strictly speaking, both the check on modelName and that on the
machine type are redundant, because we make sure targetIndex is only
set for PHBs and thus only for pSeries guests.

That said, checking again doesn't hurt and makes the reason for
skipping more obvious. It might also help catch bugs introduced in
the validate callback that would result in a controller that was
supposed to be skipped showing up in the output for some test case.

-- 
Andrea Bolognani / Red Hat / Virtualization