On 20.06.2016 10:32, Ján Tomko wrote: > On Fri, Jun 10, 2016 at 05:32:55PM +0200, Michal Privoznik wrote: >> There's currently just one limitation: redirdevs that want to go >> on USB bus require a USB controller, surprisingly. >> At the same time, since I'm using virDomainDefHasUSB() in this >> new validator function, it has to be moved a few lines up and >> also its header needed to be changed a bit: it is now taking a >> const pointer to domain def since it's not changing anything in >> there. >> >> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >> --- >> src/conf/domain_conf.c | 56 >> ++++++++++++++++++++++++++++---------------------- >> 1 file changed, 32 insertions(+), 24 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 85f6e31..c75279d 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -4567,14 +4567,45 @@ virDomainDiskDefValidate(const >> virDomainDiskDef *disk) >> return 0; >> } >> >> +static bool >> +virDomainDefHasUSB(const virDomainDef *def) >> +{ >> + size_t i; >> + >> + for (i = 0; i < def->ncontrollers; i++) { >> + if (def->controllers[i]->type == >> VIR_DOMAIN_CONTROLLER_TYPE_USB && >> + def->controllers[i]->model != >> VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) >> + return true; >> + } > > This function returns false either if there is a model='none' USB > controller or if there is none at all, But there's an implicit controlled being added after domain XML is parsed (if none was provided in the XML), unless this behaviour has been suppressed by model='none'. > >> + >> + return false; >> +} >> + > >> @@ -17060,14 +17090,6 @@ virDomainDefParseXML(xmlDocPtr xml, >> if (!redirdev) >> goto error; >> >> - if (redirdev->bus == VIR_DOMAIN_REDIRDEV_BUS_USB && usb_none) { >> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> - _("Can't add redirected USB device: " >> - "USB is disabled for this domain")); >> - virDomainRedirdevDefFree(redirdev); > > but this error was only reported for model="none'. > > This broke virt-manager's test: > https://ci.centos.org/job/virt-manager-test/systems=libvirt-fedora-rawhide/1363/console I don't see the problem. Virt-manager relied on a bug and I've fixed it. I mean, what is happening here is: virt-manager tries to open a test connection whilst providing its own test driver state XML. This big XML has domain XMLs too. These are missing usb controllers and since we are not adding implicit ones in post parse callback (there are none defined for the test driver), domains really are missing USB controllers and thus USB bus. It's just our buggy behaviour that caused virt-manager to work by chance. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list