[Kimchi-devel] [PATCH] [Kimchi] Improve multifunction attach/detach operations

[Lucio Correia]

Multifunction PCI device attach/detach operations now
need to be performed on all functions simultaneously.
For older libvirt versions, without that support, the
functions must be attached/detached individually. This
patch tries to attach all functions simultaneously and
fallback to individual attach/detach in case it fails.

Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
---
 model/vmhostdevs.py | 64 ++++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 37 deletions(-)

diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py
index 0c89959..e289f03 100644
--- a/model/vmhostdevs.py
+++ b/model/vmhostdevs.py
@@ -232,15 +232,7 @@ class VMHostDevsModel(object):
                 raise
 
             dom = VMModel.get_vm(vmid, self.conn)
-            # Due to libvirt limitation, we don't support live assigne device
-            # to vfio driver.
-            driver = ('vfio' if DOM_STATE_MAP[dom.info()[0]] == "shutoff" and
-                      self.caps.kernel_vfio else 'kvm')
-
-            # on powerkvm systems it must be vfio driver.
-            distro, _, _ = platform.linux_distribution()
-            if distro == 'IBM_PowerKVM':
-                driver = 'vfio'
+            driver = 'vfio' if self.caps.kernel_vfio else 'kvm'
 
             # Attach all PCI devices in the same IOMMU group
             affected_names = self.devs_model.get_list(
@@ -296,34 +288,27 @@ class VMHostDevsModel(object):
                 slot = self._available_slot(dom)
 
             with RollbackContext() as rollback:
-                # multifunction hotplug is a special case where all functions
-                # must be attached together within one xml file, the same does
-                # not happen to multifunction coldplug - where each function
-                # is attached individually
-                if DOM_STATE_MAP[dom.info()[0]] != 'shutoff' and \
-                   is_multifunction:
+                # multifuction: try to attach all functions together within one
+                # xml file. It requires libvirt support.
+                if is_multifunction:
                     xmlstr = self._get_pci_devices_xml(pci_infos, slot, driver)
 
                     try:
                         dom.attachDeviceFlags(xmlstr, device_flags)
 
                     except libvirt.libvirtError:
-                        msg = WokMessage('KCHVMHDEV0007E',
-                                         {'device': pci_info['name'],
-                                          'vm': vmid})
-                        cb(msg.get_text(), False)
-                        wok_log.error(
-                            'Failed to attach mutifunction device VM %s: \n%s',
-                            vmid, xmlstr)
-                        raise
-
-                    rollback.prependDefer(dom.detachDeviceFlags, xmlstr,
-                                          device_flags)
-                    rollback.commitAll()
-                    if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
-                        cb('OK', True)
-                    return
-
+                        # If operation fails, we try the other way, where each
+                        # function is attached individually
+                        pass
+                    else:
+                        rollback.prependDefer(dom.detachDeviceFlags, xmlstr,
+                                              device_flags)
+                        rollback.commitAll()
+                        if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
+                            cb('OK', True)
+                        return
+
+                # attach each function individually (multi or single function)
                 for pci_info in pci_infos:
                     pci_info['detach_driver'] = driver
                     xmlstr = self._get_pci_device_xml(pci_info,
@@ -638,7 +623,14 @@ class VMHostDevModel(object):
                 raise InvalidOperation('KCHVMHDEV0006E',
                                        {'name': dev_info['name']})
 
-            if self._hotunplug_multifunction_pci(dom, hostdev, dev_name):
+            # check for multifunction and detach all functions together
+            try:
+                multi = self._unplug_multifunction_pci(dom, hostdev, dev_name)
+            except libvirt.libvirtError:
+                multi = False
+
+            # successfully detached all functions: finish operation
+            if multi:
                 if is_3D_device:
                     devsmodel = VMHostDevsModel(conn=self.conn)
                     devsmodel.update_mmio_guest(vmid, False)
@@ -647,6 +639,7 @@ class VMHostDevModel(object):
                     cb('OK', True)
                 return
 
+            # detach each function individually
             for e in hostdev:
                 if DeviceModel.deduce_dev_name(e, self.conn) == dev_name:
                     xmlstr = etree.tostring(e)
@@ -685,14 +678,11 @@ class VMHostDevModel(object):
 
         return devices
 
-    def _hotunplug_multifunction_pci(self, dom, hostdev, dev_name):
-        if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
-            return False
-
-        domain, bus, slot, _ = dev_name.split('_')[1:]
+    def _unplug_multifunction_pci(self, dom, hostdev, dev_name):
         # get all devices attached to the guest in the same domain+bus+slot
         # that the one we are going to detach because they must be detached
         # together
+        domain, bus, slot, _ = dev_name.split('_')[1:]
         devices = self._get_devices_same_addr(hostdev,
                                               int(domain, 16),
                                               int(bus, 16),
-- 
2.7.4