diff --git a/cinder/tests/unit/test_smbfs.py b/cinder/tests/unit/test_smbfs.py index 92f0d312050..bb1a65a5ceb 100644 --- a/cinder/tests/unit/test_smbfs.py +++ b/cinder/tests/unit/test_smbfs.py @@ -13,19 +13,37 @@ # under the License. import copy +import functools import os import mock +from oslo_utils import fileutils from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.volume.drivers import remotefs from cinder.volume.drivers import smbfs +def requires_allocation_data_update(expected_size): + def wrapper(func): + @functools.wraps(func) + def inner(inst, *args, **kwargs): + with mock.patch.object( + inst._smbfs_driver, + 'update_disk_allocation_data') as fake_update: + func(inst, *args, **kwargs) + fake_update.assert_called_once_with(inst._FAKE_VOLUME, + expected_size) + return inner + return wrapper + + class SmbFsTestCase(test.TestCase): _FAKE_SHARE = '//1.2.3.4/share1' + _FAKE_SHARE_HASH = 'db0bf952c1734092b83e8990bd321131' _FAKE_MNT_BASE = '/mnt' _FAKE_VOLUME_NAME = 'volume-4f711859-4928-4cb7-801a-a50c37ceaccc' _FAKE_TOTAL_SIZE = '2048' @@ -36,7 +54,7 @@ class SmbFsTestCase(test.TestCase): 'provider_location': _FAKE_SHARE, 'name': _FAKE_VOLUME_NAME, 'status': 'available'} - _FAKE_MNT_POINT = os.path.join(_FAKE_MNT_BASE, 'fake_hash') + _FAKE_MNT_POINT = os.path.join(_FAKE_MNT_BASE, _FAKE_SHARE_HASH) _FAKE_VOLUME_PATH = os.path.join(_FAKE_MNT_POINT, _FAKE_VOLUME_NAME) _FAKE_SNAPSHOT_ID = '5g811859-4928-4cb7-801a-a50c37ceacba' _FAKE_SNAPSHOT = {'id': _FAKE_SNAPSHOT_ID, @@ -48,9 +66,9 @@ class SmbFsTestCase(test.TestCase): _FAKE_SHARE_OPTS = '-o username=Administrator,password=12345' _FAKE_OPTIONS_DICT = {'username': 'Administrator', 'password': '12345'} + _FAKE_ALLOCATION_DATA_PATH = os.path.join('fake_dir', + 'fake_allocation_data') - _FAKE_LISTDIR = [_FAKE_VOLUME_NAME, _FAKE_VOLUME_NAME + '.vhd', - _FAKE_VOLUME_NAME + '.vhdx', 'fake_folder'] _FAKE_SMBFS_CONFIG = mock.MagicMock() _FAKE_SMBFS_CONFIG.smbfs_oversub_ratio = 2 _FAKE_SMBFS_CONFIG.smbfs_used_ratio = 0.5 @@ -67,7 +85,88 @@ class SmbFsTestCase(test.TestCase): return_value=self._FAKE_MNT_POINT) self._smbfs_driver._execute = mock.Mock() self._smbfs_driver.base = self._FAKE_MNT_BASE + self._smbfs_driver._alloc_info_file_path = ( + self._FAKE_ALLOCATION_DATA_PATH) + def _get_fake_allocation_data(self): + return {self._FAKE_SHARE_HASH: { + 'total_allocated': self._FAKE_TOTAL_ALLOCATED}} + + @mock.patch.object(smbfs, 'open', create=True) + @mock.patch('os.path.exists') + @mock.patch.object(fileutils, 'ensure_tree') + @mock.patch('json.load') + def _test_setup_allocation_data(self, mock_json_load, mock_ensure_tree, + mock_exists, mock_open, + allocation_data_exists=False): + mock_exists.return_value = allocation_data_exists + self._smbfs_driver._update_allocation_data_file = mock.Mock() + + self._smbfs_driver._setup_allocation_data() + + if allocation_data_exists: + fd = mock_open.return_value.__enter__.return_value + mock_json_load.assert_called_once_with(fd) + self.assertEqual(mock_json_load.return_value, + self._smbfs_driver._allocation_data) + else: + mock_ensure_tree.assert_called_once_with( + os.path.dirname(self._FAKE_ALLOCATION_DATA_PATH)) + update_func = self._smbfs_driver._update_allocation_data_file + update_func.assert_called_once_with() + + def test_setup_allocation_data_file_unexisting(self): + self._test_setup_allocation_data() + + def test_setup_allocation_data_file_existing(self): + self._test_setup_allocation_data(allocation_data_exists=True) + + def _test_update_allocation_data(self, virtual_size_gb=None, + volume_exists=True): + self._smbfs_driver._update_allocation_data_file = mock.Mock() + update_func = self._smbfs_driver._update_allocation_data_file + + fake_alloc_data = self._get_fake_allocation_data() + if volume_exists: + fake_alloc_data[self._FAKE_SHARE_HASH][ + self._FAKE_VOLUME_NAME] = self._FAKE_VOLUME['size'] + + self._smbfs_driver._allocation_data = fake_alloc_data + + self._smbfs_driver.update_disk_allocation_data(self._FAKE_VOLUME, + virtual_size_gb) + + vol_allocated_size = fake_alloc_data[self._FAKE_SHARE_HASH].get( + self._FAKE_VOLUME_NAME, None) + if not virtual_size_gb: + expected_total_allocated = (self._FAKE_TOTAL_ALLOCATED - + self._FAKE_VOLUME['size']) + + self.assertIsNone(vol_allocated_size) + else: + expected_total_allocated = (self._FAKE_TOTAL_ALLOCATED + + virtual_size_gb - + self._FAKE_VOLUME['size']) + self.assertEqual(virtual_size_gb, vol_allocated_size) + + update_func.assert_called_once_with() + + self.assertEqual( + expected_total_allocated, + fake_alloc_data[self._FAKE_SHARE_HASH]['total_allocated']) + + def test_update_allocation_data_volume_deleted(self): + self._test_update_allocation_data() + + def test_update_allocation_data_volume_extended(self): + self._test_update_allocation_data( + virtual_size_gb=self._FAKE_VOLUME['size'] + 1) + + def test_update_allocation_data_volume_created(self): + self._test_update_allocation_data( + virtual_size_gb=self._FAKE_VOLUME['size']) + + @requires_allocation_data_update(expected_size=None) def test_delete_volume(self): drv = self._smbfs_driver fake_vol_info = self._FAKE_VOLUME_PATH + '.info' @@ -208,9 +307,8 @@ class SmbFsTestCase(test.TestCase): self._smbfs_driver._mounted_shares = mounted_shares self._smbfs_driver._is_share_eligible = mock.Mock( return_value=eligible_shares) - fake_capacity_info = ((2, 1, 5), (2, 1, 4), (2, 1, 1)) - self._smbfs_driver._get_capacity_info = mock.Mock( - side_effect=fake_capacity_info) + self._smbfs_driver._get_total_allocated = mock.Mock( + side_effect=[3, 2, 1]) if not mounted_shares: self.assertRaises(exception.SmbfsNoSharesMounted, @@ -425,12 +523,11 @@ class SmbFsTestCase(test.TestCase): fake_convert: if extend_failed: self.assertRaises(exception.ExtendVolumeError, - drv._extend_volume, + drv.extend_volume, self._FAKE_VOLUME, mock.sentinel.new_size) else: - drv._extend_volume( - self._FAKE_VOLUME, - mock.sentinel.new_size) + drv.extend_volume(self._FAKE_VOLUME, mock.sentinel.new_size) + if image_format in (drv._DISK_FORMAT_VHDX, drv._DISK_FORMAT_VHD_LEGACY): fake_tmp_path = self._FAKE_VOLUME_PATH + '.tmp' @@ -445,12 +542,14 @@ class SmbFsTestCase(test.TestCase): fake_resize.assert_called_once_with( self._FAKE_VOLUME_PATH, mock.sentinel.new_size) + @requires_allocation_data_update(expected_size=mock.sentinel.new_size) def test_extend_volume(self): self._test_extend_volume() def test_extend_volume_failed(self): self._test_extend_volume(extend_failed=True) + @requires_allocation_data_update(expected_size=mock.sentinel.new_size) def test_extend_vhd_volume(self): self._test_extend_volume(image_format='vpc') @@ -492,6 +591,29 @@ class SmbFsTestCase(test.TestCase): def test_check_extend_volume_uneligible_share(self): self._test_check_extend_support(is_eligible=False) + @requires_allocation_data_update(expected_size=_FAKE_VOLUME['size']) + @mock.patch.object(remotefs.RemoteFSSnapDriver, 'create_volume') + def test_create_volume_base(self, mock_create_volume): + self._smbfs_driver.create_volume(self._FAKE_VOLUME) + mock_create_volume.assert_called_once_with(self._FAKE_VOLUME) + + @requires_allocation_data_update(expected_size=_FAKE_VOLUME['size']) + @mock.patch.object(smbfs.SmbfsDriver, + '_create_volume_from_snapshot') + def test_create_volume_from_snapshot(self, mock_create_volume): + self._smbfs_driver.create_volume_from_snapshot(self._FAKE_VOLUME, + self._FAKE_SNAPSHOT) + mock_create_volume.assert_called_once_with(self._FAKE_VOLUME, + self._FAKE_SNAPSHOT) + + @requires_allocation_data_update(expected_size=_FAKE_VOLUME['size']) + @mock.patch.object(smbfs.SmbfsDriver, '_create_cloned_volume') + def test_create_cloned_volume(self, mock_create_volume): + self._smbfs_driver.create_cloned_volume(self._FAKE_VOLUME, + mock.sentinel.src_vol) + mock_create_volume.assert_called_once_with(self._FAKE_VOLUME, + mock.sentinel.src_vol) + def test_create_volume_from_in_use_snapshot(self): fake_snapshot = {'status': 'in-use'} self.assertRaises( @@ -597,19 +719,18 @@ class SmbFsTestCase(test.TestCase): fake_block_size = 4096.0 fake_total_blocks = 1024 fake_avail_blocks = 512 - fake_total_allocated = fake_total_blocks * fake_block_size fake_df = ('%s %s %s' % (fake_block_size, fake_total_blocks, fake_avail_blocks), None) - fake_du = (str(fake_total_allocated), None) self._smbfs_driver._get_mount_point_for_share = mock.Mock( return_value=self._FAKE_MNT_POINT) - self._smbfs_driver._execute = mock.Mock( - side_effect=(fake_df, fake_du)) + self._smbfs_driver._get_total_allocated = mock.Mock( + return_value=self._FAKE_TOTAL_ALLOCATED) + self._smbfs_driver._execute.return_value = fake_df ret_val = self._smbfs_driver._get_capacity_info(self._FAKE_SHARE) expected = (fake_block_size * fake_total_blocks, fake_block_size * fake_avail_blocks, - fake_total_allocated) + self._FAKE_TOTAL_ALLOCATED) self.assertEqual(expected, ret_val) diff --git a/cinder/tests/unit/windows/test_smbfs.py b/cinder/tests/unit/windows/test_smbfs.py index b02bbf22a19..caff008ed31 100644 --- a/cinder/tests/unit/windows/test_smbfs.py +++ b/cinder/tests/unit/windows/test_smbfs.py @@ -30,8 +30,6 @@ class WindowsSmbFsTestCase(test.TestCase): _FAKE_MNT_POINT = os.path.join(_FAKE_MNT_BASE, 'fake_hash') _FAKE_VOLUME_NAME = 'volume-4f711859-4928-4cb7-801a-a50c37ceaccc' _FAKE_SNAPSHOT_NAME = _FAKE_VOLUME_NAME + '-snapshot.vhdx' - _FAKE_VOLUME_PATH = os.path.join(_FAKE_MNT_POINT, - _FAKE_VOLUME_NAME) _FAKE_SNAPSHOT_PATH = os.path.join(_FAKE_MNT_POINT, _FAKE_SNAPSHOT_NAME) _FAKE_TOTAL_SIZE = '2048' @@ -46,8 +44,6 @@ class WindowsSmbFsTestCase(test.TestCase): _FAKE_SHARE_OPTS = '-o username=Administrator,password=12345' _FAKE_VOLUME_PATH = os.path.join(_FAKE_MNT_POINT, _FAKE_VOLUME_NAME + '.vhdx') - _FAKE_LISTDIR = [_FAKE_VOLUME_NAME + '.vhd', - _FAKE_VOLUME_NAME + '.vhdx', 'fake_folder'] def setUp(self): super(WindowsSmbFsTestCase, self).setUp() @@ -105,24 +101,6 @@ class WindowsSmbFsTestCase(test.TestCase): self._FAKE_TOTAL_ALLOCATED]] self.assertEqual(expected_ret_val, ret_val) - def test_get_total_allocated(self): - fake_listdir = mock.Mock(side_effect=[self._FAKE_LISTDIR, - self._FAKE_LISTDIR[:-1]]) - fake_folder_path = os.path.join(self._FAKE_SHARE, 'fake_folder') - fake_isdir = lambda x: x == fake_folder_path - self._smbfs_driver._remotefsclient.is_symlink = mock.Mock( - return_value=False) - fake_getsize = mock.Mock(return_value=self._FAKE_VOLUME['size']) - self._smbfs_driver.vhdutils.get_vhd_size = mock.Mock( - return_value={'VirtualSize': 1}) - - with mock.patch.multiple('os.path', isdir=fake_isdir, - getsize=fake_getsize): - with mock.patch('os.listdir', fake_listdir): - ret_val = self._smbfs_driver._get_total_allocated( - self._FAKE_SHARE) - self.assertEqual(4, ret_val) - def _test_get_img_info(self, backing_file=None): self._smbfs_driver.vhdutils.get_vhd_parent_path.return_value = ( backing_file) diff --git a/cinder/volume/drivers/smbfs.py b/cinder/volume/drivers/smbfs.py index 312b922a8e2..59640388301 100644 --- a/cinder/volume/drivers/smbfs.py +++ b/cinder/volume/drivers/smbfs.py @@ -13,12 +13,17 @@ # License for the specific language governing permissions and limitations # under the License. +import decorator + +import inspect +import json import os from os_brick.remotefs import remotefs from oslo_concurrency import processutils as putils from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import fileutils from oslo_utils import units from cinder import exception @@ -36,6 +41,10 @@ volume_opts = [ cfg.StrOpt('smbfs_shares_config', default='/etc/cinder/smbfs_shares', help='File with the list of available smbfs shares.'), + cfg.StrOpt('smbfs_allocation_info_file_path', + default='$state_path/allocation_data', + help=('The path of the automatically generated file containing ' + 'information about volume disk space allocation.')), cfg.StrOpt('smbfs_default_volume_format', default='qcow2', choices=['raw', 'qcow2', 'vhd', 'vhdx'], @@ -69,6 +78,25 @@ CONF = cfg.CONF CONF.register_opts(volume_opts) +def update_allocation_data(delete=False): + @decorator.decorator + def wrapper(func, inst, *args, **kwargs): + ret_val = func(inst, *args, **kwargs) + + call_args = inspect.getcallargs(func, inst, *args, **kwargs) + volume = call_args['volume'] + requested_size = call_args.get('size_gb', None) + + if delete: + allocated_size_gb = None + else: + allocated_size_gb = requested_size or volume['size'] + + inst.update_disk_allocation_data(volume, allocated_size_gb) + return ret_val + return wrapper + + class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): """SMBFS based cinder volume driver.""" @@ -100,6 +128,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): smbfs_mount_point_base=self.base, smbfs_mount_options=opts) self.img_suffix = None + self._alloc_info_file_path = CONF.smbfs_allocation_info_file_path def _qemu_img_info(self, path, volume_name): return super(SmbfsDriver, self)._qemu_img_info_base( @@ -163,6 +192,51 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): self.shares = {} # address : options self._ensure_shares_mounted() + self._setup_allocation_data() + + def _setup_allocation_data(self): + if not os.path.exists(self._alloc_info_file_path): + fileutils.ensure_tree( + os.path.dirname(self._alloc_info_file_path)) + self._allocation_data = {} + self._update_allocation_data_file() + else: + with open(self._alloc_info_file_path, 'r') as f: + self._allocation_data = json.load(f) + + def update_disk_allocation_data(self, volume, virtual_size_gb=None): + volume_name = volume['name'] + smbfs_share = volume['provider_location'] + if smbfs_share: + share_hash = self._get_hash_str(smbfs_share) + else: + return + + share_alloc_data = self._allocation_data.get(share_hash, {}) + old_virtual_size = share_alloc_data.get(volume_name, 0) + total_allocated = share_alloc_data.get('total_allocated', 0) + + if virtual_size_gb: + share_alloc_data[volume_name] = virtual_size_gb + total_allocated += virtual_size_gb - old_virtual_size + elif share_alloc_data.get(volume_name): + # The volume is deleted. + del share_alloc_data[volume_name] + total_allocated -= old_virtual_size + + share_alloc_data['total_allocated'] = total_allocated + self._allocation_data[share_hash] = share_alloc_data + self._update_allocation_data_file() + + def _update_allocation_data_file(self): + with open(self._alloc_info_file_path, 'w') as f: + json.dump(self._allocation_data, f) + + def _get_total_allocated(self, smbfs_share): + share_hash = self._get_hash_str(smbfs_share) + share_alloc_data = self._allocation_data.get(share_hash, {}) + total_allocated = share_alloc_data.get('total_allocated', 0) << 30 + return float(total_allocated) def local_path(self, volume): """Get volume path (mounted locally fs path) for given volume. @@ -224,6 +298,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): return volume_format @remotefs_drv.locked_volume_id_operation + @update_allocation_data(delete=True) def delete_volume(self, volume): """Deletes a logical volume.""" if not volume['provider_location']: @@ -254,6 +329,11 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): volume_path, str(volume_size * units.Gi), run_as_root=True) + @remotefs_drv.locked_volume_id_operation + @update_allocation_data() + def create_volume(self, volume): + return super(SmbfsDriver, self).create_volume(volume) + def _do_create_volume(self, volume): """Create a volume on given smbfs_share. @@ -298,9 +378,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): 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=True) - total_allocated = float(du.split()[0]) + total_allocated = self._get_total_allocated(smbfs_share) return total_size, total_available, total_allocated def _find_share(self, volume_size_in_gib): @@ -321,7 +399,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): for smbfs_share in self._mounted_shares: if not self._is_share_eligible(smbfs_share, volume_size_in_gib): continue - total_allocated = self._get_capacity_info(smbfs_share)[2] + total_allocated = self._get_total_allocated(smbfs_share) if target_share is not None: if target_share_reserved > total_allocated: target_share = smbfs_share @@ -398,6 +476,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): raise exception.InvalidVolume(err_msg) @remotefs_drv.locked_volume_id_operation + @update_allocation_data() def extend_volume(self, volume, size_gb): LOG.info(_LI('Extending volume %s.'), volume['id']) self._extend_volume(volume, size_gb) @@ -449,6 +528,11 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): 'extend volume %s to %sG.' % (volume['id'], size_gb)) + @remotefs_drv.locked_volume_id_operation + @update_allocation_data() + def create_volume_from_snapshot(self, volume, snapshot): + return self._create_volume_from_snapshot(volume, snapshot) + def _copy_volume_from_snapshot(self, snapshot, volume, volume_size): """Copy data from snapshot to destination volume. @@ -506,6 +590,12 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): reason=(_("Expected volume size was %d") % volume['size']) + (_(" but size is now %d.") % virt_size)) + @remotefs_drv.locked_volume_id_operation + @update_allocation_data() + def create_cloned_volume(self, volume, src_vref): + """Creates a clone of the specified volume.""" + return self._create_cloned_volume(volume, src_vref) + def _ensure_share_mounted(self, smbfs_share): mnt_flags = [] if self.shares.get(smbfs_share) is not None: diff --git a/cinder/volume/drivers/windows/smbfs.py b/cinder/volume/drivers/windows/smbfs.py index eb31ab4c1e9..37844d0d104 100644 --- a/cinder/volume/drivers/windows/smbfs.py +++ b/cinder/volume/drivers/windows/smbfs.py @@ -15,7 +15,6 @@ import os -import re import sys from oslo_config import cfg @@ -26,7 +25,7 @@ from oslo_utils import units from cinder import exception from cinder.i18n import _, _LI from cinder.image import image_utils -from cinder import utils +from cinder.volume.drivers import remotefs as remotefs_drv from cinder.volume.drivers import smbfs from cinder.volume.drivers.windows import remotefs from cinder.volume.drivers.windows import vhdutils @@ -38,6 +37,8 @@ LOG = logging.getLogger(__name__) CONF = cfg.CONF CONF.set_default('smbfs_shares_config', r'C:\OpenStack\smbfs_shares.txt') +CONF.set_default('smbfs_allocation_info_file_path', + r'C:\OpenStack\allocation_data.txt') CONF.set_default('smbfs_mount_point_base', r'C:\OpenStack\_mnt') CONF.set_default('smbfs_default_volume_format', 'vhd') @@ -111,25 +112,6 @@ class WindowsSmbfsDriver(smbfs.SmbfsDriver): 'allocated': total_allocated}) return [float(x) for x in return_value] - def _get_total_allocated(self, smbfs_share): - elements = os.listdir(smbfs_share) - total_allocated = 0 - for element in elements: - element_path = os.path.join(smbfs_share, element) - if not self._remotefsclient.is_symlink(element_path): - if "snapshot" in element: - continue - if re.search(r'\.vhdx?$', element): - total_allocated += self.vhdutils.get_vhd_size( - element_path)['VirtualSize'] - continue - if os.path.isdir(element_path): - total_allocated += self._get_total_allocated(element_path) - continue - total_allocated += os.path.getsize(element_path) - - return total_allocated - def _img_commit(self, snapshot_path): self.vhdutils.merge_vhd(snapshot_path) self._delete(snapshot_path) @@ -172,7 +154,7 @@ class WindowsSmbfsDriver(smbfs.SmbfsDriver): def _do_extend_volume(self, volume_path, size_gb, volume_name=None): self.vhdutils.resize_vhd(volume_path, size_gb * units.Gi) - @utils.synchronized('smbfs', external=False) + @remotefs_drv.locked_volume_id_operation def copy_volume_to_image(self, context, volume, image_service, image_meta): """Copy the volume to the specified image."""