Re: [Kimchi-devel] [PATCH v3 1/3] PCI passthrough: list all types of host devices

looks good for me Just a minor inline comment below On 05/22/2014 10:06 AM, Zhou Zheng Sheng wrote: > The URI /host/devices only presents scsi_host (particularly fc_host) > device information. To implement host PCI pass through, we should list > all > types of host devices. This patch adds support for parsing various host > devices information, and listing them on /host/devices. So the user is > free > to choose any listed PCI device to pass through to guest. Since the > patch changes the device information dictionary format, the existing code > consuming the device information is also changed accordingly. > > To get all types of host device, access the following URL. > > curl -k -u root -H "Content-Type: application/json" \ > -H "Accept: application/json" \ > https://127.0.0.1:8001/host/devices > > To get only fc_host devices, change the URL to > "https://127.0.0.1:8001/host/devices?_cap=fc_host" > > To get only pci device, change the URL to > "https://127.0.0.1:8001/host/devices?_cap=pci" > > v1: > Parse the node device XML using xpath. > > v2: > Write a "dictize" function and parse the node device XML using > dictize. > > v3: > Fix a naming mistake. > > Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; > > PCI passthrough: list all types of host devices > --- > docs/API.md | 11 +- > src/kimchi/hostdev.py | 212 > +++++++++++++++++++++++++++++++++ > src/kimchi/mockmodel.py | 7 +- > src/kimchi/model/host.py | 15 +-- > src/kimchi/model/libvirtstoragepool.py | 18 +-- > src/kimchi/xmlutils.py | 26 +++- > tests/test_rest.py | 6 +- > tests/test_storagepool.py | 7 +- > 8 files changed, 265 insertions(+), 37 deletions(-) > create mode 100644 src/kimchi/hostdev.py > > diff --git a/docs/API.md b/docs/API.md > index 9217a37..484a4c8 100644 > --- a/docs/API.md > +++ b/docs/API.md > @@ -869,12 +869,11 @@ stats history > * **GET**: Retrieve information of a single pci device. > Currently only scsi_host devices are supported: > * name: The name of the device. > - * adapter_type: The capability type of the scsi_host device > (fc_host). > - Empty if pci device is not scsi_host. > - * wwnn: The HBA Word Wide Node Name. > - Empty if pci device is not scsi_host. > - * wwpn: The HBA Word Wide Port Name > - Empty if pci device is not scsi_host. > + * path: Path of device in sysfs. > + * adapter: Host adapter information. Empty if pci device is not > scsi_host. > + * type: The capability type of the scsi_host device (fc_host, > vport_ops). > + * wwnn: The HBA Word Wide Node Name. Empty if pci device is > not fc_host. > + * wwpn: The HBA Word Wide Port Name. Empty if pci device is > not fc_host. > > ### Collection: Host Packages Update > > diff --git a/src/kimchi/hostdev.py b/src/kimchi/hostdev.py > new file mode 100644 > index 0000000..009501b > --- /dev/null > +++ b/src/kimchi/hostdev.py > @@ -0,0 +1,212 @@ > +# > +# Kimchi > +# > +# Copyright IBM Corp, 2014 > +# > +# This library is free software; you can redistribute it and/or > +# modify it under the terms of the GNU Lesser General Public > +# License as published by the Free Software Foundation; either > +# version 2.1 of the License, or (at your option) any later version. > +# > +# This library is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# Lesser General Public License for more details. > +# > +# You should have received a copy of the GNU Lesser General Public > +# License along with this library; if not, write to the Free Software > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA > + > +from kimchi.model.libvirtconnection import LibvirtConnection > +from kimchi.utils import kimchi_log > +from kimchi.xmlutils import dictize > + > + > +def _get_all_host_dev_infos(): > + libvirt_conn = LibvirtConnection('qemu:///system').get() > + node_devs = libvirt_conn.listAllDevices() > + return [get_dev_info(node_dev) for node_dev in node_devs] > + > + > +def _get_dev_info_tree(dev_infos): > + devs = dict([(dev_info['name'], dev_info) for dev_info in > dev_infos]) > + root = None > + for dev_info in dev_infos: > + if dev_info['parent'] is None: > + root = dev_info > + continue > + try: > + parent = devs[dev_info['parent']] > + except KeyError: why KeyError, it look strange for me. but I have a try. there is a KeyError, it is "scsi_host6"

