From 758a32f7cef6c675b35c04dd8d276c918be188dd Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 8 Dec 2015 04:39:17 -0800 Subject: HyperV: use os-brick for volume related operations This patch refactors volumeops.py to use os-brick for volume related operations. The immediate benefits are: * FC support * improved iSCSI MPIO support * simplified volume connect/disconnect workflow Change-Id: Ib21947141aadca1fa6cb99afc07a175ce14d192e Depends-On: I19dfc8dd2e9e8a1b17675b55c63de903804480e4 Implements: blueprint hyperv-use-os-brick --- nova/conf/hyperv.py | 34 +- nova/tests/unit/virt/hyperv/test_driver.py | 3 +- .../unit/virt/hyperv/test_livemigrationops.py | 3 +- nova/tests/unit/virt/hyperv/test_volumeops.py | 711 +++++++++------------ nova/virt/hyperv/constants.py | 4 + nova/virt/hyperv/driver.py | 2 +- nova/virt/hyperv/livemigrationops.py | 2 +- nova/virt/hyperv/volumeops.py | 525 ++++++--------- .../bp-hyperv-use-os-brick-bf576a5bc97f0ea2.yaml | 11 + 9 files changed, 563 insertions(+), 732 deletions(-) create mode 100644 releasenotes/notes/bp-hyperv-use-os-brick-bf576a5bc97f0ea2.yaml diff --git a/nova/conf/hyperv.py b/nova/conf/hyperv.py index 159f0ba203..3ee2589f2e 100644 --- a/nova/conf/hyperv.py +++ b/nova/conf/hyperv.py @@ -84,11 +84,9 @@ in order to limit the CPU features used by the instance. help=""" Mounted disk query retry count -The number of times to retry checking for a disk mounted via iSCSI. -During long stress runs the WMI query that is looking for the iSCSI -device number can incorrectly return no data. If the query is -retried the appropriate data can then be obtained. The query runs -until the device can be found or the retry count is reached. +The number of times to retry checking for a mounted disk. +The query runs until the device can be found or the retry +count is reached. Possible values: @@ -106,7 +104,7 @@ Related options: help=""" Mounted disk query retry interval -Interval between checks for a mounted iSCSI disk, in seconds. +Interval between checks for a mounted disk, in seconds. Possible values: @@ -259,12 +257,8 @@ Related options: help=""" Volume attach retry count -The number of times to retry to attach a volume. This option is used -to avoid incorrectly returned no data when the system is under load. -Volume attachment is retried until success or the given retry count -is reached. To prepare the Hyper-V node to be able to attach to -volumes provided by cinder you must first make sure the Windows iSCSI -initiator service is running and started automatically. +The number of times to retry attaching a volume. Volume attachment +is retried until success or the given retry count is reached. Possible values: @@ -321,6 +315,22 @@ extra specs: Windows / Hyper-V Server 2016. Acceptable values:: 64, 128, 256, 512, 1024 +"""), + cfg.BoolOpt('use_multipath_io', + default=False, + help=""" +Use multipath connections when attaching iSCSI or FC disks. + +This requires the Multipath IO Windows feature to be enabled. MPIO must be +configured to claim such devices. +"""), + cfg.ListOpt('iscsi_initiator_list', + default=[], + help=""" +List of iSCSI initiators that will be used for estabilishing iSCSI sessions. + +If none are specified, the Microsoft iSCSI initiator service will choose the +initiator. """) ] diff --git a/nova/tests/unit/virt/hyperv/test_driver.py b/nova/tests/unit/virt/hyperv/test_driver.py index 4a57858722..1d10878935 100644 --- a/nova/tests/unit/virt/hyperv/test_driver.py +++ b/nova/tests/unit/virt/hyperv/test_driver.py @@ -196,8 +196,7 @@ class HyperVDriverTestCase(test_base.HyperVBaseTestCase): def test_get_volume_connector(self): self.driver.get_volume_connector(mock.sentinel.instance) - self.driver._volumeops.get_volume_connector.assert_called_once_with( - mock.sentinel.instance) + self.driver._volumeops.get_volume_connector.assert_called_once_with() def test_get_available_resource(self): self.driver.get_available_resource(mock.sentinel.nodename) diff --git a/nova/tests/unit/virt/hyperv/test_livemigrationops.py b/nova/tests/unit/virt/hyperv/test_livemigrationops.py index b27aa6bf6c..8979473da6 100644 --- a/nova/tests/unit/virt/hyperv/test_livemigrationops.py +++ b/nova/tests/unit/virt/hyperv/test_livemigrationops.py @@ -135,8 +135,7 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase): @mock.patch('nova.virt.hyperv.volumeops.VolumeOps.get_disk_path_mapping') @mock.patch('nova.virt.hyperv.imagecache.ImageCache.get_cached_image') - @mock.patch('nova.virt.hyperv.volumeops.VolumeOps' - '.initialize_volumes_connection') + @mock.patch('nova.virt.hyperv.volumeops.VolumeOps.connect_volumes') def _test_pre_live_migration(self, mock_initialize_connection, mock_get_cached_image, mock_get_disk_path_mapping, diff --git a/nova/tests/unit/virt/hyperv/test_volumeops.py b/nova/tests/unit/virt/hyperv/test_volumeops.py index f0e42d1c40..22d1b1aa50 100644 --- a/nova/tests/unit/virt/hyperv/test_volumeops.py +++ b/nova/tests/unit/virt/hyperv/test_volumeops.py @@ -14,10 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. -import os - import mock -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 @@ -138,58 +136,114 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): 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'] - block_device_mapping[0]['connection_info'] = { - 'driver_volume_type': mock.sentinel.fake_vol_type} fake_volume_driver = mock_get_volume_driver.return_value self._volumeops.disconnect_volumes(block_device_info) - fake_volume_driver.disconnect_volumes.assert_called_once_with( - block_device_mapping) + fake_volume_driver.disconnect_volume.assert_called_once_with( + block_device_mapping[0]['connection_info']) + @mock.patch('time.sleep') @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} - } + 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 - mock_volume_driver = mock_get_volume_driver.return_value + expected_try_count = 1 + if attach_failed: + expected_try_count += CONF.hyperv.volume_attach_retry_count - self._volumeops.attach_volume(fake_conn_info, - mock.sentinel.instance_name, - disk_bus=mock.sentinel.disk_bus) + fake_volume_driver.set_disk_qos_specs.side_effect = ( + test.TestingException) - 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) + 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) - def test_get_volume_connector(self): - mock_instance = mock.DEFAULT - initiator = self._volumeops._volutils.get_iscsi_initiator.return_value - expected = {'ip': CONF.my_ip, - 'host': CONF.host, - 'initiator': initiator} + 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() - response = self._volumeops.get_volume_connector(instance=mock_instance) + def test_attach_volume(self): + self._test_attach_volume(attach_failed=False) - self._volumeops._volutils.get_iscsi_initiator.assert_called_once_with() - self.assertEqual(expected, response) + def test_attach_volume_exc(self): + self._test_attach_volume(attach_failed=True) @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') - def test_initialize_volumes_connection(self, mock_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( + 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): + 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( + 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(connector, 'get_connector_properties') + def test_get_volume_connector(self, mock_get_connector): + conn = self._volumeops.get_volume_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() - self._volumeops.initialize_volumes_connection(block_device_info) + self._volumeops.connect_volumes(block_device_info) init_vol_conn = ( - mock_get_volume_driver.return_value.initialize_volume_connection) + mock_get_volume_driver.return_value.connect_volume) init_vol_conn.assert_called_once_with( block_device_info['block_device_mapping'][0]['connection_info']) @mock.patch.object(volumeops.VolumeOps, - 'get_mounted_disk_path_from_volume') + 'get_disk_resource_path') def test_get_disk_path_mapping(self, mock_get_disk_path): block_device_info = get_fake_block_dev_info() block_device_mapping = block_device_info['block_device_mapping'] @@ -208,27 +262,16 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): self.assertEqual(expected_disk_path_mapping, resulted_disk_path_mapping) - def test_group_block_devices_by_type(self): - block_device_map = get_fake_block_dev_info()['block_device_mapping'] - block_device_map[0]['connection_info'] = { - 'driver_volume_type': 'iscsi'} - result = self._volumeops._group_block_devices_by_type( - block_device_map) - - expected = {'iscsi': [block_device_map[0]]} - self.assertEqual(expected, result) - @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver') - def test_get_mounted_disk_path_from_volume(self, mock_get_volume_driver): + def test_get_disk_resource_path(self, mock_get_volume_driver): fake_conn_info = get_fake_connection_info() fake_volume_driver = mock_get_volume_driver.return_value - resulted_disk_path = self._volumeops.get_mounted_disk_path_from_volume( + resulted_disk_path = self._volumeops.get_disk_resource_path( fake_conn_info) - mock_get_volume_driver.assert_called_once_with( - connection_info=fake_conn_info) - get_mounted_disk = fake_volume_driver.get_mounted_disk_path_from_volume + mock_get_volume_driver.assert_called_once_with(fake_conn_info) + 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) @@ -251,397 +294,280 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase): self.assertTrue(mock_warning.called) -class ISCSIVolumeDriverTestCase(test_base.HyperVBaseTestCase): - """Unit tests for Hyper-V ISCSIVolumeDriver class.""" +class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase): + """Unit tests for Hyper-V BaseVolumeDriver class.""" def setUp(self): - super(ISCSIVolumeDriverTestCase, self).setUp() - self._volume_driver = volumeops.ISCSIVolumeDriver() - self._volume_driver._vmutils = mock.MagicMock() - self._volume_driver._volutils = mock.MagicMock() - - def test_login_storage_target_auth_exception(self): - connection_info = get_fake_connection_info( - auth_method='fake_auth_method') - - self.assertRaises(exception.UnsupportedBDMVolumeAuthMethod, - self._volume_driver.login_storage_target, - connection_info) - - @mock.patch.object(volumeops.ISCSIVolumeDriver, - '_get_mounted_disk_from_lun') - def _check_login_storage_target(self, mock_get_mounted_disk_from_lun, - dev_number): - connection_info = get_fake_connection_info() - login_target = self._volume_driver._volutils.login_storage_target - get_number = self._volume_driver._volutils.get_device_number_for_target - get_number.return_value = dev_number - - self._volume_driver.login_storage_target(connection_info) - - get_number.assert_called_once_with(mock.sentinel.fake_iqn, - mock.sentinel.fake_lun) - if not dev_number: - login_target.assert_called_once_with( - mock.sentinel.fake_lun, mock.sentinel.fake_iqn, - mock.sentinel.fake_portal, mock.sentinel.fake_user, - mock.sentinel.fake_pass) - mock_get_mounted_disk_from_lun.assert_called_once_with( - mock.sentinel.fake_iqn, mock.sentinel.fake_lun, True) + super(BaseVolumeDriverTestCase, self).setUp() + + self._base_vol_driver = volumeops.BaseVolumeDriver() + + 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.assertFalse(login_target.called) + self.assertRaises(exception.DiskNotFound, + self._base_vol_driver.get_disk_resource_path, + conn_info) - def test_login_storage_target_already_logged(self): - self._check_login_storage_target(dev_number=1) + def test_get_existing_disk_res_path(self): + self._test_get_disk_resource_path_by_conn_info() - def test_login_storage_target(self): - self._check_login_storage_target(dev_number=0) + def test_get_unfound_disk_res_path(self): + self._test_get_disk_resource_path_by_conn_info(disk_found=False) - def _check_logout_storage_target(self, disconnected_luns_count=0): - self._volume_driver._volutils.get_target_lun_count.return_value = 1 + def test_get_block_dev_res_path(self): + self._base_vol_driver._is_block_dev = True - self._volume_driver.logout_storage_target( - target_iqn=mock.sentinel.fake_iqn, - disconnected_luns_count=disconnected_luns_count) - - logout_storage = self._volume_driver._volutils.logout_storage_target + 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) - if disconnected_luns_count: - logout_storage.assert_called_once_with(mock.sentinel.fake_iqn) - else: - self.assertFalse(logout_storage.called) + disk_path = self._base_vol_driver._get_disk_res_path( + mock.sentinel.dev_name) - def test_logout_storage_target_skip(self): - self._check_logout_storage_target() + 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) - def test_logout_storage_target(self): - self._check_logout_storage_target(disconnected_luns_count=1) + self.assertEqual(mock.sentinel.disk_path, disk_path) - @mock.patch.object(volumeops.ISCSIVolumeDriver, - '_get_mounted_disk_from_lun') - def test_get_mounted_disk_path_from_volume(self, - mock_get_mounted_disk_from_lun): - connection_info = get_fake_connection_info() - resulted_disk_path = ( - self._volume_driver.get_mounted_disk_path_from_volume( - connection_info)) - - mock_get_mounted_disk_from_lun.assert_called_once_with( - connection_info['data']['target_iqn'], - connection_info['data']['target_lun'], - wait_for_device=True) - self.assertEqual(mock_get_mounted_disk_from_lun.return_value, - resulted_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 - @mock.patch.object(volumeops.ISCSIVolumeDriver, - '_get_mounted_disk_from_lun') - @mock.patch.object(volumeops.ISCSIVolumeDriver, 'logout_storage_target') - @mock.patch.object(volumeops.ISCSIVolumeDriver, 'login_storage_target') - def test_attach_volume_exception(self, mock_login_storage_target, - mock_logout_storage_target, - mock_get_mounted_disk): - connection_info = get_fake_connection_info() - mock_get_mounted_disk.side_effect = os_win_exc.HyperVException - - self.assertRaises(os_win_exc.HyperVException, - self._volume_driver.attach_volume, connection_info, - mock.sentinel.instance_name) - mock_logout_storage_target.assert_called_with(mock.sentinel.fake_iqn) - - @mock.patch.object(volumeops.ISCSIVolumeDriver, - '_get_mounted_disk_from_lun') - @mock.patch.object(volumeops.ISCSIVolumeDriver, 'login_storage_target') - def _check_attach_volume(self, mock_login_storage_target, - mock_get_mounted_disk_from_lun, disk_bus): + 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_res_path') + @mock.patch.object(volumeops.BaseVolumeDriver, '_get_disk_ctrl_and_slot') + @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 + mock_connect_volume.return_value = dict(path=mock.sentinel.raw_path) - get_ide_path = self._volume_driver._vmutils.get_vm_ide_controller - get_scsi_path = self._volume_driver._vmutils.get_vm_scsi_controller - fake_ide_path = get_ide_path.return_value - fake_scsi_path = get_scsi_path.return_value - fake_mounted_disk_path = mock_get_mounted_disk_from_lun.return_value - attach_vol = self._volume_driver._vmutils.attach_volume_to_controller - - get_free_slot = self._volume_driver._vmutils.get_free_controller_slot - get_free_slot.return_value = 1 + mock_get_disk_res_path.return_value = ( + mock.sentinel.disk_path) + mock_get_disk_ctrl_and_slot.return_value = ( + mock.sentinel.ctrller_path, + mock.sentinel.slot) - self._volume_driver.attach_volume( + self._base_vol_driver.attach_volume( connection_info=connection_info, instance_name=mock.sentinel.instance_name, - disk_bus=disk_bus) - - mock_login_storage_target.assert_called_once_with(connection_info) - mock_get_mounted_disk_from_lun.assert_called_once_with( - mock.sentinel.fake_iqn, - mock.sentinel.fake_lun, - wait_for_device=True) - if disk_bus == constants.CTRL_TYPE_IDE: - get_ide_path.assert_called_once_with( - mock.sentinel.instance_name, 0) - attach_vol.assert_called_once_with(mock.sentinel.instance_name, - fake_ide_path, 0, - fake_mounted_disk_path, - serial=mock.sentinel.serial) + disk_bus=mock.sentinel.disk_bus) + + if is_block_dev: + 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: - get_scsi_path.assert_called_once_with(mock.sentinel.instance_name) - get_free_slot.assert_called_once_with(fake_scsi_path) - attach_vol.assert_called_once_with(mock.sentinel.instance_name, - fake_scsi_path, 1, - fake_mounted_disk_path, - serial=mock.sentinel.serial) - - def test_attach_volume_ide(self): - self._check_attach_volume(disk_bus=constants.CTRL_TYPE_IDE) - - def test_attach_volume_scsi(self): - self._check_attach_volume(disk_bus=constants.CTRL_TYPE_SCSI) - - @mock.patch.object(volumeops.ISCSIVolumeDriver, - '_get_mounted_disk_from_lun') - @mock.patch.object(volumeops.ISCSIVolumeDriver, 'logout_storage_target') - def test_detach_volume(self, mock_logout_storage_target, - mock_get_mounted_disk_from_lun): + 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_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=False) + + def test_attach_volume_block_dev(self): + self._test_attach_volume(is_block_dev=True) + + @mock.patch.object(volumeops.BaseVolumeDriver, + 'get_disk_resource_path') + def test_detach_volume(self, mock_get_disk_resource_path): connection_info = get_fake_connection_info() - self._volume_driver.detach_volume(connection_info, - mock.sentinel.instance_name) + self._base_vol_driver.detach_volume(connection_info, + mock.sentinel.instance_name) - mock_get_mounted_disk_from_lun.assert_called_once_with( - mock.sentinel.fake_iqn, - mock.sentinel.fake_lun, - wait_for_device=True) - self._volume_driver._vmutils.detach_vm_disk.assert_called_once_with( + mock_get_disk_resource_path.assert_called_once_with( + connection_info) + self._vmutils.detach_vm_disk.assert_called_once_with( mock.sentinel.instance_name, - mock_get_mounted_disk_from_lun.return_value) - mock_logout_storage_target.assert_called_once_with( - mock.sentinel.fake_iqn) - - def test_get_mounted_disk_from_lun(self): - with test.nested( - mock.patch.object(self._volume_driver._volutils, - 'get_device_number_for_target'), - mock.patch.object(self._volume_driver._vmutils, - 'get_mounted_disk_by_drive_number') - ) as (mock_get_device_number_for_target, - mock_get_mounted_disk_by_drive_number): - - mock_get_device_number_for_target.return_value = 0 - mock_get_mounted_disk_by_drive_number.return_value = ( - mock.sentinel.disk_path) + mock_get_disk_resource_path.return_value, + is_physical=self._base_vol_driver._is_block_dev) - disk = self._volume_driver._get_mounted_disk_from_lun( - mock.sentinel.target_iqn, - mock.sentinel.target_lun) - self.assertEqual(mock.sentinel.disk_path, disk) + 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=constants.CTRL_TYPE_IDE) - def test_get_target_from_disk_path(self): - result = self._volume_driver.get_target_from_disk_path( - mock.sentinel.physical_drive_path) + expected_ctrl = self._vmutils.get_vm_ide_controller.return_value + expected_slot = 0 - mock_get_target = ( - self._volume_driver._volutils.get_target_from_disk_path) - mock_get_target.assert_called_once_with( - mock.sentinel.physical_drive_path) - self.assertEqual(mock_get_target.return_value, result) + self._vmutils.get_vm_ide_controller.assert_called_once_with( + mock.sentinel.instance_name, 0) - @mock.patch('time.sleep') - def test_get_mounted_disk_from_lun_failure(self, fake_sleep): - self.flags(mounted_disk_query_retry_count=1, group='hyperv') + self.assertEqual(expected_ctrl, ctrl) + self.assertEqual(expected_slot, slot) - with mock.patch.object(self._volume_driver._volutils, - 'get_device_number_for_target') as m_device_num: - m_device_num.side_effect = [None, -1] + def test_get_disk_ctrl_and_slot_scsi(self): + ctrl, slot = self._base_vol_driver._get_disk_ctrl_and_slot( + mock.sentinel.instance_name, + disk_bus=constants.CTRL_TYPE_SCSI) - self.assertRaises(exception.NotFound, - self._volume_driver._get_mounted_disk_from_lun, - mock.sentinel.target_iqn, - mock.sentinel.target_lun) + expected_ctrl = self._vmutils.get_vm_scsi_controller.return_value + expected_slot = ( + self._vmutils.get_free_controller_slot.return_value) - @mock.patch.object(volumeops.ISCSIVolumeDriver, 'logout_storage_target') - def test_disconnect_volumes(self, mock_logout_storage_target): - block_device_info = get_fake_block_dev_info() - connection_info = get_fake_connection_info() - block_device_mapping = block_device_info['block_device_mapping'] - block_device_mapping[0]['connection_info'] = connection_info + 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) + + self.assertEqual(expected_ctrl, ctrl) + self.assertEqual(expected_slot, slot) + + 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) - self._volume_driver.disconnect_volumes(block_device_mapping) - mock_logout_storage_target.assert_called_once_with( - mock.sentinel.fake_iqn, 1) +class ISCSIVolumeDriverTestCase(test_base.HyperVBaseTestCase): + """Unit tests for Hyper-V BaseVolumeDriver class.""" - def test_get_target_lun_count(self): - result = self._volume_driver.get_target_lun_count( - mock.sentinel.target_iqn) + 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]) - mock_get_lun_count = self._volume_driver._volutils.get_target_lun_count - mock_get_lun_count.assert_called_once_with(mock.sentinel.target_iqn) - self.assertEqual(mock_get_lun_count.return_value, result) + vol_driver = volumeops.ISCSIVolumeDriver() - @mock.patch.object(volumeops.ISCSIVolumeDriver, 'login_storage_target') - def test_initialize_volume_connection(self, mock_login_storage_target): - self._volume_driver.initialize_volume_connection( - mock.sentinel.connection_info) - mock_login_storage_target.assert_called_once_with( - mock.sentinel.connection_info) + self.assertEqual(expected_extra_conn_args, + vol_driver._extra_connector_args) 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._volutils = mock.MagicMock() - - @mock.patch.object(volumeops.SMBFSVolumeDriver, - '_get_disk_path') - def test_get_mounted_disk_path_from_volume(self, mock_get_disk_path): - disk_path = self._volume_driver.get_mounted_disk_path_from_volume( - 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) - - @mock.patch.object(volumeops.SMBFSVolumeDriver, 'ensure_share_mounted') - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path') - def _check_attach_volume(self, mock_get_disk_path, - mock_ensure_share_mounted, disk_bus): - mock_get_disk_path.return_value = mock.sentinel.disk_path + self._volume_driver._conn = mock.Mock() + self._conn = self._volume_driver._conn + def test_get_export_path(self): + 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) + + @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_CONNECTION_INFO, + self._FAKE_CONN_INFO, mock.sentinel.instance_name, - disk_bus) - - if disk_bus == constants.CTRL_TYPE_IDE: - get_vm_ide_controller = ( - self._volume_driver._vmutils.get_vm_ide_controller) - get_vm_ide_controller.assert_called_once_with( - mock.sentinel.instance_name, 0) - ctrller_path = get_vm_ide_controller.return_value - slot = 0 - else: - get_vm_scsi_controller = ( - self._volume_driver._vmutils.get_vm_scsi_controller) - get_vm_scsi_controller.assert_called_once_with( - mock.sentinel.instance_name) - get_free_controller_slot = ( - self._volume_driver._vmutils.get_free_controller_slot) - get_free_controller_slot.assert_called_once_with( - get_vm_scsi_controller.return_value) - - ctrller_path = get_vm_scsi_controller.return_value - slot = get_free_controller_slot.return_value - - mock_ensure_share_mounted.assert_called_once_with( - self._FAKE_CONNECTION_INFO) - mock_get_disk_path.assert_called_once_with(self._FAKE_CONNECTION_INFO) - self._volume_driver._vmutils.attach_drive.assert_called_once_with( - mock.sentinel.instance_name, mock.sentinel.disk_path, - ctrller_path, slot) - - def test_attach_volume_ide(self): - self._check_attach_volume(disk_bus=constants.CTRL_TYPE_IDE) - - def test_attach_volume_scsi(self): - self._check_attach_volume(disk_bus=constants.CTRL_TYPE_SCSI) - - @mock.patch.object(volumeops.SMBFSVolumeDriver, 'ensure_share_mounted') - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path') - def test_attach_non_existing_image(self, mock_get_disk_path, - mock_ensure_share_mounted): - self._volume_driver._vmutils.attach_drive.side_effect = ( - os_win_exc.HyperVException) - self.assertRaises(exception.VolumeAttachFailed, - self._volume_driver.attach_volume, - self._FAKE_CONNECTION_INFO, - mock.sentinel.instance_name) - - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path') - def test_detach_volume(self, mock_get_disk_path): - mock_get_disk_path.return_value = ( - mock.sentinel.disk_path) - - self._volume_driver.detach_volume(self._FAKE_CONNECTION_INFO, - mock.sentinel.instance_name) - - self._volume_driver._vmutils.detach_vm_disk.assert_called_once_with( - mock.sentinel.instance_name, mock.sentinel.disk_path, - is_physical=False) - - 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) - - def test_get_export_path(self): - result = self._volume_driver._get_export_path( - self._FAKE_CONNECTION_INFO) - - expected = self._FAKE_SHARE.replace('/', '\\') - self.assertEqual(expected, result) - - def test_get_disk_path(self): - expected = os.path.join(self._FAKE_SHARE_NORMALIZED, - self._FAKE_DISK_NAME) - - disk_path = self._volume_driver._get_disk_path( - self._FAKE_CONNECTION_INFO) - - self.assertEqual(expected, disk_path) - - @mock.patch.object(volumeops.SMBFSVolumeDriver, '_parse_credentials') - def _test_ensure_mounted(self, mock_parse_credentials, is_mounted=False): - mock_mount_smb_share = self._volume_driver._smbutils.mount_smb_share - self._volume_driver._smbutils.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) - - if is_mounted: - self.assertFalse( - mock_mount_smb_share.called) - else: - mock_mount_smb_share.assert_called_once_with( - self._FAKE_SHARE_NORMALIZED, - username=self._FAKE_USERNAME, - password=self._FAKE_PASSWORD) + disk_bus=mock.sentinel.disk_bus) - def test_ensure_mounted_new_share(self): - self._test_ensure_mounted() + mock_attach.assert_called_once_with( + self._FAKE_CONN_INFO, + mock.sentinel.instance_name, + disk_bus=mock.sentinel.disk_bus) - def test_ensure_already_mounted(self): - self._test_ensure_mounted(is_mounted=True) + @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) - def test_disconnect_volumes(self): - block_device_mapping = [ - {'connection_info': self._FAKE_CONNECTION_INFO}] - self._volume_driver.disconnect_volumes(block_device_mapping) - 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): @@ -652,17 +578,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) diff --git a/nova/virt/hyperv/constants.py b/nova/virt/hyperv/constants.py index c636fafe37..e938e7e4ec 100644 --- a/nova/virt/hyperv/constants.py +++ b/nova/virt/hyperv/constants.py @@ -85,3 +85,7 @@ FLAVOR_ESPEC_REMOTEFX_MONITORS = 'os:monitors' 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' diff --git a/nova/virt/hyperv/driver.py b/nova/virt/hyperv/driver.py index 6fd6710c30..0748335d11 100644 --- a/nova/virt/hyperv/driver.py +++ b/nova/virt/hyperv/driver.py @@ -181,7 +181,7 @@ class HyperVDriver(driver.ComputeDriver): instance.name) def get_volume_connector(self, instance): - return self._volumeops.get_volume_connector(instance) + return self._volumeops.get_volume_connector() def get_available_resource(self, nodename): return self._hostops.get_available_resource() diff --git a/nova/virt/hyperv/livemigrationops.py b/nova/virt/hyperv/livemigrationops.py index 13521e83fe..644ba9cdaa 100644 --- a/nova/virt/hyperv/livemigrationops.py +++ b/nova/virt/hyperv/livemigrationops.py @@ -96,7 +96,7 @@ class LiveMigrationOps(object): if not boot_from_volume and instance.image_ref: self._imagecache.get_cached_image(context, instance) - self._volumeops.initialize_volumes_connection(block_device_info) + self._volumeops.connect_volumes(block_device_info) disk_path_mapping = self._volumeops.get_disk_path_mapping( block_device_info) diff --git a/nova/virt/hyperv/volumeops.py b/nova/virt/hyperv/volumeops.py index 9f6a92bc8a..86aa12779a 100644 --- a/nova/virt/hyperv/volumeops.py +++ b/nova/virt/hyperv/volumeops.py @@ -17,16 +17,12 @@ """ Management class for Storage-related functions (attach, detach, etc). """ -import collections -import os -import re import time -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 six.moves import range +from oslo_utils import strutils import nova.conf from nova import exception @@ -46,15 +42,14 @@ class VolumeOps(object): def __init__(self): self._vmutils = utilsfactory.get_vmutils() - self._volutils = utilsfactory.get_iscsi_initiator_utils() - self._initiator = None self._default_root_device = 'vda' - self.volume_drivers = {'smbfs': SMBFSVolumeDriver(), - 'iscsi': ISCSIVolumeDriver()} + self.volume_drivers = { + constants.STORAGE_PROTOCOL_SMBFS: SMBFSVolumeDriver(), + constants.STORAGE_PROTOCOL_ISCSI: ISCSIVolumeDriver(), + constants.STORAGE_PROTOCOL_FC: FCVolumeDriver()} - def _get_volume_driver(self, driver_type=None, connection_info=None): - if connection_info: - driver_type = connection_info.get('driver_volume_type') + def _get_volume_driver(self, connection_info): + driver_type = connection_info.get('driver_volume_type') if driver_type not in self.volume_drivers: raise exception.VolumeDriverNotFound(driver_type=driver_type) return self.volume_drivers[driver_type] @@ -65,28 +60,74 @@ class VolumeOps(object): def disconnect_volumes(self, block_device_info): mapping = driver.block_device_info_get_mapping(block_device_info) - block_devices = self._group_block_devices_by_type( - mapping) - for driver_type, block_device_mapping in block_devices.items(): - volume_driver = self._get_volume_driver(driver_type) - volume_driver.disconnect_volumes(block_device_mapping) + for vol in mapping: + self.disconnect_volume(vol['connection_info']) 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_driver.attach_volume(connection_info, instance_name, - disk_bus=disk_bus) + 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): - volume_driver = self._get_volume_driver( - connection_info=connection_info) + 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) def fix_instance_volume_disk_paths(self, instance_name, block_device_info): # Mapping containing the current disk paths for each volume. @@ -107,25 +148,23 @@ class VolumeOps(object): self._vmutils.set_disk_host_res(vm_disk['resource_path'], actual_disk_path) - def get_volume_connector(self, instance): - if not self._initiator: - self._initiator = self._volutils.get_iscsi_initiator() - if not self._initiator: - LOG.warning(_LW('Could not determine iscsi initiator name'), - instance=instance) - return { - 'ip': CONF.my_block_storage_ip, - 'host': CONF.host, - 'initiator': self._initiator, - } - - def initialize_volumes_connection(self, block_device_info): + def get_volume_connector(self): + # 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) for vol in mapping: connection_info = vol['connection_info'] - volume_driver = self._get_volume_driver( - connection_info=connection_info) - volume_driver.initialize_volume_connection(connection_info) + volume_driver = self._get_volume_driver(connection_info) + volume_driver.connect_volume(connection_info) def get_disk_path_mapping(self, block_device_info): block_mapping = driver.block_device_info_get_mapping(block_device_info) @@ -134,23 +173,13 @@ class VolumeOps(object): connection_info = vol['connection_info'] disk_serial = connection_info['serial'] - disk_path = self.get_mounted_disk_path_from_volume(connection_info) + disk_path = self.get_disk_resource_path(connection_info) disk_path_mapping[disk_serial] = disk_path return disk_path_mapping - def _group_block_devices_by_type(self, block_device_mapping): - block_devices = collections.defaultdict(list) - for volume in block_device_mapping: - connection_info = volume['connection_info'] - volume_type = connection_info.get('driver_volume_type') - block_devices[volume_type].append(volume) - return block_devices - - def get_mounted_disk_path_from_volume(self, connection_info): - volume_driver = self._get_volume_driver( - connection_info=connection_info) - return volume_driver.get_mounted_disk_path_from_volume( - connection_info) + 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): @@ -173,301 +202,150 @@ class VolumeOps(object): LOG.warning(msg) -class ISCSIVolumeDriver(object): +class BaseVolumeDriver(object): + _is_block_dev = True + _protocol = None + _extra_connector_args = {} + def __init__(self): + self._conn = None + self._diskutils = utilsfactory.get_diskutils() self._vmutils = utilsfactory.get_vmutils() - self._volutils = utilsfactory.get_iscsi_initiator_utils() - - def login_storage_target(self, connection_info): - data = connection_info['data'] - target_lun = data['target_lun'] - target_iqn = data['target_iqn'] - target_portal = data['target_portal'] - auth_method = data.get('auth_method') - auth_username = data.get('auth_username') - auth_password = data.get('auth_password') - - if auth_method and auth_method.upper() != 'CHAP': - LOG.error(_LE("Cannot log in target %(target_iqn)s. Unsupported " - "iSCSI authentication method: %(auth_method)s."), - {'target_iqn': target_iqn, - 'auth_method': auth_method}) - raise exception.UnsupportedBDMVolumeAuthMethod( - auth_method=auth_method) - - # Check if we already logged in - if self._volutils.get_device_number_for_target(target_iqn, target_lun): - LOG.debug("Already logged in on storage target. No need to " - "login. Portal: %(target_portal)s, " - "IQN: %(target_iqn)s, LUN: %(target_lun)s", - {'target_portal': target_portal, - 'target_iqn': target_iqn, 'target_lun': target_lun}) - else: - LOG.debug("Logging in on storage target. Portal: " - "%(target_portal)s, IQN: %(target_iqn)s, " - "LUN: %(target_lun)s", - {'target_portal': target_portal, - 'target_iqn': target_iqn, 'target_lun': target_lun}) - self._volutils.login_storage_target(target_lun, target_iqn, - target_portal, auth_username, - auth_password) - # Wait for the target to be mounted - self._get_mounted_disk_from_lun(target_iqn, target_lun, True) - - def disconnect_volumes(self, block_device_mapping): - iscsi_targets = collections.defaultdict(int) - for vol in block_device_mapping: - target_iqn = vol['connection_info']['data']['target_iqn'] - iscsi_targets[target_iqn] += 1 - - for target_iqn, disconnected_luns in iscsi_targets.items(): - self.logout_storage_target(target_iqn, disconnected_luns) - - def logout_storage_target(self, target_iqn, disconnected_luns_count=1): - total_available_luns = self._volutils.get_target_lun_count( - target_iqn) - - if total_available_luns == disconnected_luns_count: - LOG.debug("Logging off storage target %s", target_iqn) - self._volutils.logout_storage_target(target_iqn) - else: - LOG.debug("Skipping disconnecting target %s as there " - "are LUNs still being used.", target_iqn) - - def get_mounted_disk_path_from_volume(self, connection_info): - data = connection_info['data'] - target_lun = data['target_lun'] - target_iqn = data['target_iqn'] - # Getting the mounted disk - return self._get_mounted_disk_from_lun(target_iqn, target_lun, - wait_for_device=True) + @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): + return self._connector.connect_volume(connection_info['data']) + + def disconnect_volume(self, connection_info): + self._connector.disconnect_volume(connection_info['data']) + + 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) + + 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 - """ - target_iqn = None - LOG.debug("Attach_volume: %(connection_info)s to %(instance_name)s", - {'connection_info': connection_info, - 'instance_name': instance_name}) - try: - self.login_storage_target(connection_info) - - serial = connection_info['serial'] - # Getting the mounted disk - mounted_disk_path = self.get_mounted_disk_path_from_volume( - connection_info) - - if disk_bus == constants.CTRL_TYPE_IDE: - # Find the IDE controller for the vm. - ctrller_path = self._vmutils.get_vm_ide_controller( - instance_name, 0) - # Attaching to the first slot - slot = 0 - 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) - + 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: + # We need to tag physical disk resources with the volume + # serial number, in order to be able to retrieve them + # during live migration. self._vmutils.attach_volume_to_controller(instance_name, ctrller_path, slot, - mounted_disk_path, + disk_path, serial=serial) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('Unable to attach volume to instance %s'), - instance_name) - target_iqn = connection_info['data']['target_iqn'] - if target_iqn: - self.logout_storage_target(target_iqn) - - def detach_volume(self, connection_info, instance_name): - """Detach a volume to the SCSI controller.""" - LOG.debug("Detach_volume: %(connection_info)s " - "from %(instance_name)s", - {'connection_info': connection_info, - 'instance_name': instance_name}) - - target_iqn = connection_info['data']['target_iqn'] - mounted_disk_path = self.get_mounted_disk_path_from_volume( - connection_info) - - LOG.debug("Detaching physical disk from instance: %s", - mounted_disk_path) - self._vmutils.detach_vm_disk(instance_name, mounted_disk_path) - - self.logout_storage_target(target_iqn) - - def _get_mounted_disk_from_lun(self, target_iqn, target_lun, - wait_for_device=False): - # The WMI query in get_device_number_for_target can incorrectly - # return no data when the system is under load. This issue can - # be avoided by adding a retry. - for i in range(CONF.hyperv.mounted_disk_query_retry_count): - device_number = self._volutils.get_device_number_for_target( - target_iqn, target_lun) - if device_number in (None, -1): - attempt = i + 1 - LOG.debug('Attempt %d to get device_number ' - 'from get_device_number_for_target failed. ' - 'Retrying...', attempt) - time.sleep(CONF.hyperv.mounted_disk_query_retry_interval) - else: - break - - if device_number in (None, -1): - raise exception.NotFound(_('Unable to find a mounted disk for ' - 'target_iqn: %s') % target_iqn) - LOG.debug('Device number: %(device_number)s, ' - 'target lun: %(target_lun)s', - {'device_number': device_number, 'target_lun': target_lun}) - # Finding Mounted disk drive - for i in range(0, CONF.hyperv.volume_attach_retry_count): - mounted_disk_path = self._vmutils.get_mounted_disk_by_drive_number( - device_number) - if mounted_disk_path or not wait_for_device: - break - time.sleep(CONF.hyperv.volume_attach_retry_interval) - - if not mounted_disk_path: - raise exception.NotFound(_('Unable to find a mounted disk for ' - 'target_iqn: %s. Please ensure that ' - 'the host\'s SAN policy is set to ' - '"OfflineAll" or "OfflineShared"') % - target_iqn) - return mounted_disk_path - - def get_target_from_disk_path(self, physical_drive_path): - return self._volutils.get_target_from_disk_path(physical_drive_path) - - def get_target_lun_count(self, target_iqn): - return self._volutils.get_target_lun_count(target_iqn) - - def initialize_volume_connection(self, connection_info): - self.login_storage_target(connection_info) - - def set_disk_qos_specs(self, connection_info, disk_qos_specs): - LOG.info(_LI("The iSCSI Hyper-V volume driver does not support QoS. " - "Ignoring QoS specs.")) - - -def export_path_synchronized(f): - def wrapper(inst, connection_info, *args, **kwargs): - export_path = inst._get_export_path(connection_info) - - @utils.synchronized(export_path) - def inner(): - return f(inst, connection_info, *args, **kwargs) - return inner() - return wrapper - - -class SMBFSVolumeDriver(object): - def __init__(self): - self._smbutils = utilsfactory.get_smbutils() - self._vmutils = utilsfactory.get_vmutils() - self._username_regex = re.compile(r'user(?:name)?=([^, ]+)') - self._password_regex = re.compile(r'pass(?:word)?=([^, ]+)') - - def get_mounted_disk_path_from_volume(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): - self.ensure_share_mounted(connection_info) - - disk_path = self._get_disk_path(connection_info) - - try: - if disk_bus == constants.CTRL_TYPE_IDE: - ctrller_path = self._vmutils.get_vm_ide_controller( - instance_name, 0) - slot = 0 - else: - ctrller_path = self._vmutils.get_vm_scsi_controller( - instance_name) - slot = self._vmutils.get_free_controller_slot(ctrller_path) - + else: self._vmutils.attach_drive(instance_name, disk_path, ctrller_path, slot) - except os_win_exc.HyperVException as exn: - LOG.exception(_LE('Attach volume failed to %(instance_name)s: ' - '%(exn)s'), {'instance_name': instance_name, - 'exn': exn}) - raise exception.VolumeAttachFailed( - volume_id=connection_info['data']['volume_id'], - reason=exn.message) def detach_volume(self, connection_info, instance_name): - LOG.debug("Detaching volume: %(connection_info)s " - "from %(instance_name)s", - {'connection_info': connection_info, - 'instance_name': instance_name}) - - disk_path = self._get_disk_path(connection_info) - export_path = self._get_export_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=disk_path, + instance_name=instance_name)) self._vmutils.detach_vm_disk(instance_name, disk_path, - is_physical=False) - self._unmount_smb_share(export_path) + is_physical=self._is_block_dev) + + def _get_disk_ctrl_and_slot(self, instance_name, disk_bus): + if disk_bus == constants.CTRL_TYPE_IDE: + # Find the IDE controller for the vm. + ctrller_path = self._vmutils.get_vm_ide_controller( + instance_name, 0) + # Attaching to the first slot + slot = 0 + 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) + return ctrller_path, slot - def disconnect_volumes(self, block_device_mapping): - export_paths = set() - for vol in block_device_mapping: - connection_info = vol['connection_info'] - export_path = self._get_export_path(connection_info) - export_paths.add(export_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)) - for export_path in export_paths: - self._unmount_smb_share(export_path) - def _get_export_path(self, connection_info): - return connection_info['data']['export'].replace('/', '\\') +class ISCSIVolumeDriver(BaseVolumeDriver): + _is_block_dev = True + _protocol = constants.STORAGE_PROTOCOL_ISCSI - def _get_disk_path(self, connection_info): - export = self._get_export_path(connection_info) - disk_name = connection_info['data']['name'] - disk_path = os.path.join(export, disk_name) - return disk_path + def __init__(self, *args, **kwargs): + self._extra_connector_args = dict( + initiator_list=CONF.hyperv.iscsi_initiator_list) - def ensure_share_mounted(self, connection_info): - export_path = self._get_export_path(connection_info) + super(ISCSIVolumeDriver, self).__init__(*args, **kwargs) - if not self._smbutils.check_smb_mapping(export_path): - opts_str = connection_info['data'].get('options', '') - 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 +class SMBFSVolumeDriver(BaseVolumeDriver): + _is_block_dev = False + _protocol = constants.STORAGE_PROTOCOL_SMBFS + _extra_connector_args = dict(local_path_for_loopback=True) - match = self._password_regex.findall(opts_str) - password = match[0] if match else None + def export_path_synchronized(f): + def wrapper(inst, connection_info, *args, **kwargs): + export_path = inst._get_export_path(connection_info) - return username, password + @utils.synchronized(export_path) + def inner(): + return f(inst, connection_info, *args, **kwargs) + return inner() + return wrapper - def initialize_volume_connection(self, connection_info): - self.ensure_share_mounted(connection_info) + def _get_export_path(self, connection_info): + return connection_info['data']['export'].replace('/', '\\') - def _unmount_smb_share(self, export_path): - # 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. - @utils.synchronized(export_path) - def unmount_synchronized(): - self._smbutils.unmount_smb_share(export_path) - unmount_synchronized() + @export_path_synchronized + def attach_volume(self, *args, **kwargs): + super(SMBFSVolumeDriver, self).attach_volume(*args, **kwargs) + + @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'] @@ -479,5 +357,10 @@ class SMBFSVolumeDriver(object): 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): + _is_block_dev = True + _protocol = constants.STORAGE_PROTOCOL_FC diff --git a/releasenotes/notes/bp-hyperv-use-os-brick-bf576a5bc97f0ea2.yaml b/releasenotes/notes/bp-hyperv-use-os-brick-bf576a5bc97f0ea2.yaml new file mode 100644 index 0000000000..68ecdb95e6 --- /dev/null +++ b/releasenotes/notes/bp-hyperv-use-os-brick-bf576a5bc97f0ea2.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + The Hyper-V driver now uses os-brick for volume related + operations, introducing the following new features: + + - Attaching volumes over fibre channel on a passthrough + basis. + - Improved iSCSI MPIO support, by connecting to multiple + iSCSI targets/portals when available and allowing using + a predefined list of initiator HBAs. -- cgit v1.2.1