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

On 02/05/2014 12: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(a)linux.vnet.ibm.com&gt; > --- > 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' > * 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. > > ### 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$", > "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 Does it a single value? At least from UI I am just able to select a single LUN to create the vm

> + } > } > }, > "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 You can remove the above line

> + 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 Did you run it when libvirt does not support fc_host? Does libvirt display some error message in this case? If so it would be good to silence the errors by disable_screen_error_logging() enable_screen_error_logging() The same mechanism used for iso streaming tests

> 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} > It is only used in backend. There is no need to expose it to API

> 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 Please, join those changes when you modify the file at first time > 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_','') From API.md the SCSI host will be in "adapter_name" instead of "name"

> + # 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' When do you set the adapter_type to fc_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 '' _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel