On 07/11/2016 04:31 PM, Jose Ricardo Ziviani wrote:

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

Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- model/libvirtevents.py | 48 ++++++++++++++++++++++++++++++++++++++++ model/vmhostdevs.py | 60 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 98 insertions(+), 10 deletions(-)

diff --git a/model/libvirtevents.py b/model/libvirtevents.py index dfd22c6..0f3bfd5 100644 --- a/model/libvirtevents.py +++ b/model/libvirtevents.py @@ -89,3 +89,51 @@ 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) + + def deregisterAttachDevicesEvent(self, conn, id): + """ + deregister libvirt event to listen to devices attachment + """ + try: + conn.get().domainEventDeregisterAny(id) + + except libvirt.libvirtError as e: + wok_log.error("deregister attach event failed: %s" % e.message) + + def deregisterDetachDevicesEvent(self, conn, id): + """ + deregister libvirt event to listen to devices detachment + """ + try: + conn.get().domainEventDeregisterAny(id) + + except libvirt.libvirtError as e: + wok_log.error("deregister detach event failed: %s" % e.message)

These 2 functions do the same thing but with a different string at wok_log. I think you can do a generic detach events function that can be reused in other cases: def deregisterEvent(self, conn, id): """ deregister libvirt event based on event id """ + try: + conn.get().domainEventDeregisterAny(id) + + except libvirt.libvirtError as e: + wok_log.error("deregister event failed: %s" % e.message) The information provided in the e.message string should be enough to differentiate whether the failure was on a detach device, attach device or any other kind of libvirt event.

diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py index 2130ac4..676e40a 100644 --- a/model/vmhostdevs.py +++ b/model/vmhostdevs.py @@ -47,8 +47,10 @@ 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._eid = None

def get_list(self, vmid): dom = VMModel.get_vm(vmid, self.conn) @@ -67,6 +69,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 self._eid is None: + wok_log.error("callbackID cannot be None") + return + + wok_log.info("Device %s added successfuly" % alias) + opaque('OK', True) + def create(self, vmid, params): dev_name = params['name'] self._passthrough_device_validate(dev_name) @@ -224,6 +237,12 @@ class VMHostDevsModel(object): raise InvalidOperation('KCHVMHDEV0006E', {'name': dev_info['name']})

+ if self._eid is None: + self._eid = self.events.registerAttachDevicesEvent( + self.conn, + self._event_devices, + cb) +

This code is repeated 3 times in methods of the same class. I propose to create something like: --- a/model/vmhostdevs.py +++ b/model/vmhostdevs.py @@ -200,6 +200,13 @@ class VMHostDevsModel(object): return free + 1 + def register_attach_devices_event(self, cb): + if self._eid is None: + self._eid = self.events.registerAttachDevicesEvent( + self.conn, + self._event_devices, + cb) + And then just call register_attach_devices_event(cb) wherever you need. Note: don't need to use this exact method 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: @@ -273,7 +292,6 @@ class VMHostDevsModel(object): rollback.prependDefer(dom.detachDeviceFlags, xmlstr, device_flags) rollback.commitAll() - cb('OK', True) return

for pci_info in pci_infos: @@ -294,8 +312,6 @@ class VMHostDevsModel(object): xmlstr, device_flags) rollback.commitAll()

- cb('OK', True) - def _count_3D_devices_attached(self, dom): counter = 0 root = objectify.fromstring(dom.XMLDesc(0)) @@ -407,6 +423,12 @@ class VMHostDevsModel(object): dev_info = params['dev_info'] dom = VMModel.get_vm(vmid, self.conn)

+ if self._eid is None: + self._eid = self.events.registerAttachDevicesEvent( + self.conn, + self._event_devices, + cb) + with RollbackContext() as rollback: cb('Reading source device XML') xmlstr = self._get_scsi_device_xml(dev_info) @@ -421,8 +443,6 @@ class VMHostDevsModel(object): rollback.prependDefer(dom.detachDeviceFlags, xmlstr, device_flags) rollback.commitAll()

- cb('OK', True) - def _get_usb_device_xml(self, dev_info): source = E.source( E.vendor(id=dev_info['vendor']['id']), @@ -440,6 +460,12 @@ class VMHostDevsModel(object): dev_info = params['dev_info'] dom = VMModel.get_vm(vmid, self.conn)

+ if self._eid is None: + self._eid = self.events.registerAttachDevicesEvent( + self.conn, + self._event_devices, + cb) + with RollbackContext() as rollback: cb('Reading source device XML') xmlstr = self._get_usb_device_xml(dev_info) @@ -454,14 +480,14 @@ class VMHostDevsModel(object): rollback.prependDefer(dom.detachDeviceFlags, xmlstr, device_flags) rollback.commitAll()

- 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._eid = None

def lookup(self, vmid, dev_name): dom = VMModel.get_vm(vmid, self.conn) @@ -509,6 +535,17 @@ class VMHostDevModel(object): task_params) return self.task.lookup(taskid)

+ def _event_devices(self, conn, dom, alias, opaque): + """ + Callback to handle add/remove devices event + """ + if self._eid is None: + wok_log.error("callbackID cannot be None") + return + + wok_log.info("Device %s removed successfuly" % alias) + opaque('OK', True) + def _detach_device(self, cb, params): cb('Detaching device.') vmid = params['vmid'] @@ -526,12 +563,17 @@ class VMHostDevModel(object): raise InvalidOperation('KCHVMHDEV0006E', {'name': dev_info['name']})

+ if self._eid is None: + self._eid = self.events.registerDetachDevicesEvent( + self.conn, + self._event_devices, + cb) + 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

for e in hostdev: @@ -552,8 +594,6 @@ class VMHostDevModel(object): raise NotFoundError('KCHVMHDEV0001E', {'vmid': vmid, 'dev_name': dev_name})

- cb('OK', True) - def _get_devices_same_addr(self, hostdev, domain, bus, slot): devices = [] for device in hostdev: