[Kimchi-devel] [PATCH v2] [Kimchi] Handle libvirt events for device attachment/detachment

Jose Ricardo Ziviani joserz at linux.vnet.ibm.com
Thu Jul 21 16:34:57 UTC 2016


 - kimchi will only finish the device attachment/detachment after
   receiving the appropriate event from libvirt.

Signed-off-by: Jose Ricardo Ziviani <joserz at linux.vnet.ibm.com>
---
v2:
  - removed functions to deregister events (unused functions)
  - improved the event handler
  - add a lock for tasks

Depends on [Kimchi] Issue #956: Unable to assign pci passthrough devices through kimchi

 model/libvirtevents.py |  28 ++++
 model/vmhostdevs.py    | 382 ++++++++++++++++++++++++++++---------------------
 2 files changed, 248 insertions(+), 162 deletions(-)

diff --git a/model/libvirtevents.py b/model/libvirtevents.py
index dfd22c6..2dd9904 100644
--- a/model/libvirtevents.py
+++ b/model/libvirtevents.py
@@ -89,3 +89,31 @@ class LibvirtEvents(object):
             else:
                 reason = e.message
             wok_log.error("Register of ENOSPC event failed: %s" % reason)
+
+    def registerAttachDevicesEvent(self, conn, cb, arg):
+        """
+        register libvirt event to listen to devices attachment
+        """
+        try:
+            return conn.get().domainEventRegisterAny(
+                None,
+                libvirt.VIR_DOMAIN_EVENT_ID_DEVICE_ADDED,
+                cb,
+                arg)
+
+        except libvirt.libvirtError as e:
+            wok_log.error("register attach event failed: %s" % e.message)
+
+    def registerDetachDevicesEvent(self, conn, cb, arg):
+        """
+        register libvirt event to listen to devices detachment
+        """
+        try:
+            return conn.get().domainEventRegisterAny(
+                None,
+                libvirt.VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED,
+                cb,
+                arg)
+
+        except libvirt.libvirtError as e:
+            wok_log.error("register detach event failed: %s" % e.message)
diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py
index b57cbf0..5a63bb1 100644
--- a/model/vmhostdevs.py
+++ b/model/vmhostdevs.py
@@ -21,6 +21,7 @@ import glob
 import libvirt
 import os
 import platform
+import threading
 from lxml import etree, objectify
 from lxml.builder import E, ElementMaker
 from operator import itemgetter
@@ -47,8 +48,14 @@ class VMHostDevsModel(object):
     def __init__(self, **kargs):
         self.conn = kargs['conn']
         self.objstore = kargs['objstore']
+        self.events = kargs['eventsloop']
         self.caps = CapabilitiesModel(**kargs)
         self.task = TaskModel(**kargs)
+        self._cb = None
+        self.events.registerAttachDevicesEvent(
+            self.conn,
+            self._event_devices,
+            self)
 
     def get_list(self, vmid):
         dom = VMModel.get_vm(vmid, self.conn)
@@ -67,6 +74,17 @@ class VMHostDevsModel(object):
         if dev_name not in eligible_dev_names:
             raise InvalidParameter('KCHVMHDEV0002E', {'dev_name': dev_name})
 
+    def _event_devices(self, conn, dom, alias, opaque):
+        """
+        Callback to handle add/remove devices event
+        """
+        if opaque._cb is None:
+            wok_log.error('opaque must be valid')
+            return
+
+        wok_log.info("Device %s added successfuly" % alias)
+        opaque._cb('OK', True)
+
     def create(self, vmid, params):
         dev_name = params['name']
         self._passthrough_device_validate(dev_name)
@@ -76,7 +94,8 @@ class VMHostDevsModel(object):
             taskid = add_task(u'/plugins/kimchi/vms/%s/hostdevs/' %
                               VMModel.get_vm(vmid, self.conn).name(),
                               self._attach_pci_device, self.objstore,
-                              {'vmid': vmid, 'dev_info': dev_info})
+                              {'vmid': vmid, 'dev_info': dev_info,
+                               'lock': threading.RLock()})
             return self.task.lookup(taskid)
 
         with RollbackContext() as rollback:
