diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index cb6bcb8ea59..61d8c07e002 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -145,7 +145,8 @@ def _convert_image(prefix, source, dest, out_format, run_as_root=True): if duration < 1: duration = 1 try: - image_size = qemu_img_info(source, run_as_root=True).virtual_size + image_size = qemu_img_info(source, + run_as_root=run_as_root).virtual_size except ValueError as e: msg = _LI("The image was successfully converted, but image size " "is unavailable. src %(src)s, dest %(dest)s. %(error)s") diff --git a/cinder/tests/unit/volume/drivers/test_coho.py b/cinder/tests/unit/volume/drivers/test_coho.py index 8e0c63732ab..6324c01a464 100644 --- a/cinder/tests/unit/volume/drivers/test_coho.py +++ b/cinder/tests/unit/volume/drivers/test_coho.py @@ -25,6 +25,7 @@ import xdrlib from cinder import context from cinder import exception from cinder import test +from cinder.tests.unit import fake_volume from cinder.volume import configuration as conf from cinder.volume.drivers import coho from cinder.volume.drivers import nfs @@ -42,7 +43,7 @@ VOLUME = { 'volume_id': 'bcc48c61-9691-4e5f-897c-793686093190', 'size': 128, 'volume_type': 'silver', - 'volume_type_id': 'type-id', + 'volume_type_id': 'deadbeef-aaaa-bbbb-cccc-deadbeefbeef', 'metadata': [{'key': 'type', 'service_label': 'silver'}], 'provider_location': 'coho-datastream-addr:/test/path', @@ -72,7 +73,7 @@ VOLUME_TYPE = { 'updated_at': None, 'extra_specs': {}, 'deleted_at': None, - 'id': 'type-id' + 'id': 'deadbeef-aaaa-bbbb-cccc-deadbeefbeef' } QOS_SPEC = { @@ -161,6 +162,11 @@ class CohoDriverTest(test.TestCase): def test_create_volume_with_qos(self): drv = coho.CohoDriver(configuration=self.configuration) + volume = fake_volume.fake_volume_obj(self.context, + **{'volume_type_id': + VOLUME['volume_type_id'], + 'provider_location': + VOLUME['provider_location']}) mock_remotefs_create = self.mock_object(remotefs.RemoteFSDriver, 'create_volume') mock_rpc_client = self.mock_object(coho, 'CohoRPCClient') @@ -172,18 +178,18 @@ class CohoDriverTest(test.TestCase): mock_get_admin_context = self.mock_object(context, 'get_admin_context') mock_get_admin_context.return_value = 'test' - drv.create_volume(VOLUME) + drv.create_volume(volume) self.assertTrue(mock_remotefs_create.called) self.assertTrue(mock_get_admin_context.called) - mock_remotefs_create.assert_has_calls([mock.call(VOLUME)]) + mock_remotefs_create.assert_has_calls([mock.call(volume)]) mock_get_volume_type.assert_has_calls( - [mock.call('test', VOLUME_TYPE['id'])]) + [mock.call('test', volume.volume_type_id)]) mock_get_qos_specs.assert_has_calls( [mock.call('test', QOS_SPEC['id'])]) mock_rpc_client.assert_has_calls( [mock.call(ADDR, self.configuration.coho_rpc_port), - mock.call().set_qos_policy(os.path.join(PATH, VOLUME['name']), + mock.call().set_qos_policy(os.path.join(PATH, volume.name), QOS)]) def test_create_snapshot(self): diff --git a/cinder/tests/unit/volume/drivers/test_nfs.py b/cinder/tests/unit/volume/drivers/test_nfs.py index 5257d973896..cfff772a39c 100644 --- a/cinder/tests/unit/volume/drivers/test_nfs.py +++ b/cinder/tests/unit/volume/drivers/test_nfs.py @@ -17,14 +17,18 @@ import ddt import errno import os +import six +import uuid import mock +from oslo_utils import imageutils from oslo_utils import units from cinder import context from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.volume import configuration as conf from cinder.volume.drivers import nfs @@ -43,6 +47,7 @@ class RemoteFsDriverTestCase(test.TestCase): self.configuration.append_config_values(mock.ANY) self.configuration.nas_secure_file_permissions = 'false' self.configuration.nas_secure_file_operations = 'false' + self.configuration.nfs_snapshot_support = True self.configuration.max_over_subscription_ratio = 1.0 self.configuration.reserved_percentage = 5 self._driver = remotefs.RemoteFSDriver( @@ -293,6 +298,81 @@ class RemoteFsDriverTestCase(test.TestCase): ret_flag = drv.secure_file_operations_enabled() self.assertFalse(ret_flag) +# NFS configuration scenarios +NFS_CONFIG1 = {'max_over_subscription_ratio': 1.0, + 'reserved_percentage': 0, + 'nfs_sparsed_volumes': True, + 'nfs_qcow2_volumes': False, + 'nas_secure_file_permissions': 'false', + 'nas_secure_file_operations': 'false'} + +NFS_CONFIG2 = {'max_over_subscription_ratio': 10.0, + 'reserved_percentage': 5, + 'nfs_sparsed_volumes': False, + 'nfs_qcow2_volumes': True, + 'nas_secure_file_permissions': 'true', + 'nas_secure_file_operations': 'true'} + +NFS_CONFIG3 = {'max_over_subscription_ratio': 15.0, + 'reserved_percentage': 10, + 'nfs_sparsed_volumes': False, + 'nfs_qcow2_volumes': False, + 'nas_secure_file_permissions': 'auto', + 'nas_secure_file_operations': 'auto'} + +NFS_CONFIG4 = {'max_over_subscription_ratio': 20.0, + 'reserved_percentage': 60, + 'nfs_sparsed_volumes': True, + 'nfs_qcow2_volumes': True, + 'nas_secure_file_permissions': 'false', + 'nas_secure_file_operations': 'true'} + +QEMU_IMG_INFO_OUT1 = """image: %(volid)s + file format: raw + virtual size: %(size_gb)sG (%(size_b)s bytes) + disk size: 173K + """ + +QEMU_IMG_INFO_OUT2 = """image: %(volid)s +file format: qcow2 +virtual size: %(size_gb)sG (%(size_b)s bytes) +disk size: 196K +cluster_size: 65536 +Format specific information: + compat: 1.1 + lazy refcounts: false + refcount bits: 16 + corrupt: false + """ + +QEMU_IMG_INFO_OUT3 = """image: volume-%(volid)s.%(snapid)s +file format: qcow2 +virtual size: %(size_gb)sG (%(size_b)s bytes) +disk size: 196K +cluster_size: 65536 +backing file: volume-%(volid)s +backing file format: qcow2 +Format specific information: + compat: 1.1 + lazy refcounts: false + refcount bits: 16 + corrupt: false + """ + +QEMU_IMG_INFO_OUT4 = """image: volume-%(volid)s.%(snapid)s +file format: raw +virtual size: %(size_gb)sG (%(size_b)s bytes) +disk size: 196K +cluster_size: 65536 +backing file: volume-%(volid)s +backing file format: raw +Format specific information: + compat: 1.1 + lazy refcounts: false + refcount bits: 16 + corrupt: false + """ + @ddt.ddt class NfsDriverTestCase(test.TestCase): @@ -312,6 +392,7 @@ class NfsDriverTestCase(test.TestCase): TEST_SHARES_CONFIG_FILE = '/etc/cinder/test-shares.conf' TEST_NFS_EXPORT_SPACES = 'nfs-host3:/export this' TEST_MNT_POINT_SPACES = '/ 0 0 0 /foo' + VOLUME_UUID = '69ad4ff6-b892-4215-aaaa-aaaaaaaaaaaa' def setUp(self): super(NfsDriverTestCase, self).setUp() @@ -332,16 +413,25 @@ class NfsDriverTestCase(test.TestCase): self.configuration.nas_share_path = None self.configuration.nas_mount_options = None self.configuration.volume_dd_blocksize = '1M' - self._driver = nfs.NfsDriver(configuration=self.configuration) - self._driver.shares = {} - mock_exc = mock.patch.object(self._driver, '_execute') - self._execute = mock_exc.start() - self.addCleanup(mock_exc.stop) + self.context = context.get_admin_context() - def test_local_path(self): + def _set_driver(self, extra_confs=None): + + # Overide the default configs + if extra_confs: + for config_name, config_value in extra_confs.items(): + setattr(self.configuration, config_name, config_value) + + self._driver = nfs.NfsDriver(configuration=self.configuration) + self._driver.shares = {} + self.mock_object(self._driver, '_execute') + + @ddt.data(NFS_CONFIG1, NFS_CONFIG2, NFS_CONFIG3, NFS_CONFIG4) + def test_local_path(self, nfs_config): """local_path common use case.""" self.configuration.nfs_mount_point_base = self.TEST_MNT_POINT_BASE + self._set_driver(extra_confs=nfs_config) drv = self._driver volume = fake_volume.fake_volume_obj( @@ -352,31 +442,36 @@ class NfsDriverTestCase(test.TestCase): '/mnt/test/2f4f60214cf43c595666dd815f0360a4/%s' % volume.name, drv.local_path(volume)) - @mock.patch.object(image_utils, 'qemu_img_info') - @mock.patch.object(image_utils, 'resize_image') - @mock.patch.object(image_utils, 'fetch_to_raw') - def test_copy_image_to_volume(self, mock_fetch, mock_resize, mock_qemu): + @ddt.data(NFS_CONFIG1, NFS_CONFIG2, NFS_CONFIG3, NFS_CONFIG4) + def test_copy_image_to_volume(self, nfs_config): """resize_image common case usage.""" + + mock_resize = self.mock_object(image_utils, 'resize_image') + mock_fetch = self.mock_object(image_utils, 'fetch_to_raw') + + self._set_driver() drv = self._driver volume = fake_volume.fake_volume_obj(self.context, size=self.TEST_SIZE_IN_GB) - TEST_IMG_SOURCE = 'volume-%s' % volume.id + test_img_source = 'volume-%s' % volume.id - with mock.patch.object(drv, 'local_path', - return_value=TEST_IMG_SOURCE): - data = mock.Mock() - data.virtual_size = 1 * units.Gi - mock_qemu.return_value = data - drv.copy_image_to_volume(None, volume, None, None) - mock_fetch.assert_called_once_with( - None, None, None, TEST_IMG_SOURCE, mock.ANY, run_as_root=True, - size=self.TEST_SIZE_IN_GB) - mock_resize.assert_called_once_with(TEST_IMG_SOURCE, - self.TEST_SIZE_IN_GB, - run_as_root=True) + self.mock_object(drv, 'local_path', return_value=test_img_source) + + data = mock.Mock() + data.virtual_size = 1 * units.Gi + self.mock_object(image_utils, 'qemu_img_info', return_value=data) + drv.copy_image_to_volume(None, volume, None, None) + + mock_fetch.assert_called_once_with( + None, None, None, test_img_source, mock.ANY, run_as_root=True, + size=self.TEST_SIZE_IN_GB) + mock_resize.assert_called_once_with(test_img_source, + self.TEST_SIZE_IN_GB, + run_as_root=True) def test_get_mount_point_for_share(self): """_get_mount_point_for_share should calculate correct value.""" + self._set_driver() drv = self._driver self.configuration.nfs_mount_point_base = self.TEST_MNT_POINT_BASE @@ -402,6 +497,7 @@ class NfsDriverTestCase(test.TestCase): def test_get_capacity_info(self): """_get_capacity_info should calculate correct value.""" + self._set_driver() drv = self._driver stat_total_size = 2620544 stat_avail = 2129984 @@ -413,8 +509,8 @@ class NfsDriverTestCase(test.TestCase): with mock.patch.object( drv, '_get_mount_point_for_share') as mock_get_mount: mock_get_mount.return_value = self.TEST_MNT_POINT - self._execute.side_effect = [(stat_output, None), - (du_output, None)] + drv._execute.side_effect = [(stat_output, None), + (du_output, None)] self.assertEqual((stat_total_size, stat_avail, du_used), drv._get_capacity_info(self.TEST_NFS_EXPORT1)) @@ -427,10 +523,11 @@ class NfsDriverTestCase(test.TestCase): '--exclude', '*snapshot*', self.TEST_MNT_POINT, run_as_root=True)] - self._execute.assert_has_calls(calls) + drv._execute.assert_has_calls(calls) def test_get_capacity_info_for_share_and_mount_point_with_spaces(self): """_get_capacity_info should calculate correct value.""" + self._set_driver() drv = self._driver stat_total_size = 2620544 stat_avail = 2129984 @@ -442,8 +539,8 @@ class NfsDriverTestCase(test.TestCase): with mock.patch.object( drv, '_get_mount_point_for_share') as mock_get_mount: mock_get_mount.return_value = self.TEST_MNT_POINT_SPACES - self._execute.side_effect = [(stat_output, None), - (du_output, None)] + drv._execute.side_effect = [(stat_output, None), + (du_output, None)] self.assertEqual((stat_total_size, stat_avail, du_used), drv._get_capacity_info( @@ -458,10 +555,12 @@ class NfsDriverTestCase(test.TestCase): '--exclude', '*snapshot*', self.TEST_MNT_POINT_SPACES, run_as_root=True)] - self._execute.assert_has_calls(calls) + drv._execute.assert_has_calls(calls) def test_load_shares_config(self): + self._set_driver() drv = self._driver + drv.configuration.nfs_shares_config = self.TEST_SHARES_CONFIG_FILE with mock.patch.object( @@ -487,6 +586,7 @@ class NfsDriverTestCase(test.TestCase): drv.shares[self.TEST_NFS_EXPORT2]) def test_load_shares_config_nas_opts(self): + self._set_driver() drv = self._driver drv.configuration.nas_host = self.TEST_NFS_HOST drv.configuration.nas_share_path = self.TEST_NFS_SHARE_PATH @@ -499,6 +599,7 @@ class NfsDriverTestCase(test.TestCase): def test_ensure_shares_mounted_should_save_mounting_successfully(self): """_ensure_shares_mounted should save share if mounted with success.""" + self._set_driver() drv = self._driver config_data = [] config_data.append(self.TEST_NFS_EXPORT1) @@ -516,6 +617,7 @@ class NfsDriverTestCase(test.TestCase): def test_ensure_shares_mounted_should_not_save_mounting_with_error(self, LOG): """_ensure_shares_mounted should not save share if failed to mount.""" + self._set_driver() drv = self._driver config_data = [] config_data.append(self.TEST_NFS_EXPORT1) @@ -531,6 +633,7 @@ class NfsDriverTestCase(test.TestCase): def test_find_share_should_throw_error_if_there_is_no_mounted_share(self): """_find_share should throw error if there is no mounted shares.""" + self._set_driver() drv = self._driver drv._mounted_shares = [] @@ -540,6 +643,7 @@ class NfsDriverTestCase(test.TestCase): def test_find_share(self): """_find_share simple use case.""" + self._set_driver() drv = self._driver drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2] @@ -557,6 +661,7 @@ class NfsDriverTestCase(test.TestCase): def test_find_share_should_throw_error_if_there_is_not_enough_space(self): """_find_share should throw error if there is no share to host vol.""" + self._set_driver() drv = self._driver drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2] @@ -573,13 +678,15 @@ class NfsDriverTestCase(test.TestCase): mock_get_capacity_info.assert_has_calls(calls) self.assertEqual(2, mock_get_capacity_info.call_count) - def _simple_volume(self): + def _simple_volume(self, size=10): + loc = self.TEST_NFS_EXPORT1 return fake_volume.fake_volume_obj(self.context, display_name='volume_name', - provider_location='127.0.0.1:/mnt', - size=10) + provider_location=loc, + size=size) def test_create_sparsed_volume(self): + self._set_driver() drv = self._driver volume = self._simple_volume() @@ -596,6 +703,7 @@ class NfsDriverTestCase(test.TestCase): mock_set_rw_permissions.assert_called_once_with(mock.ANY) def test_create_nonsparsed_volume(self): + self._set_driver() drv = self._driver self.configuration.nfs_sparsed_volumes = False volume = self._simple_volume() @@ -615,6 +723,7 @@ class NfsDriverTestCase(test.TestCase): @mock.patch.object(nfs, 'LOG') def test_create_volume_should_ensure_nfs_mounted(self, mock_log): """create_volume ensures shares provided in config are mounted.""" + self._set_driver() drv = self._driver drv._find_share = mock.Mock() drv._find_share.return_value = self.TEST_NFS_EXPORT1 @@ -632,6 +741,7 @@ class NfsDriverTestCase(test.TestCase): @mock.patch.object(nfs, 'LOG') def test_create_volume_should_return_provider_location(self, mock_log): """create_volume should return provider_location with found share.""" + self._set_driver() drv = self._driver drv._ensure_shares_mounted = mock.Mock() drv._do_create_volume = mock.Mock() @@ -647,6 +757,7 @@ class NfsDriverTestCase(test.TestCase): def test_delete_volume(self): """delete_volume simple test case.""" + self._set_driver() drv = self._driver drv._ensure_share_mounted = mock.Mock() @@ -658,26 +769,24 @@ class NfsDriverTestCase(test.TestCase): with mock.patch.object(drv, 'local_path') as mock_local_path: mock_local_path.return_value = self.TEST_LOCAL_PATH drv.delete_volume(volume) - mock_local_path.assert_called_once_with(volume) - self._execute.assert_called_once_with('rm', '-f', - self.TEST_LOCAL_PATH, - run_as_root=True) + mock_local_path.assert_called_with(volume) + drv._execute.assert_called_once() def test_delete_should_ensure_share_mounted(self): """delete_volume should ensure that corresponding share is mounted.""" + self._set_driver() drv = self._driver volume = fake_volume.fake_volume_obj( self.context, display_name='volume-123', provider_location=self.TEST_NFS_EXPORT1) - with mock.patch.object( - drv, '_ensure_share_mounted') as mock_ensure_share: + with mock.patch.object(drv, '_ensure_share_mounted'): drv.delete_volume(volume) - mock_ensure_share.assert_called_once_with(self.TEST_NFS_EXPORT1) def test_delete_should_not_delete_if_provider_location_not_provided(self): """delete_volume shouldn't delete if provider_location missed.""" + self._set_driver() drv = self._driver volume = fake_volume.fake_volume_obj(self.context, name='volume-123', @@ -685,10 +794,11 @@ class NfsDriverTestCase(test.TestCase): with mock.patch.object(drv, '_ensure_share_mounted'): drv.delete_volume(volume) - self.assertFalse(self._execute.called) + self.assertFalse(drv._execute.called) def test_get_volume_stats(self): """get_volume_stats must fill the correct values.""" + self._set_driver() drv = self._driver drv._mounted_shares = [self.TEST_NFS_EXPORT1, self.TEST_NFS_EXPORT2] @@ -742,7 +852,7 @@ class NfsDriverTestCase(test.TestCase): @ddt.data(True, False) def test_update_volume_stats(self, thin): - + self._set_driver() self._driver.configuration.max_over_subscription_ratio = 20.0 self._driver.configuration.reserved_percentage = 5.0 self._driver.configuration.nfs_sparsed_volumes = thin @@ -780,6 +890,7 @@ class NfsDriverTestCase(test.TestCase): def _check_is_share_eligible(self, total_size, total_available, total_allocated, requested_volume_size): + self._set_driver() with mock.patch.object(self._driver, '_get_capacity_info')\ as mock_get_capacity_info: mock_get_capacity_info.return_value = (total_size, @@ -789,6 +900,7 @@ class NfsDriverTestCase(test.TestCase): requested_volume_size) def test_is_share_eligible(self): + self._set_driver() total_size = 100.0 * units.Gi total_available = 90.0 * units.Gi total_allocated = 10.0 * units.Gi @@ -800,6 +912,7 @@ class NfsDriverTestCase(test.TestCase): requested_volume_size)) def test_share_eligibility_with_reserved_percentage(self): + self._set_driver() total_size = 100.0 * units.Gi total_available = 4.0 * units.Gi total_allocated = 96.0 * units.Gi @@ -812,6 +925,7 @@ class NfsDriverTestCase(test.TestCase): requested_volume_size)) def test_is_share_eligible_above_oversub_ratio(self): + self._set_driver() total_size = 100.0 * units.Gi total_available = 10.0 * units.Gi total_allocated = 90.0 * units.Gi @@ -824,6 +938,7 @@ class NfsDriverTestCase(test.TestCase): requested_volume_size)) def test_is_share_eligible_reserved_space_above_oversub_ratio(self): + self._set_driver() total_size = 100.0 * units.Gi total_available = 10.0 * units.Gi total_allocated = 100.0 * units.Gi @@ -838,6 +953,7 @@ class NfsDriverTestCase(test.TestCase): def test_extend_volume(self): """Extend a volume by 1.""" + self._set_driver() drv = self._driver volume = fake_volume.fake_volume_obj( self.context, @@ -860,6 +976,7 @@ class NfsDriverTestCase(test.TestCase): def test_extend_volume_failure(self): """Error during extend operation.""" + self._set_driver() drv = self._driver volume = fake_volume.fake_volume_obj( self.context, @@ -878,6 +995,7 @@ class NfsDriverTestCase(test.TestCase): def test_extend_volume_insufficient_space(self): """Insufficient space on nfs_share during extend operation.""" + self._set_driver() drv = self._driver volume = fake_volume.fake_volume_obj( self.context, @@ -896,6 +1014,7 @@ class NfsDriverTestCase(test.TestCase): def test_is_file_size_equal(self): """File sizes are equal.""" + self._set_driver() drv = self._driver path = 'fake/path' size = 2 @@ -908,6 +1027,7 @@ class NfsDriverTestCase(test.TestCase): def test_is_file_size_equal_false(self): """File sizes are not equal.""" + self._set_driver() drv = self._driver path = 'fake/path' size = 2 @@ -925,6 +1045,7 @@ class NfsDriverTestCase(test.TestCase): The NFS driver overrides the base method with a driver specific version. """ + self._set_driver() drv = self._driver drv._mounted_shares = [self.TEST_NFS_EXPORT1] is_new_install = True @@ -948,6 +1069,7 @@ class NfsDriverTestCase(test.TestCase): The NFS driver overrides the base method with a driver specific version. """ + self._set_driver() drv = self._driver drv._mounted_shares = [self.TEST_NFS_EXPORT1] is_new_install = False @@ -968,6 +1090,7 @@ class NfsDriverTestCase(test.TestCase): def test_set_nas_security_options_exception_if_no_mounted_shares(self): """Ensure proper exception is raised if there are no mounted shares.""" + self._set_driver() drv = self._driver drv._ensure_shares_mounted = mock.Mock() drv._mounted_shares = [] @@ -980,6 +1103,7 @@ class NfsDriverTestCase(test.TestCase): def test_ensure_share_mounted(self): """Case where the mount works the first time.""" + self._set_driver() self.mock_object(self._driver._remotefsclient, 'mount') drv = self._driver drv.configuration.nfs_mount_attempts = 3 @@ -995,6 +1119,7 @@ class NfsDriverTestCase(test.TestCase): num_attempts = 3 + self._set_driver() self.mock_object(self._driver._remotefsclient, 'mount', side_effect=Exception) drv = self._driver @@ -1011,6 +1136,7 @@ class NfsDriverTestCase(test.TestCase): min_num_attempts = 1 num_attempts = 0 + self._set_driver() self.mock_object(self._driver._remotefsclient, 'mount', side_effect=Exception) drv = self._driver @@ -1023,6 +1149,217 @@ class NfsDriverTestCase(test.TestCase): self.assertEqual(min_num_attempts, drv._remotefsclient.mount.call_count) + @ddt.data([NFS_CONFIG1, QEMU_IMG_INFO_OUT3], + [NFS_CONFIG2, QEMU_IMG_INFO_OUT4], + [NFS_CONFIG3, QEMU_IMG_INFO_OUT3], + [NFS_CONFIG4, QEMU_IMG_INFO_OUT4]) + @ddt.unpack + def test_copy_volume_from_snapshot(self, nfs_conf, qemu_img_info): + self._set_driver(extra_confs=nfs_conf) + drv = self._driver + dest_volume = self._simple_volume() + src_volume = self._simple_volume() + + fake_snap = fake_snapshot.fake_snapshot_obj(self.context) + fake_snap.volume = src_volume + + img_out = qemu_img_info % {'volid': src_volume.id, + 'snapid': fake_snap.id, + 'size_gb': src_volume.size, + 'size_b': src_volume.size * units.Gi} + + img_info = imageutils.QemuImgInfo(img_out) + mock_img_info = self.mock_object(image_utils, 'qemu_img_info') + mock_img_info.return_value = img_info + mock_convert_image = self.mock_object(image_utils, 'convert_image') + + vol_dir = os.path.join(self.TEST_MNT_POINT_BASE, + drv._get_hash_str(src_volume.provider_location)) + src_vol_path = os.path.join(vol_dir, img_info.backing_file) + dest_vol_path = os.path.join(vol_dir, dest_volume.name) + info_path = os.path.join(vol_dir, src_volume.name) + '.info' + + snap_file = dest_volume.name + '.' + fake_snap.id + snap_path = os.path.join(vol_dir, snap_file) + size = dest_volume.size + + mock_read_info_file = self.mock_object(drv, '_read_info_file') + mock_read_info_file.return_value = {'active': snap_file, + fake_snap.id: snap_file} + + mock_permission = self.mock_object(drv, '_set_rw_permissions_for_all') + + drv._copy_volume_from_snapshot(fake_snap, dest_volume, size) + + mock_read_info_file.assert_called_once_with(info_path) + mock_img_info.assert_called_once_with(snap_path, run_as_root=True) + used_qcow = nfs_conf['nfs_qcow2_volumes'] + mock_convert_image.assert_called_once_with( + src_vol_path, dest_vol_path, 'qcow2' if used_qcow else 'raw', + run_as_root=True) + mock_permission.assert_called_once_with(dest_vol_path) + + @ddt.data([NFS_CONFIG1, QEMU_IMG_INFO_OUT3], + [NFS_CONFIG2, QEMU_IMG_INFO_OUT4], + [NFS_CONFIG3, QEMU_IMG_INFO_OUT3], + [NFS_CONFIG4, QEMU_IMG_INFO_OUT4]) + @ddt.unpack + def test_create_volume_from_snapshot(self, nfs_conf, qemu_img_info): + self._set_driver(extra_confs=nfs_conf) + drv = self._driver + + # Volume source of the snapshot we are trying to clone from. We need it + # to have a different id than the default provided. + src_volume = self._simple_volume(size=10) + src_volume.id = six.text_type(uuid.uuid4()) + src_volume_dir = os.path.join(self.TEST_MNT_POINT_BASE, + drv._get_hash_str( + src_volume.provider_location)) + src_volume_path = os.path.join(src_volume_dir, src_volume.name) + fake_snap = fake_snapshot.fake_snapshot_obj(self.context) + + # Fake snapshot based in the previous created volume + snap_file = src_volume.name + '.' + fake_snap.id + fake_snap.volume = src_volume + fake_snap.status = 'available' + fake_snap.size = 10 + + # New fake volume where the snap will be copied + new_volume = self._simple_volume(size=10) + new_volume_dir = os.path.join(self.TEST_MNT_POINT_BASE, + drv._get_hash_str( + src_volume.provider_location)) + new_volume_path = os.path.join(new_volume_dir, new_volume.name) + + # Mocks + img_out = qemu_img_info % {'volid': src_volume.id, + 'snapid': fake_snap.id, + 'size_gb': src_volume.size, + 'size_b': src_volume.size * units.Gi} + img_info = imageutils.QemuImgInfo(img_out) + mock_img_info = self.mock_object(image_utils, 'qemu_img_info') + mock_img_info.return_value = img_info + + mock_ensure = self.mock_object(drv, '_ensure_shares_mounted') + mock_find_share = self.mock_object(drv, '_find_share', + return_value=self.TEST_NFS_EXPORT1) + mock_read_info_file = self.mock_object(drv, '_read_info_file') + mock_read_info_file.return_value = {'active': snap_file, + fake_snap.id: snap_file} + mock_convert_image = self.mock_object(image_utils, 'convert_image') + self.mock_object(drv, '_create_qcow2_file') + self.mock_object(drv, '_create_regular_file') + self.mock_object(drv, '_create_regular_file') + self.mock_object(drv, '_set_rw_permissions') + self.mock_object(drv, '_read_file') + + ret = drv.create_volume_from_snapshot(new_volume, fake_snap) + + # Test asserts + self.assertEqual(self.TEST_NFS_EXPORT1, ret['provider_location']) + used_qcow = nfs_conf['nfs_qcow2_volumes'] + mock_convert_image.assert_called_once_with( + src_volume_path, new_volume_path, 'qcow2' if used_qcow else 'raw', + run_as_root=True) + mock_ensure.assert_called_once() + mock_find_share.assert_called_once_with(new_volume.size) + + def test_create_volume_from_snapshot_status_not_available(self): + """Expect an error when the snapshot's status is not 'available'.""" + self._set_driver() + drv = self._driver + + src_volume = self._simple_volume() + + fake_snap = fake_snapshot.fake_snapshot_obj(self.context) + fake_snap.volume = src_volume + + new_volume = self._simple_volume() + new_volume['size'] = fake_snap['volume_size'] + + self.assertRaises(exception.InvalidSnapshot, + drv.create_volume_from_snapshot, + new_volume, + fake_snap) + + @ddt.data([NFS_CONFIG1, QEMU_IMG_INFO_OUT1], + [NFS_CONFIG2, QEMU_IMG_INFO_OUT2], + [NFS_CONFIG3, QEMU_IMG_INFO_OUT1], + [NFS_CONFIG4, QEMU_IMG_INFO_OUT2]) + @ddt.unpack + def test_initialize_connection(self, nfs_confs, qemu_img_info): + self._set_driver(extra_confs=nfs_confs) + drv = self._driver + + volume = self._simple_volume() + vol_dir = os.path.join(self.TEST_MNT_POINT_BASE, + drv._get_hash_str(volume.provider_location)) + vol_path = os.path.join(vol_dir, volume.name) + + mock_img_utils = self.mock_object(image_utils, 'qemu_img_info') + img_out = qemu_img_info % {'volid': volume.id, 'size_gb': volume.size, + 'size_b': volume.size * units.Gi} + mock_img_utils.return_value = imageutils.QemuImgInfo(img_out) + self.mock_object(drv, '_read_info_file', + return_value={'active': "volume-%s" % volume.id}) + + conn_info = drv.initialize_connection(volume, None) + + mock_img_utils.assert_called_once_with(vol_path, run_as_root=True) + self.assertEqual('nfs', conn_info['driver_volume_type']) + self.assertEqual(volume.name, conn_info['data']['name']) + self.assertEqual(self.TEST_MNT_POINT_BASE, + conn_info['mount_point_base']) + + @mock.patch.object(image_utils, 'qemu_img_info') + def test_initialize_connection_raise_exception(self, mock_img_info): + self._set_driver() + drv = self._driver + volume = self._simple_volume() + + qemu_img_output = """image: %s + file format: iso + virtual size: 1.0G (1073741824 bytes) + disk size: 173K + """ % volume['name'] + mock_img_info.return_value = imageutils.QemuImgInfo(qemu_img_output) + + self.assertRaises(exception.InvalidVolume, + drv.initialize_connection, + volume, + None) + + def test_create_snapshot(self): + self._set_driver() + drv = self._driver + volume = self._simple_volume() + self.configuration.nfs_snapshot_support = True + fake_snap = fake_snapshot.fake_snapshot_obj(self.context) + fake_snap.volume = volume + vol_dir = os.path.join(self.TEST_MNT_POINT_BASE, + drv._get_hash_str(self.TEST_NFS_EXPORT1)) + snap_file = volume['name'] + '.' + fake_snap.id + snap_path = os.path.join(vol_dir, snap_file) + info_path = os.path.join(vol_dir, volume['name']) + '.info' + + with mock.patch.object(drv, '_local_path_volume_info', + return_value=info_path), \ + mock.patch.object(drv, '_read_info_file', return_value={}), \ + mock.patch.object(drv, '_do_create_snapshot') \ + as mock_do_create_snapshot, \ + mock.patch.object(drv, '_write_info_file') \ + as mock_write_info_file, \ + mock.patch.object(drv, 'get_active_image_from_info', + return_value=volume['name']), \ + mock.patch.object(drv, '_get_new_snap_path', + return_value=snap_path): + self._driver.create_snapshot(fake_snap) + + mock_do_create_snapshot.assert_called_with(fake_snap, volume['name'], + snap_path) + mock_write_info_file.assert_called_with( + info_path, {'active': snap_file, fake_snap.id: snap_file}) + class NfsDriverDoSetupTestCase(test.TestCase): diff --git a/cinder/tests/unit/volume/drivers/test_quobyte.py b/cinder/tests/unit/volume/drivers/test_quobyte.py index e5f08bef608..de5169cf9e5 100644 --- a/cinder/tests/unit/volume/drivers/test_quobyte.py +++ b/cinder/tests/unit/volume/drivers/test_quobyte.py @@ -611,7 +611,8 @@ class QuobyteDriverTestCase(test.TestCase): drv.extend_volume(volume, 3) drv.get_active_image_from_info.assert_called_once_with(volume) - image_utils.qemu_img_info.assert_called_once_with(volume_path) + image_utils.qemu_img_info.assert_called_once_with(volume_path, + run_as_root=False) image_utils.resize_image.assert_called_once_with(volume_path, 3) def test_copy_volume_from_snapshot(self): @@ -662,7 +663,8 @@ class QuobyteDriverTestCase(test.TestCase): drv._copy_volume_from_snapshot(snapshot, dest_volume, size) drv._read_info_file.assert_called_once_with(info_path) - image_utils.qemu_img_info.assert_called_once_with(snap_path) + image_utils.qemu_img_info.assert_called_once_with(snap_path, + run_as_root=False) (image_utils.convert_image. assert_called_once_with(src_vol_path, dest_vol_path, @@ -744,7 +746,8 @@ class QuobyteDriverTestCase(test.TestCase): conn_info = drv.initialize_connection(volume, None) drv.get_active_image_from_info.assert_called_once_with(volume) - image_utils.qemu_img_info.assert_called_once_with(vol_path) + image_utils.qemu_img_info.assert_called_once_with(vol_path, + run_as_root=False) self.assertEqual('raw', conn_info['data']['format']) self.assertEqual('quobyte', conn_info['driver_volume_type']) @@ -789,9 +792,10 @@ class QuobyteDriverTestCase(test.TestCase): mock_get_active_image_from_info.assert_called_once_with(volume) mock_local_volume_dir.assert_called_once_with(volume) - mock_qemu_img_info.assert_called_once_with(volume_path) + mock_qemu_img_info.assert_called_once_with(volume_path, + run_as_root=False) mock_upload_volume.assert_called_once_with( - mock.ANY, mock.ANY, mock.ANY, upload_path) + mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False) self.assertTrue(mock_create_temporary_file.called) def test_copy_volume_to_image_qcow2_image(self): @@ -834,11 +838,12 @@ class QuobyteDriverTestCase(test.TestCase): mock_get_active_image_from_info.assert_called_once_with(volume) mock_local_volume_dir.assert_called_with(volume) - mock_qemu_img_info.assert_called_once_with(volume_path) + mock_qemu_img_info.assert_called_once_with(volume_path, + run_as_root=False) mock_convert_image.assert_called_once_with( - volume_path, upload_path, 'raw') + volume_path, upload_path, 'raw', run_as_root=False) mock_upload_volume.assert_called_once_with( - mock.ANY, mock.ANY, mock.ANY, upload_path) + mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False) self.assertTrue(mock_create_temporary_file.called) def test_copy_volume_to_image_snapshot_exists(self): @@ -883,11 +888,12 @@ class QuobyteDriverTestCase(test.TestCase): mock_get_active_image_from_info.assert_called_once_with(volume) mock_local_volume_dir.assert_called_with(volume) - mock_qemu_img_info.assert_called_once_with(volume_path) + mock_qemu_img_info.assert_called_once_with(volume_path, + run_as_root=False) mock_convert_image.assert_called_once_with( - volume_path, upload_path, 'raw') + volume_path, upload_path, 'raw', run_as_root=False) mock_upload_volume.assert_called_once_with( - mock.ANY, mock.ANY, mock.ANY, upload_path) + mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False) self.assertTrue(mock_create_temporary_file.called) def test_set_nas_security_options_default(self): diff --git a/cinder/tests/unit/volume/drivers/test_remotefs.py b/cinder/tests/unit/volume/drivers/test_remotefs.py index 739cd8015ab..4e1d8f0c16c 100644 --- a/cinder/tests/unit/volume/drivers/test_remotefs.py +++ b/cinder/tests/unit/volume/drivers/test_remotefs.py @@ -192,7 +192,11 @@ class RemoteFsSnapDriverTestCase(test.TestCase): self._driver._write_info_file.assert_called_once_with( mock.sentinel.fake_info_path, expected_info) - def test_do_create_snapshot(self): + @mock.patch.object(remotefs.RemoteFSDriver, + 'secure_file_operations_enabled', + return_value=True) + @mock.patch.object(os, 'stat') + def test_do_create_snapshot(self, _mock_stat, _mock_sec_enabled): self._driver._local_volume_dir = mock.Mock( return_value=self._fake_volume_path) fake_backing_path = os.path.join( @@ -391,7 +395,8 @@ class RemoteFsSnapDriverTestCase(test.TestCase): mock.sentinel.image_path, fake_vol_name, basedir) - mock_qemu_img_info.assert_called_with(mock.sentinel.image_path) + mock_qemu_img_info.assert_called_with(mock.sentinel.image_path, + run_as_root=True) @ddt.data([None, '/fake_basedir'], ['/fake_basedir/cb2016/fake_vol_name', '/fake_basedir'], diff --git a/cinder/tests/unit/volume/drivers/test_zfssa.py b/cinder/tests/unit/volume/drivers/test_zfssa.py index 6f526670d61..d972b3aefdf 100644 --- a/cinder/tests/unit/volume/drivers/test_zfssa.py +++ b/cinder/tests/unit/volume/drivers/test_zfssa.py @@ -30,6 +30,7 @@ from cinder.tests.unit import fake_utils from cinder.tests.unit import utils from cinder.volume import configuration as conf from cinder.volume import driver +from cinder.volume.drivers import nfs as nfsdriver from cinder.volume.drivers import remotefs from cinder.volume.drivers.zfssa import restclient as client from cinder.volume.drivers.zfssa import webdavclient @@ -1050,7 +1051,7 @@ class TestZFSSANFSDriver(test.TestCase): def tearDown(self): super(TestZFSSANFSDriver, self).tearDown() - @mock.patch.object(remotefs.RemoteFSDriver, 'delete_volume') + @mock.patch.object(nfsdriver.NfsDriver, 'delete_volume') @mock.patch.object(zfssanfs.ZFSSANFSDriver, '_check_origin') def test_delete_volume(self, _check_origin, _delete_vol): self.drv.zfssa.get_volume.side_effect = self._get_volume_side_effect @@ -1175,7 +1176,7 @@ class TestZFSSANFSDriver(test.TestCase): img_props_nfs) @mock.patch.object(zfssanfs.ZFSSANFSDriver, '_create_cache_volume') - @mock.patch.object(remotefs.RemoteFSDriver, 'delete_volume') + @mock.patch.object(nfsdriver.NfsDriver, 'delete_volume') def test_verify_cache_vol_updated_vol(self, _del_vol, _create_cache_vol): updated_vol = { 'updated_at': date(3000, 12, 12), @@ -1192,7 +1193,7 @@ class TestZFSSANFSDriver(test.TestCase): img_props_nfs) @mock.patch.object(remotefs.RemoteFSDriver, 'copy_image_to_volume') - @mock.patch.object(remotefs.RemoteFSDriver, 'create_volume') + @mock.patch.object(nfsdriver.NfsDriver, 'create_volume') def test_create_cache_volume(self, _create_vol, _copy_image): self.drv.zfssa.webdavclient = mock.Mock() self.drv._create_cache_volume(fakecontext, diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index c4b6fa9391d..4b85aafdaa7 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -1,4 +1,5 @@ # Copyright (c) 2012 NetApp, Inc. +# Copyright (c) 2016 Red Hat, Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -29,9 +30,11 @@ from cinder.i18n import _, _LE, _LI, _LW from cinder.image import image_utils from cinder import interface from cinder import utils +from cinder.volume import driver from cinder.volume.drivers import remotefs +from cinder.volume.drivers.remotefs import locked_volume_id_operation -VERSION = '1.3.1' +VERSION = '1.4.0' LOG = logging.getLogger(__name__) @@ -39,24 +42,32 @@ LOG = logging.getLogger(__name__) nfs_opts = [ cfg.StrOpt('nfs_shares_config', default='/etc/cinder/nfs_shares', - help='File with the list of available NFS shares'), + help='File with the list of available NFS shares.'), cfg.BoolOpt('nfs_sparsed_volumes', default=True, - help=('Create volumes as sparsed files which take no space.' - 'If set to False volume is created as regular file.' - 'In such case volume creation takes a lot of time.')), + help='Create volumes as sparsed files which take no space. ' + 'If set to False volume is created as regular file. ' + 'In such case volume creation takes a lot of time.'), + cfg.BoolOpt('nfs_qcow2_volumes', + default=False, + help='Create volumes as QCOW2 files rather than raw files.'), cfg.StrOpt('nfs_mount_point_base', default='$state_path/mnt', - help=('Base dir containing mount points for NFS shares.')), + help='Base dir containing mount points for NFS shares.'), cfg.StrOpt('nfs_mount_options', - help=('Mount options passed to the NFS client. See section ' - 'of the NFS man page for details.')), + help='Mount options passed to the NFS client. See section ' + 'of the NFS man page for details.'), cfg.IntOpt('nfs_mount_attempts', default=3, - help=('The number of attempts to mount NFS shares before ' - 'raising an error. At least one attempt will be ' - 'made to mount an NFS share, regardless of the ' - 'value specified.')), + help='The number of attempts to mount NFS shares before ' + 'raising an error. At least one attempt will be ' + 'made to mount an NFS share, regardless of the ' + 'value specified.'), + cfg.BoolOpt('nfs_snapshot_support', + default=False, + help='Enable support for snapshots on the NFS driver. ' + 'Platforms using libvirt <1.2.7 will encounter issues ' + 'with this feature.'), ] CONF = cfg.CONF @@ -64,7 +75,7 @@ CONF.register_opts(nfs_opts) @interface.volumedriver -class NfsDriver(remotefs.RemoteFSDriver): +class NfsDriver(remotefs.RemoteFSSnapDriver, driver.ExtendVD): """NFS based cinder driver. Creates file on NFS share for using it as block device on hypervisor. @@ -109,6 +120,36 @@ class NfsDriver(remotefs.RemoteFSDriver): self.max_over_subscription_ratio = ( self.configuration.max_over_subscription_ratio) + def initialize_connection(self, volume, connector): + + LOG.debug('Initializing connection to volume %(vol)s. ' + 'Connector: %(con)s', {'vol': volume.id, 'con': connector}) + + active_vol = self.get_active_image_from_info(volume) + volume_dir = self._local_volume_dir(volume) + path_to_vol = os.path.join(volume_dir, active_vol) + info = self._qemu_img_info(path_to_vol, volume['name']) + + data = {'export': volume.provider_location, + 'name': active_vol} + if volume.provider_location in self.shares: + data['options'] = self.shares[volume.provider_location] + + conn_info = { + 'driver_volume_type': self.driver_volume_type, + 'data': data, + 'mount_point_base': self._get_mount_point_base() + } + + # Test file for raw vs. qcow2 format + if info.file_format not in ['raw', 'qcow2']: + msg = _('nfs volume must be a valid raw or qcow2 image.') + raise exception.InvalidVolume(reason=msg) + + conn_info['data']['format'] = info.file_format + LOG.debug('NfsDriver: conn_info: %s', conn_info) + return conn_info + def do_setup(self, context): """Any initialization the volume driver does while starting.""" super(NfsDriver, self).do_setup(context) @@ -155,6 +196,7 @@ class NfsDriver(remotefs.RemoteFSDriver): # Now that all configuration data has been loaded (shares), # we can "set" our final NAS file security options. self.set_nas_security_options(self._is_voldb_empty_at_startup) + self._check_snapshot_support(setup_checking=True) def _ensure_share_mounted(self, nfs_share): mnt_flags = [] @@ -294,19 +336,17 @@ class NfsDriver(remotefs.RemoteFSDriver): :param nfs_share: example 172.18.194.100:/var/nfs """ - run_as_root = self._execute_as_root - mount_point = self._get_mount_point_for_share(nfs_share) df, _ = self._execute('stat', '-f', '-c', '%S %b %a', mount_point, - run_as_root=run_as_root) + run_as_root=self._execute_as_root) block_size, blocks_total, blocks_avail = map(float, df.split()) total_available = block_size * blocks_avail total_size = block_size * blocks_total du, _ = self._execute('du', '-sb', '--apparent-size', '--exclude', '*snapshot*', mount_point, - run_as_root=run_as_root) + run_as_root=self._execute_as_root) total_allocated = float(du.split()[0]) return total_size, total_available, total_allocated @@ -379,10 +419,15 @@ class NfsDriver(remotefs.RemoteFSDriver): # If secure NAS, update the '_execute_as_root' flag to not # run as the root user; run as process' user ID. + + # TODO(eharney): need to separate secure NAS vs. execute as root. + # There are requirements to run some commands as root even + # when running in secure NAS mode. (i.e. read volume file + # attached to an instance and owned by qemu:qemu) if self.configuration.nas_secure_file_operations == 'true': self._execute_as_root = False - LOG.debug('NAS variable secure_file_operations setting is: %s', + LOG.debug('NAS secure file operations setting is: %s', self.configuration.nas_secure_file_operations) if self.configuration.nas_secure_file_operations == 'false': @@ -407,8 +452,6 @@ class NfsDriver(remotefs.RemoteFSDriver): :param original_volume_status: The status of the original volume :returns: model_update to update DB with any needed changes """ - # TODO(vhou) This method may need to be updated after - # NFS snapshots are introduced. name_id = None if original_volume_status == 'available': current_name = CONF.volume_name_template % new_volume.id @@ -455,3 +498,108 @@ class NfsDriver(remotefs.RemoteFSDriver): data['thick_provisioning_support'] = not thin_enabled self._stats = data + + @locked_volume_id_operation + def create_volume(self, volume): + """Apply locking to the create volume operation.""" + + return super(NfsDriver, self).create_volume(volume) + + @locked_volume_id_operation + def delete_volume(self, volume): + """Deletes a logical volume.""" + + LOG.debug('Deleting volume %(vol)s, provider_location: %(loc)s', + {'vol': volume.id, 'loc': volume.provider_location}) + + if not volume.provider_location: + LOG.warning(_LW('Volume %s does not have provider_location ' + 'specified, skipping'), volume.name) + return + + info_path = self._local_path_volume_info(volume) + info = self._read_info_file(info_path, empty_if_missing=True) + + if info: + base_volume_path = os.path.join(self._local_volume_dir(volume), + info['active']) + self._delete(info_path) + else: + base_volume_path = self._local_path_volume(volume) + + self._delete(base_volume_path) + + def _qemu_img_info(self, path, volume_name): + return super(NfsDriver, self)._qemu_img_info_base( + path, volume_name, self.configuration.nfs_mount_point_base) + + def _check_snapshot_support(self, setup_checking=False): + """Ensure snapshot support is enabled in config.""" + + if (not self.configuration.nfs_snapshot_support and + not setup_checking): + msg = _("NFS driver snapshot support is disabled in cinder.conf.") + raise exception.VolumeDriverException(message=msg) + + if (self.configuration.nas_secure_file_operations == 'true' and + self.configuration.nfs_snapshot_support): + msg = _("Snapshots are not supported with " + "nas_secure_file_operations enabled ('true' or 'auto'). " + "Please set it to 'false' if you intend to have " + "it enabled.") + LOG.error(msg) + raise exception.VolumeDriverException(message=msg) + + @locked_volume_id_operation + def create_snapshot(self, snapshot): + """Apply locking to the create snapshot operation.""" + + self._check_snapshot_support() + return self._create_snapshot(snapshot) + + @locked_volume_id_operation + def delete_snapshot(self, snapshot): + """Apply locking to the delete snapshot operation.""" + + self._check_snapshot_support() + return self._delete_snapshot(snapshot) + + def _copy_volume_from_snapshot(self, snapshot, volume, volume_size): + """Copy data from snapshot to destination volume. + + This is done with a qemu-img convert to raw/qcow2 from the snapshot + qcow2. + """ + + LOG.debug("Copying snapshot: %(snap)s -> volume: %(vol)s, " + "volume_size: %(size)s GB", + {'snap': snapshot.id, + 'vol': volume.id, + 'size': volume_size}) + + info_path = self._local_path_volume_info(snapshot.volume) + snap_info = self._read_info_file(info_path) + vol_path = self._local_volume_dir(snapshot.volume) + forward_file = snap_info[snapshot.id] + forward_path = os.path.join(vol_path, forward_file) + + # Find the file which backs this file, which represents the point + # when this snapshot was created. + img_info = self._qemu_img_info(forward_path, snapshot.volume.name) + path_to_snap_img = os.path.join(vol_path, img_info.backing_file) + + path_to_new_vol = self._local_path_volume(volume) + + LOG.debug("will copy from snapshot at %s", path_to_snap_img) + + if self.configuration.nfs_qcow2_volumes: + out_format = 'qcow2' + else: + out_format = 'raw' + + image_utils.convert_image(path_to_snap_img, + path_to_new_vol, + out_format, + run_as_root=self._execute_as_root) + + self._set_rw_permissions_for_all(path_to_new_vol) diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index 65605f56015..e120ddad93b 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -187,6 +187,8 @@ class RemoteFSDriver(driver.BaseVD): self.configuration.nas_secure_file_permissions, 'nas_secure_file_operations': self.configuration.nas_secure_file_operations} + + LOG.debug('NAS config: %s', secure_options) for opt_name, opt_value in secure_options.items(): if opt_value not in valid_secure_opts: err_parms = {'name': opt_name, 'value': opt_value} @@ -205,7 +207,7 @@ class RemoteFSDriver(driver.BaseVD): for share in self.shares.keys(): mount_path = self._get_mount_point_for_share(share) out, _ = self._execute('du', '--bytes', mount_path, - run_as_root=True) + run_as_root=self._execute_as_root) provisioned_size += int(out.split()[0]) return round(provisioned_size / units.Gi, 2) @@ -231,6 +233,8 @@ class RemoteFSDriver(driver.BaseVD): :param volume: volume reference :returns: provider_location update dict for database """ + + LOG.debug('Creating volume %(vol)s', {'vol': volume.id}) self._ensure_shares_mounted() volume.provider_location = self._find_share(volume.size) @@ -250,7 +254,12 @@ class RemoteFSDriver(driver.BaseVD): volume_size = volume.size if getattr(self.configuration, - self.driver_prefix + '_sparsed_volumes'): + self.driver_prefix + '_qcow2_volumes', False): + # QCOW2 volumes are inherently sparse, so this setting + # will override the _sparsed_volumes setting. + self._create_qcow2_file(volume_path, volume_size) + elif getattr(self.configuration, + self.driver_prefix + '_sparsed_volumes', False): self._create_sparsed_file(volume_path, volume_size) else: self._create_regular_file(volume_path, volume_size) @@ -282,6 +291,9 @@ class RemoteFSDriver(driver.BaseVD): :param volume: volume reference """ + + LOG.debug('Deleting volume %(vol)s, provider_location: %(loc)s', + {'vol': volume.id, 'loc': volume.provider_location}) if not volume.provider_location: LOG.warning(_LW('Volume %s does not have ' 'provider_location specified, ' @@ -342,7 +354,7 @@ class RemoteFSDriver(driver.BaseVD): def _fallocate(self, path, size): """Creates a raw file of given size in GiB using fallocate.""" self._execute('fallocate', '--length=%sG' % size, - path, run_as_root=True) + path, run_as_root=self._execute_as_root) def _create_qcow2_file(self, path, size_gb): """Creates a QCOW2 file of a given size in GiB.""" @@ -395,7 +407,6 @@ class RemoteFSDriver(driver.BaseVD): def copy_image_to_volume(self, context, volume, image_service, image_id): """Fetch the image from image_service and write it to the volume.""" - run_as_root = self._execute_as_root image_utils.fetch_to_raw(context, image_service, @@ -403,7 +414,7 @@ class RemoteFSDriver(driver.BaseVD): self.local_path(volume), self.configuration.volume_dd_blocksize, size=volume.size, - run_as_root=run_as_root) + run_as_root=self._execute_as_root) # NOTE (leseb): Set the virtual size of the image # the raw conversion overwrote the destination file @@ -413,10 +424,10 @@ class RemoteFSDriver(driver.BaseVD): # this sets the size to the one asked in the first place by the user # and then verify the final virtual size image_utils.resize_image(self.local_path(volume), volume.size, - run_as_root=run_as_root) + run_as_root=self._execute_as_root) data = image_utils.qemu_img_info(self.local_path(volume), - run_as_root=run_as_root) + run_as_root=self._execute_as_root) virt_size = data.virtual_size // units.Gi if virt_size != volume.size: raise exception.ImageUnacceptable( @@ -429,7 +440,8 @@ class RemoteFSDriver(driver.BaseVD): image_utils.upload_volume(context, image_service, image_meta, - self.local_path(volume)) + self.local_path(volume), + run_as_root=self._execute_as_root) def _read_config_file(self, config_file): # Returns list of lines in file @@ -613,7 +625,7 @@ class RemoteFSDriver(driver.BaseVD): # Set the permissions on our special marker file to # protect from accidental removal (owner write only). self._execute('chmod', '640', file_path, - run_as_root=False) + run_as_root=self._execute_as_root) LOG.info(_LI('New Cinder secure environment indicator' ' file created at path %s.'), file_path) except IOError as err: @@ -691,7 +703,8 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): This code expects to deal only with relative filenames. """ - info = image_utils.qemu_img_info(path) + info = image_utils.qemu_img_info(path, + run_as_root=self._execute_as_root) if info.image: info.image = os.path.basename(info.image) if info.backing_file: @@ -722,11 +735,19 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): raise NotImplementedError() def _img_commit(self, path): + # TODO(eharney): this is not using the correct permissions for + # NFS snapshots + # It needs to run as root for volumes attached to instances, but + # does not when in secure mode. self._execute('qemu-img', 'commit', path, run_as_root=self._execute_as_root) self._delete(path) def _rebase_img(self, image, backing_file, volume_format): + # qemu-img create must run as root, because it reads from the + # backing file, which will be owned by qemu:qemu if attached to an + # instance. + # TODO(erlon): Sanity check this. self._execute('qemu-img', 'rebase', '-u', '-b', backing_file, image, '-F', volume_format, run_as_root=self._execute_as_root) @@ -880,7 +901,8 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): # Convert due to snapshots # or volume data not being stored in raw format # (upload_volume assumes raw format input) - image_utils.convert_image(active_file_path, temp_path, 'raw') + image_utils.convert_image(active_file_path, temp_path, 'raw', + run_as_root=self._execute_as_root) upload_path = temp_path else: upload_path = active_file_path @@ -888,7 +910,8 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): image_utils.upload_volume(context, image_service, image_meta, - upload_path) + upload_path, + run_as_root=self._execute_as_root) def get_active_image_from_info(self, volume): """Returns filename of the active image from the info file.""" @@ -909,8 +932,10 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): {'src': src_vref.id, 'dst': volume.id}) - if src_vref.status != 'available': - msg = _("Volume status must be 'available'.") + if src_vref.status not in ['available', 'backing-up']: + msg = _("Source volume status must be 'available', or " + "'backing-up' but is: " + "%(status)s.") % {'status': src_vref.status} raise exception.InvalidVolume(msg) volume_name = CONF.volume_name_template % volume.id @@ -981,11 +1006,17 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): """ - LOG.debug('Deleting snapshot %s:', snapshot.id) + LOG.debug('Deleting %(type)s snapshot %(snap)s of volume %(vol)s', + {'snap': snapshot.id, 'vol': snapshot.volume.id, + 'type': ('online' if snapshot.volume.status == 'in-use' + else 'offline')}) volume_status = snapshot.volume.status - if volume_status not in ['available', 'in-use']: - msg = _('Volume status must be "available" or "in-use".') + if volume_status not in ['available', 'in-use', 'backing-up']: + msg = _("Volume status must be 'available', 'in-use' or " + "'backing-up' but is: " + "%(status)s.") % {'status': volume_status} + raise exception.InvalidVolume(msg) vol_path = self._local_volume_dir(snapshot.volume) @@ -1111,8 +1142,13 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): Snapshot must not be the active snapshot. (offline) """ + LOG.debug('Creating volume %(vol)s from snapshot %(snap)s', + {'vol': volume.id, 'snap': snapshot.id}) + if snapshot.status != 'available': - msg = _('Snapshot status must be "available" to clone.') + msg = _('Snapshot status must be "available" to clone. ' + 'But is: %(status)s') % {'status': snapshot.status} + raise exception.InvalidSnapshot(msg) self._ensure_shares_mounted() @@ -1142,6 +1178,15 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): backing_path_full_path = os.path.join( self._local_volume_dir(snapshot.volume), backing_filename) + + command = ['qemu-img', 'create', '-f', 'qcow2', '-o', + 'backing_file=%s' % backing_path_full_path, new_snap_path] + + # qemu-img create must run as root, because it reads from the + # backing file, which will be owned by qemu:qemu if attached to an + # instance. (TODO(eharney): sanity check this) + self._execute(*command, run_as_root=self._execute_as_root) + info = self._qemu_img_info(backing_path_full_path, snapshot.volume.name) backing_fmt = info.file_format @@ -1157,10 +1202,24 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): '-b', backing_filename, '-F', backing_fmt, new_snap_path] + + # qemu-img rebase must run as root for the same reasons as above self._execute(*command, run_as_root=self._execute_as_root) self._set_rw_permissions(new_snap_path) + # if in secure mode, chown new file + if self.secure_file_operations_enabled(): + ref_file = backing_path_full_path + log_msg = 'Setting permissions: %(file)s -> %(user)s:%(group)s' % { + 'file': ref_file, 'user': os.stat(ref_file).st_uid, + 'group': os.stat(ref_file).st_gid} + LOG.debug(log_msg) + command = ['chown', + '--reference=%s' % ref_file, + new_snap_path] + self._execute(*command, run_as_root=self._execute_as_root) + def _create_snapshot(self, snapshot): """Create a snapshot. @@ -1265,10 +1324,17 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): info file: { 'active': 'volume-1234' } (* changed!) """ + LOG.debug('Creating %(type)s snapshot %(snap)s of volume %(vol)s', + {'snap': snapshot.id, 'vol': snapshot.volume.id, + 'type': ('online' if snapshot.volume.status == 'in-use' + else 'offline')}) + status = snapshot.volume.status - if status not in ['available', 'in-use']: - msg = _('Volume status must be "available" or "in-use"' - ' for snapshot. (is %s)') % status + if status not in ['available', 'in-use', 'backing-up']: + msg = _("Volume status must be 'available', 'in-use' or " + "'backing-up' but is: " + "%(status)s.") % {'status': status} + raise exception.InvalidVolume(msg) info_path = self._local_path_volume_info(snapshot.volume) @@ -1451,7 +1517,8 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): # Delete stale file path_to_delete = os.path.join( self._local_volume_dir(snapshot.volume), file_to_delete) - self._execute('rm', '-f', path_to_delete, run_as_root=True) + self._execute('rm', '-f', path_to_delete, + run_as_root=self._execute_as_root) class RemoteFSSnapDriver(RemoteFSSnapDriverBase): diff --git a/releasenotes/notes/nfs-snapshots-21b641300341cba1.yaml b/releasenotes/notes/nfs-snapshots-21b641300341cba1.yaml new file mode 100644 index 00000000000..222df03e589 --- /dev/null +++ b/releasenotes/notes/nfs-snapshots-21b641300341cba1.yaml @@ -0,0 +1,5 @@ +--- +features: + - Added support to snapshots in NFS driver. This functionality is only + enabled if "nfs_snapshot_support" is set to True in cinder.conf. Cloning + volumes is only supported if the source volume is not attached.