If the user tries to define a domain that has

  <controller type='usb' model='none'/>

and also some USB devices, we report an error:
  error: internal error: No free USB ports

Which is technically still correct for a domain with no USB ports.

Change it to:

USB is disabled for this domain, but USB devices are present in the domain XML

https://bugzilla.redhat.com/show_bug.cgi?id=1347550
---
 src/conf/domain_conf.c         | 2 +-
 src/conf/domain_conf.h         | 2 ++
 src/libvirt_private.syms       | 1 +
 src/qemu/qemu_domain_address.c | 7 +++++++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f3b4dd33d..028fcb5be 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5116,7 +5116,7 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk)
     return 0;
 }
 
-static bool
+bool
 virDomainDefHasUSB(const virDomainDef *def)
 {
     size_t i;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bb3b6f0c3..32e191e0a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2656,6 +2656,8 @@ int virDomainDefPostParse(virDomainDefPtr def,
                           unsigned int parseFlags,
                           virDomainXMLOptionPtr xmlopt,
                           void *parseOpaque);
+bool
+virDomainDefHasUSB(const virDomainDef *def);
 
 int virDomainDefValidate(virDomainDefPtr def,
                          virCapsPtr caps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5b1bc5e4f..21f226258 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -267,6 +267,7 @@ virDomainDefGetVcpusTopology;
 virDomainDefHasDeviceAddress;
 virDomainDefHasMemballoon;
 virDomainDefHasMemoryHotplug;
+virDomainDefHasUSB;
 virDomainDefHasVcpusOffline;
 virDomainDefMaybeAddController;
 virDomainDefMaybeAddInput;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 69c0c8bf2..0d565b3ea 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -2644,6 +2644,13 @@ qemuDomainUSBAddressAddHubs(virDomainDefPtr def)
                                               &data,
                                               false));
 
+    if (data.count && !virDomainDefHasUSB(def)) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("USB is disabled for this domain, but USB devices "
+                         "are present in the domain XML"));
+        return -1;
+    }
+
     if (data.count > available_ports)
         hubs_needed = VIR_DIV_UP(data.count - available_ports + 1,
                                  VIR_DOMAIN_USB_HUB_PORTS - 1);
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, 2017-09-20 at 16:18 +0200, Ján Tomko wrote:
> If the user tries to define a domain that has
> 
>   <controller type='usb' model='none'/>
> 
> and also some USB devices, we report an error:
>   error: internal error: No free USB ports
> 
> Which is technically still correct for a domain with no USB ports.
> 
> Change it to:
> 
> USB is disabled for this domain, but USB devices are present in the domain XML
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1347550

[...]
> @@ -2656,6 +2656,8 @@ int virDomainDefPostParse(virDomainDefPtr def,
>                            unsigned int parseFlags,
>                            virDomainXMLOptionPtr xmlopt,
>                            void *parseOpaque);
> +bool
> +virDomainDefHasUSB(const virDomainDef *def);

Have everything on a single line here.

[...]
> @@ -2644,6 +2644,13 @@ qemuDomainUSBAddressAddHubs(virDomainDefPtr def)
>                                                &data,
>                                                false));
>  
> +    if (data.count && !virDomainDefHasUSB(def)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("USB is disabled for this domain, but USB devices "
> +                         "are present in the domain XML"));
> +        return -1;
> +    }

I would prefer 'data.count > 0' here, but your version works
fine too.


Reviewed-by: Andrea Bolognani <abologna@redhat.com>
List-necromancy-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list