devicehostdev: Drop device creation helper

Goes against other device paradigms, and is cleaner when we unwind it.

tests/cli-test-xml/compare/many-devices.xml

@@ -76,6 +76,11 @@
     <channel type="spicevmc">
       <target type="virtio" name="com.redhat.spice.0"/>
     </channel>
+    <hostdev mode="subsystem" type="pci" managed="yes">
+      <source>
+        <address domain="0" bus="0" slot="25" function="0"/>
+      </source>
+    </hostdev>
     <smartcard mode="passthrough" type="spicevmc"/>
     <tpm model="tpm-tis">
       <backend type="passthrough">
@@ -164,6 +169,11 @@
     <channel type="spicevmc">
       <target type="virtio" name="com.redhat.spice.0"/>
     </channel>
+    <hostdev mode="subsystem" type="pci" managed="yes">
+      <source>
+        <address domain="0" bus="0" slot="25" function="0"/>
+      </source>
+    </hostdev>
     <smartcard mode="passthrough" type="spicevmc"/>
     <tpm model="tpm-tis">
       <backend type="passthrough">

tests/clitest.py

@@ -479,7 +479,7 @@ c = vinst.add_category("misc", "--nographics --noautoconsole")
 c.add_compare("", "noargs-fail")  # No arguments
 c.add_compare("--hvm --nodisks --pxe --print-step all", "simple-pxe")  # Diskless PXE install
 c.add_compare("--hvm --cdrom %(EXISTIMG2)s --file %(EXISTIMG1)s --os-variant win2k3 --wait 0 --vcpus cores=4", "w2k3-cdrom")  # HVM windows install with disk
-c.add_compare("""--hvm --pxe --controller usb,model=ich9-ehci1,address=0:0:4.7,index=0 --controller usb,model=ich9-uhci1,address=0:0:4.0,index=0,master=0 --controller usb,model=ich9-uhci2,address=0:0:4.1,index=0,master=2 --controller usb,model=ich9-uhci3,address=0:0:4.2,index=0,master=4 --disk %(MANAGEDEXISTUPPER)s,cache=writeback,io=threads,perms=sh,serial=WD-WMAP9A966149 --disk %(NEWIMG1)s,sparse=false,size=.001,perms=ro,error_policy=enospace --disk device=cdrom,bus=sata --serial tcp,host=:2222,mode=bind,protocol=telnet --filesystem /source,/target,mode=squash --network user,mac=12:34:56:78:11:22 --network bridge=foobar,model=virtio --channel spicevmc --smartcard passthrough,type=spicevmc --tpm passthrough,model=tpm-tis,path=/dev/tpm0 --security type=static,label='system_u:object_r:svirt_image_t:s0:c100,c200',relabel=yes  --numatune \\"1-3,5\\",mode=preferred --boot loader=/foo/bar """, "many-devices")  # Lots of devices
+c.add_compare("""--hvm --pxe --controller usb,model=ich9-ehci1,address=0:0:4.7,index=0 --controller usb,model=ich9-uhci1,address=0:0:4.0,index=0,master=0 --controller usb,model=ich9-uhci2,address=0:0:4.1,index=0,master=2 --controller usb,model=ich9-uhci3,address=0:0:4.2,index=0,master=4 --disk %(MANAGEDEXISTUPPER)s,cache=writeback,io=threads,perms=sh,serial=WD-WMAP9A966149 --disk %(NEWIMG1)s,sparse=false,size=.001,perms=ro,error_policy=enospace --disk device=cdrom,bus=sata --serial tcp,host=:2222,mode=bind,protocol=telnet --filesystem /source,/target,mode=squash --network user,mac=12:34:56:78:11:22 --network bridge=foobar,model=virtio --channel spicevmc --smartcard passthrough,type=spicevmc --tpm passthrough,model=tpm-tis,path=/dev/tpm0 --security type=static,label='system_u:object_r:svirt_image_t:s0:c100,c200',relabel=yes  --numatune \\"1-3,5\\",mode=preferred --boot loader=/foo/bar --host-device net_00_1c_25_10_b1_e4""", "many-devices")  # Lots of devices
 c.add_valid("--hvm --disk path=virt-install,device=cdrom")  # Specifying cdrom media via --disk
 c.add_valid("--hvm --import --disk path=virt-install")  # FV Import install
 c.add_valid("--hvm --import --disk path=virt-install --prompt --force")  # Working scenario w/ prompt shouldn't ask anything

tests/nodedev.py

