SMBFS: Fix retrieving total allocated size

Currently, the Windows SMBFS driver parses all the VHD/X images
present on the configured share in order to retrieve the allocated
size. Not only that this can be slow when having many volumes, but
it can also fail in certain environments when the images are in use.

The Linux SMBFS driver uses 'du', which gives incorrect values in
case of VHD/X images.

In order to avoid this, the driver can keep track of the allocated
size according to each share, the most basic solution being a JSON
stored in a file.

As the Windows SMBFS driver inherits the Linux SMBFS driver, only
overwriting the os specific methods, the fix is applied to the base
SMBFS driver.

DocImpact

Closes-Bug: #1425100

Change-Id: I14aa7b001605ae14fe0b6d0a45ee6b1abf8c4f60
This commit is contained in:
Lucian Petrut 2015-02-17 18:28:07 +02:00 committed by Petrut Lucian
parent 9ba2d75a5a
commit 667c1dc511
4 changed files with 234 additions and 63 deletions

View File

@ -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)

View File

@ -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)

View File

@ -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:

View File

@ -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."""