[virt-tools-list] [virt-manager PATCH] hostdev: add an address element for USB host devs if necessary

[Giuseppe Scrivano]

Pavel Hrdina <phrdina at redhat.com> writes:

> This issue was fixed for few years but only in virt-manager,
> virt-install has the same bug.  If you have two USB devices with same
> vendor and product ID, you need to use also address element to create
> a valid XML to define that device into a guest.
>
> This patch moves the logic from vmmAddHardware into VirtualHostDevice in
> order to not duplicate that code for virt-manager and virt-install.
>
> Also update the tests files to properly check this functionality.  I've
> changed the USB device according the 'tests/testdriver.xml' and picked
> one of the USB HUBs, because they have the same vendor and product ID.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1230611
>
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  tests/nodedev-xml/devxml/usbdev2.xml |  6 +++---
>  tests/nodedev.py                     | 10 ++++------
>  virtManager/addhardware.py           | 18 +++---------------
>  virtinst/devicehostdev.py            | 19 ++++++++++++++++---
>  4 files changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/tests/nodedev-xml/devxml/usbdev2.xml b/tests/nodedev-xml/devxml/usbdev2.xml
> index 73364b5..e648c20 100644
> --- a/tests/nodedev-xml/devxml/usbdev2.xml
> +++ b/tests/nodedev-xml/devxml/usbdev2.xml
> @@ -1,8 +1,8 @@
>  <hostdev mode="subsystem" type="usb" managed="yes">
>    <source>
> -    <vendor id="0x0781"/>
> -    <product id="0x5151"/>
> -    <address bus="1" device="4"/>
> +    <vendor id="0x1d6b"/>
> +    <product id="0x0002"/>
> +    <address bus="2" device="1"/>
>    </source>
>  </hostdev>
>  
> diff --git a/tests/nodedev.py b/tests/nodedev.py
> index 28e0603..5d9fdec 100644
> --- a/tests/nodedev.py
> +++ b/tests/nodedev.py
> @@ -81,14 +81,13 @@ class TestNodeDev(unittest.TestCase):
>                      "expect=%s\nactual=%s" % (devname, attr, expect, actual))
>              self.assertEqual(vals[attr], getattr(dev, attr))
>  
> -    def _testNode2DeviceCompare(self, nodename, devfile,
> -                                nodedev=None, is_dup=False):
> +    def _testNode2DeviceCompare(self, nodename, devfile, nodedev=None):
>          devfile = os.path.join("tests/nodedev-xml/devxml", devfile)
>          if not nodedev:
>              nodedev = self._nodeDevFromName(nodename)
>  
>          dev = VirtualHostDevice(conn)
> -        dev.set_from_nodedev(nodedev, use_full_usb=is_dup)
> +        dev.set_from_nodedev(nodedev)
>          utils.diff_compare(dev.get_xml_config() + "\n", devfile)
>  
>      def testSystemDevice(self):
> @@ -238,12 +237,11 @@ class TestNodeDev(unittest.TestCase):
>          self._testNode2DeviceCompare(nodename, devfile)
>  
>      def testNodeDev2USB2(self):
> -        nodename = "usb_device_781_5151_2004453082054CA1BEEE"
> +        nodename = "usb_device_1d6b_2_0000_00_1d_7"
>          devfile = "usbdev2.xml"
>          nodedev = self._nodeDevFromName(nodename)
>  
> -        self._testNode2DeviceCompare(nodename, devfile, nodedev=nodedev,
> -                                     is_dup=True)
> +        self._testNode2DeviceCompare(nodename, devfile, nodedev=nodedev)
>  
>      def testNodeDev2PCI(self):
>          nodename = "pci_1180_592"
> diff --git a/virtManager/addhardware.py b/virtManager/addhardware.py
> index 7607529..87c71d5 100644
> --- a/virtManager/addhardware.py
> +++ b/virtManager/addhardware.py
> @@ -246,7 +246,7 @@ class vmmAddHardware(vmmGObjectUI):
>          # Host device list
>          # model = [ Description, nodedev name ]
>          host_dev = self.widget("host-device")
> -        host_dev_model = Gtk.ListStore(str, str, str, object)
> +        host_dev_model = Gtk.ListStore(str, str, object)
>          host_dev.set_model(host_dev_model)
>  
>          host_col = Gtk.TreeViewColumn()
> @@ -1620,24 +1620,12 @@ class vmmAddHardware(vmmGObjectUI):
>  
>      def _validate_page_hostdev(self):
>          row = uiutil.get_list_selected_row(self.widget("host-device"))
> -        is_dup = False
>  
>          if row is None:
>              return self.err.val_err(_("Physical Device Required"),
>                                      _("A device must be selected."))
>  
> -        devtype = row[2]
> -        nodedev = row[3]
> -        if devtype == "usb_device":
> -            vendor = nodedev.vendor_id
> -            product = nodedev.product_id
> -            count = self.conn.get_nodedev_count(devtype, vendor, product)
> -            if not count:
> -                raise RuntimeError(_("Could not find USB device "
> -                                     "(vendorId: %s, productId: %s) "
> -                                     % (vendor, product)))

should be self.vendor and self.product, I will push the fix as trivial.

Regards,
Giuseppe