From 1991e5768e0ef47d69214063370fd527a6444f7b Mon Sep 17 00:00:00 2001 From: Tom Swanson Date: Tue, 2 Jun 2015 14:02:33 -0500 Subject: [PATCH] Dell SC: Expanded comments and update var names Additional comments have been added to help explain what is happening in the driver. This revolves mostly around dell_storagecenter_api.py which is not an obvious file for a reviewer to review. Hopefully additional comments will aid this. Updated single character variable names except for r which is the REST calls return throughout the code by convention. Reworked create_volume for clarity. No functional changes. Changed two obvious error messages from LOG.debug to LOG.error. Driver version changed to 1.0.2. Change-Id: Ib41d9bc45d20af4fc179b691549fe52f8bd6ec6d --- .../drivers/dell/dell_storagecenter_api.py | 721 ++++++++++++------ .../drivers/dell/dell_storagecenter_common.py | 39 +- .../drivers/dell/dell_storagecenter_fc.py | 7 +- .../drivers/dell/dell_storagecenter_iscsi.py | 2 +- 4 files changed, 531 insertions(+), 238 deletions(-) diff --git a/cinder/volume/drivers/dell/dell_storagecenter_api.py b/cinder/volume/drivers/dell/dell_storagecenter_api.py index 89fb072ed1b..4e190516b7d 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_api.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_api.py @@ -35,7 +35,8 @@ class PayloadFilter(object): Simple class for creating filters for interacting with the Dell Storage API. - Note that this defaults to "AND" filter types. + Note that this defaults to "AND" filter types. This is a pretty limited + class. It only does the trivial filters required for this driver. ''' def __init__(self): @@ -60,6 +61,15 @@ class HttpClient(object): ''' def __init__(self, host, port, user, password, verify): + '''HttpClient handles the REST requests. + + :param host: IP address of the Dell Data Collector. + :param port: Port the Data Collector is listening on. + :param user: User account to login with. + :param password: Password. + :param verify: Boolean indicating whether certificate verification + should be turned on or not. + ''' self.baseUrl = 'https://%s:%s/api/rest/' % (host, port) self.session = requests.Session() self.session.auth = (user, password) @@ -115,17 +125,22 @@ class StorageCenterApiHelper(object): '''StorageCenterApiHelper Helper class for API access. Handles opening and closing the - connection to the Storage Center. + connection to the Dell Enterprise Manager. ''' def __init__(self, config): self.config = config def open_connection(self): + '''Creates the StorageCenterApi object. + + :return: StorageCenterApi object. + :raises: VolumeBackendAPIException + ''' connection = None ssn = self.config.dell_sc_ssn - LOG.info(_LI('open_connection to %(s)s at %(ip)s'), - {'s': ssn, + LOG.info(_LI('open_connection to %(ssn)s at %(ip)s'), + {'ssn': ssn, 'ip': self.config.san_ip}) if ssn: '''Open connection to Enterprise Manager.''' @@ -151,12 +166,21 @@ class StorageCenterApi(object): '''StorageCenterApi - Handles calls to EnterpriseManager via the REST API interface. + Handles calls to Dell Enterprise Manager (EM) via the REST API interface. ''' - APIVERSION = '1.0.1' + APIVERSION = '1.0.2' def __init__(self, host, port, user, password, verify): + '''This creates a connection to Dell Enterprise Manager. + + :param host: IP address of the Dell Data Collector. + :param port: Port the Data Collector is listening on. + :param user: User account to login with. + :param password: Password. + :param verify: Boolean indicating whether certificate verification + should be turned on or not. + ''' self.notes = 'Created by Dell Cinder Driver' self.ssn = None self.vfname = 'openstack' @@ -174,6 +198,11 @@ class StorageCenterApi(object): self.close_connection() def _path_to_array(self, path): + '''Breaks a path into a reversed string array. + + :param path: Path to a folder on the Storage Center. + :return: A reversed array of each path element. + ''' array = [] while True: (path, tail) = os.path.split(path) @@ -183,9 +212,31 @@ class StorageCenterApi(object): array.append(tail) def _first_result(self, blob): + '''Get the first result from the JSON return value. + + :param blob: Full return from a REST call. + :return: The JSON encoded dict or the first item in a JSON encoded + list. + ''' return self._get_result(blob, None, None) def _get_result(self, blob, attribute, value): + '''Find the result specified by attribute and value. + + If the JSON blob is a list then it will be searched for the attribute + and value combination. If attribute and value are not specified then + the the first item is returned. If the JSON blob is a dict then it + will be returned so long as the dict matches the attribute and value + combination or attribute is None. + + :param blob: The REST call's JSON response. Can be a list or dict. + :param attribute: The attribute we are looking for. If it is None + the first item in the list, or the dict, is returned. + :param value: The attribute value we are looking for. If the attribute + is None this value is ignored. + :returns: The JSON content in blob, the dict specified by matching the + attribute and value or None. + ''' rsp = None content = self._get_json(blob) if content is not None: @@ -209,6 +260,11 @@ class StorageCenterApi(object): return rsp def _get_json(self, blob): + '''Returns a dict from the JSON of a REST response. + + :param blob: The response from a REST call. + :returns: JSON or None on error. + ''' try: return blob.json() except AttributeError: @@ -217,6 +273,11 @@ class StorageCenterApi(object): return None def _get_id(self, blob): + '''Returns the instanceId from a Dell REST object. + + :param blob: A Dell SC REST call's response. + :returns: The instanceId from the Dell SC object or None on error. + ''' try: if isinstance(blob, dict): return blob.get('instanceId') @@ -226,31 +287,38 @@ class StorageCenterApi(object): return None def open_connection(self): - # Authenticate against EM + '''Authenticate against Dell Enterprise Manager. + + :raises: VolumeBackendAPIException. + ''' + payload = {} payload['Application'] = 'Cinder REST Driver' payload['ApplicationVersion'] = self.APIVERSION r = self.client.post('ApiConnection/Login', payload) if r.status_code != 200: - LOG.error(_LE('Login error: %(c)d %(r)s'), - {'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('Login error: %(code)d %(reason)s'), + {'code': r.status_code, + 'reason': r.reason}) raise exception.VolumeBackendAPIException( _('Failed to connect to Enterprise Manager')) def close_connection(self): + '''Logout of Dell Enterprise Manager.''' r = self.client.post('ApiConnection/Logout', {}) if r.status_code != 204: - LOG.warning(_LW('Logout error: %(c)d %(r)s'), - {'c': r.status_code, - 'r': r.reason}) + LOG.warning(_LW('Logout error: %(code)d %(reason)s'), + {'code': r.status_code, + 'reason': r.reason}) self.client = None def find_sc(self): - '''This is really just a check that the sc is there and being managed - by EM. + '''Check that the SC is there and being managed by EM. + + :returns: The SC SSN. + :raises: VolumeBackendAPIException ''' r = self.client.get('StorageCenter/StorageCenter') result = self._get_result(r, @@ -268,9 +336,19 @@ class StorageCenterApi(object): # Folder functions def _create_folder(self, url, parent, folder): - '''This is generic to server and volume folders. + '''Creates folder under parent. + + This can create both to server and volume folders. The REST url + sent in defines the folder type being created on the Dell Storage + Center backend. + + :param url: This is the Dell SC rest url for creating the specific + (server or volume) folder type. + :param parent: The instance ID of this folder's parent folder. + :param folder: The folder name to be created. This is one level deep. + :returns: The REST folder object. ''' - f = None + scfolder = None payload = {} payload['Name'] = folder payload['StorageCenter'] = self.ssn @@ -281,53 +359,72 @@ class StorageCenterApi(object): r = self.client.post(url, payload) if r.status_code != 201: - LOG.debug('%(u)s error: %(c)d %(r)s', - {'u': url, - 'c': r.status_code, - 'r': r.reason}) + LOG.debug('%(url)s error: %(code)d %(reason)s', + {'url': url, + 'code': r.status_code, + 'reason': r.reason}) else: - f = self._first_result(r) - return f + scfolder = self._first_result(r) + return scfolder def _create_folder_path(self, url, foldername): - '''This is generic to server and volume folders. + '''Creates a folder path from a fully qualified name. + + The REST url sent in defines the folder type being created on the Dell + Storage Center backend. Thus this is generic to server and volume + folders. + + :param url: This is the Dell SC REST url for creating the specific + (server or volume) folder type. + :param foldername: The full folder name with path. + :returns: The REST folder object. ''' path = self._path_to_array(foldername) folderpath = '' instanceId = '' # Technically the first folder is the root so that is already created. found = True - f = None + scfolder = None for folder in path: folderpath = folderpath + folder # If the last was found see if this part of the path exists too if found: listurl = url + '/GetList' - f = self._find_folder(listurl, - folderpath) - if f is None: + scfolder = self._find_folder(listurl, + folderpath) + if scfolder is None: found = False # We didn't find it so create it if found is False: - f = self._create_folder(url, - instanceId, - folder) + scfolder = self._create_folder(url, + instanceId, + folder) # If we haven't found a folder or created it then leave - if f is None: + if scfolder is None: LOG.error(_LE('Unable to create folder path %s'), folderpath) break # Next part of the path will need this - instanceId = self._get_id(f) + instanceId = self._get_id(scfolder) folderpath = folderpath + '/' - return f + return scfolder def _find_folder(self, url, foldername): - '''Most of the time the folder will already have been created so + '''Find a folder on the SC using the specified url. + + Most of the time the folder will already have been created so we look for the end folder and check that the rest of the path is right. - This is generic to server and volume folders. + The REST url sent in defines the folder type being created on the Dell + Storage Center backend. Thus this is generic to server and volume + folders. + + :param url: The portion of the url after the base url (see http class) + to use for this operation. (Can be for Server or Volume + folders.) + :param foldername: Full path to the folder we are looking for. + :returns: Dell folder object. ''' pf = PayloadFilter() pf.append('scSerialNumber', self.ssn) @@ -347,13 +444,20 @@ class StorageCenterApi(object): 'folderPath', folderpath) else: - LOG.debug('%(u)s error: %(c)d %(r)s', - {'u': url, - 'c': r.status_code, - 'r': r.reason}) + LOG.debug('%(url)s error: %(code)d %(reason)s', + {'url': url, + 'code': r.status_code, + 'reason': r.reason}) return folder def _find_volume_folder(self, create=False): + '''Looks for the volume folder where backend volumes will be created. + + Volume folder is specified in the cindef.conf. See __init. + + :param create: If True will create the folder if not found. + :returns: Folder object. + ''' folder = self._find_folder('StorageCenter/ScVolumeFolder/GetList', self.vfname) # Doesn't exist? make it @@ -363,10 +467,13 @@ class StorageCenterApi(object): return folder def _init_volume(self, scvolume): - '''Maps the volume to a random server and immediately unmaps + '''Initializes the volume. + + Maps the volume to a random server and immediately unmaps it. This initializes the volume. Don't wig out if this fails. + :param scvolume: Dell Volume object. ''' pf = PayloadFilter() pf.append('scSerialNumber', scvolume.get('scSerialNumber'), 'Equals') @@ -385,13 +492,26 @@ class StorageCenterApi(object): scserver) self.unmap_volume(scvolume, scserver) - break + return + # We didn't map/unmap the volume. So no initialization done. + # Warn the user before we leave. Note that this is almost certainly + # a tempest test failure we are trying to catch here. A snapshot + # has likely been attempted before the volume has been instantiated + # on the Storage Center. In the real world no one will snapshot + # a volume without first putting some data in that volume. + LOG.warning(_LW('Volume initialization failure. (%s)'), + self._get_id(scvolume)) def create_volume(self, name, size): - '''This creates a new volume on the storage center. It - will create it in a folder called self.vfname. If self.vfname + '''Creates a new volume on the Storage Center. + + It will create it in a folder called self.vfname. If self.vfname does not exist it will create it. If it cannot create it the volume will be created in the root. + + :param name: Name of the volume to be created on the Dell SC backend. + This is the cinder volume ID. + :returns: Dell Volume object or None. ''' LOG.debug('Create Volume %(name)s %(ssn)s %(folder)s', {'name': name, @@ -421,29 +541,31 @@ class StorageCenterApi(object): payload) if r.status_code == 201: scvolume = self._get_json(r) + if scvolume: + LOG.info(_LI('Created volume %(instanceId)s: %(name)s'), + {'instanceId': scvolume['instanceId'], + 'name': scvolume['name']}) + else: + LOG.error(_LE('ScVolume returned success with empty payload.' + ' Attempting to locate volume')) + # In theory it is there since success was returned. + # Try one last time to find it before returning. + scvolume = self.find_volume(name) else: - LOG.error(_LE('ScVolume create error %(name)s: %(c)d %(r)s'), + LOG.error(_LE('ScVolume create error ' + '%(name)s: %(code)d %(reason)s'), {'name': name, - 'c': r.status_code, - 'r': r.reason}) - return None - if scvolume: - LOG.info(_LI('Created volume %(instanceId)s: %(name)s'), - {'instanceId': scvolume['instanceId'], - 'name': scvolume['name']}) - else: - LOG.error(_LE('ScVolume returned success with empty payload.' - ' Attempting to locate volume')) - # In theory it is there since success was returned. - # Try one last time to find it before returning. - scvolume = self.find_volume(name) + 'code': r.status_code, + 'reason': r.reason}) return scvolume def _get_volume_list(self, name, filterbyvfname=True): - '''Return the list of volumes with name of name. + '''Return the specified list of volumes. + :param name: Volume name. - :param filterbyvfname: If true filters by the preset folder name. + :param filterbyvfname: If set to true then this filters by the preset + folder name. :return: Returns the scvolume or None. ''' result = None @@ -462,10 +584,10 @@ class StorageCenterApi(object): r = self.client.post('StorageCenter/ScVolume/GetList', pf.payload) if r.status_code != 200: - LOG.debug('ScVolume GetList error %(n)s: %(c)d %(r)s', - {'n': name, - 'c': r.status_code, - 'r': r.reason}) + LOG.debug('ScVolume GetList error %(name)s: %(code)d %(reason)s', + {'name': name, + 'code': r.status_code, + 'reason': r.reason}) else: result = self._get_json(r) # We return None if there was an error and a list if the command @@ -474,6 +596,15 @@ class StorageCenterApi(object): def find_volume(self, name): '''Search self.ssn for volume of name. + + This searches the folder self.vfname (specified in the cinder.conf) + for the volume first. If not found it searches the entire array for + the volume. + + :param name: Name of the volume to search for. This is the cinder + volume ID. + :returns: Dell Volume object or None if not found. + :raises VolumeBackendAPIException: If multiple copies are found. ''' LOG.debug('Searching %(sn)s for %(name)s', {'sn': self.ssn, @@ -505,18 +636,26 @@ class StorageCenterApi(object): return None if not vollist else vollist[0] def delete_volume(self, name): - # Delete our volume. + '''Deletes the volume from the SC backend array. + + If the volume cannot be found we claim success. + + :param name: Name of the volume to search for. This is the cinder + volume ID. + :returns: Boolean indicating success or failure. + ''' vol = self.find_volume(name) if vol is not None: r = self.client.delete('StorageCenter/ScVolume/%s' % self._get_id(vol)) if r.status_code != 200: raise exception.VolumeBackendAPIException( - _('Error deleting volume %(ssn)s: %(sn)s: %(c)d %(r)s') % + _('Error deleting volume ' + '%(ssn)s: %(volume)s: %(code)d %(reason)s') % {'ssn': self.ssn, - 'sn': name, - 'c': r.status_code, - 'r': r.reason}) + 'volume': name, + 'code': r.status_code, + 'reason': r.reason}) # json return should be true or false return self._get_json(r) LOG.warning(_LW('delete_volume: unable to find volume %s'), @@ -525,6 +664,14 @@ class StorageCenterApi(object): return True def _find_server_folder(self, create=False): + '''Looks for the server folder on the Dell Storage Center. + + This is the folder where a server objects for mapping volumes will be + created. Server folder is specified in cinder.conf. See __init. + + :param create: If True will create the folder if not found. + :return: Folder object. + ''' folder = self._find_folder('StorageCenter/ScServerFolder/GetList', self.sfname) if folder is None and create is True: @@ -533,8 +680,17 @@ class StorageCenterApi(object): return folder def _add_hba(self, scserver, wwnoriscsiname, isfc=False): - '''Adds an HBA to the scserver. The HBA will be added - even if it has not been seen by the storage center. + '''This adds a server HBA to the Dell server object. + + The HBA is taken from the connector provided in initialize_connection. + The Dell server object is largely a container object for the list of + HBAs associated with a single server (or vm or cluster) for the + purposes of mapping volumes. + + :param scserver: Dell server object. + :param wwnoriscsiname: The WWN or IQN to add to this server. + :param isfc: Boolean indicating whether this is an FC HBA or not. + :returns: Boolean indicating success or failure. ''' payload = {} if isfc is True: @@ -547,19 +703,25 @@ class StorageCenterApi(object): % self._get_id(scserver), payload) if r.status_code != 200: - LOG.error(_LE('AddHba error: %(i)s to %(s)s : %(c)d %(r)s'), - {'i': wwnoriscsiname, - 's': scserver['name'], - 'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('AddHba error: ' + '%(wwn)s to %(srvname)s : %(code)d %(reason)s'), + {'wwn': wwnoriscsiname, + 'srvname': scserver['name'], + 'code': r.status_code, + 'reason': r.reason}) return False return True - # We do not know that we are red hat linux 6.x but that works - # best for red hat and ubuntu. So, there. def _find_serveros(self, osname='Red Hat Linux 6.x'): '''Returns the serveros instance id of the specified osname. - Required to create a server. + + Required to create a Dell server object. + + We do not know that we are Red Hat Linux 6.x but that works + best for Red Hat and Ubuntu. So we use that. + + :param osname: The name of the OS to look for. + :returns: InstanceId of the ScServerOperatingSystem object. ''' pf = PayloadFilter() pf.append('scSerialNumber', self.ssn) @@ -573,16 +735,21 @@ class StorageCenterApi(object): # Found it return the id return self._get_id(srvos) - LOG.warning(_LW('ScServerOperatingSystem GetList return: %(c)d %(r)s'), - {'c': r.status_code, - 'r': r.reason}) + LOG.warning(_LW('ScServerOperatingSystem GetList return: ' + '%(code)d %(reason)s'), + {'code': r.status_code, + 'reason': r.reason}) return None def create_server_multiple_hbas(self, wwns): - '''Same as create_server except it can take a list of hbas. hbas - can be wwns or iqns. + '''Creates a server with multiple WWNS associated with it. + + Same as create_server except it can take a list of HBAs. + + :param wwns: A list of FC WWNs or iSCSI IQNs associated with this + server. + :returns: Dell server object. ''' - # Add hbas scserver = None # Our instance names for wwn in wwns: @@ -598,8 +765,14 @@ class StorageCenterApi(object): return scserver def create_server(self, wwnoriscsiname, isfc=False): - '''creates a server on the the storage center self.ssn. Adds the first - HBA to it. + '''Creates a Dell server object on the the Storage Center. + + Adds the first HBA identified by wwnoriscsiname to it. + + :param wwnoriscsiname: A list of FC WWNs or iSCSI IQNs associated with + this Dell server object. + :param isfc: Boolean indicating whether this is an FC HBA or not. + :returns: Dell server object. ''' scserver = None payload = {} @@ -625,10 +798,11 @@ class StorageCenterApi(object): r = self.client.post('StorageCenter/ScPhysicalServer', payload) if r.status_code != 201: - LOG.error(_LE('ScPhysicalServer create error: %(i)s: %(c)d %(r)s'), - {'i': wwnoriscsiname, - 'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('ScPhysicalServer create error: ' + '%(wwn)s: %(code)d %(reason)s'), + {'wwn': wwnoriscsiname, + 'code': r.status_code, + 'reason': r.reason}) else: # Server was created scserver = self._first_result(r) @@ -646,10 +820,16 @@ class StorageCenterApi(object): return scserver def find_server(self, instance_name): - '''Hunts for a server by looking for an HBA with the server's IQN - or wwn. + '''Hunts for a server on the Dell backend by instance_name. - If found, the server the HBA is attached to, if any, is returned. + The instance_name is the same as the server's HBA. This is the IQN or + WWN listed in the connector. If found, the server the HBA is attached + to, if any, is returned. + + :param instance_name: instance_name is a FC WWN or iSCSI IQN from + the connector. In cinder a server is identified + by its HBA. + :returns: Dell server object or None. ''' scserver = None # We search for our server by first finding our HBA @@ -664,9 +844,9 @@ class StorageCenterApi(object): r = self.client.post('StorageCenter/ScServer/GetList', pf.payload) if r.status_code != 200: - LOG.error(_LE('ScServer error: %(c)d %(r)s'), - {'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('ScServer error: %(code)d %(reason)s'), + {'code': r.status_code, + 'reason': r.reason}) else: scserver = self._first_result(r) if scserver is None: @@ -675,10 +855,14 @@ class StorageCenterApi(object): return scserver def _find_serverhba(self, instance_name): - '''Hunts for a sc server HBA by looking for an HBA with the - server's IQN or wwn. + '''Hunts for a server HBA on the Dell backend by instance_name. - If found, the sc server HBA is returned. + Instance_name is the same as the IQN or WWN specified in the + connector. + + :param instance_name: Instance_name is a FC WWN or iSCSI IQN from + the connector. + :returns: Dell server HBA object. ''' scserverhba = None # We search for our server by first finding our HBA @@ -688,29 +872,42 @@ class StorageCenterApi(object): r = self.client.post('StorageCenter/ScServerHba/GetList', pf.payload) if r.status_code != 200: - LOG.debug('ScServerHba error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('ScServerHba error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) else: scserverhba = self._first_result(r) return scserverhba def _find_domains(self, cportid): + '''Find the list of Dell domain objects associated with the cportid. + + :param cportid: The Instance ID of the Dell controller port. + :returns: List of fault domains associated with this controller port. + ''' r = self.client.get('StorageCenter/ScControllerPort/%s/FaultDomainList' % cportid) if r.status_code == 200: domains = self._get_json(r) return domains else: - LOG.debug('FaultDomainList error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('FaultDomainList error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) LOG.error(_LE('Error getting FaultDomainList')) return None def _find_domain(self, cportid, domainip): - '''Returns the fault domain which a given controller port can - be seen by the server + '''Find the Dell fault domain object on cportid with domainip address. + + Returns the fault domain which a given controller port can + be seen by the server. + + :param cportid: The Instance ID of the Dell controller port. + :param domainip: The IP address specified in the cinder.conf file + for the iSCSI address. + :returns: The fault domain associated with this controller port and + specified domainip or None. ''' domains = self._find_domains(cportid) if domains: @@ -724,9 +921,10 @@ class StorageCenterApi(object): return None def _find_fc_initiators(self, scserver): - '''_find_fc_initiators + '''Returns a list of FC WWNs associated with the specified Dell server. - returns the server's fc HBA's wwns + :param scserver: The Dell backend server object. + :returns: A list of FC WWNs associated with this server. ''' initiators = [] r = self.client.get('StorageCenter/ScServer/%s/HbaList' @@ -739,13 +937,18 @@ class StorageCenterApi(object): wwn is not None): initiators.append(wwn) else: - LOG.debug('HbaList error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('HbaList error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) LOG.error(_LE('Unable to find FC initiators')) return initiators def get_volume_count(self, scserver): + '''Returns the number of volumes attached to specified Dell server. + + :param scserver: The Dell backend server object. + :returns: Mapping count. -1 if there was an error. + ''' r = self.client.get('StorageCenter/ScServer/%s/MappingList' % self._get_id(scserver)) if r.status_code == 200: @@ -755,9 +958,10 @@ class StorageCenterApi(object): return -1 def _find_mappings(self, scvolume): - '''find mappings + '''Find the Dell volume object mappings. - returns the volume's mappings + :param scvolume: Dell volume object. + :returns: A list of Dell mappings objects. ''' mappings = [] if scvolume.get('active', False): @@ -766,9 +970,9 @@ class StorageCenterApi(object): if r.status_code == 200: mappings = self._get_json(r) else: - LOG.debug('MappingList error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('MappingList error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) LOG.error(_LE('Unable to find volume mappings: %s'), scvolume.get('name')) else: @@ -776,9 +980,10 @@ class StorageCenterApi(object): return mappings def _find_controller_port(self, cportid): - '''_find_controller_port + '''Finds the SC controller port object for the specified cportid. - returns the controller port dict + :param cportid: The instanceID of the Dell backend controller port. + :returns: The controller port object. ''' controllerport = None r = self.client.get('StorageCenter/ScControllerPort/%s' @@ -786,16 +991,20 @@ class StorageCenterApi(object): if r.status_code == 200: controllerport = self._first_result(r) else: - LOG.debug('ScControllerPort error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('ScControllerPort error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) LOG.error(_LE('Unable to find controller port: %s'), cportid) return controllerport def find_wwns(self, scvolume, scserver): - '''returns the lun and wwns of the mapped volume''' - # Our returnables + '''Finds the lun and wwns of the mapped volume. + + :param scvolume: Storage Center volume object. + :param scserver: Storage Center server opbject. + :returns: Lun, wwns, initiator target map + ''' lun = None # our lun. We return the first lun. wwns = [] # list of targets itmap = {} # dict of initiators and the associated targets @@ -845,7 +1054,14 @@ class StorageCenterApi(object): return lun, wwns, itmap def _find_active_controller(self, scvolume): - # Find the controller on which scvolume is active. + '''Finds the controller on which the Dell volume is active. + + There can be more than one Dell backend controller per Storage center + but a given volume can only be active on one of them at a time. + + :param scvolume: Dell backend volume object. + :returns: Active controller ID. + ''' actvctrl = None r = self.client.get('StorageCenter/ScVolume/%s/VolumeConfiguration' % self._get_id(scvolume)) @@ -854,9 +1070,9 @@ class StorageCenterApi(object): controller = volconfig.get('controller') actvctrl = self._get_id(controller) else: - LOG.debug('VolumeConfiguration error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('VolumeConfiguration error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) LOG.error(_LE('Unable to retrieve VolumeConfiguration: %s'), self._get_id(scvolume)) LOG.debug('activecontroller %s', actvctrl) @@ -871,7 +1087,7 @@ class StorageCenterApi(object): return self._find_domains(self._get_id(mapping.get('controllerPort'))) def _get_iqn(self, mapping): - # Get our iqn from our controller port. + # Get our iqn from the controller port listed in our our mapping. iqn = None cportid = self._get_id(mapping.get('controllerPort')) controllerport = self._find_controller_port(cportid) @@ -881,6 +1097,16 @@ class StorageCenterApi(object): return iqn def find_iscsi_properties(self, scvolume, ip=None, port=None): + '''Finds target information for a given Dell scvolume object mapping. + + The data coming back is both the preferred path and all the paths. + + :param scvolume: The dell sc volume object. + :param ip: The preferred target portal ip. + :param port: The preferred target portal port. + :returns: iSCSI property dictionary. + :raises: VolumeBackendAPIException + ''' LOG.debug('enter find_iscsi_properties') LOG.debug('scvolume: %s', scvolume) active = -1 @@ -907,11 +1133,11 @@ class StorageCenterApi(object): iqn = self._get_iqn(mapping) ctrlid = self._get_controller_id(mapping) if domains and iqn is not None: - for d in domains: - LOG.debug('domain: %s', d) - ipaddress = d.get('targetIpv4Address', - d.get('wellKnownIpAddress')) - portnumber = d.get('portNumber') + for dom in domains: + LOG.debug('domain: %s', dom) + ipaddress = dom.get('targetIpv4Address', + dom.get('wellKnownIpAddress')) + portnumber = dom.get('portNumber') # We save our portal. portals.append(ipaddress + ':' + six.text_type(portnumber)) @@ -971,10 +1197,14 @@ class StorageCenterApi(object): return data def map_volume(self, scvolume, scserver): - '''map_volume + '''Maps the Dell backend volume object to the Dell server object. - The check for server existence is elsewhere; does not create the - server. + The check for the Dell server object existence is elsewhere; does not + create the Dell server object. + + :param scvolume: Storage Center volume object. + :param scserver: Storage Center server opbject. + :returns: scmapping or None ''' # Make sure we have what we think we have serverid = self._get_id(scserver) @@ -992,9 +1222,9 @@ class StorageCenterApi(object): # We just return our mapping return self._first_result(r) # Should not be here. - LOG.debug('MapToServer error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('MapToServer error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) # Error out LOG.error(_LE('Unable to map %(vol)s to %(srv)s'), {'vol': scvolume['name'], @@ -1002,10 +1232,14 @@ class StorageCenterApi(object): return None def unmap_volume(self, scvolume, scserver): - '''unmap_volume + '''Unmaps the Dell volume object from the Dell server object. - deletes all mappings to a server, not just the ones on the path - defined in cinder.conf. + Deletes all mappings to a Dell server object, not just the ones on + the path defined in cinder.conf. + + :param scvolume: Storage Center volume object. + :param scserver: Storage Center server opbject. + :returns: True or False. ''' rtn = True serverid = self._get_id(scserver) @@ -1022,27 +1256,33 @@ class StorageCenterApi(object): 'StorageCenter/ScMappingProfile/%s' % self._get_id(profile)) if (r.status_code != 200 or r.ok is False): - LOG.debug('ScMappingProfile error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('ScMappingProfile error: ' + '%(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) LOG.error(_LE('Unable to unmap Volume %s'), volumeid) # 1 failed unmap is as good as 100. # Fail it and leave rtn = False break - LOG.debug('Volume %(v)s unmapped from %(s)s', - {'v': volumeid, - 's': serverid}) + LOG.debug('Volume %(vol)s unmapped from %(srv)s', + {'vol': volumeid, + 'srv': serverid}) else: - LOG.debug('MappingProfileList error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('MappingProfileList error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) rtn = False return rtn def get_storage_usage(self): - '''get_storage_usage''' + '''Gets the storage usage object from the Dell backend. + + This contains capacity and usage information for the SC. + + :returns: The SC storageusage object. + ''' storageusage = None if self.ssn is not None: r = self.client.get('StorageCenter/StorageCenter/%s/StorageUsage' @@ -1050,19 +1290,28 @@ class StorageCenterApi(object): if r.status_code == 200: storageusage = self._get_json(r) else: - LOG.debug('StorageUsage error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.debug('StorageUsage error: %(code)d %(reason)s', + {'code': r.status_code, + 'reason': r.reason}) return storageusage def create_replay(self, scvolume, replayid, expire): - '''create_replay + '''Takes a snapshot of a volume. - expire is in minutes. - one could snap a volume before it has been activated, so activate + One could snap a volume before it has been activated, so activate by mapping and unmapping to a random server and let them. This should be a fail but the Tempest tests require it. + + :param scvolume: Volume to snapshot. + :param replayid: Name to use for the snapshot. This is a portion of + the snapshot ID as we do not have space for the + entire GUID in the replay description. + :param expire: Time in minutes before the replay expires. For most + snapshots this will be 0 (never expire) but if we are + cloning a volume we will snap it right before creating + the clone. + :returns: The Dell replay object or None. ''' replay = None if scvolume is not None: @@ -1076,56 +1325,68 @@ class StorageCenterApi(object): % self._get_id(scvolume), payload) if r.status_code != 200: - LOG.debug('CreateReplay error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) - LOG.error(_LE('Error creating replay.')) + LOG.error(_LE('CreateReplay error: %(code)d %(reason)s'), + {'code': r.status_code, + 'reason': r.reason}) else: replay = self._first_result(r) + + # Quick double check. + if replay is None: + LOG.warning(_LW('Unable to create snapshot %s'), + replayid) + # Return replay or None. return replay def find_replay(self, scvolume, replayid): - '''find_replay + '''Searches for the replay by replayid. - searches for the replay by replayid which we store in the - replay's description attribute + replayid is stored in the replay's description attribute. + + :param scvolume: Dell volume object. + :param replayid: Name to search for. This is a portion of the + snapshot ID as we do not have space for the entire + GUID in the replay description. + :returns: Dell replay object or None. ''' - replay = None r = self.client.get('StorageCenter/ScVolume/%s/ReplayList' % self._get_id(scvolume)) try: - content = self._get_json(r) + replays = self._get_json(r) # This will be a list. If it isn't bail - if isinstance(content, list): - for r in content: + if isinstance(replays, list): + for replay in replays: # The only place to save our information with the public # api is the description field which isn't quite long # enough. So we check that our description is pretty much # the max length and we compare that to the start of # the snapshot id. - description = r.get('description') + description = replay.get('description') if (len(description) >= 30 and replayid.startswith(description) is True and - r.get('markedForExpiration') is not True): - replay = r - break + replay.get('markedForExpiration') is not True): + # We found our replay so return it. + return replay except Exception: LOG.error(_LE('Invalid ReplayList return: %s'), r) + # If we are here then we didn't find the replay so warn and leave. + LOG.warning(_LW('Unable to find snapshot %s'), + replayid) - if replay is None: - LOG.debug('Unable to find snapshot %s', - replayid) - - return replay + return None def delete_replay(self, scvolume, replayid): - '''delete_replay + '''Finds a Dell replay by replayid string and expires it. - hunts down a replay by replayid string and expires it. + Once marked for expiration we do not return the replay as a snapshot + even though it might still exist. (Backend requirements.) - once marked for expiration we do not return the replay as - a snapshot. + :param scvolume: Dell volume object. + :param replayid: Name to search for. This is a portion of the snapshot + ID as we do not have space for the entire GUID in the + replay description. + :returns: Boolean for success or failure. ''' LOG.debug('Expiring replay %s', replayid) replay = self.find_replay(scvolume, @@ -1135,17 +1396,19 @@ class StorageCenterApi(object): % self._get_id(replay), {}) if r.status_code != 204: - LOG.debug('ScReplay Expire error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('ScReplay Expire error: %(code)d %(reason)s'), + {'code': r.status_code, + 'reason': r.reason}) return False # We either couldn't find it or expired it. return True def create_view_volume(self, volname, screplay): - '''create_view_volume + '''Creates a new volume named volname from the screplay. - creates a new volume named volname from the screplay. + :param volname: Name of new volume. This is the cinder volume ID. + :param screplay: Dell replay object from which to make a new volume. + :returns: Dell volume object or None. ''' folder = self._find_volume_folder(True) @@ -1162,9 +1425,9 @@ class StorageCenterApi(object): if r.status_code == 200: volume = self._first_result(r) else: - LOG.debug('ScReplay CreateView error: %(c)d %(r)s', - {'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('ScReplay CreateView error: %(code)d %(reason)s'), + {'code': r.status_code, + 'reason': r.reason}) if volume is None: LOG.error(_LE('Unable to create volume %s from replay'), @@ -1173,10 +1436,17 @@ class StorageCenterApi(object): return volume def create_cloned_volume(self, volumename, scvolume): - '''create_cloned_volume + '''Creates a volume named volumename from a copy of scvolume. - creates a temporary replay and then creates a - view volume from that. + This is done by creating a replay and then a view volume from + that replay. The replay is set to expire after an hour. It is only + needed long enough to create the volume. (1 minute should be enough + but we set an hour in case the world has gone mad.) + + + :param volumename: Name of new volume. This is the cinder volume ID. + :param scvolume: Dell volume object. + :returns: The new volume's Dell volume object. ''' clone = None replay = self.create_replay(scvolume, @@ -1190,7 +1460,12 @@ class StorageCenterApi(object): return clone def expand_volume(self, scvolume, newsize): - '''expand_volume''' + '''Expands scvolume to newsize GBs. + + :param scvolume: Dell volume object to be expanded. + :param newsize: The new size of the volume object. + :returns: The updated Dell volume object on success or None on failure. + ''' payload = {} payload['NewSize'] = '%d GB' % newsize r = self.client.post('StorageCenter/ScVolume/%s/ExpandToSize' @@ -1200,42 +1475,60 @@ class StorageCenterApi(object): if r.status_code == 200: vol = self._get_json(r) else: - LOG.error(_LE('Error expanding volume %(n)s: %(c)d %(r)s'), - {'n': scvolume['name'], - 'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('Error expanding volume ' + '%(name)s: %(code)d %(reason)s'), + {'name': scvolume['name'], + 'code': r.status_code, + 'reason': r.reason}) if vol is not None: - LOG.debug('Volume expanded: %(i)s %(s)s', - {'i': vol['instanceId'], - 's': vol['configuredSize']}) + LOG.debug('Volume expanded: %(name)s %(size)s', + {'name': vol['name'], + 'size': vol['configuredSize']}) return vol def rename_volume(self, scvolume, name): + '''Rename scvolume to name. + + This is mostly used by update_migrated_volume. + + :param scvolume: The Dell volume object to be renamed. + :param name: The new volume name. + :returns: Boolean indicating success or failure. + ''' payload = {} payload['Name'] = name r = self.client.post('StorageCenter/ScVolume/%s/Modify' % self._get_id(scvolume), payload) if r.status_code != 200: - LOG.error(_LE('Error renaming volume %(o)s to %(n)s: %(c)d %(r)s'), - {'o': scvolume['name'], - 'n': name, - 'c': r.status_code, - 'r': r.reason}) + LOG.error(_LE('Error renaming volume ' + '%(original)s to %(name)s: %(code)d %(reason)s'), + {'original': scvolume['name'], + 'name': name, + 'code': r.status_code, + 'reason': r.reason}) return False return True def _delete_server(self, scserver): - '''_delete_server + '''Deletes scserver from the backend. - Just give it a shot. If it fails it doesn't matter to cinder. + Just give it a shot. If it fails it doesn't matter to cinder. This + is generally used when a create_server call fails in the middle of + creation. Cinder knows nothing of the servers objects on Dell backends + so success or failure is purely an internal thing. + + Note that we do not delete a server object in normal operation. + + :param scserver: Dell server object to delete. + :returns: Nothing. Only logs messages. ''' if scserver.get('deleteAllowed') is True: r = self.client.delete('StorageCenter/ScServer/%s' % self._get_id(scserver)) - LOG.debug('ScServer %(i)s delete return: %(c)d %(r)s', - {'i': self._get_id(scserver), - 'c': r.status_code, - 'r': r.reason}) + LOG.debug('ScServer %(id)s delete return: %(code)d %(reason)s', + {'id': self._get_id(scserver), + 'code': r.status_code, + 'reason': r.reason}) else: LOG.debug('_delete_server: deleteAllowed is False.') diff --git a/cinder/volume/drivers/dell/dell_storagecenter_common.py b/cinder/volume/drivers/dell/dell_storagecenter_common.py index 596d8811783..f6acb95b1ea 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_common.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_common.py @@ -92,9 +92,9 @@ class DellCommonDriver(san.SanDriver): # We use id as our name as it is unique. volume_name = volume.get('id') volume_size = volume.get('size') - LOG.debug('Creating volume %(n)s of size %(s)s', - {'n': volume_name, - 's': volume_size}) + LOG.debug('Creating volume %(name)s of size %(size)s', + {'name': volume_name, + 'size': volume_size}) scvolume = None with self._client.open_connection() as api: try: @@ -183,9 +183,9 @@ class DellCommonDriver(san.SanDriver): LOG.error(_LE('Failed to create volume %s'), volume_name) if scvolume is not None: - LOG.debug('Volume %(n)s created from %(s)s', - {'n': volume_name, - 's': snapshot_id}) + LOG.debug('Volume %(vol)s created from %(snap)s', + {'vol': volume_name, + 'snap': snapshot_id}) else: raise exception.VolumeBackendAPIException( _('Failed to create volume %s') % volume_name) @@ -210,9 +210,9 @@ class DellCommonDriver(san.SanDriver): LOG.error(_LE('Failed to create volume %s'), volume_name) if scvolume is not None: - LOG.debug('Volume %(n)s cloned from %(s)s', - {'n': volume_name, - 's': src_volume_name}) + LOG.debug('Volume %(vol)s cloned from %(src)s', + {'vol': volume_name, + 'src': src_volume_name}) else: raise exception.VolumeBackendAPIException( _('Failed to create volume %s') % volume_name) @@ -263,7 +263,7 @@ class DellCommonDriver(san.SanDriver): volume_name) if scvolume is None: raise exception.VolumeBackendAPIException( - _('unable to find volume %s') % volume_name) + _('Unable to find volume %s') % volume_name) def remove_export(self, context, volume): '''Remove an export of a volume. @@ -277,7 +277,8 @@ class DellCommonDriver(san.SanDriver): '''Extend the size of the volume.''' volume_name = volume.get('id') LOG.debug('Extending volume %(vol)s to %(size)s', - {'vol': volume_name, 'size': new_size}) + {'vol': volume_name, + 'size': new_size}) if volume is not None: with self._client.open_connection() as api: if api.find_sc(): @@ -323,25 +324,25 @@ class DellCommonDriver(san.SanDriver): data['free_capacity_gb'] = freespacegb data['QoS_support'] = False self._stats = data - LOG.debug('Total cap %(t)s Free cap %(f)s', - {'t': data['total_capacity_gb'], - 'f': data['free_capacity_gb']}) + LOG.debug('Total cap %(total)s Free cap %(free)s', + {'total': data['total_capacity_gb'], + 'free': data['free_capacity_gb']}) def update_migrated_volume(self, ctxt, volume, new_volume): - """Return model update for migrated volume. + '''Return model update for migrated volume. :param volume: The original volume that was migrated to this backend :param new_volume: The migration volume object that was created on this backend as part of the migration process :return model_update to update DB with any needed changes - """ + ''' # We use id as our volume name so we need to rename the backend # volume to the original volume name. original_volume_name = volume.get('id') current_name = new_volume.get('id') - LOG.debug('update_migrated_volume: %(c)s to %(o)s', - {'c': current_name, - 'o': original_volume_name}) + LOG.debug('update_migrated_volume: %(current)s to %(original)s', + {'current': current_name, + 'original': original_volume_name}) if original_volume_name: with self._client.open_connection() as api: if api.find_sc(): diff --git a/cinder/volume/drivers/dell/dell_storagecenter_fc.py b/cinder/volume/drivers/dell/dell_storagecenter_fc.py index 502f21af6c3..2bb3f5a6f85 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_fc.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_fc.py @@ -35,7 +35,7 @@ class DellStorageCenterFCDriver(dell_storagecenter_common.DellCommonDriver, volume_driver=cinder.volume.drivers.dell.DellStorageCenterFCDriver ''' - VERSION = '1.0.1' + VERSION = '1.0.2' def __init__(self, *args, **kwargs): super(DellStorageCenterFCDriver, self).__init__(*args, **kwargs) @@ -95,11 +95,10 @@ class DellStorageCenterFCDriver(dell_storagecenter_common.DellCommonDriver, except Exception: with excutils.save_and_reraise_exception(): - LOG.error(_LE('Failed to initialize connection ')) + LOG.error(_LE('Failed to initialize connection.')) # We get here because our mapping is none so blow up. - raise exception.VolumeBackendAPIException( - _('unable to map volume')) + raise exception.VolumeBackendAPIException(_('Unable to map volume.')) @fczm_utils.RemoveFCZone def terminate_connection(self, volume, connector, force=False, **kwargs): diff --git a/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py b/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py index 141d22ec6e6..32d63248a6f 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py @@ -34,7 +34,7 @@ class DellStorageCenterISCSIDriver(san.SanISCSIDriver, volume_driver=cinder.volume.drivers.dell.DellStorageCenterISCSIDriver ''' - VERSION = '1.0.1' + VERSION = '1.0.2' def __init__(self, *args, **kwargs): super(DellStorageCenterISCSIDriver, self).__init__(*args, **kwargs)