@@ -64,8 +64,8 @@ class TestNodeDev(unittest.TestCase):
         if not nodedev:
             nodedev = self._nodeDevFromName(nodename)
 
-        dev = VirtualHostDevice.device_from_node(conn, nodedev=nodedev,
+        dev = VirtualHostDevice(conn)
-                                                 is_dup=is_dup)
+        dev.set_from_nodedev(nodedev, use_full_usb=is_dup)
         utils.diff_compare(dev.get_xml_config() + "\n", devfile)
 
     def testSystemDevice(self):

virtManager/addhardware.py

@@ -1485,16 +1485,13 @@ class vmmAddHardware(vmmGObjectUI):
                 raise RuntimeError(_("Could not find USB device "
                                      "(vendorId: %s, productId: %s) "
                                      % (vendor, product)))
-
             if count > 1:
                 is_dup = True
 
         try:
-            self._dev = virtinst.VirtualHostDevice.device_from_node(
+            dev = virtinst.VirtualHostDevice(self.conn.get_backend())
-                            conn=self.conn.get_backend(),
+            dev.set_from_nodedev(nodedev, use_full_usb=is_dup)
-                            name=nodedev_name,
+            self._dev = dev
-                            nodedev=nodedev,
-                            is_dup=is_dup)
         except Exception, e:
             return self.err.val_err(_("Host device parameter error"), e)
 

virtManager/details.py

@@ -278,8 +278,7 @@ def lookup_nodedev(vmmconn, hostdev):
     devtype = hostdev.type
     found_dev = None
 
-    vendor_id = product_id = bus = device = \
+    vendor_id = product_id = bus = device = domain = slot = func = None
-        domain = slot = func = None
 
     # For USB we want a device, not a bus
     if devtype == 'usb':

virtinst/cli.py

@@ -1919,8 +1919,11 @@ def parse_sound(guest, optstr, dev=None):
 #####################
 
 def parse_hostdev(guest, optstr, dev=None):
-    return virtinst.VirtualHostDevice.device_from_node(guest.conn,
+    if not dev:
-                                                       name=optstr,
+        dev = virtinst.VirtualHostDevice(guest.conn)
-                                                       dev=dev)
+
+    nodedev = virtinst.NodeDevice.lookupNodeName(guest.conn, optstr)
+    dev.set_from_nodedev(nodedev)
+    return dev
 
 get_hostdevs = _make_handler("hostdev", parse_hostdev)

virtinst/devicehostdev.py

@@ -25,42 +25,17 @@ from virtinst.xmlbuilder import XMLProperty
 class VirtualHostDevice(VirtualDevice):
     virtual_device_type = VirtualDevice.VIRTUAL_DEV_HOSTDEV
 
-    @staticmethod
+    def set_from_nodedev(self, nodedev, use_full_usb=None):
-    def device_from_node(conn, name=None, nodedev=None, is_dup=False,
-                         dev=None):
         """
-        Convert the passed device name to a VirtualHostDevice
+        @use_full_usb: If set, and nodedev is USB, specify both
-        instance, with proper error reporting. Name can be any of the
+            vendor and product. Used if user requests bus/add on virt-install
-        values accepted by NodeDevice.lookupNodeName. If a node
+            command line, or if virt-manager detects a dup USB device
-        device name is not specified, a virtinst.NodeDevice instance can
+            and we need to differentiate
-        be passed in to create a dev from.
-
-        @param conn: libvirt.virConnect instance to perform the lookup on
-        @param name: optional libvirt node device name to lookup
-        @param nodedev: optional L{virtinst.NodeDevice} instance to use
-
-        @rtype: L{virtinst.VirtualHostDevice} instance
         """
-        if not name and not nodedev:
+        if (use_full_usb is None and
-            raise ValueError("'name' or 'nodedev' required.")
+            nodedev.addr_type == nodedev.HOSTDEV_ADDR_TYPE_USB_BUSADDR):
+            use_full_usb = True
 
-        if nodedev:
-            nodeinst = nodedev
-        else:
-            nodeinst, addr_type = NodeDevice.lookupNodeName(conn, name)
-            if addr_type == NodeDevice.HOSTDEV_ADDR_TYPE_USB_BUSADDR:
-                is_dup = True
-
-        if nodeinst.device_type == nodeinst.CAPABILITY_TYPE_NET:
-            parentname = nodeinst.parent
-            return VirtualHostDevice.device_from_node(conn,
-                                                      name=parentname)
-        if not dev:
-            dev = VirtualHostDevice(conn)
-        dev.set_from_nodedev(nodeinst, is_dup)
-        return dev
-
-    def set_from_nodedev(self, nodedev, is_dup=False):
         if nodedev.device_type == NodeDevice.CAPABILITY_TYPE_PCI:
             self.type = "pci"
             self.domain = nodedev.domain
