[Kimchi-devel] [PATCH V4 2/5] Storagepool SCSI/FC: Backend implementation

Rodrigo Trujillo rodrigo.trujillo at linux.vnet.ibm.com
Tue Feb 11 23:08:00 UTC 2014


On 02/10/2014 01:58 PM, Sheldon wrote:
> comments below
>
> On 02/05/2014 10:18 PM, Rodrigo Trujillo wrote:
>> This patch creates functions that allow kimchi users to create an 
>> libvirt
>> SCSI storagepool using the rest API. This patch creates the feature test
>> to check fc_host capability in libvirt. This patch implements basic
>> routines to add a disk (scsi) to a new vm template, based on given
>> volumes (LUN name) from UI or API directly.
>>
>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>> ---
>>   docs/API.md                            |  5 +++-
>>   src/kimchi/API.json                    | 14 ++++++++--
>>   src/kimchi/featuretests.py             | 27 +++++++++++++++++++
>>   src/kimchi/model/config.py             |  5 +++-
>>   src/kimchi/model/host.py               |  4 +--
>>   src/kimchi/model/libvirtstoragepool.py | 48 
>> ++++++++++++++++++++++++++++++++--
>>   src/kimchi/model/storagepools.py       | 22 +++++++++++++---
>>   src/kimchi/model/templates.py          |  5 ++++
>>   src/kimchi/model/vms.py                | 25 ++++++++++++++++--
>>   src/kimchi/vmtemplate.py               | 31 +++++++++++++++++++++-
>>   10 files changed, 172 insertions(+), 14 deletions(-)
>>
>> diff --git a/docs/API.md b/docs/API.md
>> index 580728c..7f0628d 100644
>> --- a/docs/API.md
>> +++ b/docs/API.md
>> @@ -55,6 +55,8 @@ the following general conventions:
>>                        Independent Computing Environments
>>               * null: Graphics is disabled or type not supported
>>           * listen: The network which the vnc/spice server listens on.
>> +    * volumes *(optional)*: List of Fibre channel LUN names to be 
>> assigned as
>> +                            disk to VM. Required if pool is type SCSI.
>>
>>
>>   ### Resource: Virtual Machine
>> @@ -269,7 +271,7 @@ A interface represents available network 
>> interface on VM.
>>   * **POST**: Create a new Storage Pool
>>       * name: The name of the Storage Pool.
>>       * type: The type of the defined Storage Pool.
>> -            Supported types: 'dir', 'kimchi-iso', 'netfs', 
>> 'logical', 'iscsi'
>> +            Supported types: 'dir', 'kimchi-iso', 'netfs', 
>> 'logical', 'iscsi, scsi'
> 'iscsi, scsi' -> 'iscsi', 'scsi' ?
ack

