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

[Zhou Zheng Sheng]

on 2014/10/04 00:36, Aline Manera wrote:
> 
> On 09/30/2014 07:00 AM, Zhou Zheng Sheng wrote:
>> 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.
> 

Yes, you are correct. I only added the most simple and necessary docs in
this patch. The full docs are added in the patch "Host device
passthrough: Add unit tests and documents".

>> +    * 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/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)
> 

Agree. Thank you.

-- 
Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou at linux.vnet.ibm.com
Telephone: 86-10-82454397