@@ -93,7 +112,8 @@ class VMHostDevsModel(object):
         taskid = add_task(u'/plugins/kimchi/vms/%s/hostdevs/' %
                           VMModel.get_vm(vmid, self.conn).name(),
                           '_attach_%s_device' % dev_info['device_type'],
-                          self.objstore, {'vmid': vmid, 'dev_info': dev_info})
+                          self.objstore, {'vmid': vmid, 'dev_info': dev_info,
+                                          'lock': threading.RLock()})
 
         return self.task.lookup(taskid)
 
@@ -189,112 +209,118 @@ class VMHostDevsModel(object):
 
     def _attach_pci_device(self, cb, params):
         cb('Attaching PCI device')
+        self._cb = cb
         vmid = params['vmid']
         dev_info = params['dev_info']
-        self._validate_pci_passthrough_env()
-
-        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'
+        lock = params['lock']
+
+        with lock:
+            self._validate_pci_passthrough_env()
+
+            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'
+
+            # Attach all PCI devices in the same IOMMU group
+            dev_model = DeviceModel(conn=self.conn)
+            devs_model = DevicesModel(conn=self.conn)
+            affected_names = devs_model.get_list(
+                _passthrough_affected_by=dev_info['name'])
+            passthrough_names = devs_model.get_list(
+                _cap='pci', _passthrough='true')
+            group_names = list(set(affected_names) & set(passthrough_names))
+            pci_infos = [dev_model.lookup(dev_name) for dev_name in
+                         group_names]
+            pci_infos.append(dev_info)
+
+            is_multifunction = len(pci_infos) > 1
+            pci_infos = sorted(pci_infos, key=itemgetter('name'))
+
+            # does not allow hot-plug of 3D graphic cards
+            is_3D_device = dev_model.is_device_3D_controller(dev_info)
+            if is_3D_device and DOM_STATE_MAP[dom.info()[0]] != "shutoff":
+                raise InvalidOperation('KCHVMHDEV0006E',
+                                       {'name': dev_info['name']})
+
+            # all devices in the group that is going to be attached to the vm
+            # must be detached from the host first
+            with RollbackContext() as rollback:
+                for pci_info in pci_infos:
+                    try:
+                        dev = self.conn.get().nodeDeviceLookupByName(
+                            pci_info['name'])
+                        dev.dettach()
+                    except Exception:
+                        raise OperationFailed('KCHVMHDEV0005E',
+                                              {'name': pci_info['name']})
+                    else:
+                        rollback.prependDefer(dev.reAttach)
 
-        # Attach all PCI devices in the same IOMMU group
-        dev_model = DeviceModel(conn=self.conn)
-        devs_model = DevicesModel(conn=self.conn)
-        affected_names = devs_model.get_list(
-            _passthrough_affected_by=dev_info['name'])
-        passthrough_names = devs_model.get_list(
-            _cap='pci', _passthrough='true')
-        group_names = list(set(affected_names) & set(passthrough_names))
-        pci_infos = [dev_model.lookup(dev_name) for dev_name in group_names]
-        pci_infos.append(dev_info)
-
-        is_multifunction = len(pci_infos) > 1
-        pci_infos = sorted(pci_infos, key=itemgetter('name'))
-
-        # does not allow hot-plug of 3D graphic cards
-        is_3D_device = dev_model.is_device_3D_controller(dev_info)
-        if is_3D_device and DOM_STATE_MAP[dom.info()[0]] != "shutoff":
-            raise InvalidOperation('KCHVMHDEV0006E',
-                                   {'name': dev_info['name']})
-
-        # all devices in the group that is going to be attached to the vm
-        # must be detached from the host first
-        with RollbackContext() as rollback:
-            for pci_info in pci_infos:
-                try:
-                    dev = self.conn.get().nodeDeviceLookupByName(
-                        pci_info['name'])
-                    dev.dettach()
-                except Exception:
-                    raise OperationFailed('KCHVMHDEV0005E',
-                                          {'name': pci_info['name']})
-                else:
-                    rollback.prependDefer(dev.reAttach)
-
-            rollback.commitAll()
-
-        device_flags = get_vm_config_flag(dom, mode='all')
-
-        # when attaching a 3D graphic device it might be necessary to increase
-        # the window size memory in order to be able to attach more than one
-        # device to the same guest
-        if is_3D_device:
-            self.update_mmio_guest(vmid, True)
-
-        slot = 0
-        if is_multifunction:
-            # search for the first available slot in guest xml
-            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:
-                xmlstr = self._get_pci_devices_xml(pci_infos, slot, driver)
-
-                try:
-                    dom.attachDeviceFlags(xmlstr, device_flags)
+                rollback.commitAll()
 
