Re: [libvirt] [PATCH REBASE 2/2] qemu: Clean up PCI controller options
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