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