SMBFS: drop JSON file storing allocation data
Due to the fact that we cannot query in-use images, some time ago we've started storing the total disk image allocated size within a JSON file. At the same time, we're caching those values within the service. As we're aiming to support A-A deployments, also considering the risk of having this file corrupted, we're now dropping it. Instead of using a separate DB, we're just going to rely on the Cinder DB when retrieving the allocated space for a specific share. One reason why we did not go with this approach from the beginning is the fact that we needed support for reporting each share as a pool, which is accomplished by the other patches from this chain. Change-Id: I2085c040d7140f8ba685f6a36c4d3b5b30b14103
This commit is contained in:
Lucian Petrut
parent
d60f1a8a7c
commit
792da5dbbf
@ -13,7 +13,6 @@
|
||||
# under the License.
|
||||
|
||||
import copy
|
||||
import functools
|
||||
import os
|
||||
|
||||
import ddt
|
||||
@ -31,20 +30,6 @@ from cinder.volume.drivers import remotefs
|
||||
from cinder.volume.drivers.windows 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.volume,
|
||||
expected_size)
|
||||
return inner
|
||||
return wrapper
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class WindowsSmbFsTestCase(test.TestCase):
|
||||
|
||||
@ -66,8 +51,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)
|
||||
_FAKE_ALLOCATION_DATA_PATH = os.path.join('fake_dir',
|
||||
'fake_allocation_data')
|
||||
_FAKE_SHARE_OPTS = '-o username=Administrator,password=12345'
|
||||
|
||||
@mock.patch.object(smbfs, 'utilsfactory')
|
||||
@ -90,8 +73,6 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
self._smbfs_driver._local_volume_dir = mock.Mock(
|
||||
return_value=self._FAKE_MNT_POINT)
|
||||
self._smbfs_driver.base = self._FAKE_MNT_BASE
|
||||
self._smbfs_driver._alloc_info_file_path = (
|
||||
self._FAKE_ALLOCATION_DATA_PATH)
|
||||
|
||||
self._vhdutils = self._smbfs_driver._vhdutils
|
||||
|
||||
@ -118,7 +99,6 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
return snapshot
|
||||
|
||||
@mock.patch.object(smbfs.WindowsSmbfsDriver, '_check_os_platform')
|
||||
@mock.patch.object(smbfs.WindowsSmbfsDriver, '_setup_allocation_data')
|
||||
@mock.patch.object(remotefs.RemoteFSSnapDriver, 'do_setup')
|
||||
@mock.patch('os.path.exists')
|
||||
@mock.patch('os.path.isabs')
|
||||
@ -126,7 +106,7 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
def _test_setup(self, mock_check_qemu_img_version,
|
||||
mock_is_abs, mock_exists,
|
||||
mock_remotefs_do_setup,
|
||||
mock_setup_alloc_data, mock_check_os_platform,
|
||||
mock_check_os_platform,
|
||||
config, share_config_exists=True):
|
||||
mock_exists.return_value = share_config_exists
|
||||
fake_ensure_mounted = mock.MagicMock()
|
||||
@ -148,7 +128,6 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
mock_is_abs.assert_called_once_with(self._smbfs_driver.base)
|
||||
self.assertEqual({}, self._smbfs_driver.shares)
|
||||
fake_ensure_mounted.assert_called_once_with()
|
||||
mock_setup_alloc_data.assert_called_once_with()
|
||||
self._smbfs_driver._setup_pool_mappings.assert_called_once_with()
|
||||
|
||||
mock_check_os_platform.assert_called_once_with()
|
||||
@ -230,83 +209,32 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
fake_config.smbfs_used_ratio = 1.1
|
||||
self._test_setup(config=fake_config)
|
||||
|
||||
@mock.patch.object(smbfs, 'open', create=True)
|
||||
@mock.patch('os.path.exists')
|
||||
@mock.patch.object(smbfs.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()
|
||||
@mock.patch.object(smbfs, 'context')
|
||||
@mock.patch.object(smbfs.WindowsSmbfsDriver,
|
||||
'_get_pool_name_from_share')
|
||||
def test_get_total_allocated(self, mock_get_pool_name, mock_ctxt):
|
||||
fake_pool_name = 'pool0'
|
||||
fake_host_name = 'fake_host@fake_backend'
|
||||
fake_vol_sz_sum = 5
|
||||
|
||||
self._smbfs_driver._setup_allocation_data()
|
||||
mock_db = mock.Mock()
|
||||
mock_db.volume_data_get_for_host.return_value = [
|
||||
mock.sentinel.vol_count, fake_vol_sz_sum]
|
||||
|
||||
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()
|
||||
self._smbfs_driver.host = fake_host_name
|
||||
self._smbfs_driver.db = mock_db
|
||||
|
||||
def test_setup_allocation_data_file_unexisting(self):
|
||||
self._test_setup_allocation_data()
|
||||
mock_get_pool_name.return_value = fake_pool_name
|
||||
|
||||
def test_setup_allocation_data_file_existing(self):
|
||||
self._test_setup_allocation_data(allocation_data_exists=True)
|
||||
allocated = self._smbfs_driver._get_total_allocated(
|
||||
mock.sentinel.share)
|
||||
self.assertEqual(fake_vol_sz_sum << 30,
|
||||
allocated)
|
||||
|
||||
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._FAKE_SHARE_HASH: {
|
||||
'total_allocated': self._FAKE_TOTAL_ALLOCATED}}
|
||||
if volume_exists:
|
||||
fake_alloc_data[self._FAKE_SHARE_HASH][
|
||||
self.volume.name] = self.volume.size
|
||||
|
||||
self._smbfs_driver._allocation_data = fake_alloc_data
|
||||
|
||||
self._smbfs_driver.update_disk_allocation_data(self.volume,
|
||||
virtual_size_gb)
|
||||
|
||||
vol_allocated_size = fake_alloc_data[self._FAKE_SHARE_HASH].get(
|
||||
self.volume.name, None)
|
||||
if not virtual_size_gb:
|
||||
expected_total_allocated = (self._FAKE_TOTAL_ALLOCATED -
|
||||
self.volume.size)
|
||||
|
||||
self.assertIsNone(vol_allocated_size)
|
||||
else:
|
||||
exp_added = (self.volume.size if not volume_exists
|
||||
else virtual_size_gb - self.volume.size)
|
||||
expected_total_allocated = (self._FAKE_TOTAL_ALLOCATED +
|
||||
exp_added)
|
||||
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.volume.size + 1)
|
||||
|
||||
def test_update_allocation_data_volume_created(self):
|
||||
self._test_update_allocation_data(
|
||||
virtual_size_gb=self.volume.size,
|
||||
volume_exists=False)
|
||||
mock_get_pool_name.assert_called_once_with(mock.sentinel.share)
|
||||
mock_db.volume_data_get_for_host.assert_called_once_with(
|
||||
context=mock_ctxt.get_admin_context.return_value,
|
||||
host='fake_host@fake_backend#pool0')
|
||||
|
||||
def _test_is_share_eligible(self, capacity_info, volume_size):
|
||||
self._smbfs_driver._get_capacity_info = mock.Mock(
|
||||
@ -494,7 +422,6 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
|
||||
self.assertEqual(expected_fmt, resulted_fmt)
|
||||
|
||||
@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.volume)
|
||||
@ -527,7 +454,6 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
def test_create_volume_invalid_volume(self):
|
||||
self._test_create_volume(volume_format="qcow")
|
||||
|
||||
@requires_allocation_data_update(expected_size=None)
|
||||
def test_delete_volume(self):
|
||||
drv = self._smbfs_driver
|
||||
fake_vol_info = self._FAKE_VOLUME_PATH + '.info'
|
||||
@ -684,23 +610,6 @@ class WindowsSmbFsTestCase(test.TestCase):
|
||||
mock_nova_assisted_snap_del.assert_not_called()
|
||||
mock_write_info_file.assert_not_called()
|
||||
|
||||
@requires_allocation_data_update(expected_size=_FAKE_VOLUME_SIZE)
|
||||
@mock.patch.object(smbfs.WindowsSmbfsDriver,
|
||||
'_create_volume_from_snapshot')
|
||||
def test_create_volume_from_snapshot(self, mock_create_volume):
|
||||
self._smbfs_driver.create_volume_from_snapshot(self.volume,
|
||||
self.snapshot)
|
||||
mock_create_volume.assert_called_once_with(self.volume,
|
||||
self.snapshot)
|
||||
|
||||
@requires_allocation_data_update(expected_size=_FAKE_VOLUME_SIZE)
|
||||
@mock.patch.object(smbfs.WindowsSmbfsDriver, '_create_cloned_volume')
|
||||
def test_create_cloned_volume(self, mock_create_volume):
|
||||
self._smbfs_driver.create_cloned_volume(self.volume,
|
||||
mock.sentinel.src_vol)
|
||||
mock_create_volume.assert_called_once_with(self.volume,
|
||||
mock.sentinel.src_vol)
|
||||
|
||||
def test_create_volume_from_unavailable_snapshot(self):
|
||||
self.snapshot.status = fields.SnapshotStatus.ERROR
|
||||
self.assertRaises(
|
||||
|
||||
@ -13,12 +13,9 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import inspect
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
|
||||
import decorator
|
||||
from os_brick.remotefs import windows_remotefs as remotefs_brick
|
||||
from os_win import utilsfactory
|
||||
from oslo_config import cfg
|
||||
@ -26,6 +23,7 @@ from oslo_log import log as logging
|
||||
from oslo_utils import fileutils
|
||||
from oslo_utils import units
|
||||
|
||||
from cinder import context
|
||||
from cinder import exception
|
||||
from cinder.i18n import _
|
||||
from cinder.image import image_utils
|
||||
@ -43,7 +41,10 @@ volume_opts = [
|
||||
cfg.StrOpt('smbfs_allocation_info_file_path',
|
||||
default=r'C:\OpenStack\allocation_data.txt',
|
||||
help=('The path of the automatically generated file containing '
|
||||
'information about volume disk space allocation.')),
|
||||
'information about volume disk space allocation.'),
|
||||
deprecated_for_removal=True,
|
||||
deprecated_since="11.0.0",
|
||||
deprecated_reason="This allocation file is no longer used."),
|
||||
cfg.StrOpt('smbfs_default_volume_format',
|
||||
default='vhd',
|
||||
choices=['vhd', 'vhdx'],
|
||||
@ -79,25 +80,6 @@ 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
|
||||
|
||||
|
||||
@interface.volumedriver
|
||||
class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
remotefs_drv.RemoteFSSnapDriver):
|
||||
@ -138,9 +120,6 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
self._pathutils = utilsfactory.get_pathutils()
|
||||
self._smbutils = utilsfactory.get_smbutils()
|
||||
|
||||
self._alloc_info_file_path = (
|
||||
self.configuration.smbfs_allocation_info_file_path)
|
||||
|
||||
def do_setup(self, context):
|
||||
self._check_os_platform()
|
||||
super(WindowsSmbfsDriver, self).do_setup(context)
|
||||
@ -177,7 +156,6 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
|
||||
self.shares = {} # address : options
|
||||
self._ensure_shares_mounted()
|
||||
self._setup_allocation_data()
|
||||
self._setup_pool_mappings()
|
||||
|
||||
def _setup_pool_mappings(self):
|
||||
@ -230,49 +208,14 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
"driver supports only Win32 platforms.") % sys.platform
|
||||
raise exception.SmbfsException(_msg)
|
||||
|
||||
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)
|
||||
pool_name = self._get_pool_name_from_share(smbfs_share)
|
||||
host = "#".join([self.host, pool_name])
|
||||
|
||||
vol_sz_sum = self.db.volume_data_get_for_host(
|
||||
context=context.get_admin_context(),
|
||||
host=host)[1]
|
||||
return float(vol_sz_sum * units.Gi)
|
||||
|
||||
def _is_share_eligible(self, smbfs_share, volume_size_in_gib):
|
||||
"""Verifies SMBFS share is eligible to host volume with given size.
|
||||
@ -382,7 +325,6 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
self.configuration.smbfs_default_volume_format)
|
||||
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
@update_allocation_data()
|
||||
def create_volume(self, volume):
|
||||
return super(WindowsSmbfsDriver, self).create_volume(volume)
|
||||
|
||||
@ -408,7 +350,6 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
self._remotefsclient.mount(smbfs_share, mnt_flags)
|
||||
|
||||
@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:
|
||||
@ -491,7 +432,6 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
backing_file_full_path)
|
||||
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
@update_allocation_data()
|
||||
def extend_volume(self, volume, size_gb):
|
||||
LOG.info('Extending volume %s.', volume.id)
|
||||
|
||||
@ -616,17 +556,6 @@ class WindowsSmbfsDriver(remotefs_drv.RemoteFSPoolMixin,
|
||||
volume.size * units.Gi,
|
||||
is_file_max_size=False)
|
||||
|
||||
@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)
|
||||
|
||||
@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 _copy_volume_from_snapshot(self, snapshot, volume, volume_size):
|
||||
"""Copy data from snapshot to destination volume."""
|
||||
|
||||
|
||||
@ -0,0 +1,6 @@
|
||||
---
|
||||
deprecations:
|
||||
- |
|
||||
The 'smbfs_allocation_info_file_path' SMBFS driver config option is now
|
||||
deprecated as we're no longer using a JSON file to store volume allocation
|
||||
data. This file had a considerable chance of getting corrupted.
|
||||
Loading…
x
Reference in New Issue
Block a user