[libvirt] [PATCH REBASE 2/2] qemu: Clean up PCI controller options

[Andrea Bolognani]

On Sun, 2018-02-11 at 08:22 -0500, John Ferlan wrote:
> On 02/05/2018 11:08 AM, Andrea Bolognani wrote:
> > Most of the options are only applicable to one or two controller
> > types, so they should be filtered out everywhere else.
> > 
> > This will reduce user confusion and, in at least one corner case,
> > prevent guests from disappearing on daemon restart.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1483816
> 
> So, if I'm reading things correctly - rather than fail because someone
> set an option on a controller (and sometimes for a machine) for which
> it's not supported, the choice is to ignore the value and enforce the
> default.
> 
> This does seem to go against what's been traditionally done (at least my
> recollection of it) for other XML options being wrongly applied to some
> other device.
> 
> Even the bz is a big vague:
> 
> Expected results:
> Schema for the 'target' field in <controller model='pci-bridge'> should
> not accept 'chassis' and 'port' parameters for 'q35' machine type
> 
> But I'd read that as some sort of failure is expected rather than
> acceptance and quietly changing.

Yeah, I'm not sure why I implemented it that way myself. I'll
move everything to a Validate() sub-callback and error out if any
unexpected option is set for a controller.

That will make the test cases added in 1/2 slightly less useful,
though, as it will error out on the first such option instead of
showing that it's resetting all of them.

> > +static void
> > +qemuDomainPCIControllerCleanupOpts(const virDomainDef *def,
> > +                                   virDomainControllerDefPtr cont)
> > +{
> > +    if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI)
> > +        return;
> 
> This is redundant with [1]
> 
> > @@ -4799,6 +4914,8 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
> >  
> >      case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
> 
> [1] only called when type is PCI...

I prefer not embedding in a function knowledge about its callers,
because callers change all the time. In this specific case, I guess
the name is explicit enough that nobody would try calling it on a
USB controller, so we can probably skip the check.

-- 
Andrea Bolognani / Red Hat / Virtualization