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