@@ -73,9 +48,14 @@ class VirtualHostDevice(VirtualDevice):
             self.vendor = nodedev.vendor_id
             self.product = nodedev.product_id
 
-            if is_dup:
+            if use_full_usb:
                 self.bus = nodedev.bus
                 self.device = nodedev.device
+
+        elif nodedev.device_type == nodedev.CAPABILITY_TYPE_NET:
+            parentnode = nodedev.lookupNodeName(self.conn, nodedev.parent)
+            self.set_from_nodedev(parentnode, use_full_usb=use_full_usb)
+
         else:
             raise ValueError("Unknown node device type %s" % nodedev)
 

virtinst/nodedev.py

@@ -36,7 +36,13 @@ class XMLProperty(OrigXMLProperty):
 
 
 def _lookupNodeName(conn, name):
-    nodedev = conn.nodeDeviceLookupByName(name)
+    try:
+        nodedev = conn.nodeDeviceLookupByName(name)
+    except libvirt.libvirtError, e:
+        raise libvirt.libvirtError(
+            _("Did not find node device '%s': %s" %
+            (name, str(e))))
+
     xml = nodedev.XMLDesc(0)
     return NodeDevice.parse(conn, xml)
 
@@ -66,7 +72,7 @@ class NodeDevice(XMLBuilder):
 
         @param conn: libvirt.virConnect instance to perform the lookup on
         @param name: libvirt node device name to lookup, or address for
-                     devAddressToNodedev
+                     _devAddressToNodedev
 
         @rtype: L{NodeDevice} instance
         """
@@ -75,14 +81,13 @@ class NodeDevice(XMLBuilder):
                                "enumeration."))
 
         try:
-            return (_lookupNodeName(conn, name),
+            return _lookupNodeName(conn, name)
-                     NodeDevice.HOSTDEV_ADDR_TYPE_LIBVIRT)
         except libvirt.libvirtError, e:
             ret = _isAddressStr(name)
             if not ret:
                 raise e
 
-            return devAddressToNodedev(conn, name)
+            return _devAddressToNodedev(conn, name)
 
     @staticmethod
     def parse(conn, xml):
@@ -94,6 +99,9 @@ class NodeDevice(XMLBuilder):
         instantiate = kwargs.pop("allow_node_instantiate", False)
         if self.__class__ is NodeDevice and not instantiate:
             raise RuntimeError("Can not instantiate NodeDevice directly")
+
+        self.addr_type = None
+
         XMLBuilder.__init__(self, *args, **kwargs)
 
     _XML_ROOT_NAME = "device"
@@ -316,6 +324,8 @@ def _isAddressStr(addrstr):
                         (int(nodedev.device) == addr))
             cmp_func = usbaddr_cmp
             addr_type = NodeDevice.HOSTDEV_ADDR_TYPE_USB_BUSADDR
+        else:
+            return None
     except:
         logging.exception("Error parsing node device string.")
         return None
@@ -323,7 +333,7 @@ def _isAddressStr(addrstr):
     return cmp_func, devtype, addr_type
 
 
-def devAddressToNodedev(conn, addrstr):
+def _devAddressToNodedev(conn, addrstr):
     """
     Look up the passed host device address string as a libvirt node device,
     parse its xml, and return a NodeDevice instance.
@@ -335,10 +345,6 @@ def devAddressToNodedev(conn, addrstr):
         - (domain:)bus:slot.func (ex. 00:10.0 for a pci device)
     @param addrstr: C{str}
     """
-    if not conn.check_conn_support(conn.SUPPORT_CONN_NODEDEV):
-        raise ValueError(_("Connection does not support host device "
-                           "enumeration."))
-
     ret = _isAddressStr(addrstr)
     if not ret:
         raise ValueError(_("Could not determine format of '%s'") % addrstr)
@@ -357,7 +363,8 @@ def devAddressToNodedev(conn, addrstr):
             count += 1
 
     if count == 1:
-        return nodedev, addr_type
+        nodedev.addr_type = addr_type
+        return nodedev
     elif count > 1:
         raise ValueError(_("%s corresponds to multiple node devices") %
                          addrstr)