-                except libvirt.libvirtError:
-                    wok_log.error(
-                        'Failed to attach mutifunction device VM %s: \n%s',
-                        vmid, xmlstr)
-                    raise
+            device_flags = get_vm_config_flag(dom, mode='all')
 
-                rollback.prependDefer(dom.detachDeviceFlags, xmlstr,
-                                      device_flags)
+            # when attaching a 3D graphic device it might be necessary to
+            # increase the window size memory in order to be able to attach
+            # more than one device to the same guest
+            if is_3D_device:
+                self.update_mmio_guest(vmid, True)
+
+            slot = 0
+            if is_multifunction:
+                # search for the first available slot in guest xml
+                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:
+                    xmlstr = self._get_pci_devices_xml(pci_infos, slot, driver)
+
+                    try:
+                        dom.attachDeviceFlags(xmlstr, device_flags)
+
+                    except libvirt.libvirtError:
+                        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
+
+                for pci_info in pci_infos:
+                    pci_info['detach_driver'] = driver
+                    xmlstr = self._get_pci_device_xml(pci_info,
+                                                      slot,
+                                                      is_multifunction)
+                    try:
+                        dom.attachDeviceFlags(xmlstr, device_flags)
+                    except libvirt.libvirtError:
+                        wok_log.error(
+                            'Failed to attach host device %s to VM %s: \n%s',
+                            pci_info['name'], vmid, xmlstr)
+                        raise
+                    rollback.prependDefer(dom.detachDeviceFlags,
+                                          xmlstr, device_flags)
                 rollback.commitAll()
-                cb('OK', True)
-                return
 
-            for pci_info in pci_infos:
-                pci_info['detach_driver'] = driver
-                cb('Reading source device XML')
-                xmlstr = self._get_pci_device_xml(pci_info,
-                                                  slot,
-                                                  is_multifunction)
-                try:
-                    cb('Attaching device to VM')
-                    dom.attachDeviceFlags(xmlstr, device_flags)
-                except libvirt.libvirtError:
-                    wok_log.error(
-                        'Failed to attach host device %s to VM %s: \n%s',
-                        pci_info['name'], vmid, xmlstr)
-                    raise
-                rollback.prependDefer(dom.detachDeviceFlags,
-                                      xmlstr, device_flags)
-            rollback.commitAll()
-
-        cb('OK', True)
+        if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
+            cb('OK', True)
 
     def _count_3D_devices_attached(self, dom):
         counter = 0
@@ -403,25 +429,31 @@ class VMHostDevsModel(object):
 
     def _attach_scsi_device(self, cb, params):
         cb('Attaching SCSI device...')
+        self._cb = cb
         vmid = params['vmid']
         dev_info = params['dev_info']
-        dom = VMModel.get_vm(vmid, self.conn)
+        lock = params['lock']
 
