[libvirt] [PATCH v2 1/7] domain_conf: Validate redirdev after parsing

[Michal Privoznik]

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 at redhat.com>
>> ---
>> 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