[PATCH] [Kimchi 0/2] Improve PCI passthrough experience

This patch has improvements to both PCI tab (Edit Guest) and PCI Passthrough performances. Jose Ricardo Ziviani (2): Improve PCI passthrough performance Replace device companion check before the passthrough model/vmhostdevs.py | 34 ++++++++--------- ui/js/src/kimchi.guest_edit_main.js | 74 ++++++++++++++++++++++--------------- ui/pages/i18n.json.tmpl | 1 + 3 files changed, 61 insertions(+), 48 deletions(-) -- 2.7.4

- This commit improves PCI passthrough performance by re-using the device(s) model instead of creating new instances, which was the source cause of a huge overhead. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- model/vmhostdevs.py | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/model/vmhostdevs.py b/model/vmhostdevs.py index 70f0b17..15b5bec 100644 --- a/model/vmhostdevs.py +++ b/model/vmhostdevs.py @@ -51,6 +51,8 @@ class VMHostDevsModel(object): self.objstore = kargs['objstore'] self.events = kargs['eventsloop'] self.caps = CapabilitiesModel(**kargs) + self.devs_model = DevicesModel(**kargs) + self.dev_model = DeviceModel(**kargs) self.task = TaskModel(**kargs) self._cb = None self.events.registerAttachDevicesEvent( @@ -70,8 +72,7 @@ class VMHostDevsModel(object): return [DeviceModel.deduce_dev_name(e, self.conn) for e in hostdev] def _passthrough_device_validate(self, dev_name): - eligible_dev_names = \ - DevicesModel(conn=self.conn).get_list(_passthrough='true') + eligible_dev_names = self.devs_model.get_list(_passthrough='true') if dev_name not in eligible_dev_names: raise InvalidParameter('KCHVMHDEV0002E', {'dev_name': dev_name}) @@ -89,7 +90,7 @@ class VMHostDevsModel(object): def create(self, vmid, params): dev_name = params['name'] self._passthrough_device_validate(dev_name) - dev_info = DeviceModel(conn=self.conn).lookup(dev_name) + dev_info = self.dev_model.lookup(dev_name) if dev_info['device_type'] == 'pci': taskid = add_task(u'/plugins/kimchi/vms/%s/hostdevs/' % @@ -192,7 +193,7 @@ class VMHostDevsModel(object): slots = [] try: devices = root.devices - slots = [DeviceModel._toint(dev.attrib['slot']) + slots = [self.dev_model._toint(dev.attrib['slot']) for dev in devices.findall('.//address') if 'slot' in dev.attrib] @@ -235,14 +236,12 @@ class VMHostDevsModel(object): 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( + affected_names = self.devs_model.get_list( _passthrough_affected_by=dev_info['name']) - passthrough_names = devs_model.get_list( + passthrough_names = self.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 + pci_infos = [self.dev_model.lookup(dev_name) for dev_name in group_names] pci_infos.append(dev_info) @@ -250,7 +249,7 @@ class VMHostDevsModel(object): 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) + is_3D_device = self.dev_model.is_device_3D_controller(dev_info) if is_3D_device and DOM_STATE_MAP[dom.info()[0]] != "shutoff": msg = WokMessage('KCHVMHDEV0006E', {'name': dev_info['name']}) cb(msg.get_text(), False) @@ -359,7 +358,7 @@ class VMHostDevsModel(object): continue name = DeviceModel.deduce_dev_name(device, self.conn) - info = DeviceModel(conn=self.conn).lookup(name) + info = self.dev_model.lookup(name) if 'vga3d' in info and info['vga3d']: counter += 1 @@ -534,6 +533,8 @@ class VMHostDevModel(object): self.objstore = kargs['objstore'] self.events = kargs['eventsloop'] self.task = TaskModel(**kargs) + self.devs_model = DevicesModel(**kargs) + self.dev_model = DeviceModel(**kargs) self._cb = None self.events.registerDetachDevicesEvent( self.conn, @@ -550,11 +551,10 @@ class VMHostDevModel(object): raise NotFoundError('KCHVMHDEV0001E', {'vmid': vmid, 'dev_name': dev_name}) - dev_model = DeviceModel(conn=self.conn) for e in hostdev: deduced_name = DeviceModel.deduce_dev_name(e, self.conn) if deduced_name == dev_name: - dev_info = dev_model.lookup(dev_name) + dev_info = self.dev_model.lookup(dev_name) return {'name': dev_name, 'type': e.attrib['type'], 'product': dev_info.get('product', None), @@ -612,9 +612,8 @@ class VMHostDevModel(object): 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) + dev_info = self.dev_model.lookup(dev_name) + is_3D_device = self.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']}) @@ -685,9 +684,8 @@ class VMHostDevModel(object): return True def _delete_affected_pci_devices(self, dom, dev_name, pci_devs): - dev_model = DeviceModel(conn=self.conn) try: - dev_model.lookup(dev_name) + self.dev_model.lookup(dev_name) except NotFoundError: return -- 2.7.4

- Instead of checking the companion for each device (with performance penalties when opening the Edit Guest window) we now check for companions before plugging a device to a guest. Then, we popup a confirmation window showing devices affected by such action and asks for user confirmation. Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_edit_main.js | 74 ++++++++++++++++++++++--------------- ui/pages/i18n.json.tmpl | 1 + 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/ui/js/src/kimchi.guest_edit_main.js b/ui/js/src/kimchi.guest_edit_main.js index cabd497..9099f39 100644 --- a/ui/js/src/kimchi.guest_edit_main.js +++ b/ui/js/src/kimchi.guest_edit_main.js @@ -467,30 +467,6 @@ kimchi.guest_edit_main = function() { if (kimchi.thisVMState === "running" && device.vga3d) { $('button', deviceHtml).prop("disabled", true); } - kimchi.getPCIDeviceCompanions(device.name, function(infoData) { - var pciTitle = i18n['KCHVMED6007M'] + '\n'; - var haveCompanions = false; - for (var p = 0; p < infoData.length; p++) { - if (infoData[p].device_type === 'net') { - haveCompanions = true; - pciTitle += ' ' + infoData[p].name + '\n'; - pciTitle += ' ' + i18n['KCHVMED6001M'] + ' ' + infoData[p].interface; - pciTitle += ', ' + i18n['KCHVMED6002M'] + ' ' + infoData[p].address; - pciTitle += ', ' + i18n['KCHVMED6003M'] + ' ' + infoData[p].link_type + '\n'; - } else if (infoData[p].device_type === 'storage') { - haveCompanions = true; - pciTitle += ' ' + infoData[p].name + '\n'; - pciTitle += ' ' + i18n['KCHVMED6004M'] + ' ' + infoData[p].block; - pciTitle += ', ' + i18n['KCHVMED6005M'] + ' ' + infoData[p].drive_type; - pciTitle += ', ' + i18n['KCHVMED6006M'] + ' ' + infoData[p].model + '\n'; - } - } - for (var q = 0; q < infoData.length; q++) { - haveCompanions && $('.name', '#' + infoData[q].parent).attr('title', pciTitle); - haveCompanions && $('.product', '#' + infoData[q].parent).attr('title', pciTitle); - haveCompanions && $('.vendor', '#' + infoData[q].parent).attr('title', pciTitle); - } - }); device = deviceHtml[0].outerHTML; $('.body', '#form-guest-edit-pci').append(device); }); @@ -565,12 +541,50 @@ kimchi.guest_edit_main = function() { wok.message.error(err.responseJSON.reason, '#alert-modal-container'); }); } else { - kimchi.addVMPCIDevice(kimchi.selectedGuest, { - name: id - }, function(task) { - getOngoingAttachingDevices(task); - }, function(err) { - wok.message.error(err.responseJSON.reason, '#alert-modal-container'); + kimchi.getPCIDeviceCompanions(id, function(infoData) { + var pciTitle = i18n['KCHVMED6007M'] + '\n'; + var haveCompanions = false; + for (var p = 0; p < infoData.length; p++) { + if (infoData[p].device_type === 'net') { + haveCompanions = true; + pciTitle += ' ' + infoData[p].name + '\n'; + pciTitle += ' ' + i18n['KCHVMED6001M'] + ' ' + infoData[p].interface; + pciTitle += ', ' + i18n['KCHVMED6002M'] + ' ' + infoData[p].address; + pciTitle += ', ' + i18n['KCHVMED6003M'] + ' ' + infoData[p].link_type + '\n'; + } else if (infoData[p].device_type === 'storage') { + haveCompanions = true; + pciTitle += ' ' + infoData[p].name + '\n'; + pciTitle += ' ' + i18n['KCHVMED6004M'] + ' ' + infoData[p].block; + pciTitle += ', ' + i18n['KCHVMED6005M'] + ' ' + infoData[p].drive_type; + pciTitle += ', ' + i18n['KCHVMED6006M'] + ' ' + infoData[p].model + '\n'; + } + } + var settings = { + title: i18n['KCHVMED6012M'], + content: pciTitle, + confirm: i18n['KCHAPI6002M'], + cancel: i18n['KCHAPI6003M'] + }; + + if (haveCompanions) { + wok.confirm(settings, function() { + kimchi.addVMPCIDevice(kimchi.selectedGuest, { + name: id + }, function(task) { + getOngoingAttachingDevices(task); + }, function(err) { + wok.message.error(err.responseJSON.reason, '#alert-modal-container'); + }); + }); + } else { + kimchi.addVMPCIDevice(kimchi.selectedGuest, { + name: id + }, function(task) { + getOngoingAttachingDevices(task); + }, function(err) { + wok.message.error(err.responseJSON.reason, '#alert-modal-container'); + }); + } }); } }); diff --git a/ui/pages/i18n.json.tmpl b/ui/pages/i18n.json.tmpl index 610debc..a5185b1 100644 --- a/ui/pages/i18n.json.tmpl +++ b/ui/pages/i18n.json.tmpl @@ -86,6 +86,7 @@ "KCHVMED6009M": "$_("Less")", "KCHVMED6010M": "$_("Successfully attached device to VM")", "KCHVMED6011M": "$_("Successfully detached device from VM")", + "KCHVMED6012M": "$_("Following devices will be affected, confirm?")", "KCHNET6001M": "$_("unavailable")", "KCHNET6002M": "$_("This action will interrupt network connectivity for any virtual machine that depend on this network.")", -- 2.7.4

Applied to master. Thanks! On 07/27/2016 06:49 PM, Jose Ricardo Ziviani wrote:
This patch has improvements to both PCI tab (Edit Guest) and PCI Passthrough performances.
Jose Ricardo Ziviani (2): Improve PCI passthrough performance Replace device companion check before the passthrough
model/vmhostdevs.py | 34 ++++++++--------- ui/js/src/kimchi.guest_edit_main.js | 74 ++++++++++++++++++++++--------------- ui/pages/i18n.json.tmpl | 1 + 3 files changed, 61 insertions(+), 48 deletions(-)
participants (2)
-
Daniel Henrique Barboza
-
Jose Ricardo Ziviani