Ensure sensible default USB controller model used (#1362)

When users adding devices either via the pass through or USB redirector
approach, ensure a default controller is added using the xhci stack.
This should work for all versions of USB, where as the previous default
of piix3-uhci appears to only work for some USB 1 controllers.

Fixes: #1346

README.md

@@ -1111,13 +1111,14 @@ The USB controller can be configured using `libvirt.usb_controller`, with the fo
 Vagrant.configure("2") do |config|
   config.vm.provider :libvirt do |libvirt|
     # Set up a USB3 controller
-    libvirt.usb_controller :model => "nec-xhci"
+    libvirt.usb_controller :model => "qemu-xhci"
   end
 end
 ```
 
 See the [libvirt documentation](https://libvirt.org/formatdomain.html#elementsControllers) for a list of valid models.
 
+If any USB devices are passed through by setting `libvirt.usb` or `libvirt.redirdev`, a default controller will be added using the model `qemu-xhci` in the absence of a user specified one. This should help ensure more devices work out of the box as the default configured by libvirt is pii3-uhci, which appears to only work for USB 1 devices and does not work as expected when connected via a USB 2 controller, while the xhci stack should work for all versions of USB.
 
 ### USB Device Passthrough
 
@@ -1138,6 +1139,17 @@ The example values above match the device from the following output of `lsusb`:
 Bus 001 Device 002: ID 1234:abcd Example device
 ```
 
+```ruby
+Vagrant.configure("2") do |config|
+  config.vm.provider :libvirt do |libvirt|
+    # pass through specific device based on identifying it
+    libvirt.usbdev :vendor => '0x1234', :product => '0xabcd'
+    # pass through a host device where multiple of the same vendor/product exist
+    libvirt.usbdev :bus => '1', :device => '1'
+  end
+end
+```
+
 Additionally, the following options can be used:
 
 * `startupPolicy` - Is passed through to Libvirt and controls if the device has
@@ -1194,7 +1206,7 @@ In this case, the USB device with `class 0x0b`, `vendor 0x08e6`, `product 0x3437
 Vagrant.configure("2") do |config|
   config.vm.provider :libvirt do |libvirt|
     libvirt.redirdev :type => "spicevmc"
-    libvirt.redirfilter :class => "0x0b" :vendor => "0x08e6" :product => "0x3437" :version => "2.00" :allow => "yes"
+    libvirt.redirfilter :class => "0x0b", :vendor => "0x08e6", :product => "0x3437", :version => "2.00", :allow => "yes"
     libvirt.redirfilter :allow => "no"
   end
 end

lib/vagrant-libvirt/config.rb

@@ -563,7 +563,7 @@ module VagrantPlugins
         end
 
         @usbctl_dev[:model] = options[:model]
-        @usbctl_dev[:ports] = options[:ports]
+        @usbctl_dev[:ports] = options[:ports] if options[:ports]
       end
 
       def usb(options = {})
@@ -915,9 +915,6 @@ module VagrantPlugins
         # Watchdog device
         @watchdog_dev = {} if @watchdog_dev == UNSET_VALUE
 
-        # USB controller
-        @usbctl_dev = {} if @usbctl_dev == UNSET_VALUE
-
         # USB device passthrough
         @usbs = [] if @usbs == UNSET_VALUE
 
@@ -925,6 +922,11 @@ module VagrantPlugins
         @redirdevs = [] if @redirdevs == UNSET_VALUE
         @redirfilters = [] if @redirfilters == UNSET_VALUE
 
+        # USB controller
+        if @usbctl_dev == UNSET_VALUE
+          @usbctl_dev = if !@usbs.empty? or !@redirdevs.empty? then {:model => 'qemu-xhci'} else {} end
+        end
+
         # smartcard device
         @smartcard_dev = {} if @smartcard_dev == UNSET_VALUE
 

spec/unit/config_spec.rb

@@ -435,6 +435,54 @@ describe VagrantPlugins::ProviderLibvirt::Config do
         end
       end
     end
+
+    context '@usbctl_dev' do
+      it 'should be empty by default' do
+        subject.finalize!
+
+        expect(subject.usbctl_dev).to eq({})
+      end
+
+      context 'when usb devices added' do
+        it 'should inject a default controller' do
+          subject.usb :vendor => '0x1234', :product => '0xabcd'
+
+          subject.finalize!
+
+          expect(subject.usbctl_dev).to eq({:model => 'qemu-xhci'})
+        end
+
+        context 'when user specified a controller' do
+          it 'should retain the user setting' do
+            subject.usb :vendor => '0x1234', :product => '0xabcd'
+            subject.usb_controller :model => 'pii3-uchi'
+
+            subject.finalize!
+            expect(subject.usbctl_dev).to eq({:model => 'pii3-uchi'})
+          end
+        end
+      end
+
+      context 'when redirdevs entries added' do
+        it 'should inject a default controller' do
+          subject.redirdev :type => 'spicevmc'
+
+          subject.finalize!
+
+          expect(subject.usbctl_dev).to eq({:model => 'qemu-xhci'})
+        end
+
+        context 'when user specified a controller' do
+          it 'should retain the user setting' do
+            subject.redirdev :type => 'spicevmc'
+            subject.usb_controller :model => 'pii3-uchi'
+
+            subject.finalize!
+            expect(subject.usbctl_dev).to eq({:model => 'pii3-uchi'})
+          end
+        end
+      end
+    end
   end
 
   def assert_invalid