> + continue > + > + try: > + children = parent['children'] > + except KeyError: > + parent['children'] = [dev_info] > + else: > + children.append(dev_info) > + return root > + > + > +def get_dev_info(node_dev): > + ''' Parse the node device XML string into dict according to > + http://libvirt.org/formatnode.html. ''' > + > + def shift_subdict(d, toshift): > + subdict = d.pop(toshift) > + d.update(subdict) > + return d > + > + xmlstr = node_dev.XMLDesc() > + info = dictize(xmlstr)['device'] > + dev_type = info['capability'].pop('type') > + info['device_type'] = dev_type > + shift_subdict(info, 'capability') > + info['parent'] = node_dev.parent() > + > + get_dev_type_info = { > + 'net': _get_net_dev_info, > + 'pci': _get_pci_dev_info, > + 'scsi': _get_scsi_dev_info, > + 'scsi_generic': _get_scsi_generic_dev_info, > + 'scsi_host': _get_scsi_host_dev_info, > + 'scsi_target': _get_scsi_target_dev_info, > + 'storage': _get_storage_dev_info, > + 'system': _get_system_dev_info, > + 'usb': _get_usb_dev_info, > + 'usb_device': _get_usb_device_dev_info, > + } > + try: > + get_detail_info = get_dev_type_info[dev_type] > + except KeyError: > + kimchi_log.error("Unknown device type: %s", dev_type) > + return info > + > + return get_detail_info(info) > + > + > +def _get_net_dev_info(info): > + cap = info.pop('capability') > + links = {"80203": "IEEE 802.3", "80211": "IEEE 802.11"} > + link_raw = cap['type'] > + info['link_type'] = links.get(link_raw, link_raw) > + > + return info > + > + > +def _get_pci_dev_info(info): > + for k in ('vendor', 'product'): > + info[k]['description'] = info[k].pop('pyval') > + try: > + info['iommuGroup'] = info['iommuGroup']['number'] > + except KeyError: > + # No IOMMU group support. > + pass > + return info > + > + > +def _get_scsi_dev_info(info): > + return info > + > + > +def _get_scsi_generic_dev_info(info): > + # scsi_generic is not documented in libvirt official website. Try to > + # parse scsi_generic according to the following libvirt path series. > + # > https://www.redhat.com/archives/libvir-list/2013-June/msg00014.html > + return info > + > + > +def _get_scsi_host_dev_info(info): > + try: > + cap_info = info.pop('capability') > + except KeyError: > + # kimchi.model.libvirtstoragepool.ScsiPoolDef assumes > + # info['adapter']['type'] always exists. > + info['adapter'] = {'type': ''} > + return info > + info['adapter'] = cap_info > + return info > + > + > +def _get_scsi_target_dev_info(info): > + # scsi_target is not documented in libvirt official website. Try to > + # parse scsi_target according to the libvirt commit db19834a0a. > + return info > + > + > +def _get_storage_dev_info(info): > + try: > + cap_info = info.pop('capability') > + except KeyError: > + return info > + > + if cap_info['type'] == 'removable': > + cap_info['available'] = bool(cap_info.pop('media_available')) > + if cap_info['available']: > + cap_info.update({'size': cap_info.pop('media_size'), > + 'label': cap_info.pop('media_label')}) > + info['media'] = cap_info > + return info > + > + > +def _get_system_dev_info(info): > + return info > + > + > +def _get_usb_dev_info(info): > + return info > + > + > +def _get_usb_device_dev_info(info): > + for k in ('vendor', 'product'): > + try: > + info[k]['description'] = info[k].pop('pyval') > + except KeyError: > + # Some USB devices don't provide vendor/product description. > + pass > + return info > + > + > +# For test and debug > +def _print_host_dev_tree(): > + dev_infos = _get_all_host_dev_infos() > + root = _get_dev_info_tree(dev_infos) > + if root is None: > + print "No device found" > + return > + print '-----------------' > + print '\n'.join(_format_dev_node(root)) > + > + > +def _format_dev_node(node): > + from pprint import pformat > + > + try: > + children = node['children'] > + del node['children'] > + except KeyError: > + children = [] > + > + lines = [] > + lines.extend([' ~' + line for line in pformat(node).split('\n')]) > + > + count = len(children) > + for i, child in enumerate(children): > + if count == 1: > + lines.append(' \-----------------') > + else: > + lines.append(' +-----------------') > + clines = _format_dev_node(child) > + if i == count - 1: > + p = ' ' > + else: > + p = ' |' > + lines.extend([p + cline for cline in clines]) > + lines.append('') > + > + return lines > + > + > +if __name__ == '__main__': > + _print_host_dev_tree() > diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py > index 05720f4..7420e7e 100644 > --- a/src/kimchi/mockmodel.py > +++ b/src/kimchi/mockmodel.py > @@ -493,9 +493,10 @@ class MockModel(object): > def device_lookup(self, nodedev_name): > return { > 'name': nodedev_name, > - 'adapter_type': 'fc_host', > - 'wwnn': uuid.uuid4().hex[:16], > - 'wwpn': uuid.uuid4().hex[:16]} > + 'adapter': { > + 'type': 'fc_host', > + 'wwnn': uuid.uuid4().hex[:16], > + 'wwpn': uuid.uuid4().hex[:16]}} > > def isopool_lookup(self, name): > return {'state': 'active', > diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py > index e9ac487..f4bd613 100644 > --- a/src/kimchi/model/host.py > +++ b/src/kimchi/model/host.py > @@ -28,6 +28,7 @@ import psutil > from cherrypy.process.plugins import BackgroundTask > > from kimchi import disks > +from kimchi import hostdev > from kimchi import netinfo > from kimchi import xmlutils > from kimchi.basemodel import Singleton > @@ -285,20 +286,10 @@ class DeviceModel(object): > def lookup(self, nodedev_name): > conn = self.conn.get() > try: > - dev_xml = > conn.nodeDeviceLookupByName(nodedev_name).XMLDesc(0) > + dev = conn.nodeDeviceLookupByName(nodedev_name) > except: > raise NotFoundError('KCHHOST0003E', {'name': nodedev_name}) > - cap_type = xmlutils.xpath_get_text( > - dev_xml, '/device/capability/capability/@type') > - wwnn = xmlutils.xpath_get_text( > - dev_xml, '/device/capability/capability/wwnn') > - wwpn = xmlutils.xpath_get_text( > - dev_xml, '/device/capability/capability/wwpn') > - return { > - 'name': nodedev_name, > - 'adapter_type': cap_type[0] if len(cap_type) >= 1 else '', > - 'wwnn': wwnn[0] if len(wwnn) == 1 else '', > - 'wwpn': wwpn[0] if len(wwpn) == 1 else ''} > + return hostdev.get_dev_info(dev) > > > class PackagesUpdateModel(object): > diff --git a/src/kimchi/model/libvirtstoragepool.py > b/src/kimchi/model/libvirtstoragepool.py > index 47b239b..b15bf1a 100644 > --- a/src/kimchi/model/libvirtstoragepool.py > +++ b/src/kimchi/model/libvirtstoragepool.py > @@ -180,34 +180,34 @@ class ScsiPoolDef(StoragePoolDef): > self.poolArgs['source']['name'] = tmp_name.replace('scsi_', '') > # fc_host adapters type are only available in libvirt >= 1.0.5 > if not self.poolArgs['fc_host_support']: > - self.poolArgs['source']['adapter_type'] = 'scsi_host' > + self.poolArgs['source']['adapter']['type'] = 'scsi_host' > msg = "Libvirt version <= 1.0.5. Setting SCSI host name > as '%s'; "\ > "setting SCSI adapter type as 'scsi_host'; "\ > "ignoring wwnn and wwpn." % tmp_name > kimchi_log.info(msg) > # Path for Fibre Channel scsi hosts > self.poolArgs['path'] = '/dev/disk/by-path' > - if not self.poolArgs['source']['adapter_type']: > - self.poolArgs['source']['adapter_type'] = 'scsi_host' > + if not self.poolArgs['source']['adapter']['type']: > + self.poolArgs['source']['adapter']['type'] = 'scsi_host' > > @property > def xml(self): > # Required parameters > # name: > - # source[adapter_type]: > + # source[adapter][type]: > # source[name]: > - # source[wwnn]: > - # source[wwpn]: > + # source[adapter][wwnn]: > + # source[adapter][wwpn]: > # path: > > xml = """ > <pool type='scsi'> > <name>{name}</name> > <source> > - <adapter type='{source[adapter_type]}'\ > + <adapter type='{source[adapter][type]}'\ > name='{source[name]}'\ > - wwnn='{source[wwnn]}'\ > - wwpn='{source[wwpn]}'/> > + wwnn='{source[adapter][wwnn]}'\ > + wwpn='{source[adapter][wwpn]}'/> > </source> > <target> > <path>{path}</path> > diff --git a/src/kimchi/xmlutils.py b/src/kimchi/xmlutils.py > index 76f0696..56517f2 100644 > --- a/src/kimchi/xmlutils.py > +++ b/src/kimchi/xmlutils.py > @@ -18,6 +18,7 @@ > # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA > > import libxml2 > +from lxml import objectify > > > from xml.etree import ElementTree > @@ -26,7 +27,7 @@ from xml.etree import ElementTree > def xpath_get_text(xml, expr): > doc = libxml2.parseDoc(xml) > res = doc.xpathEval(expr) > - ret = [None if x.children == None else x.children.content for x > in res] > + ret = [None if x.children is None else x.children.content for x > in res] > > doc.freeDoc() > return ret > @@ -37,3 +38,26 @@ def xml_item_update(xml, xpath, value): > item = root.find(xpath) > item.text = value > return ElementTree.tostring(root, encoding="utf-8") > + > + > +def dictize(xmlstr): > + root = objectify.fromstring(xmlstr) > + return {root.tag: _dictize(root)} > + > + > +def _dictize(e): > + d = {} > + if e.text is not None: > + if not e.attrib and e.countchildren() == 0: > + return e.pyval > + d['pyval'] = e.pyval > + d.update(e.attrib) > + for child in e.iterchildren(): > + if child.tag in d: > + continue > + if len(child) > 1: > + d[child.tag] = [ > + _dictize(same_tag_child) for same_tag_child in child] > + else: > + d[child.tag] = _dictize(child) > + return d > diff --git a/tests/test_rest.py b/tests/test_rest.py > index 7ed94cb..cb1ae9a 100644 > --- a/tests/test_rest.py > +++ b/tests/test_rest.py > @@ -158,9 +158,9 @@ class RestTests(unittest.TestCase): > nodedev = > json.loads(self.request('/host/devices/scsi_host4').read()) > # Mockmodel generates random wwpn and wwnn > self.assertEquals('scsi_host4', nodedev['name']) > - self.assertEquals('fc_host', nodedev['adapter_type']) > - self.assertEquals(16, len(nodedev['wwpn'])) > - self.assertEquals(16, len(nodedev['wwnn'])) > + self.assertEquals('fc_host', nodedev['adapter']['type']) > + self.assertEquals(16, len(nodedev['adapter']['wwpn'])) > + self.assertEquals(16, len(nodedev['adapter']['wwnn'])) > > def test_get_vms(self): > vms = json.loads(self.request('/vms').read()) > diff --git a/tests/test_storagepool.py b/tests/test_storagepool.py > index 22b4943..3e3ad83 100644 > --- a/tests/test_storagepool.py > +++ b/tests/test_storagepool.py > @@ -145,9 +145,10 @@ class storagepoolTests(unittest.TestCase): > 'path': '/dev/disk/by-path', > 'source': { > 'name': 'scsi_host3', > - 'adapter_type': 'fc_host', > - 'wwpn': '0123456789abcdef', > - 'wwnn': 'abcdef0123456789'}}, > + 'adapter': { > + 'type': 'fc_host', > + 'wwpn': '0123456789abcdef', > + 'wwnn': 'abcdef0123456789'}}}, > 'xml': > """ > <pool type='scsi'>