-        with RollbackContext() as rollback:
-            cb('Reading source device XML')
-            xmlstr = self._get_scsi_device_xml(dev_info)
-            device_flags = get_vm_config_flag(dom, mode='all')
-            try:
-                cb('Attaching device to VM')
-                dom.attachDeviceFlags(xmlstr, device_flags)
-            except libvirt.libvirtError:
-                wok_log.error('Failed to attach host device %s to VM %s: \n%s',
-                              dev_info['name'], vmid, xmlstr)
-                raise
-            rollback.prependDefer(dom.detachDeviceFlags, xmlstr, device_flags)
-            rollback.commitAll()
+        with lock:
+            dom = VMModel.get_vm(vmid, self.conn)
+
+            with RollbackContext() as rollback:
+                xmlstr = self._get_scsi_device_xml(dev_info)
+                device_flags = get_vm_config_flag(dom, mode='all')
+                try:
+                    cb('Attaching device to VM')
+                    dom.attachDeviceFlags(xmlstr, device_flags)
+                except libvirt.libvirtError:
+                    wok_log.error(
+                        'Failed to attach host device %s to VM %s: \n%s',
+                        dev_info['name'], vmid, xmlstr)
+                    raise
+                rollback.prependDefer(dom.detachDeviceFlags, xmlstr,
+                                      device_flags)
+                rollback.commitAll()
 
-        cb('OK', True)
+        if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
+            cb('OK', True)
 
     def _get_usb_device_xml(self, dev_info):
         source = E.source(
@@ -436,32 +468,43 @@ class VMHostDevsModel(object):
 
     def _attach_usb_device(self, cb, params):
         cb('Attaching USB device...')
+        self._cb = cb
         vmid = params['vmid']
         dev_info = params['dev_info']
         dom = VMModel.get_vm(vmid, self.conn)
+        lock = params['lock']
 
-        with RollbackContext() as rollback:
-            cb('Reading source device XML')
-            xmlstr = self._get_usb_device_xml(dev_info)
-            device_flags = get_vm_config_flag(dom, mode='all')
-            try:
-                cb('Attaching device to VM')
-                dom.attachDeviceFlags(xmlstr, device_flags)
-            except libvirt.libvirtError:
-                wok_log.error('Failed to attach host device %s to VM %s: \n%s',
-                              dev_info['name'], vmid, xmlstr)
-                raise
-            rollback.prependDefer(dom.detachDeviceFlags, xmlstr, device_flags)
-            rollback.commitAll()
+        with lock:
+            with RollbackContext() as rollback:
+                xmlstr = self._get_usb_device_xml(dev_info)
+                device_flags = get_vm_config_flag(dom, mode='all')
+                try:
+                    cb('Attaching device to VM')
+                    dom.attachDeviceFlags(xmlstr, device_flags)
+                except libvirt.libvirtError:
+                    wok_log.error(
+                        'Failed to attach host device %s to VM %s: \n%s',
+                        dev_info['name'], vmid, xmlstr)
+                    raise
+                rollback.prependDefer(dom.detachDeviceFlags, xmlstr,
+                                      device_flags)
+                rollback.commitAll()
 
-        cb('OK', True)
+        if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
+            cb('OK', True)
 
 
 class VMHostDevModel(object):
     def __init__(self, **kargs):
         self.conn = kargs['conn']
         self.objstore = kargs['objstore']
+        self.events = kargs['eventsloop']
         self.task = TaskModel(**kargs)
+        self._cb = None
+        self.events.registerDetachDevicesEvent(
+            self.conn,
+            self._event_devices,
+            self)
 
     def lookup(self, vmid, dev_name):
         dom = VMModel.get_vm(vmid, self.conn)
@@ -502,57 +545,72 @@ class VMHostDevModel(object):
         task_params = {'vmid': vmid,
                        'dev_name': dev_name,
                        'dom': dom,
-                       'hostdev': hostdev}
+                       'hostdev': hostdev,
+                       'lock': threading.RLock()}
         task_uri = u'/plugins/kimchi/vms/%s/hostdevs/%s' % \
             (VMModel.get_vm(vmid, self.conn).name(), dev_name)
         taskid = add_task(task_uri, self._detach_device, self.objstore,
                           task_params)
         return self.task.lookup(taskid)
 
