[Kimchi-devel] [PATCH V4 1/5] Storagepool SCSI/FC: Implement node devices API backend

[Rodrigo Trujillo]

On 02/10/2014 01:39 PM, Sheldon wrote:
> Just a minor comment below
>
> On 02/10/2014 11:28 PM, Sheldon wrote:
>> On 02/05/2014 10:18 PM, Rodrigo Trujillo wrote:
>>> In order to implement support to SCSI/FC UI, it is necessary to 
>>> retrieve
>>> node devices.
>>>
>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>> ---
>>>   src/kimchi/control/host.py | 16 ++++++++++++++
>>>   src/kimchi/model/host.py   | 54 
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 70 insertions(+)
>>>
>>> diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py
>>> index 053c822..936d298 100644
>>> --- a/src/kimchi/control/host.py
>>> +++ b/src/kimchi/control/host.py
>>> @@ -36,6 +36,7 @@ class Host(Resource):
>>>           self.shutdown = self.generate_action_handler('shutdown')
>>>           self.stats = HostStats(self.model)
>>>           self.partitions = Partitions(self.model)
>>> +        self.devices = Devices(self.model)
>>>
>>>       @property
>>>       def data(self):
>>> @@ -61,3 +62,18 @@ class Partition(Resource):
>>>       @property
>>>       def data(self):
>>>           return self.info
>>> +
>>> +
>>> +class Devices(Collection):
>>> +    def __init__(self, model):
>>> +        super(Devices, self).__init__(model)
>>> +        self.resource = Device
>>> +
>>> +
>>> +class Device(Resource):
>>> +    def __init__(self, model, id):
>>> +        super(Device, self).__init__(model, id)
>>> +
>>> +    @property
>>> +    def data(self):
>>> +        return self.info
>>> diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py
>>> index a3d9e38..0545a88 100644
>>> --- a/src/kimchi/model/host.py
>>> +++ b/src/kimchi/model/host.py
>>> @@ -30,6 +30,7 @@ from cherrypy.process.plugins import BackgroundTask
>>>
>>>   from kimchi import disks
>>>   from kimchi import netinfo
>>> +from kimchi import xmlutils
>>>   from kimchi.basemodel import Singleton
>>>   from kimchi.exception import NotFoundError, OperationFailed
>>>   from kimchi.model.vms import DOM_STATE_MAP
>>> @@ -199,3 +200,56 @@ class PartitionModel(object):
>>>               raise NotFoundError("Partition %s not found in the host"
>>>                                   % name)
>>>           return disks.get_partition_details(name)
>>> +
>>> +
>>> +class DevicesModel(object):
>>> +    def __init__(self, **kargs):
>>> +        self.conn = kargs['conn']
>>> +
>>> +    def get_list(self, _cap=None):
>>> +        conn = self.conn.get()
>>> +        if _cap == None:
>>> +            dev_names = [name.name() for name in 
>>> conn.listAllDevices(0)]
>>> +        elif _cap == 'fc_host':
>>> +            dev_names = self._get_devices_fc_host()
>>> +        else:
>>> +            # Get devices with required capability
>>> +            dev_names = conn.listDevices(_cap,0)
>>> +        return dev_names
>>> +
>>> +    def _get_devices_fc_host(self):
>>> +       conn = self.conn.get()
>>> +        # Libvirt < 1.0.5 does not support fc_host capability
>>> +        if not self.fc_host_support:
>>> +            ret = []
>>> +            scsi_hosts = conn.listDevices('scsi_host',0)
>>> +            for host in scsi_hosts:
>>> +                xml = conn.nodeDeviceLookupByName(host).XMLDesc(0)
>>> +                path = '/device/capability/capability/@type'
>>> +               if 'fc_host' in xmlutils.xpath_get_text(xml, path):
>>> +                    ret.append(host)
>>> +            return ret
>>> +        return conn.listDevices('fc_host',0)
>>> +
>>> +
>>> +class DeviceModel(object):
>>> +    def __init__(self, **kargs):
>>> +        self.conn = kargs['conn']
>>> +
>>> +    def lookup(self, nodedev_name):
>>> +        conn = self.conn.get()
>>> +        try:
>>> +            dev_xml = 
>>> conn.nodeDeviceLookupByName(nodedev_name).XMLDesc(0)
>>> +        except:
>>> +            raise NotFoundError('Node device "%s" not found' % 
>>> nodedev_name)
>>> +        cap_type = xmlutils.xpath_get_text(
>>> +            dev_xml, '/device/capability/capability/@type')
>> capability/capability/
>> double capability?
>>
>>> +        wwnn = xmlutils.xpath_get_text(
>>> +            dev_xml, '/device/capability/capability/wwnn')
> so this DeviceModel is not only for 'fc_host', right?
> Then other Device may not have wwnn and wwpn.
Yeap you are right... at this moment, the NodeDevices is going to 
support only the scsi_host type,
which is the device that provides fc_host functionality.
As kimchi evolves we can increase the support.
wwpn and wwnn will be "" if not present.

>>> +        wwpn = xmlutils.xpath_get_text(
>>> +            dev_xml, '/device/capability/capability/wwpn')
>>> +        return {
>>> +            'name': nodedev_name,
>>> +            'adapter_type': cap_type,
>>> +            'wwnn': wwnn,
>>> +            'wwpn': wwpn}
As my code is working, I think it is better to change this later.
If xmlutils.xpath_get_text should not be used, then we need to refactor  
all the kimchi code.

>> use lxml?
>>
>> from lxml import objectify
>>
>> root = objectify.fromstring(dev_xml)
>> cap = root.capability.capability
>> return {
>> 'adapter_type': cap.get("type "),
>> "wwnn": cap.wwnn,
>> "wwpn": cap.wwpn
>> }
>>
>
>