[Kimchi-devel] [PATCH V2 4/7] (WIP) Storagepool SCSI/FC: Implement node devices API backend

[Rodrigo Trujillo]

On 01/25/2014 09:37 PM, Aline Manera wrote:
> On 01/23/2014 10:29 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/devices.py | 40 
>> ++++++++++++++++++++++++++++++++++++++++
>>   src/kimchi/control/host.py    |  3 +++
>>   src/kimchi/model.py           | 28 ++++++++++++++++++++++++++++
>>   3 files changed, 71 insertions(+)
>>   create mode 100644 src/kimchi/control/devices.py
>>
>> diff --git a/src/kimchi/control/devices.py 
>> b/src/kimchi/control/devices.py
>> new file mode 100644
>> index 0000000..d5ef8fe
>> --- /dev/null
>> +++ b/src/kimchi/control/devices.py
>> @@ -0,0 +1,40 @@
>> +#
>> +# Project Kimchi
>> +#
>> +# Copyright IBM, Corp. 2014
>> +#
>> +# Authors:
>> +#  Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>> +#
>> +# 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.control.base import Collection, Resource
>> +
>> +
>> +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/control/host.py b/src/kimchi/control/host.py
>> index 9b19577..82c8c13 100644
>> --- a/src/kimchi/control/host.py
>> +++ b/src/kimchi/control/host.py
>> @@ -24,6 +24,7 @@
>>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
>> 02110-1301 USA
>>
>>   from kimchi.control.base import Collection, Resource
>> +from kimchi.control.devices import Devices
>>
>>
>>   class Host(Resource):
>> @@ -36,6 +37,8 @@ class Host(Resource):
>>           self.stats.exposed = True
>>           self.partitions = Partitions(self.model)
>>           self.partitions.exposed = True
>> +        self.devices = Devices(self.model)
>> +        self.devices.exposed = True
>
> If "Devices" is a sub-resource of host you should put the 
> control/devices.py content in control/host.py
> The files in control/ are named according to uri.

Ok, makes sense, but I think we reached a Kimchi architectural decision 
here.
Should devices be a sub-resource of Host or not ?
While coding I thought more appropriated to add then under host, like 
volumes are under storagepool.
But, in libvirt, 'nodedev' are not restricted to any resource ... they 
are listed and treated as Network or StoragePool.
So, should the API be <kimchi>/devices  ??

>
>>       @property
>>       def data(self):
>> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
>> index 552f69d..4a5f344 100644
>> --- a/src/kimchi/model.py
>> +++ b/src/kimchi/model.py
>> @@ -361,6 +361,34 @@ class Model(object):
>>           self.stats[vm_uuid].update({'disk_io': rate, 'max_disk_io': 
>> max_disk_io,
>>                                    'diskRdKB': diskRdKB, 'diskWrKB': 
>> diskWrKB})
>>
>> +    def device_lookup(self, nodedev_name):
>> +        return {'name': nodedev_name}
>> +
>> +    def devices_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 is_libvirt_version_lesser('1.0.5'):
>> +            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
>
> What about the "else" code?
This is code stile ... the else would have the return right below
>
>> +        return conn.listDevices('fc_host',0)
>> +
>>       def debugreport_lookup(self, name):
>>           path = config.get_debugreports_path()
>>           file_pattern = os.path.join(path, name)
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>