+    def _event_devices(self, conn, dom, alias, opaque):
+        """
+        Callback to handle add/remove devices event
+        """
+        if opaque._cb is None:
+            wok_log.error('opaque must be valid')
+            return
+
+        wok_log.info("Device %s removed successfuly" % alias)
+        opaque._cb('OK', True)
+
     def _detach_device(self, cb, params):
         cb('Detaching device')
+        self._cb = cb
         vmid = params['vmid']
         dev_name = params['dev_name']
         dom = params['dom']
         hostdev = params['hostdev']
+        lock = params['lock']
 
-        pci_devs = [(DeviceModel.deduce_dev_name(e, self.conn), e)
-                    for e in hostdev if e.attrib['type'] == 'pci']
+        with lock:
+            pci_devs = [(DeviceModel.deduce_dev_name(e, self.conn), e)
+                        for e in hostdev if e.attrib['type'] == 'pci']
 
-        dev_model = DeviceModel(conn=self.conn)
-        dev_info = dev_model.lookup(dev_name)
-        is_3D_device = dev_model.is_device_3D_controller(dev_info)
-        if is_3D_device and DOM_STATE_MAP[dom.info()[0]] != "shutoff":
-            raise InvalidOperation('KCHVMHDEV0006E',
-                                   {'name': dev_info['name']})
-
-        if self._hotunplug_multifunction_pci(dom, hostdev, dev_name):
-            if is_3D_device:
-                cb('Updating MMIO from VM...')
-                devsmodel = VMHostDevsModel(conn=self.conn)
-                devsmodel.update_mmio_guest(vmid, False)
-            cb('OK', True)
-            return
+            dev_model = DeviceModel(conn=self.conn)
+            dev_info = dev_model.lookup(dev_name)
+            is_3D_device = dev_model.is_device_3D_controller(dev_info)
+            if is_3D_device and DOM_STATE_MAP[dom.info()[0]] != "shutoff":
+                raise InvalidOperation('KCHVMHDEV0006E',
+                                       {'name': dev_info['name']})
 
-        for e in hostdev:
-            if DeviceModel.deduce_dev_name(e, self.conn) == dev_name:
-                xmlstr = etree.tostring(e)
-                cb('Detaching device from VM...')
-                dom.detachDeviceFlags(
-                    xmlstr, get_vm_config_flag(dom, mode='all'))
-                if e.attrib['type'] == 'pci':
-                    cb('Deleting affected PCI devices...')
-                    self._delete_affected_pci_devices(dom, dev_name, pci_devs)
+            if self._hotunplug_multifunction_pci(dom, hostdev, dev_name):
                 if is_3D_device:
-                    cb('Updating MMIO from VM...')
                     devsmodel = VMHostDevsModel(conn=self.conn)
                     devsmodel.update_mmio_guest(vmid, False)
-                break
-        else:
-            raise NotFoundError('KCHVMHDEV0001E',
-                                {'vmid': vmid, 'dev_name': dev_name})
 
-        cb('OK', True)
+                if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
+                    cb('OK', True)
+                return
+
+            for e in hostdev:
+                if DeviceModel.deduce_dev_name(e, self.conn) == dev_name:
+                    xmlstr = etree.tostring(e)
+                    dom.detachDeviceFlags(
+                        xmlstr, get_vm_config_flag(dom, mode='all'))
+                    if e.attrib['type'] == 'pci':
+                        self._delete_affected_pci_devices(dom, dev_name,
+                                                          pci_devs)
+                    if is_3D_device:
+                        devsmodel = VMHostDevsModel(conn=self.conn)
+                        devsmodel.update_mmio_guest(vmid, False)
+                    break
+            else:
+                raise NotFoundError('KCHVMHDEV0001E',
+                                    {'vmid': vmid, 'dev_name': dev_name})
+
+        if DOM_STATE_MAP[dom.info()[0]] == "shutoff":
+            cb('OK', True)
 
     def _get_devices_same_addr(self, hostdev, domain, bus, slot):
         devices = []
-- 
2.7.4




More information about the Kimchi-devel mailing list