[Kimchi-devel] [PATCH v11 1/6] Host device passthrough: List all types of host devices

Aline Manera alinefm at linux.vnet.ibm.com
Fri Oct 3 16:36:16 UTC 2014


On 09/30/2014 07:00 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.
>
> v4:
>    It is observed that sometimes the parent devices is not listed by
>    libvirt but the child device is listed. In previous version we catch
>    this exception and ignore it. The root cause is unknown, and we failed
>    to re-produce the problem. In v4 we do not catch it. It seems to be
>    related to USB removable disk, and the problem is gone after we
>    upgraded Linux kernel.
>
> v8:
>    Move hostdev.py from src/kimchi to src/kimchi/model.
>
> v9:
>    Improve Ubuntu and Fedora 19 compatibility. Gather device information
>    if libvirt does not provide enough information. Share the same
>    LibvirtConnection object with the Model class, to prevent connection
>    exhausting.
>
> v10:
>    Adapt to RHEL 6. RHEL 6 does not provide iommu group information in
>    sysfs. For now we just ignore this error and live with it. The device
>    passthrough for PCI devices will not work, but the basic devices
>    informations are still provided to the user. In future we'll develope
>    code to gather iommu group information.
>
> v11:
>    Properly Parse NPIV Capable HBA Card Information.
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
> ---
>   docs/API.md                            |  11 +-
>   src/kimchi/mockmodel.py                |   7 +-
>   src/kimchi/model/host.py               |  15 +--
>   src/kimchi/model/hostdev.py            | 238 +++++++++++++++++++++++++++++++++
>   src/kimchi/model/libvirtstoragepool.py |  18 +--
>   src/kimchi/xmlutils.py                 |  26 +++-
>   tests/test_rest.py                     |   6 +-
>   tests/test_storagepool.py              |   7 +-
>   8 files changed, 291 insertions(+), 37 deletions(-)
>   create mode 100644 src/kimchi/model/hostdev.py
>
> diff --git a/docs/API.md b/docs/API.md
> index cc438cc..b65f211 100644
> --- a/docs/API.md
> +++ b/docs/API.md
> @@ -912,12 +912,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.

Only 'name' and 'path' are expected for pci devices?
 From the code below, seems more values will be returned according to 
the pci device type.
We should document it as well.

> +    * 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/mockmodel.py b/src/kimchi/mockmodel.py
> index ab4357d..d807eec 100644
> --- a/src/kimchi/mockmodel.py
> +++ b/src/kimchi/mockmodel.py
> @@ -617,9 +617,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 3975a10..7d7cd66 100644
> --- a/src/kimchi/model/host.py
> +++ b/src/kimchi/model/host.py
> @@ -33,6 +33,7 @@ from kimchi import netinfo
>   from kimchi import xmlutils
>   from kimchi.basemodel import Singleton
>   from kimchi.exception import InvalidOperation, NotFoundError, OperationFailed
> +from kimchi.model import hostdev
>   from kimchi.model.config import CapabilitiesModel
>   from kimchi.model.tasks import TaskModel
>   from kimchi.model.vms import DOM_STATE_MAP
> @@ -341,20 +342,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/hostdev.py b/src/kimchi/model/hostdev.py
> new file mode 100644
> index 0000000..1d660d8
> --- /dev/null
> +++ b/src/kimchi/model/hostdev.py
> @@ -0,0 +1,238 @@
> +#
> +# 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 pprint import pformat
> +
> +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):
> +    node_devs = libvirt_conn.listAllDevices(0)
> +    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
> +        parent = devs[dev_info['parent']]
> +
> +        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. '''
> +
> +    xmlstr = node_dev.XMLDesc(0)
> +    info = dictize(xmlstr)['device']
> +    dev_type = info['capability'].pop('type')
> +    info['device_type'] = dev_type
> +    cap_dict = info.pop('capability')
> +    info.update(cap_dict)
> +    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]

As the functions has the same pattern name, isn't possible to do 
something like:

get_detail_info = globals()["_get_%s_dev_info" % dev_type]

?

That way we avoid having to maintain a big map for it.

I noticed below, some of those functions only return the same info 
without additional process.

Maybe we could exclude them:

if dev_type in [...]
     return info

# for the other types call the get_detail_info()
get_detail_info = globals()["_get_%s_dev_info" % dev_type]
return get_detail_info(info)

> +    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'):
> +        try:
> +            description = info[k].pop('pyval')
> +        except KeyError:
> +            description = None
> +        info[k]['description'] = description
> +    if 'path' not in info:
> +        # Old libvirt does not provide syspath info
> +        info['path'] = \
> +            "/sys/bus/pci/devices/" \
> +            "%(domain)04x:%(bus)02x:%(slot)02x.%(function)01x" % {
> +                'domain': info['domain'], 'bus': info['bus'],
> +                'slot': info['slot'], 'function': info['function']}
> +    try:
> +        info['iommuGroup'] = int(info['iommuGroup']['number'])
> +    except KeyError:
> +        # Old libvirt does not provide syspath info, figure it out ourselves
> +        iommu_link = os.path.join(info['path'], 'iommu_group')
> +        if os.path.exists(iommu_link):
> +            iommu_path = os.path.realpath(iommu_link)
> +            try:
> +                info['iommuGroup'] = int(iommu_path.rsplit('/', 1)[1])
> +            except (ValueError, IndexError):
> +                # No IOMMU group support at all.
> +                pass
> +        else:
> +            # No IOMMU group support at all.
> +            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
> +    if isinstance(cap_info, list):
> +        info['adapter'] = {}
> +        for cap in cap_info:
> +            if cap['type'] == 'vport_ops':
> +                del cap['type']
> +                info['adapter']['vport_ops'] = cap
> +            else:
> +                info['adapter'].update(cap)
> +    else:
> +        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']:
> +            for k in ('size', 'label'):
> +                try:
> +                    cap_info[k] = cap_info.pop('media_' + k)
> +                except KeyError:
> +                    cap_info[k] = None
> +    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():
> +    libvirt_conn = LibvirtConnection('qemu:///system').get()
> +    dev_infos = _get_all_host_dev_infos(libvirt_conn)
> +    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):
> +    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/model/libvirtstoragepool.py b/src/kimchi/model/libvirtstoragepool.py
> index d39835b..d7b49e2 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 8612e66..00a9d55 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 7eb6233..0d4e822 100644
> --- a/tests/test_rest.py
> +++ b/tests/test_rest.py
> @@ -164,9 +164,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 2f5e376..3fbeed0 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'>




More information about the Kimchi-devel mailing list