From 03e125a22e2ec493d7dbd0b6d715c763aee7234d Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Mon, 21 Nov 2016 10:52:32 +0200 Subject: [PATCH] resync: HyperV: use os-brick for volume related operations Co-Authored-By: Lucian Petrut (cherry-picked from commit 758a32f7cef6c675b35c04dd8d276c918be188dd) Change-Id: Ie06bfc056ef1ecf5950d780d3e02dab757a73de1 --- hyperv/nova/constants.py | 4 + hyperv/nova/volumeops.py | 591 ++++--------- hyperv/tests/unit/test_livemigrationops.py | 3 +- hyperv/tests/unit/test_volumeops.py | 939 +++++++-------------- requirements.txt | 1 + 5 files changed, 476 insertions(+), 1062 deletions(-) diff --git a/hyperv/nova/constants.py b/hyperv/nova/constants.py index 53a1ffe9..bcfae50b 100644 --- a/hyperv/nova/constants.py +++ b/hyperv/nova/constants.py @@ -97,6 +97,10 @@ FLAVOR_ESPEC_REMOTEFX_VRAM = 'os:vram' IOPS_BASE_SIZE = 8 * units.Ki +STORAGE_PROTOCOL_ISCSI = 'iscsi' +STORAGE_PROTOCOL_FC = 'fibre_channel' +STORAGE_PROTOCOL_SMBFS = 'smbfs' + MAX_CONSOLE_LOG_FILE_SIZE = units.Mi // 2 IMAGE_PROP_SECURE_BOOT = "os_secure_boot" diff --git a/hyperv/nova/volumeops.py b/hyperv/nova/volumeops.py index 0f4a1b94..97e5a397 100644 --- a/hyperv/nova/volumeops.py +++ b/hyperv/nova/volumeops.py @@ -17,27 +17,19 @@ """ Management class for Storage-related functions (attach, detach, etc). """ -import abc -import collections -import os -import platform -import re -import sys +import time import nova.conf from nova import exception from nova import utils from nova.virt import driver -from os_win import exceptions as os_win_exc +from os_brick.initiator import connector from os_win import utilsfactory from oslo_log import log as logging -from oslo_utils import excutils -from oslo_utils import units -import six +from oslo_utils import strutils from hyperv.i18n import _, _LI, _LE, _LW from hyperv.nova import constants -from hyperv.nova import pathutils LOG = logging.getLogger(__name__) @@ -50,29 +42,14 @@ class VolumeOps(object): _SUPPORTED_QOS_SPECS = ['total_bytes_sec', 'min_bytes_sec', 'total_iops_sec', 'min_iops_sec'] - _IOPS_BASE_SIZE = 8 * units.Ki def __init__(self): - self._default_root_device = 'vda' - - self._hostutils = utilsfactory.get_hostutils() self._vmutils = utilsfactory.get_vmutils() - - self._verify_setup() - self.volume_drivers = {'smbfs': SMBFSVolumeDriver(), - 'iscsi': ISCSIVolumeDriver(), - 'fibre_channel': FCVolumeDriver()} - - def _verify_setup(self): - if CONF.hyperv.use_multipath_io: - mpio_enabled = self._hostutils.check_server_feature( - self._hostutils.FEATURE_MPIO) - if not mpio_enabled: - err_msg = _LE( - "Using multipath connections for iSCSI and FC disks " - "requires the Multipath IO Windows feature to be " - "enabled. MPIO must be configured to claim such devices.") - raise exception.ServiceUnavailable(err_msg) + self._default_root_device = 'vda' + self.volume_drivers = { + constants.STORAGE_PROTOCOL_SMBFS: SMBFSVolumeDriver(), + constants.STORAGE_PROTOCOL_ISCSI: ISCSIVolumeDriver(), + constants.STORAGE_PROTOCOL_FC: FCVolumeDriver()} def _get_volume_driver(self, connection_info): driver_type = connection_info.get('driver_volume_type') @@ -84,41 +61,73 @@ class VolumeOps(object): for vol in volumes: self.attach_volume(vol['connection_info'], instance_name) - def attach_volume(self, connection_info, instance_name, - disk_bus=constants.CTRL_TYPE_SCSI): - volume_driver = self._get_volume_driver( - connection_info=connection_info) - - volume_connected = False - try: - volume_driver.connect_volume(connection_info) - volume_connected = True - - volume_driver.attach_volume(connection_info, instance_name, - disk_bus=disk_bus) - - qos_specs = connection_info['data'].get('qos_specs') or {} - if qos_specs: - volume_driver.set_disk_qos_specs(connection_info, - qos_specs) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.exception(_LE('Unable to attach volume to instance %s'), - instance_name) - # Even if the attach failed, some cleanup may be needed. If - # the volume could not be connected, it surely is not attached. - if volume_connected: - volume_driver.detach_volume(connection_info, instance_name) - volume_driver.disconnect_volume(connection_info) - def disconnect_volumes(self, block_device_info): mapping = driver.block_device_info_get_mapping(block_device_info) - for volume in mapping: - connection_info = volume['connection_info'] - volume_driver = self._get_volume_driver(connection_info) - volume_driver.disconnect_volume(connection_info) + for vol in mapping: + self.disconnect_volume(vol['connection_info']) + + def attach_volume(self, connection_info, instance_name, + disk_bus=constants.CTRL_TYPE_SCSI): + tries_left = CONF.hyperv.volume_attach_retry_count + 1 + + while tries_left: + try: + self._attach_volume(connection_info, + instance_name, + disk_bus) + break + except Exception as ex: + tries_left -= 1 + if not tries_left: + LOG.exception( + _LE("Failed to attach volume %(connection_info)s " + "to instance %(instance_name)s. "), + {'connection_info': strutils.mask_dict_password( + connection_info), + 'instance_name': instance_name}) + + self.disconnect_volume(connection_info) + raise exception.VolumeAttachFailed( + volume_id=connection_info['serial'], + reason=ex) + else: + LOG.warning( + _LW("Failed to attach volume %(connection_info)s " + "to instance %(instance_name)s. " + "Tries left: %(tries_left)s."), + {'connection_info': strutils.mask_dict_password( + connection_info), + 'instance_name': instance_name, + 'tries_left': tries_left}) + + time.sleep(CONF.hyperv.volume_attach_retry_interval) + + def _attach_volume(self, connection_info, instance_name, + disk_bus=constants.CTRL_TYPE_SCSI): + LOG.debug( + "Attaching volume: %(connection_info)s to %(instance_name)s", + {'connection_info': strutils.mask_dict_password(connection_info), + 'instance_name': instance_name}) + volume_driver = self._get_volume_driver(connection_info) + volume_driver.attach_volume(connection_info, + instance_name, + disk_bus) + + qos_specs = connection_info['data'].get('qos_specs') or {} + if qos_specs: + volume_driver.set_disk_qos_specs(connection_info, + qos_specs) + + def disconnect_volume(self, connection_info): + volume_driver = self._get_volume_driver(connection_info) + volume_driver.disconnect_volume(connection_info) def detach_volume(self, connection_info, instance_name): + LOG.debug("Detaching volume: %(connection_info)s " + "from %(instance_name)s", + {'connection_info': strutils.mask_dict_password( + connection_info), + 'instance_name': instance_name}) volume_driver = self._get_volume_driver(connection_info) volume_driver.detach_volume(connection_info, instance_name) volume_driver.disconnect_volume(connection_info) @@ -143,17 +152,15 @@ class VolumeOps(object): actual_disk_path) def get_volume_connector(self): - connector = { - 'host': CONF.host, - 'ip': CONF.my_block_storage_ip, - 'multipath': CONF.hyperv.use_multipath_io, - 'os_type': sys.platform, - 'platform': platform.machine(), - } - for volume_driver_type, volume_driver in self.volume_drivers.items(): - connector_updates = volume_driver.get_volume_connector_props() - connector.update(connector_updates) - return connector + # NOTE(lpetrut): the Windows os-brick connectors + # do not use a root helper. + conn = connector.get_connector_properties( + root_helper=None, + my_ip=CONF.my_block_storage_ip, + multipath=CONF.hyperv.use_multipath_io, + enforce_multipath=True, + host=CONF.host) + return conn def connect_volumes(self, block_device_info): mapping = driver.block_device_info_get_mapping(block_device_info) @@ -162,6 +169,25 @@ class VolumeOps(object): volume_driver = self._get_volume_driver(connection_info) volume_driver.connect_volume(connection_info) + def get_disk_path_mapping(self, block_device_info, block_dev_only=False): + block_mapping = driver.block_device_info_get_mapping(block_device_info) + disk_path_mapping = {} + for vol in block_mapping: + connection_info = vol['connection_info'] + disk_serial = connection_info['serial'] + + volume_driver = self._get_volume_driver(connection_info) + if block_dev_only and not volume_driver._is_block_dev: + continue + + disk_path = volume_driver.get_disk_resource_path(connection_info) + disk_path_mapping[disk_serial] = disk_path + return disk_path_mapping + + def get_disk_resource_path(self, connection_info): + volume_driver = self._get_volume_driver(connection_info) + return volume_driver.get_disk_resource_path(connection_info) + @staticmethod def bytes_per_sec_to_iops(no_bytes): # Hyper-v uses normalized IOPS (8 KB increments) @@ -182,57 +208,65 @@ class VolumeOps(object): 'supported_qos_specs': supported_qos_specs}) LOG.warning(msg) - def get_disk_path_mapping(self, block_device_info, block_dev_only=False): - block_mapping = driver.block_device_info_get_mapping(block_device_info) - disk_path_mapping = {} - for vol in block_mapping: - connection_info = vol['connection_info'] - disk_serial = connection_info['serial'] - volume_driver = self._get_volume_driver(connection_info) - if block_dev_only and not volume_driver._is_block_dev: - continue - - disk_path = volume_driver.get_disk_resource_path(connection_info) - disk_path_mapping[disk_serial] = disk_path - return disk_path_mapping - - def get_disk_resource_path(self, connection_info): - volume_driver = self._get_volume_driver(connection_info) - return volume_driver.get_disk_resource_path(connection_info) - - -@six.add_metaclass(abc.ABCMeta) class BaseVolumeDriver(object): - _is_block_dev = True + _protocol = None + _extra_connector_args = {} def __init__(self): - self._vmutils = utilsfactory.get_vmutils() + self._conn = None self._diskutils = utilsfactory.get_diskutils() + self._vmutils = utilsfactory.get_vmutils() + + @property + def _connector(self): + if not self._conn: + scan_attempts = CONF.hyperv.mounted_disk_query_retry_count + scan_interval = CONF.hyperv.mounted_disk_query_retry_interval + + self._conn = connector.InitiatorConnector.factory( + protocol=self._protocol, + root_helper=None, + use_multipath=CONF.hyperv.use_multipath_io, + device_scan_attempts=scan_attempts, + device_scan_interval=scan_interval, + **self._extra_connector_args) + return self._conn def connect_volume(self, connection_info): - pass + return self._connector.connect_volume(connection_info['data']) def disconnect_volume(self, connection_info): - pass + self._connector.disconnect_volume(connection_info['data']) - def get_volume_connector_props(self): - return {} + def get_disk_resource_path(self, connection_info): + disk_paths = self._connector.get_volume_paths(connection_info['data']) + if not disk_paths: + vol_id = connection_info['serial'] + err_msg = _("Could not find disk path. Volume id: %s") + raise exception.DiskNotFound(err_msg % vol_id) - @abc.abstractmethod - def get_disk_resource_path(connection_info): - pass + return self._get_disk_res_path(disk_paths[0]) + + def _get_disk_res_path(self, disk_path): + if self._is_block_dev: + # We need the Msvm_DiskDrive resource path as this + # will be used when the disk is attached to an instance. + disk_number = self._diskutils.get_device_number_from_device_name( + disk_path) + disk_res_path = self._vmutils.get_mounted_disk_by_drive_number( + disk_number) + else: + disk_res_path = disk_path + return disk_res_path def attach_volume(self, connection_info, instance_name, disk_bus=constants.CTRL_TYPE_SCSI): - """Attach a volume to the SCSI controller or to the IDE controller if - ebs_root is True - """ - serial = connection_info['serial'] - # Getting the mounted disk - mounted_disk_path = self.get_disk_resource_path(connection_info) + dev_info = self.connect_volume(connection_info) + serial = connection_info['serial'] + disk_path = self._get_disk_res_path(dev_info['path']) ctrller_path, slot = self._get_disk_ctrl_and_slot(instance_name, disk_bus) if self._is_block_dev: @@ -242,22 +276,22 @@ class BaseVolumeDriver(object): self._vmutils.attach_volume_to_controller(instance_name, ctrller_path, slot, - mounted_disk_path, + disk_path, serial=serial) else: self._vmutils.attach_drive(instance_name, - mounted_disk_path, + disk_path, ctrller_path, slot) def detach_volume(self, connection_info, instance_name): - mounted_disk_path = self.get_disk_resource_path(connection_info) + disk_path = self.get_disk_resource_path(connection_info) LOG.debug("Detaching disk %(disk_path)s " "from instance: %(instance_name)s", - dict(disk_path=mounted_disk_path, + dict(disk_path=disk_path, instance_name=instance_name)) - self._vmutils.detach_vm_disk(instance_name, mounted_disk_path, + self._vmutils.detach_vm_disk(instance_name, disk_path, is_physical=self._is_block_dev) def _get_disk_ctrl_and_slot(self, instance_name, disk_bus): @@ -267,187 +301,34 @@ class BaseVolumeDriver(object): instance_name, 0) # Attaching to the first slot slot = 0 - elif disk_bus == constants.CTRL_TYPE_SCSI: + else: # Find the SCSI controller for the vm ctrller_path = self._vmutils.get_vm_scsi_controller( instance_name) slot = self._vmutils.get_free_controller_slot(ctrller_path) - else: - err_msg = _("Unsupported disk bus requested: %s") - raise exception.Invalid(err_msg % disk_bus) return ctrller_path, slot - def set_disk_qos_specs(self, connection_info, min_iops, max_iops): - volume_type = connection_info.get('driver_volume_type', '') - LOG.warning(_LW("The %s Hyper-V volume driver does not support QoS. " - "Ignoring QoS specs."), volume_type) - - def _check_device_paths(self, device_paths): - if len(device_paths) > 1: - err_msg = _("Multiple disk paths were found: %s. This can " - "occur if multipath is used and MPIO is not " - "properly configured, thus not claiming the device " - "paths. This issue must be addressed urgently as " - "it can lead to data corruption.") - raise exception.InvalidDevicePath(err_msg % device_paths) - elif not device_paths: - err_msg = _("Could not find the physical disk " - "path for the requested volume.") - raise exception.DiskNotFound(err_msg) - - def _get_mounted_disk_path_by_dev_name(self, device_name): - device_number = self._diskutils.get_device_number_from_device_name( - device_name) - mounted_disk_path = self._vmutils.get_mounted_disk_by_drive_number( - device_number) - return mounted_disk_path + def set_disk_qos_specs(self, connection_info, disk_qos_specs): + LOG.info(_LI("The %(protocol)s Hyper-V volume driver " + "does not support QoS. Ignoring QoS specs."), + dict(protocol=self._protocol)) class ISCSIVolumeDriver(BaseVolumeDriver): - def __init__(self): - super(ISCSIVolumeDriver, self).__init__() - self._iscsi_utils = utilsfactory.get_iscsi_initiator_utils() - self._initiator_node_name = self._iscsi_utils.get_iscsi_initiator() + _is_block_dev = True + _protocol = constants.STORAGE_PROTOCOL_ISCSI - self.validate_initiators() + def __init__(self, *args, **kwargs): + self._extra_connector_args = dict( + initiator_list=CONF.hyperv.iscsi_initiator_list) - def get_volume_connector_props(self): - props = {'initiator': self._initiator_node_name} - return props - - def validate_initiators(self): - # The MS iSCSI initiator service can manage the software iSCSI - # initiator as well as hardware initiators. - initiator_list = CONF.hyperv.iscsi_initiator_list - valid_initiators = True - - if not initiator_list: - LOG.info(_LI("No iSCSI initiator was explicitly requested. " - "The Microsoft iSCSI initiator will choose the " - "initiator when estabilishing sessions.")) - else: - available_initiators = self._iscsi_utils.get_iscsi_initiators() - for initiator in initiator_list: - if initiator not in available_initiators: - valid_initiators = False - msg = _LW("The requested initiator %(req_initiator)s " - "is not in the list of available initiators: " - "%(avail_initiators)s.") - LOG.warning(msg, - dict(req_initiator=initiator, - avail_initiators=available_initiators)) - - return valid_initiators - - def _get_all_targets(self, connection_properties): - if all([key in connection_properties for key in ('target_portals', - 'target_iqns', - 'target_luns')]): - return zip(connection_properties['target_portals'], - connection_properties['target_iqns'], - connection_properties['target_luns']) - - return [(connection_properties['target_portal'], - connection_properties['target_iqn'], - connection_properties.get('target_lun', 0))] - - def _get_all_paths(self, connection_properties): - initiator_list = CONF.hyperv.iscsi_initiator_list or [None] - all_targets = self._get_all_targets(connection_properties) - paths = [(initiator_name, target_portal, target_iqn, target_lun) - for target_portal, target_iqn, target_lun in all_targets - for initiator_name in initiator_list] - return paths - - def connect_volume(self, connection_info): - connection_properties = connection_info['data'] - auth_method = connection_properties.get('auth_method') - - if auth_method and auth_method.upper() != 'CHAP': - LOG.error(_LE("Unsupported iSCSI authentication " - "method: %(auth_method)s."), - dict(auth_method=auth_method)) - raise exception.UnsupportedBDMVolumeAuthMethod( - auth_method=auth_method) - - volume_connected = False - for (initiator_name, - target_portal, - target_iqn, - target_lun) in self._get_all_paths(connection_properties): - try: - msg = _LI("Attempting to estabilish an iSCSI session to " - "target %(target_iqn)s on portal %(target_portal)s " - "acessing LUN %(target_lun)s using initiator " - "%(initiator_name)s.") - LOG.info(msg, dict(target_portal=target_portal, - target_iqn=target_iqn, - target_lun=target_lun, - initiator_name=initiator_name)) - self._iscsi_utils.login_storage_target( - target_lun=target_lun, - target_iqn=target_iqn, - target_portal=target_portal, - auth_username=connection_properties.get('auth_username'), - auth_password=connection_properties.get('auth_password'), - mpio_enabled=CONF.hyperv.use_multipath_io, - initiator_name=initiator_name) - - volume_connected = True - if not CONF.hyperv.use_multipath_io: - break - except os_win_exc.OSWinException: - LOG.exception(_LE("Could not connect iSCSI target %s."), - target_iqn) - - if not volume_connected: - raise exception.VolumeAttachFailed( - _("Could not connect volume %s.") % - connection_properties['volume_id']) - - def disconnect_volume(self, connection_info): - # We want to refresh the cached information first. - self._diskutils.rescan_disks() - - for (target_portal, - target_iqn, - target_lun) in self._get_all_targets(connection_info['data']): - - luns = self._iscsi_utils.get_target_luns(target_iqn) - # We disconnect the target only if it does not expose other - # luns which may be in use. - if not luns or luns == [target_lun]: - self._iscsi_utils.logout_storage_target(target_iqn) - - def get_disk_resource_path(self, connection_info): - device_paths = set() - connection_properties = connection_info['data'] - - for (target_portal, - target_iqn, - target_lun) in self._get_all_targets(connection_properties): - - (device_number, - device_path) = self._iscsi_utils.get_device_number_and_path( - target_iqn, target_lun) - if device_path: - device_paths.add(device_path) - - self._check_device_paths(device_paths) - disk_path = list(device_paths)[0] - return self._get_mounted_disk_path_by_dev_name(disk_path) + super(ISCSIVolumeDriver, self).__init__(*args, **kwargs) class SMBFSVolumeDriver(BaseVolumeDriver): - _is_block_dev = False - - def __init__(self): - self._pathutils = pathutils.PathUtils() - self._smbutils = utilsfactory.get_smbutils() - self._username_regex = re.compile(r'user(?:name)?=([^, ]+)') - self._password_regex = re.compile(r'pass(?:word)?=([^, ]+)') - super(SMBFSVolumeDriver, self).__init__() + _protocol = constants.STORAGE_PROTOCOL_SMBFS + _extra_connector_args = dict(local_path_for_loopback=True) def export_path_synchronized(f): def wrapper(inst, connection_info, *args, **kwargs): @@ -459,68 +340,19 @@ class SMBFSVolumeDriver(BaseVolumeDriver): return inner() return wrapper - def get_disk_resource_path(self, connection_info): - return self._get_disk_path(connection_info) - - @export_path_synchronized - def attach_volume(self, connection_info, instance_name, - disk_bus=constants.CTRL_TYPE_SCSI): - super(SMBFSVolumeDriver, self).attach_volume( - connection_info, instance_name, disk_bus) - - @export_path_synchronized - def disconnect_volume(self, connection_info): - # We synchronize share unmount and volume attach operations based on - # the share path in order to avoid the situation when a SMB share is - # unmounted while a volume exported by it is about to be attached to - # an instance. - export_path = self._get_export_path(connection_info) - self._smbutils.unmount_smb_share(export_path) - def _get_export_path(self, connection_info): - return connection_info[ - 'data']['export'].replace('/', '\\').rstrip('\\') + return connection_info['data']['export'].replace('/', '\\') - def _get_disk_path(self, connection_info): - share_addr = self._get_export_path(connection_info) - disk_dir = share_addr + @export_path_synchronized + def attach_volume(self, *args, **kwargs): + super(SMBFSVolumeDriver, self).attach_volume(*args, **kwargs) - if self._smbutils.is_local_share(share_addr): - share_name = share_addr.lstrip('\\').split('\\')[1] - disk_dir = self._smbutils.get_smb_share_path(share_name) - if not disk_dir: - err_msg = _("Could not find the local share path for %s.") - raise exception.DiskNotFound(err_msg % share_addr) - - disk_name = connection_info['data']['name'] - disk_path = os.path.join(disk_dir, disk_name) - return disk_path - - def ensure_share_mounted(self, connection_info): - export_path = self._get_export_path(connection_info) - - if self._smbutils.is_local_share(export_path): - LOG.info(_LI("Skipping mounting share %s, " - "using local path instead."), - export_path) - elif not self._smbutils.check_smb_mapping(export_path): - opts_str = connection_info['data'].get('options') or '' - username, password = self._parse_credentials(opts_str) - self._smbutils.mount_smb_share(export_path, - username=username, - password=password) - - def _parse_credentials(self, opts_str): - match = self._username_regex.findall(opts_str) - username = match[0] if match and match[0] != 'guest' else None - - match = self._password_regex.findall(opts_str) - password = match[0] if match else None - - return username, password - - def connect_volume(self, connection_info): - self.ensure_share_mounted(connection_info) + @export_path_synchronized + def disconnect_volume(self, *args, **kwargs): + # We synchronize those operations based on the share path in order to + # avoid the situation when a SMB share is unmounted while a volume + # exported by it is about to be attached to an instance. + super(SMBFSVolumeDriver, self).disconnect_volume(*args, **kwargs) def set_disk_qos_specs(self, connection_info, qos_specs): supported_qos_specs = ['total_iops_sec', 'total_bytes_sec'] @@ -532,87 +364,10 @@ class SMBFSVolumeDriver(BaseVolumeDriver): total_bytes_sec)) if total_iops_sec: - disk_path = self._get_disk_path(connection_info) + disk_path = self.get_disk_resource_path(connection_info) self._vmutils.set_disk_qos_specs(disk_path, total_iops_sec) class FCVolumeDriver(BaseVolumeDriver): - _MAX_RESCAN_COUNT = 10 - - def __init__(self): - self._fc_utils = utilsfactory.get_fc_utils() - super(FCVolumeDriver, self).__init__() - - def get_volume_connector_props(self): - props = {} - - self._fc_utils.refresh_hba_configuration() - fc_hba_ports = self._fc_utils.get_fc_hba_ports() - - if fc_hba_ports: - wwnns = [] - wwpns = [] - for port in fc_hba_ports: - wwnns.append(port['node_name']) - wwpns.append(port['port_name']) - props['wwpns'] = wwpns - props['wwnns'] = list(set(wwnns)) - return props - - def connect_volume(self, connection_info): - self.get_disk_resource_path(connection_info) - - def get_disk_resource_path(self, connection_info): - for attempt in range(self._MAX_RESCAN_COUNT): - disk_paths = set() - - self._diskutils.rescan_disks() - volume_mappings = self._get_fc_volume_mappings(connection_info) - - LOG.debug("Retrieved volume mappings %(vol_mappings)s " - "for volume %(conn_info)s", - dict(vol_mappings=volume_mappings, - conn_info=connection_info)) - - # Because of MPIO, we may not be able to get the device name - # from a specific mapping if the disk was accessed through - # an other HBA at that moment. In that case, the device name - # will show up as an empty string. - for mapping in volume_mappings: - device_name = mapping['device_name'] - if device_name: - disk_paths.add(device_name) - - if disk_paths: - self._check_device_paths(disk_paths) - disk_path = list(disk_paths)[0] - return self._get_mounted_disk_path_by_dev_name( - disk_path) - - err_msg = _("Could not find the physical disk " - "path for the requested volume.") - raise exception.DiskNotFound(err_msg) - - def _get_fc_volume_mappings(self, connection_info): - # Note(lpetrut): All the WWNs returned by os-win are upper case. - target_wwpns = [wwpn.upper() - for wwpn in connection_info['data']['target_wwn']] - target_lun = connection_info['data']['target_lun'] - - volume_mappings = [] - hba_mapping = self._get_fc_hba_mapping() - for node_name, hba_ports in hba_mapping.items(): - target_mappings = self._fc_utils.get_fc_target_mappings(node_name) - for mapping in target_mappings: - if (mapping['port_name'] in target_wwpns - and mapping['lun'] == target_lun): - volume_mappings.append(mapping) - - return volume_mappings - - def _get_fc_hba_mapping(self): - mapping = collections.defaultdict(list) - fc_hba_ports = self._fc_utils.get_fc_hba_ports() - for port in fc_hba_ports: - mapping[port['node_name']].append(port['port_name']) - return mapping + _is_block_dev = True + _protocol = constants.STORAGE_PROTOCOL_FC diff --git a/hyperv/tests/unit/test_livemigrationops.py b/hyperv/tests/unit/test_livemigrationops.py index 52f922c3..2dafcb89 100644 --- a/hyperv/tests/unit/test_livemigrationops.py +++ b/hyperv/tests/unit/test_livemigrationops.py @@ -138,8 +138,7 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch('hyperv.nova.volumeops.VolumeOps.get_disk_path_mapping') @mock.patch('hyperv.nova.imagecache.ImageCache.get_cached_image') - @mock.patch('hyperv.nova.volumeops.VolumeOps' - '.connect_volumes') + @mock.patch('hyperv.nova.volumeops.VolumeOps.connect_volumes') def _test_pre_live_migration(self, mock_connect_volumes, mock_get_cached_image, mock_get_disk_path_mapping, diff --git a/hyperv/tests/unit/test_volumeops.py b/hyperv/tests/unit/test_volumeops.py index 2db27808..2397b95b 100644 --- a/hyperv/tests/unit/test_volumeops.py +++ b/hyperv/tests/unit/test_volumeops.py @@ -14,16 +14,11 @@ # License for the specific language governing permissions and limitations # under the License. -import copy -import os -import platform -import sys - -import ddt import mock from nova import exception +from nova import test from nova.tests.unit import fake_block_device -from os_win import exceptions as os_win_exc +from os_brick.initiator import connector from oslo_config import cfg from oslo_utils import units @@ -60,18 +55,8 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): def setUp(self): super(VolumeOpsTestCase, self).setUp() self._volumeops = volumeops.VolumeOps() + self._volumeops._volutils = mock.MagicMock() self._volumeops._vmutils = mock.Mock() - self._volumeops._hostutils = mock.Mock() - - def test_verify_setup(self): - self.flags(use_multipath_io=True, group='hyperv') - hostutils = self._volumeops._hostutils - hostutils.check_server_feature.return_value = False - - self.assertRaises(exception.ServiceUnavailable, - self._volumeops._verify_setup) - hostutils.check_server_feature.assert_called_once_with( - hostutils.FEATURE_MPIO) def test_get_volume_driver(self): fake_conn_info = {'driver_volume_type': mock.sentinel.fake_driver_type} @@ -149,136 +134,114 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): mock.sentinel.actual_disk1_path) @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') - def test_attach_volume_exc(self, mock_get_volume_driver): - fake_conn_info = { - 'data': {'qos_specs': mock.sentinel.qos_specs} - } + def test_disconnect_volumes(self, mock_get_volume_driver): + block_device_info = get_fake_block_dev_info() + block_device_mapping = block_device_info['block_device_mapping'] + fake_volume_driver = mock_get_volume_driver.return_value - mock_volume_driver = mock_get_volume_driver.return_value - mock_volume_driver.set_disk_qos_specs.side_effect = ( - exception.NovaException) + self._volumeops.disconnect_volumes(block_device_info) + fake_volume_driver.disconnect_volume.assert_called_once_with( + block_device_mapping[0]['connection_info']) - self.assertRaises(exception.NovaException, - self._volumeops.attach_volume, - fake_conn_info, - mock.sentinel.instance_name, - mock.sentinel.fake_disk_bus) + @mock.patch('time.sleep') + @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') + def _test_attach_volume(self, mock_get_volume_driver, mock_sleep, + attach_failed): + fake_conn_info = get_fake_connection_info( + qos_specs=mock.sentinel.qos_specs) + fake_volume_driver = mock_get_volume_driver.return_value + + expected_try_count = 1 + if attach_failed: + expected_try_count += CONF.hyperv.volume_attach_retry_count + + fake_volume_driver.set_disk_qos_specs.side_effect = ( + test.TestingException) + + self.assertRaises(exception.VolumeAttachFailed, + self._volumeops.attach_volume, + fake_conn_info, + mock.sentinel.inst_name, + mock.sentinel.disk_bus) + else: + self._volumeops.attach_volume( + fake_conn_info, + mock.sentinel.inst_name, + mock.sentinel.disk_bus) + + mock_get_volume_driver.assert_any_call( + fake_conn_info) + fake_volume_driver.attach_volume.assert_has_calls( + [mock.call(fake_conn_info, + mock.sentinel.inst_name, + mock.sentinel.disk_bus)] * expected_try_count) + fake_volume_driver.set_disk_qos_specs.assert_has_calls( + [mock.call(fake_conn_info, + mock.sentinel.qos_specs)] * expected_try_count) + + if attach_failed: + fake_volume_driver.disconnect_volume.assert_called_once_with( + fake_conn_info) + mock_sleep.assert_has_calls( + [mock.call(CONF.hyperv.volume_attach_retry_interval)] * + CONF.hyperv.volume_attach_retry_count) + else: + mock_sleep.assert_not_called() + + def test_attach_volume(self): + self._test_attach_volume(attach_failed=False) + + def test_attach_volume_exc(self): + self._test_attach_volume(attach_failed=True) + + @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') + def test_disconnect_volume(self, mock_get_volume_driver): + fake_volume_driver = mock_get_volume_driver.return_value + + self._volumeops.disconnect_volume(mock.sentinel.conn_info) mock_get_volume_driver.assert_called_once_with( - connection_info=fake_conn_info) - mock_volume_driver.attach_volume.assert_called_once_with( - fake_conn_info, - mock.sentinel.instance_name, - disk_bus=mock.sentinel.fake_disk_bus) - mock_volume_driver.set_disk_qos_specs.assert_called_once_with( - fake_conn_info, mock.sentinel.qos_specs) - - # We check that the volume was detached and disconnected - # after a failed attach attempt. - mock_volume_driver.detach_volume.assert_called_once_with( - fake_conn_info, mock.sentinel.instance_name) - mock_volume_driver.disconnect_volume.assert_called_once_with( - fake_conn_info) + mock.sentinel.conn_info) + fake_volume_driver.disconnect_volume.assert_called_once_with( + mock.sentinel.conn_info) @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') def test_detach_volume(self, mock_get_volume_driver): - self._volumeops.detach_volume(mock.sentinel.conn_info, - mock.sentinel.instance_name) + fake_volume_driver = mock_get_volume_driver.return_value + fake_conn_info = {'data': 'fake_conn_info_data'} + + self._volumeops.detach_volume(fake_conn_info, + mock.sentinel.inst_name) mock_get_volume_driver.assert_called_once_with( - mock.sentinel.conn_info) - mock_volume_driver = mock_get_volume_driver.return_value - mock_volume_driver.detach_volume.assert_called_once_with( - mock.sentinel.conn_info, mock.sentinel.instance_name) - mock_volume_driver.disconnect_volume.assert_called_once_with( - mock.sentinel.conn_info) + fake_conn_info) + fake_volume_driver.detach_volume.assert_called_once_with( + fake_conn_info, mock.sentinel.inst_name) + fake_volume_driver.disconnect_volume.assert_called_once_with( + fake_conn_info) - @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') - def test_disconnect_volumes(self, mock_get_volume_driver): - block_device_info = get_fake_block_dev_info() - conn_info = block_device_info[ - 'block_device_mapping'][0]['connection_info'] + @mock.patch.object(connector, 'get_connector_properties') + def test_get_volume_connector(self, mock_get_connector): + conn = self._volumeops.get_volume_connector() - self._volumeops.disconnect_volumes(block_device_info) - - mock_get_volume_driver.assert_called_once_with(conn_info) - disconnect_volume = ( - mock_get_volume_driver.return_value.disconnect_volume) - disconnect_volume.assert_called_once_with( - conn_info) - - @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') - def test_attach_volume(self, mock_get_volume_driver): - fake_conn_info = { - 'data': {'qos_specs': mock.sentinel.qos_specs} - } - - mock_volume_driver = mock_get_volume_driver.return_value - - self._volumeops.attach_volume(fake_conn_info, - mock.sentinel.instance_name, - disk_bus=mock.sentinel.disk_bus) - - mock_volume_driver.attach_volume.assert_called_once_with( - fake_conn_info, - mock.sentinel.instance_name, - disk_bus=mock.sentinel.disk_bus) - mock_volume_driver.set_disk_qos_specs.assert_called_once_with( - fake_conn_info, mock.sentinel.qos_specs) - - def test_get_volume_connector(self): - fake_connector_props = { - 'fake_vol_driver_specific_prop': - mock.sentinel.vol_driver_specific_val - } - mock_vol_driver = mock.Mock() - mock_vol_driver.get_volume_connector_props.return_value = ( - fake_connector_props) - mock_vol_drivers = { - mock.sentinel.vol_driver_type: mock_vol_driver - } - self._volumeops.volume_drivers = mock_vol_drivers - - connector = self._volumeops.get_volume_connector() - - expected_connector = dict(ip=CONF.my_ip, - host=CONF.host, - multipath=CONF.hyperv.use_multipath_io, - os_type=sys.platform, - platform=platform.machine(), - **fake_connector_props) - self.assertEqual(expected_connector, connector) + mock_get_connector.assert_called_once_with( + root_helper=None, + my_ip=CONF.my_block_storage_ip, + multipath=CONF.hyperv.use_multipath_io, + enforce_multipath=True, + host=CONF.host) + self.assertEqual(mock_get_connector.return_value, conn) @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') def test_connect_volumes(self, mock_get_volume_driver): block_device_info = get_fake_block_dev_info() - conn_info = block_device_info[ - 'block_device_mapping'][0]['connection_info'] self._volumeops.connect_volumes(block_device_info) - mock_get_volume_driver.assert_called_once_with(conn_info) - connect_volume = ( + init_vol_conn = ( mock_get_volume_driver.return_value.connect_volume) - connect_volume.assert_called_once_with( - conn_info) - - def test_bytes_per_sec_to_iops(self): - no_bytes = 15 * units.Ki - expected_iops = 2 - - resulted_iops = self._volumeops.bytes_per_sec_to_iops(no_bytes) - self.assertEqual(expected_iops, resulted_iops) - - @mock.patch.object(volumeops.LOG, 'warning') - def test_validate_qos_specs(self, mock_warning): - supported_qos_specs = [mock.sentinel.spec1, mock.sentinel.spec2] - requested_qos_specs = {mock.sentinel.spec1: mock.sentinel.val, - mock.sentinel.spec3: mock.sentinel.val2} - - self._volumeops.validate_qos_specs(requested_qos_specs, - supported_qos_specs) - self.assertTrue(mock_warning.called) + init_vol_conn.assert_called_once_with( + block_device_info['block_device_mapping'][0]['connection_info']) @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') def test_get_disk_path_mapping(self, mock_get_vol_drv): @@ -323,34 +286,152 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): fake_conn_info) mock_get_volume_driver.assert_called_once_with(fake_conn_info) - fake_volume_driver.get_disk_resource_path.assert_called_once_with( - fake_conn_info) - self.assertEqual( - fake_volume_driver.get_disk_resource_path.return_value, - resulted_disk_path) + get_mounted_disk = fake_volume_driver.get_disk_resource_path + get_mounted_disk.assert_called_once_with(fake_conn_info) + self.assertEqual(get_mounted_disk.return_value, + resulted_disk_path) + + def test_bytes_per_sec_to_iops(self): + no_bytes = 15 * units.Ki + expected_iops = 2 + + resulted_iops = self._volumeops.bytes_per_sec_to_iops(no_bytes) + self.assertEqual(expected_iops, resulted_iops) + + @mock.patch.object(volumeops.LOG, 'warning') + def test_validate_qos_specs(self, mock_warning): + supported_qos_specs = [mock.sentinel.spec1, mock.sentinel.spec2] + requested_qos_specs = {mock.sentinel.spec1: mock.sentinel.val, + mock.sentinel.spec3: mock.sentinel.val2} + + self._volumeops.validate_qos_specs(requested_qos_specs, + supported_qos_specs) + self.assertTrue(mock_warning.called) class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase): """Unit tests for Hyper-V BaseVolumeDriver class.""" - @mock.patch.object(volumeops.BaseVolumeDriver, - '__abstractmethods__', set()) def setUp(self): super(BaseVolumeDriverTestCase, self).setUp() + self._base_vol_driver = volumeops.BaseVolumeDriver() - self._base_vol_driver._vmutils = mock.MagicMock() + + self._base_vol_driver._diskutils = mock.Mock() + self._base_vol_driver._vmutils = mock.Mock() + self._base_vol_driver._conn = mock.Mock() + self._vmutils = self._base_vol_driver._vmutils + self._diskutils = self._base_vol_driver._diskutils + self._conn = self._base_vol_driver._conn + + @mock.patch.object(connector.InitiatorConnector, 'factory') + def test_connector(self, mock_conn_factory): + self._base_vol_driver._conn = None + self._base_vol_driver._protocol = mock.sentinel.protocol + self._base_vol_driver._extra_connector_args = dict( + fake_conn_arg=mock.sentinel.conn_val) + + conn = self._base_vol_driver._connector + + self.assertEqual(mock_conn_factory.return_value, conn) + mock_conn_factory.assert_called_once_with( + protocol=mock.sentinel.protocol, + root_helper=None, + use_multipath=CONF.hyperv.use_multipath_io, + device_scan_attempts=CONF.hyperv.mounted_disk_query_retry_count, + device_scan_interval=( + CONF.hyperv.mounted_disk_query_retry_interval), + **self._base_vol_driver._extra_connector_args) + + def test_connect_volume(self): + conn_info = get_fake_connection_info() + + dev_info = self._base_vol_driver.connect_volume(conn_info) + expected_dev_info = self._conn.connect_volume.return_value + + self.assertEqual(expected_dev_info, dev_info) + self._conn.connect_volume.assert_called_once_with( + conn_info['data']) + + def test_disconnect_volume(self): + conn_info = get_fake_connection_info() + + self._base_vol_driver.disconnect_volume(conn_info) + + self._conn.disconnect_volume.assert_called_once_with( + conn_info['data']) + + @mock.patch.object(volumeops.BaseVolumeDriver, '_get_disk_res_path') + def _test_get_disk_resource_path_by_conn_info(self, + mock_get_disk_res_path, + disk_found=True): + conn_info = get_fake_connection_info() + mock_vol_paths = [mock.sentinel.disk_path] if disk_found else [] + self._conn.get_volume_paths.return_value = mock_vol_paths + + if disk_found: + disk_res_path = self._base_vol_driver.get_disk_resource_path( + conn_info) + + self._conn.get_volume_paths.assert_called_once_with( + conn_info['data']) + self.assertEqual(mock_get_disk_res_path.return_value, + disk_res_path) + mock_get_disk_res_path.assert_called_once_with( + mock.sentinel.disk_path) + else: + self.assertRaises(exception.DiskNotFound, + self._base_vol_driver.get_disk_resource_path, + conn_info) + + def test_get_existing_disk_res_path(self): + self._test_get_disk_resource_path_by_conn_info() + + def test_get_unfound_disk_res_path(self): + self._test_get_disk_resource_path_by_conn_info(disk_found=False) + + def test_get_block_dev_res_path(self): + self._base_vol_driver._is_block_dev = True + + mock_get_dev_number = ( + self._diskutils.get_device_number_from_device_name) + mock_get_dev_number.return_value = mock.sentinel.dev_number + self._vmutils.get_mounted_disk_by_drive_number.return_value = ( + mock.sentinel.disk_path) + + disk_path = self._base_vol_driver._get_disk_res_path( + mock.sentinel.dev_name) + + mock_get_dev_number.assert_called_once_with(mock.sentinel.dev_name) + self._vmutils.get_mounted_disk_by_drive_number.assert_called_once_with( + mock.sentinel.dev_number) + + self.assertEqual(mock.sentinel.disk_path, disk_path) + + def test_get_virt_disk_res_path(self): + # For virtual disk images, we expect the resource path to be the + # actual image path, as opposed to passthrough disks, in which case we + # need the Msvm_DiskDrive resource path when attaching it to a VM. + self._base_vol_driver._is_block_dev = False + + path = self._base_vol_driver._get_disk_res_path( + mock.sentinel.disk_path) + self.assertEqual(mock.sentinel.disk_path, path) @mock.patch.object(volumeops.BaseVolumeDriver, - 'get_disk_resource_path') + '_get_disk_res_path') @mock.patch.object(volumeops.BaseVolumeDriver, '_get_disk_ctrl_and_slot') - def _test_attach_volume(self, mock_get_disk_ctrl_and_slot, - mock_get_disk_resource_path, + @mock.patch.object(volumeops.BaseVolumeDriver, + 'connect_volume') + def _test_attach_volume(self, mock_connect_volume, + mock_get_disk_ctrl_and_slot, + mock_get_disk_res_path, is_block_dev=True): connection_info = get_fake_connection_info() self._base_vol_driver._is_block_dev = is_block_dev - vmutils = self._base_vol_driver._vmutils + mock_connect_volume.return_value = dict(path=mock.sentinel.raw_path) - mock_get_disk_resource_path.return_value = ( + mock_get_disk_res_path.return_value = ( mock.sentinel.disk_path) mock_get_disk_ctrl_and_slot.return_value = ( mock.sentinel.ctrller_path, @@ -362,29 +443,29 @@ class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase): disk_bus=mock.sentinel.disk_bus) if is_block_dev: - vmutils.attach_volume_to_controller.assert_called_once_with( + self._vmutils.attach_volume_to_controller.assert_called_once_with( mock.sentinel.instance_name, mock.sentinel.ctrller_path, mock.sentinel.slot, mock.sentinel.disk_path, serial=connection_info['serial']) else: - vmutils.attach_drive.assert_called_once_with( + self._vmutils.attach_drive.assert_called_once_with( mock.sentinel.instance_name, mock.sentinel.disk_path, mock.sentinel.ctrller_path, mock.sentinel.slot) - mock_get_disk_resource_path.assert_called_once_with( - connection_info) + mock_get_disk_res_path.assert_called_once_with( + mock.sentinel.raw_path) mock_get_disk_ctrl_and_slot.assert_called_once_with( mock.sentinel.instance_name, mock.sentinel.disk_bus) def test_attach_volume_image_file(self): - self._test_attach_volume(is_block_dev=True) + self._test_attach_volume(is_block_dev=False) def test_attach_volume_block_dev(self): - self._test_attach_volume() + self._test_attach_volume(is_block_dev=True) @mock.patch.object(volumeops.BaseVolumeDriver, 'get_disk_resource_path') @@ -396,400 +477,112 @@ class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase): mock_get_disk_resource_path.assert_called_once_with( connection_info) - self._base_vol_driver._vmutils.detach_vm_disk.assert_called_once_with( + self._vmutils.detach_vm_disk.assert_called_once_with( mock.sentinel.instance_name, mock_get_disk_resource_path.return_value, is_physical=self._base_vol_driver._is_block_dev) - def _test_get_disk_ctrl_and_slot(self, disk_bus=constants.CTRL_TYPE_IDE): + def test_get_disk_ctrl_and_slot_ide(self): ctrl, slot = self._base_vol_driver._get_disk_ctrl_and_slot( mock.sentinel.instance_name, - disk_bus) + disk_bus=constants.CTRL_TYPE_IDE) - vmutils = self._base_vol_driver._vmutils - if disk_bus == constants.CTRL_TYPE_IDE: - expected_ctrl = vmutils.get_vm_ide_controller.return_value - expected_slot = 0 + expected_ctrl = self._vmutils.get_vm_ide_controller.return_value + expected_slot = 0 - vmutils.get_vm_ide_controller.assert_called_once_with( - mock.sentinel.instance_name, 0) - else: - expected_ctrl = vmutils.get_vm_scsi_controller.return_value - expected_slot = vmutils.get_free_controller_slot.return_value - - vmutils.get_vm_scsi_controller.assert_called_once_with( - mock.sentinel.instance_name) - vmutils.get_free_controller_slot( - vmutils.get_vm_scsi_controller.return_value) + self._vmutils.get_vm_ide_controller.assert_called_once_with( + mock.sentinel.instance_name, 0) self.assertEqual(expected_ctrl, ctrl) self.assertEqual(expected_slot, slot) - def test_get_disk_ctrl_and_slot_ide(self): - self._test_get_disk_ctrl_and_slot() - def test_get_disk_ctrl_and_slot_scsi(self): - self._test_get_disk_ctrl_and_slot( + ctrl, slot = self._base_vol_driver._get_disk_ctrl_and_slot( + mock.sentinel.instance_name, disk_bus=constants.CTRL_TYPE_SCSI) - def test_get_disk_ctrl_and_slot_unknown(self): - self.assertRaises(exception.Invalid, - self._base_vol_driver._get_disk_ctrl_and_slot, - mock.sentinel.instance_name, - 'fake bus') + expected_ctrl = self._vmutils.get_vm_scsi_controller.return_value + expected_slot = ( + self._vmutils.get_free_controller_slot.return_value) - def test_check_device_paths_multiple_found(self): - device_paths = [mock.sentinel.dev_path_0, mock.sentinel.dev_path_1] - self.assertRaises(exception.InvalidDevicePath, - self._base_vol_driver._check_device_paths, - device_paths) + self._vmutils.get_vm_scsi_controller.assert_called_once_with( + mock.sentinel.instance_name) + self._vmutils.get_free_controller_slot( + self._vmutils.get_vm_scsi_controller.return_value) - def test_check_device_paths_none_found(self): - self.assertRaises(exception.DiskNotFound, - self._base_vol_driver._check_device_paths, - []) + self.assertEqual(expected_ctrl, ctrl) + self.assertEqual(expected_slot, slot) - def test_check_device_paths_one_device_found(self): - self._base_vol_driver._check_device_paths([mock.sentinel.dev_path]) - - def test_get_mounted_disk_by_dev_name(self): - vmutils = self._base_vol_driver._vmutils - diskutils = self._base_vol_driver._diskutils - mock_get_dev_number = diskutils.get_device_number_from_device_name - - mock_get_dev_number.return_value = mock.sentinel.dev_number - vmutils.get_mounted_disk_by_drive_number.return_value = ( - mock.sentinel.disk_path) - - disk_path = self._base_vol_driver._get_mounted_disk_path_by_dev_name( - mock.sentinel.dev_name) - - mock_get_dev_number.assert_called_once_with(mock.sentinel.dev_name) - vmutils.get_mounted_disk_by_drive_number.assert_called_once_with( - mock.sentinel.dev_number) - - self.assertEqual(mock.sentinel.disk_path, disk_path) + def test_set_disk_qos_specs(self): + # This base method is a noop, we'll just make sure + # it doesn't error out. + self._base_vol_driver.set_disk_qos_specs( + mock.sentinel.conn_info, mock.sentinel.disk_qos_spes) -@ddt.ddt class ISCSIVolumeDriverTestCase(test_base.HyperVBaseTestCase): - """Unit tests for Hyper-V ISCSIVolumeDriver class.""" + """Unit tests for Hyper-V BaseVolumeDriver class.""" - def setUp(self): - super(ISCSIVolumeDriverTestCase, self).setUp() - self._volume_driver = volumeops.ISCSIVolumeDriver() - self._iscsi_utils = self._volume_driver._iscsi_utils - self._diskutils = self._volume_driver._diskutils + def test_extra_conn_args(self): + fake_iscsi_initiator = ( + 'PCI\\VEN_1077&DEV_2031&SUBSYS_17E8103C&REV_02\\' + '4&257301f0&0&0010_0') + self.flags(iscsi_initiator_list=[fake_iscsi_initiator], + group='hyperv') + expected_extra_conn_args = dict( + initiator_list=[fake_iscsi_initiator]) - def _test_get_volume_connector_props(self, initiator_present=True): - expected_initiator = self._volume_driver._initiator_node_name - expected_props = dict(initiator=expected_initiator) - resulted_props = self._volume_driver.get_volume_connector_props() - self.assertEqual(expected_props, resulted_props) + vol_driver = volumeops.ISCSIVolumeDriver() - def test_get_vol_connector_props(self): - self._test_get_volume_connector_props() - - def test_get_vol_connector_props_without_initiator(self): - self._test_get_volume_connector_props(initiator_present=False) - - @ddt.data({'requested_initiators': [mock.sentinel.initiator_0], - 'available_initiators': [mock.sentinel.initiator_0, - mock.sentinel.initiator_1]}, - {'requested_initiators': [mock.sentinel.initiator_0], - 'available_initiators': [mock.sentinel.initiator_1]}) - @ddt.unpack - def test_validate_initiators(self, requested_initiators, - available_initiators): - self.flags(iscsi_initiator_list=requested_initiators, group='hyperv') - self._iscsi_utils.get_iscsi_initiators.return_value = ( - available_initiators) - - expected_valid_initiator = not ( - set(requested_initiators).difference(set(available_initiators))) - valid_initiator = self._volume_driver.validate_initiators() - - self.assertEqual(expected_valid_initiator, valid_initiator) - - def test_get_all_targets_multipath(self): - conn_props = {'target_portals': [mock.sentinel.portal0, - mock.sentinel.portal1], - 'target_iqns': [mock.sentinel.target0, - mock.sentinel.target1], - 'target_luns': [mock.sentinel.lun0, - mock.sentinel.lun1]} - expected_targets = zip(conn_props['target_portals'], - conn_props['target_iqns'], - conn_props['target_luns']) - - resulted_targets = self._volume_driver._get_all_targets(conn_props) - self.assertEqual(list(expected_targets), list(resulted_targets)) - - def test_get_all_targets_single_path(self): - conn_props = dict(target_portal=mock.sentinel.portal, - target_iqn=mock.sentinel.target, - target_lun=mock.sentinel.lun) - expected_targets = [ - (mock.sentinel.portal, mock.sentinel.target, mock.sentinel.lun)] - resulted_targets = self._volume_driver._get_all_targets(conn_props) - self.assertEqual(expected_targets, resulted_targets) - - @ddt.data([mock.sentinel.initiator_1, mock.sentinel.initiator_2], []) - @mock.patch.object(volumeops.ISCSIVolumeDriver, '_get_all_targets') - def test_get_all_paths(self, requested_initiators, mock_get_all_targets): - self.flags(iscsi_initiator_list=requested_initiators, group='hyperv') - - target = (mock.sentinel.portal, mock.sentinel.target, - mock.sentinel.lun) - mock_get_all_targets.return_value = [target] - - paths = self._volume_driver._get_all_paths(mock.sentinel.conn_props) - - expected_initiators = requested_initiators or [None] - expected_paths = [(initiator, ) + target - for initiator in expected_initiators] - - self.assertEqual(expected_paths, paths) - mock_get_all_targets.assert_called_once_with(mock.sentinel.conn_props) - - @ddt.data(True, False) - @mock.patch.object(volumeops.ISCSIVolumeDriver, '_get_all_paths') - def test_connect_volume(self, use_multipath, - mock_get_all_paths): - self.flags(use_multipath_io=use_multipath, group='hyperv') - fake_paths = [(mock.sentinel.initiator_name, - mock.sentinel.target_portal, - mock.sentinel.target_iqn, - mock.sentinel.target_lun)] * 3 - - mock_get_all_paths.return_value = fake_paths - self._iscsi_utils.login_storage_target.side_effect = [ - os_win_exc.OSWinException, None, None] - - conn_info = get_fake_connection_info() - conn_props = conn_info['data'] - - self._volume_driver.connect_volume(conn_info) - - mock_get_all_paths.assert_called_once_with(conn_props) - expected_login_attempts = 3 if use_multipath else 2 - self._iscsi_utils.login_storage_target.assert_has_calls( - [mock.call(target_lun=mock.sentinel.target_lun, - target_iqn=mock.sentinel.target_iqn, - target_portal=mock.sentinel.target_portal, - auth_username=conn_props['auth_username'], - auth_password=conn_props['auth_password'], - mpio_enabled=use_multipath, - initiator_name=mock.sentinel.initiator_name)] * - expected_login_attempts) - - @mock.patch.object(volumeops.ISCSIVolumeDriver, '_get_all_paths') - def test_connect_volume_failed(self, mock_get_all_paths): - self.flags(use_multipath_io=True, group='hyperv') - fake_paths = [(mock.sentinel.initiator_name, - mock.sentinel.target_portal, - mock.sentinel.target_iqn, - mock.sentinel.target_lun)] * 3 - - mock_get_all_paths.return_value = fake_paths - self._iscsi_utils.login_storage_target.side_effect = ( - os_win_exc.OSWinException) - - self.assertRaises(exception.VolumeAttachFailed, - self._volume_driver.connect_volume, - get_fake_connection_info()) - - def test_connect_volume_invalid_auth_method(self): - conn_info = get_fake_connection_info(auth_method='fake_auth') - self.assertRaises(exception.UnsupportedBDMVolumeAuthMethod, - self._volume_driver.connect_volume, - conn_info) - - @mock.patch.object(volumeops.ISCSIVolumeDriver, '_get_all_targets') - def test_disconnect_volume(self, mock_get_all_targets): - targets = [ - (mock.sentinel.portal_0, mock.sentinel.tg_0, mock.sentinel.lun_0), - (mock.sentinel.portal_1, mock.sentinel.tg_1, mock.sentinel.lun_1)] - - mock_get_all_targets.return_value = targets - self._iscsi_utils.get_target_luns.return_value = [mock.sentinel.lun_0] - - conn_info = get_fake_connection_info() - self._volume_driver.disconnect_volume(conn_info) - - self._diskutils.rescan_disks.assert_called_once_with() - mock_get_all_targets.assert_called_once_with(conn_info['data']) - self._iscsi_utils.logout_storage_target.assert_called_once_with( - mock.sentinel.tg_0) - self._iscsi_utils.get_target_luns.assert_has_calls( - [mock.call(mock.sentinel.tg_0), mock.call(mock.sentinel.tg_1)]) - - @mock.patch.object(volumeops.ISCSIVolumeDriver, '_get_all_targets') - @mock.patch.object(volumeops.ISCSIVolumeDriver, '_check_device_paths') - @mock.patch.object(volumeops.ISCSIVolumeDriver, - '_get_mounted_disk_path_by_dev_name') - def test_get_disk_resource_path(self, mock_get_mounted_disk, - mock_check_dev_paths, - mock_get_all_targets): - targets = [ - (mock.sentinel.portal_0, mock.sentinel.tg_0, mock.sentinel.lun_0), - (mock.sentinel.portal_1, mock.sentinel.tg_1, mock.sentinel.lun_1)] - - mock_get_all_targets.return_value = targets - self._iscsi_utils.get_device_number_and_path.return_value = [ - mock.sentinel.dev_num, mock.sentinel.dev_path] - - conn_info = get_fake_connection_info() - volume_paths = self._volume_driver.get_disk_resource_path(conn_info) - self.assertEqual(mock_get_mounted_disk.return_value, volume_paths) - - mock_get_all_targets.assert_called_once_with(conn_info['data']) - self._iscsi_utils.get_device_number_and_path.assert_has_calls( - [mock.call(mock.sentinel.tg_0, mock.sentinel.lun_0), - mock.call(mock.sentinel.tg_1, mock.sentinel.lun_1)]) - mock_check_dev_paths.assert_called_once_with( - set([mock.sentinel.dev_path])) - mock_get_mounted_disk.assert_called_once_with(mock.sentinel.dev_path) + self.assertEqual(expected_extra_conn_args, + vol_driver._extra_connector_args) -@ddt.ddt class SMBFSVolumeDriverTestCase(test_base.HyperVBaseTestCase): """Unit tests for the Hyper-V SMBFSVolumeDriver class.""" - _FAKE_SHARE = '//1.2.3.4/fake_share' - _FAKE_SHARE_NORMALIZED = _FAKE_SHARE.replace('/', '\\') - _FAKE_DISK_NAME = 'fake_volume_name.vhdx' - _FAKE_USERNAME = 'fake_username' - _FAKE_PASSWORD = 'fake_password' - _FAKE_SMB_OPTIONS = '-o username=%s,password=%s' % (_FAKE_USERNAME, - _FAKE_PASSWORD) - _FAKE_CONNECTION_INFO = {'data': {'export': _FAKE_SHARE, - 'name': _FAKE_DISK_NAME, - 'options': _FAKE_SMB_OPTIONS, - 'volume_id': 'fake_vol_id'}} + _FAKE_EXPORT_PATH = '//ip/share/' + _FAKE_CONN_INFO = get_fake_connection_info(export=_FAKE_EXPORT_PATH) def setUp(self): super(SMBFSVolumeDriverTestCase, self).setUp() self._volume_driver = volumeops.SMBFSVolumeDriver() - self._volume_driver._vmutils = mock.MagicMock() - self._volume_driver._smbutils = mock.MagicMock() - self._volume_driver._pathutils = mock.MagicMock() - self._smbutils = self._volume_driver._smbutils - self._pathutils = self._volume_driver._pathutils - - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path') - def test_get_disk_resource_path(self, mock_get_disk_path): - disk_path = self._volume_driver.get_disk_resource_path( - mock.sentinel.conn_info) - - self.assertEqual(mock_get_disk_path.return_value, disk_path) - mock_get_disk_path.assert_called_once_with(mock.sentinel.conn_info) - - def test_parse_credentials(self): - username, password = self._volume_driver._parse_credentials( - self._FAKE_SMB_OPTIONS) - self.assertEqual(self._FAKE_USERNAME, username) - self.assertEqual(self._FAKE_PASSWORD, password) + self._volume_driver._conn = mock.Mock() + self._conn = self._volume_driver._conn def test_get_export_path(self): - result = self._volume_driver._get_export_path( - self._FAKE_CONNECTION_INFO) + export_path = self._volume_driver._get_export_path( + self._FAKE_CONN_INFO) + expected_path = self._FAKE_EXPORT_PATH.replace('/', '\\') + self.assertEqual(expected_path, export_path) - expected = self._FAKE_SHARE.replace('/', '\\') - self.assertEqual(expected, result) + @mock.patch.object(volumeops.BaseVolumeDriver, 'attach_volume') + def test_attach_volume(self, mock_attach): + # The tested method will just apply a lock before calling + # the superclass method. + self._volume_driver.attach_volume( + self._FAKE_CONN_INFO, + mock.sentinel.instance_name, + disk_bus=mock.sentinel.disk_bus) - @ddt.data(True, False) - def test_get_disk_path(self, is_local): - fake_local_share_path = 'fake_local_share_path' - self._smbutils.is_local_share.return_value = is_local - self._smbutils.get_smb_share_path.return_value = ( - fake_local_share_path) + mock_attach.assert_called_once_with( + self._FAKE_CONN_INFO, + mock.sentinel.instance_name, + disk_bus=mock.sentinel.disk_bus) - expected_dir = (fake_local_share_path if is_local - else self._FAKE_SHARE_NORMALIZED) - expected_path = os.path.join(expected_dir, - self._FAKE_DISK_NAME) + @mock.patch.object(volumeops.BaseVolumeDriver, 'detach_volume') + def test_detach_volume(self, mock_detach): + self._volume_driver.detach_volume( + self._FAKE_CONN_INFO, + instance_name=mock.sentinel.instance_name) - disk_path = self._volume_driver._get_disk_path( - self._FAKE_CONNECTION_INFO) - - self._smbutils.is_local_share.assert_called_once_with( - self._FAKE_SHARE_NORMALIZED) - if is_local: - self._smbutils.get_smb_share_path.assert_called_once_with( - 'fake_share') - self.assertEqual(expected_path, disk_path) - - def test_get_disk_path_not_found(self): - self._smbutils.is_local_share.return_value = True - self._smbutils.get_smb_share_path.return_value = False - - self.assertRaises(exception.DiskNotFound, - self._volume_driver._get_disk_path, - self._FAKE_CONNECTION_INFO) - - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_parse_credentials') - @ddt.data({}, - {'is_mounted': True}, - {'is_local': True}) - @ddt.unpack - def test_ensure_mounted(self, mock_parse_credentials, - is_mounted=False, is_local=False): - self._smbutils.is_local_share.return_value = is_local - mock_mount_smb_share = self._volume_driver._smbutils.mount_smb_share - mock_check_smb_mapping = ( - self._volume_driver._smbutils.check_smb_mapping) - mock_check_smb_mapping.return_value = is_mounted - mock_parse_credentials.return_value = ( - self._FAKE_USERNAME, self._FAKE_PASSWORD) - - self._volume_driver.ensure_share_mounted( - self._FAKE_CONNECTION_INFO) - - self._smbutils.is_local_share.assert_called_once_with( - self._FAKE_SHARE_NORMALIZED) - - if is_local or is_mounted: - self.assertFalse( - mock_mount_smb_share.called) - else: - mock_parse_credentials.assert_called_once_with( - self._FAKE_SMB_OPTIONS) - mock_mount_smb_share.assert_called_once_with( - self._FAKE_SHARE_NORMALIZED, - username=self._FAKE_USERNAME, - password=self._FAKE_PASSWORD) - - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_parse_credentials') - def test_ensure_mounted_missing_opts(self, mock_parse_credentials): - self._smbutils.is_local_share.return_value = False - mock_mount_smb_share = self._volume_driver._smbutils.mount_smb_share - mock_check_smb_mapping = ( - self._volume_driver._smbutils.check_smb_mapping) - mock_check_smb_mapping.return_value = False - mock_parse_credentials.return_value = (None, None) - - fake_conn_info = copy.deepcopy(self._FAKE_CONNECTION_INFO) - fake_conn_info['data']['options'] = None - - self._volume_driver.ensure_share_mounted(fake_conn_info) - - mock_parse_credentials.assert_called_once_with('') - mock_mount_smb_share.assert_called_once_with( - self._FAKE_SHARE_NORMALIZED, - username=None, - password=None) - - def test_disconnect_volume(self): - self._volume_driver.disconnect_volume(self._FAKE_CONNECTION_INFO) - - mock_unmount_share = self._volume_driver._smbutils.unmount_smb_share - mock_unmount_share.assert_called_once_with( - self._FAKE_SHARE_NORMALIZED) + mock_detach.assert_called_once_with( + self._FAKE_CONN_INFO, + instance_name=mock.sentinel.instance_name) @mock.patch.object(volumeops.VolumeOps, 'bytes_per_sec_to_iops') @mock.patch.object(volumeops.VolumeOps, 'validate_qos_specs') - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path') + @mock.patch.object(volumeops.BaseVolumeDriver, 'get_disk_resource_path') def test_set_disk_qos_specs(self, mock_get_disk_path, mock_validate_qos_specs, mock_bytes_per_sec_to_iops): @@ -800,154 +593,16 @@ class SMBFSVolumeDriverTestCase(test_base.HyperVBaseTestCase): expected_supported_specs = ['total_iops_sec', 'total_bytes_sec'] mock_set_qos_specs = self._volume_driver._vmutils.set_disk_qos_specs mock_bytes_per_sec_to_iops.return_value = fake_total_iops_sec + mock_get_disk_path.return_value = mock.sentinel.disk_path - self._volume_driver.set_disk_qos_specs(mock.sentinel.connection_info, + self._volume_driver.set_disk_qos_specs(self._FAKE_CONN_INFO, storage_qos_specs) mock_validate_qos_specs.assert_called_once_with( storage_qos_specs, expected_supported_specs) mock_bytes_per_sec_to_iops.assert_called_once_with( fake_total_bytes_sec) - mock_disk_path = mock_get_disk_path.return_value - mock_get_disk_path.assert_called_once_with( - mock.sentinel.connection_info) + mock_get_disk_path.assert_called_once_with(self._FAKE_CONN_INFO) mock_set_qos_specs.assert_called_once_with( - mock_disk_path, + mock.sentinel.disk_path, fake_total_iops_sec) - - -class FCVolumeDriverTestCase(test_base.HyperVBaseTestCase): - def setUp(self): - super(FCVolumeDriverTestCase, self).setUp() - self._fc_driver = volumeops.FCVolumeDriver() - self._fc_driver._fc_utils = mock.MagicMock() - self._fc_driver._vmutils = mock.MagicMock() - - self._fc_utils = self._fc_driver._fc_utils - self._vmutils = self._fc_driver._vmutils - - def _test_get_volume_connector_props(self, valid_fc_hba_ports=True): - fake_fc_hba_ports = [{'node_name': mock.sentinel.node_name, - 'port_name': mock.sentinel.port_name}, - {'node_name': mock.sentinel.second_node_name, - 'port_name': mock.sentinel.second_port_name}] - self._fc_utils.get_fc_hba_ports.return_value = ( - fake_fc_hba_ports if valid_fc_hba_ports else []) - - resulted_fc_hba_ports = self._fc_driver.get_volume_connector_props() - - self._fc_utils.refresh_hba_configuration.assert_called_once_with() - self._fc_utils.get_fc_hba_ports.assert_called_once_with() - - if valid_fc_hba_ports: - expected_fc_hba_ports = { - 'wwpns': [mock.sentinel.port_name, - mock.sentinel.second_port_name], - 'wwnns': [mock.sentinel.node_name, - mock.sentinel.second_node_name] - } - else: - expected_fc_hba_ports = {} - - self.assertItemsEqual(expected_fc_hba_ports, resulted_fc_hba_ports) - - def test_get_volume_connector_props(self): - self._test_get_volume_connector_props() - - def test_get_volume_connector_props_missing_hbas(self): - self._test_get_volume_connector_props(valid_fc_hba_ports=False) - - @mock.patch.object(volumeops.FCVolumeDriver, 'get_disk_resource_path') - def test_connect_volume(self, mock_get_disk_path): - self._fc_driver.connect_volume(mock.sentinel.conn_info) - mock_get_disk_path.assert_called_once_with(mock.sentinel.conn_info) - - @mock.patch.object(volumeops.FCVolumeDriver, - '_get_mounted_disk_path_by_dev_name') - @mock.patch.object(volumeops.FCVolumeDriver, '_get_fc_volume_mappings') - @mock.patch.object(volumeops.FCVolumeDriver, '_check_device_paths') - def _test_get_disk_resource_path(self, mock_check_dev_paths, - mock_get_fc_mappings, - mock_get_disk_path_by_dev, - fc_mappings_side_effect, - expected_rescan_count, - retrieved_dev_name=None): - mock_get_fc_mappings.side_effect = fc_mappings_side_effect - mock_get_disk_path_by_dev.return_value = mock.sentinel.disk_path - - if retrieved_dev_name: - disk_path = self._fc_driver.get_disk_resource_path( - mock.sentinel.conn_info) - self.assertEqual(mock.sentinel.disk_path, disk_path) - mock_check_dev_paths.assert_called_once_with( - set([retrieved_dev_name])) - mock_get_disk_path_by_dev.assert_called_once_with( - retrieved_dev_name) - else: - self.assertRaises( - exception.DiskNotFound, - self._fc_driver.get_disk_resource_path, - mock.sentinel.conn_info) - - mock_get_fc_mappings.assert_any_call(mock.sentinel.conn_info) - self.assertEqual( - expected_rescan_count, - self._fc_driver._diskutils.rescan_disks.call_count) - - def test_get_disk_resource_path_missing_dev_name(self): - mock_mapping = dict(device_name='') - fc_mappings_side_effect = [[]] + [[mock_mapping]] * 9 - - self._test_get_disk_resource_path( - fc_mappings_side_effect=fc_mappings_side_effect, - expected_rescan_count=self._fc_driver._MAX_RESCAN_COUNT) - - def test_get_disk_resource_path_dev_name_found(self): - dev_name = mock.sentinel.dev_name - mock_mapping = dict(device_name=dev_name) - - self._test_get_disk_resource_path( - fc_mappings_side_effect=[[mock_mapping]], - expected_rescan_count=1, - retrieved_dev_name=dev_name) - - @mock.patch.object(volumeops.FCVolumeDriver, '_get_fc_hba_mapping') - def test_get_fc_volume_mappings(self, mock_get_fc_hba_mapping): - fake_target_wwpn = 'FAKE_TARGET_WWPN' - connection_info = get_fake_connection_info( - target_lun=mock.sentinel.target_lun, - target_wwn=[fake_target_wwpn]) - - mock_hba_mapping = {mock.sentinel.node_name: mock.sentinel.hba_ports} - mock_get_fc_hba_mapping.return_value = mock_hba_mapping - - all_target_mappings = [{'device_name': mock.sentinel.dev_name, - 'port_name': fake_target_wwpn, - 'lun': mock.sentinel.target_lun}, - {'device_name': mock.sentinel.dev_name_1, - 'port_name': mock.sentinel.target_port_name_1, - 'lun': mock.sentinel.target_lun}, - {'device_name': mock.sentinel.dev_name, - 'port_name': mock.sentinel.target_port_name, - 'lun': mock.sentinel.target_lun_1}] - expected_mappings = [all_target_mappings[0]] - - self._fc_utils.get_fc_target_mappings.return_value = ( - all_target_mappings) - - volume_mappings = self._fc_driver._get_fc_volume_mappings( - connection_info) - self.assertEqual(expected_mappings, volume_mappings) - - def test_get_fc_hba_mapping(self): - fake_fc_hba_ports = [{'node_name': mock.sentinel.node_name, - 'port_name': mock.sentinel.port_name}] - - self._fc_utils.get_fc_hba_ports.return_value = fake_fc_hba_ports - - resulted_mapping = self._fc_driver._get_fc_hba_mapping() - - expected_mapping = volumeops.collections.defaultdict(list) - expected_mapping[mock.sentinel.node_name].append( - mock.sentinel.port_name) - self.assertEqual(expected_mapping, resulted_mapping) diff --git a/requirements.txt b/requirements.txt index bd063140..b52e205a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,6 +5,7 @@ pbr>=1.8 # Apache-2.0 Babel>=2.3.4 # BSD +os-brick>=1.8.0 # Apache-2.0 os-win>=1.1.0 # Apache-2.0 oslo.config!=3.18.0,>=3.14.0 # Apache-2.0 oslo.log>=3.11.0 # Apache-2.0