>>       * path: The path of the defined Storage Pool.
>>               For 'kimchi-iso' pool refers to targeted deep scan path.
>>               Pool types: 'dir', 'kimchi-iso'.
>> @@ -288,6 +290,7 @@ A interface represents available network 
>> interface on VM.
>>                                Pool types: 'iscsi'.
>>               * username: Login username of the iSCSI target.
>>               * password: Login password of the iSCSI target.
>> +        * adapter_name: *(optional) Scsi host name.
> * adapter_name *(optional)*: Scsi host name.
ack
>>
>>   ### Resource: Storage Pool
>>
>> diff --git a/src/kimchi/API.json b/src/kimchi/API.json
>> index 08c77c5..842fb11 100644
>> --- a/src/kimchi/API.json
>> +++ b/src/kimchi/API.json
>> @@ -37,7 +37,7 @@
>>                   "type": {
>>                       "description": "The type of the defined Storage 
>> Pool",
>>                       "type": "string",
>> -                    "pattern": "^dir|netfs|logical|kimchi-iso$",
>> +                    "pattern": "^dir|netfs|logical|kimchi-iso|scsi$",
> where is iscsi?
> as you say above:
Good question ! Not sure how the iscsi tag missing passed.
Humm, I think because storagepools_create was not using the validation.
Iscsi should have been committed before my patch to fix validation:
see b4ca2a87b65b66de23b395a4aa30478e28200be7

Anyway .. I will add iscsi
>
> +            Supported types: 'dir', 'kimchi-iso', 'netfs', 'logical', 
> 'iscsi, scsi'
>
>
>>                       "required": true
>>                   },
>>                   "path": {
>> @@ -76,6 +76,10 @@
>>                               "minimum": 1,
>>                               "maximum": 65535
>>                           },
>> +                        "adapter_name": {
>> +                            "description": "SCSI host name",
>> +                            "type": "string"
>> +                        },
>>                           "auth": {
>>                               "description": "Storage back-end 
>> authentication information",
>>                               "type": "object",
>> @@ -112,7 +116,13 @@
>>                       "type": "string",
>>                       "pattern": "^/storagepools/[^/]+/?$"
>>                   },
>> -                "graphics": { "$ref": "#/kimchitype/graphics" }
>> +                "graphics": { "$ref": "#/kimchitype/graphics" },
>> +                "volumes": {
>> +                    "description": "list of scsi volumes to be 
>> assigned to the new VM.",
>> +                    "type": "array",
>> +                    "items": { "type": "string" },
>> +                    "uniqueItems": true
>> +                }
>>               }
>>           },
>>           "vm_update": {
>> diff --git a/src/kimchi/featuretests.py b/src/kimchi/featuretests.py
>> index d924050..f391eb6 100644
>> --- a/src/kimchi/featuretests.py
>> +++ b/src/kimchi/featuretests.py
>> @@ -57,6 +57,18 @@ ISO_STREAM_XML = """
>>     </devices>
>>   </domain>"""
>>
>> +SCSI_FC_XML = """
>> +<pool type='scsi'>
>> +  <name>TEST_SCSI_FC_POOL</name>
>> +  <source>
>> +    <adapter type='fc_host' wwnn='1234567890abcdef' 
>> wwpn='abcdef1234567890'/>
>> +  </source>
>> +  <target>
>> +    <path>/dev/disk/by-path</path>
>> +  </target>
>> +</pool>
>> +"""
>> +
>>
>>   class FeatureTests(object):
>>
>> @@ -150,3 +162,18 @@ class FeatureTests(object):
>>               return False
>>
>>           return True
>> +
>> +    @staticmethod
>> +    def libvirt_support_fc_host():
>> +        try:
>> +            conn = libvirt.open('qemu:///system')
>> +            pool = None
>> +            pool = conn.storagePoolDefineXML(SCSI_FC_XML, 0)
>> +        except libvirt.libvirtError as e:
>> +            if e.get_error_code() == 27:
>> +                # Libvirt requires adapter name, not needed when 
>> supports to FC
>> +                return False
>> +        finally:
>> +            pool is None or pool.undefine()
>> +            conn is None or conn.close()
>> +        return True
>> diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py
>> index 0e66e02..6eb0e10 100644
>> --- a/src/kimchi/model/config.py
>> +++ b/src/kimchi/model/config.py
>> @@ -49,6 +49,7 @@ class CapabilitiesModel(object):
>>           self.qemu_stream = False
>>           self.qemu_stream_dns = False
>>           self.libvirt_stream_protocols = []
>> +        self.fc_host_support = False
>>
>>           # Subscribe function to set host capabilities to be run 
>> when cherrypy
>>           # server is up
>> @@ -60,6 +61,7 @@ class CapabilitiesModel(object):
>>           self.qemu_stream = FeatureTests.qemu_supports_iso_stream()
>>           self.qemu_stream_dns = FeatureTests.qemu_iso_stream_dns()
>>           self.nfs_target_probe = 
>> FeatureTests.libvirt_support_nfs_probe()
>> +        self.fc_host_support = FeatureTests.libvirt_support_fc_host()
>>
>>           self.libvirt_stream_protocols = []
>>           for p in ['http', 'https', 'ftp', 'ftps', 'tftp']:
>> @@ -75,7 +77,8 @@ class CapabilitiesModel(object):
>>           return {'libvirt_stream_protocols': 
>> self.libvirt_stream_protocols,
>>                   'qemu_stream': self.qemu_stream,
>>                   'screenshot': VMScreenshot.get_stream_test_result(),
>> -                'system_report_tool': bool(report_tool)}
>> +                'system_report_tool': bool(report_tool),
>> +                'fc_host_support': self.fc_host_support}
> oh, seems in your 1/5 patch, you have call fc_host_support
removed this ... I am going to use class attribute directly
>>
>>   class DistrosModel(object):
>> diff --git a/src/kimchi/model/host.py b/src/kimchi/model/host.py
>> index 0545a88..816e2e8 100644
>> --- a/src/kimchi/model/host.py
>> +++ b/src/kimchi/model/host.py
>> @@ -218,7 +218,7 @@ class DevicesModel(object):
>>           return dev_names
>>
>>       def _get_devices_fc_host(self):
>> -       conn = self.conn.get()
>> +        conn = self.conn.get()
>>           # Libvirt < 1.0.5 does not support fc_host capability
>>           if not self.fc_host_support:
>>               ret = []
>> @@ -226,7 +226,7 @@ class DevicesModel(object):
>>               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):
>> +                if 'fc_host' in xmlutils.xpath_get_text(xml, path):
>>                       ret.append(host)
>>               return ret
>>           return conn.listDevices('fc_host',0)
>> diff --git a/src/kimchi/model/libvirtstoragepool.py 
>> b/src/kimchi/model/libvirtstoragepool.py
>> index f4dbf2e..ceedbde 100644
>> --- a/src/kimchi/model/libvirtstoragepool.py
>> +++ b/src/kimchi/model/libvirtstoragepool.py
>> @@ -29,8 +29,7 @@ import libvirt
>>   from kimchi.exception import InvalidParameter, OperationFailed, 
>> TimeoutExpired
>>   from kimchi.iscsi import TargetClient
>>   from kimchi.rollbackcontext import RollbackContext
>> -from kimchi.utils import parse_cmd_output, run_command
>> -
>> +from kimchi.utils import kimchi_log, parse_cmd_output, run_command
>>
>>   class StoragePoolDef(object):
>>       @classmethod
>> @@ -175,6 +174,51 @@ class LogicalPoolDef(StoragePoolDef):
>>           return xml
>>
>>
>> +class ScsiPoolDef(StoragePoolDef):
>> +    poolType = 'scsi'
>> +
>> +    def prepare(self, conn=None):
>> +        tmp_name = self.poolArgs['source']['name']
>> +        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'
>> +            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'
>> +
>> +    @property
>> +    def xml(self):
>> +        # Required parameters
>> +        # name:
>> +        # source[adapter_type]:
>> +        # source[name]:
>> +        # source[wwnn]:
>> +        # source[wwpn]:
>> +        # path:
>> +
>> +        xml = """
>> +        <pool type='scsi'>
>> +          <name>{name}</name>
>> +          <source>
>> +            <adapter type='{source[adapter_type]}'\
>> +                     name='{source[name]}'\
>> +                     wwnn='{source[wwnn]}'\
>> +                     wwpn='{source[wwpn]}'/>
>> +          </source>
>> +          <target>
>> +            <path>{path}</path>
>> +          </target>
>> +        </pool>
>> +        """.format(**self.poolArgs)
>> +        return xml
>> +
>> +
>>   class IscsiPoolDef(StoragePoolDef):
>>       poolType = 'iscsi'
>>
>> diff --git a/src/kimchi/model/storagepools.py 
>> b/src/kimchi/model/storagepools.py
>> index 233a8a7..9be7dad 100644
>> --- a/src/kimchi/model/storagepools.py
>> +++ b/src/kimchi/model/storagepools.py
>> @@ -26,6 +26,8 @@ from kimchi import xmlutils
>>   from kimchi.scan import Scanner
>>   from kimchi.exception import InvalidOperation, MissingParameter
>>   from kimchi.exception import NotFoundError, OperationFailed
>> +from kimchi.model.config import CapabilitiesModel
>> +from kimchi.model.host import DeviceModel
>>   from kimchi.model.libvirtstoragepool import StoragePoolDef
>>   from kimchi.utils import add_task, kimchi_log
>>
>> @@ -38,7 +40,11 @@ POOL_STATE_MAP = {0: 'inactive',
>>                     4: 'inaccessible'}
>>
>>   STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name',
>> -                             'path': '/pool/source/dir/@path'}}
>> +                             'path': '/pool/source/dir/@path'},
>> +                   'scsi': {'adapter_type': 
>> '/pool/source/adapter/@type',
>> +                            'adapter_name': 
>> '/pool/source/adapter/@name',
>> +                            'wwnn': '/pool/source/adapter/@wwnn',
>> +                            'wwpn': '/pool/source/adapter/@wwpn'}}
>>
>>
>>   class StoragePoolsModel(object):
>> @@ -47,6 +53,8 @@ class StoragePoolsModel(object):
>>           self.objstore = kargs['objstore']
>>           self.scanner = Scanner(self._clean_scan)
>>           self.scanner.delete()
>> +        self.caps = CapabilitiesModel()
>> +        self.device = DeviceModel(**kargs)
>>
>>       def get_list(self):
>>           try:
>> @@ -67,6 +75,13 @@ class StoragePoolsModel(object):
>>
>>               if params['type'] == 'kimchi-iso':
>>                   task_id = self._do_deep_scan(params)
>> +
>> +            if params['type'] == 'scsi':
>> +                extra_params = self.device.lookup(
>> + params['source']['adapter_name'])
>> +                params['source'].update(extra_params)
>> +                params['fc_host_support'] = self.caps.fc_host_support
>> +
>>               poolDef = StoragePoolDef.create(params)
>>               poolDef.prepare(conn)
>>               xml = poolDef.xml
>> @@ -84,9 +99,10 @@ class StoragePoolsModel(object):
>>                   return name
>>
>>               pool = conn.storagePoolDefineXML(xml, 0)
>> -            if params['type'] in ['logical', 'dir', 'netfs']:
>> +            if params['type'] in ['logical', 'dir', 'netfs', 'scsi']:
>>                   pool.build(libvirt.VIR_STORAGE_POOL_BUILD_NEW)
>> -                # autostart dir and logical storage pool created 
>> from kimchi
>> +                # autostart dir, logical, netfs and scsi storage 
>> pools created
>> +                # from kimchi
>>                   pool.setAutostart(1)
>>               else:
>>                   # disable autostart for others
>> diff --git a/src/kimchi/model/templates.py 
>> b/src/kimchi/model/templates.py
>> index 03632a6..b004578 100644
>> --- a/src/kimchi/model/templates.py
>> +++ b/src/kimchi/model/templates.py
>> @@ -161,6 +161,11 @@ class LibvirtVMTemplate(VMTemplate):
>>           xml = pool.XMLDesc(0)
>>           return xmlutils.xpath_get_text(xml, "/pool/target/path")[0]
>>
>> +    def _get_storage_type(self):
>> +        pool = self._storage_validate()
>> +        xml = pool.XMLDesc(0)
>> +        return xmlutils.xpath_get_text(xml, "/pool/@type")[0]
>> +
>>       def fork_vm_storage(self, vm_uuid):
>>           # Provision storage:
>>           # TODO: Rebase on the storage API once upstream
>> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
>> index d4384a1..4623e28 100644
>> --- a/src/kimchi/model/vms.py
>> +++ b/src/kimchi/model/vms.py
>> @@ -155,6 +155,11 @@ class VMsModel(object):
>>                                  'diskRdKB': diskRdKB,
>>                                  'diskWrKB': diskWrKB})
>>
>> +    def _get_volume_path(self, pool, vol):
>> +         conn = self.conn.get()
>> +         pool = conn.storagePoolLookupByName(pool)
>> +         return pool.storageVolLookupByName(vol).path()
>> +
>>       def create(self, params):
>>           conn = self.conn.get()
>>           t_name = template_name_from_uri(params['template'])
>> @@ -169,6 +174,7 @@ class VMsModel(object):
>>           pool_uri = params.get('storagepool')
>>           if pool_uri:
>>               vm_overrides['storagepool'] = pool_uri
>> +            vm_overrides['fc_host_support'] = self.caps.fc_host_support
>>           t = TemplateModel.get_template(t_name, self.objstore, 
>> self.conn,
>>                                          vm_overrides)
>>
>> @@ -177,7 +183,21 @@ class VMsModel(object):
>>               raise InvalidOperation(err)
>>
>>           t.validate()
>> -        vol_list = t.fork_vm_storage(vm_uuid)
>> +
>> +        # If storagepool is SCSI, volumes will be LUNs and must be 
>> passed by
>> +        # the user from UI or manually.
>> +        vol_list = []
>> +        if t._get_storage_type() == 'scsi':
>> +            if not params.get('volumes'):
>> +                raise InvalidOperation("Volume list (LUNs names) not 
>> given.")
>> +            else:
>> +                # Get system path of the LUNs
>> +                pool = t.info['storagepool'].split('/')[-1]
>> +                for vol in params.get('volumes'):
>> +                    path = self._get_volume_path(pool, vol)
>> +                    vol_list.append((vol, path))
>> +        else:
>> +            vol_list = t.fork_vm_storage(vm_uuid)
>>
>>           # Store the icon for displaying later
>>           icon = t.info.get('icon')
>> @@ -193,7 +213,8 @@ class VMsModel(object):
>>           xml = t.to_vm_xml(name, vm_uuid,
>>                             libvirt_stream=libvirt_stream,
>> qemu_stream_dns=self.caps.qemu_stream_dns,
>> -                          graphics=graphics)
>> +                          graphics=graphics,
>> +                          volumes=vol_list)
>>
>>           try:
>>               conn.defineXML(xml.encode('utf-8'))
>> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
>> index 58147e3..368d0b4 100644
>> --- a/src/kimchi/vmtemplate.py
>> +++ b/src/kimchi/vmtemplate.py
>> @@ -49,6 +49,7 @@ class VMTemplate(object):
>>           """
>>           self.name = args['name']
>>           self.info = {}
>> +        self.fc_host_support = args.get('fc_host_support')
>>
>>           # Identify the cdrom if present
>>           iso_distro = iso_version = 'unknown'
>> @@ -180,6 +181,25 @@ class VMTemplate(object):
>>               graphics_xml = graphics_xml + spicevmc_xml
>>           return graphics_xml
>>
>> +    def _get_scsi_disks_xml(self, luns):
>> +        ret = ""
>> +        # Passthrough configuration
>> +        disk_xml = """
>> +            <disk type='volume' device='lun'>
>> +              <driver name='qemu' type='raw'/>
>> +              <source dev='%(src)s'/>
>> +              <target dev='%(dev)s' bus='scsi'/>
>> +            </disk>"""
>> +        if not self.fc_host_support:
>> +            disk_xml = disk_xml.replace('volume','block')
>> +
>> +        # Creating disk xml for each lun passed
>> +        for index,(lun, path) in enumerate(luns):
>> +            dev = "sd%s" % string.lowercase[index]
>> +            params = {'src': path, 'dev': dev}
>> +            ret = ret + disk_xml % params
>> +        return ret
>> +
>>       def to_volume_list(self, vm_uuid):
>>           storage_path = self._get_storage_path()
>>           ret = []
>> @@ -225,7 +245,6 @@ class VMTemplate(object):
>>           params = dict(self.info)
>>           params['name'] = vm_name
>>           params['uuid'] = vm_uuid
>> -        params['disks'] = self._get_disks_xml(vm_uuid)
>>           params['networks'] = self._get_networks_xml()
>>           params['qemu-namespace'] = ''
>>           params['cdroms'] = ''
>> @@ -233,6 +252,13 @@ class VMTemplate(object):
>>           graphics = kwargs.get('graphics')
>>           params['graphics'] = self._get_graphics_xml(graphics)
>>
>> +        # Current implementation just allows to create disk in one 
>> single
>> +        # storage pool, so we cannot mix the types (scsi volumes vs 
>> img file)
>> +        if self._get_storage_type() == 'scsi':
>> +            params['disks'] = 
>> self._get_scsi_disks_xml(kwargs.get('volumes'))
>> +        else:
>> +            params['disks'] = self._get_disks_xml(vm_uuid)
>> +
>>           qemu_stream_dns = kwargs.get('qemu_stream_dns', False)
>>           libvirt_stream = kwargs.get('libvirt_stream', False)
>>           cdrom_xml = self._get_cdrom_xml(libvirt_stream, 
>> qemu_stream_dns)
>> @@ -292,3 +318,6 @@ class VMTemplate(object):
>>
>>       def _get_storage_path(self):
>>           return ''
>> +
>> +    def _get_storage_type(self):
>> +        return ''
>
>




More information about the Kimchi-devel mailing list