Merge "SMBFS: Fix initialize connection issues caused by in-use images"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
7e0838fffc
|
|
@ -15,14 +15,17 @@
|
|||
import copy
|
||||
import os
|
||||
|
||||
import ddt
|
||||
import mock
|
||||
|
||||
from cinder import exception
|
||||
from cinder.image import image_utils
|
||||
from cinder import test
|
||||
from cinder import utils
|
||||
from cinder.volume.drivers import remotefs
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class RemoteFsSnapDriverTestCase(test.TestCase):
|
||||
|
||||
_FAKE_CONTEXT = 'fake_context'
|
||||
|
|
@ -356,3 +359,65 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
self._locked_volume_operation_test_helper(
|
||||
func=synchronized_func,
|
||||
expected_exception=exception.VolumeBackendAPIException)
|
||||
|
||||
@mock.patch.object(image_utils, 'qemu_img_info')
|
||||
@mock.patch('os.path.basename')
|
||||
def _test_qemu_img_info(self, mock_basename,
|
||||
mock_qemu_img_info, backing_file, basedir,
|
||||
valid_backing_file=True):
|
||||
fake_vol_name = 'fake_vol_name'
|
||||
mock_info = mock_qemu_img_info.return_value
|
||||
mock_info.image = mock.sentinel.image_path
|
||||
mock_info.backing_file = backing_file
|
||||
|
||||
self._driver._VALID_IMAGE_EXTENSIONS = ['vhd', 'vhdx', 'raw', 'qcow2']
|
||||
|
||||
mock_basename.side_effect = [mock.sentinel.image_basename,
|
||||
mock.sentinel.backing_file_basename]
|
||||
|
||||
if valid_backing_file:
|
||||
img_info = self._driver._qemu_img_info_base(
|
||||
mock.sentinel.image_path, fake_vol_name, basedir)
|
||||
self.assertEqual(mock_info, img_info)
|
||||
self.assertEqual(mock.sentinel.image_basename,
|
||||
mock_info.image)
|
||||
expected_basename_calls = [mock.call(mock.sentinel.image_path)]
|
||||
if backing_file:
|
||||
self.assertEqual(mock.sentinel.backing_file_basename,
|
||||
mock_info.backing_file)
|
||||
expected_basename_calls.append(mock.call(backing_file))
|
||||
mock_basename.assert_has_calls(expected_basename_calls)
|
||||
else:
|
||||
self.assertRaises(exception.RemoteFSException,
|
||||
self._driver._qemu_img_info_base,
|
||||
mock.sentinel.image_path,
|
||||
fake_vol_name, basedir)
|
||||
|
||||
mock_qemu_img_info.assert_called_with(mock.sentinel.image_path)
|
||||
|
||||
@ddt.data([None, '/fake_basedir'],
|
||||
['/fake_basedir/cb2016/fake_vol_name', '/fake_basedir'],
|
||||
['/fake_basedir/cb2016/fake_vol_name.vhd', '/fake_basedir'],
|
||||
['/fake_basedir/cb2016/fake_vol_name.404f-404',
|
||||
'/fake_basedir'],
|
||||
['/fake_basedir/cb2016/fake_vol_name.tmp-snap-404f-404',
|
||||
'/fake_basedir'])
|
||||
@ddt.unpack
|
||||
def test_qemu_img_info_valid_backing_file(self, backing_file, basedir):
|
||||
self._test_qemu_img_info(backing_file=backing_file,
|
||||
basedir=basedir)
|
||||
|
||||
@ddt.data(['/other_random_path', '/fake_basedir'],
|
||||
['/other_basedir/cb2016/fake_vol_name', '/fake_basedir'],
|
||||
['/fake_basedir/invalid_hash/fake_vol_name', '/fake_basedir'],
|
||||
['/fake_basedir/cb2016/invalid_vol_name', '/fake_basedir'],
|
||||
['/fake_basedir/cb2016/fake_vol_name.info', '/fake_basedir'],
|
||||
['/fake_basedir/cb2016/fake_vol_name-random-suffix',
|
||||
'/fake_basedir'],
|
||||
['/fake_basedir/cb2016/fake_vol_name.invalidext',
|
||||
'/fake_basedir'])
|
||||
@ddt.unpack
|
||||
def test_qemu_img_info_invalid_backing_file(self, backing_file, basedir):
|
||||
self._test_qemu_img_info(backing_file=backing_file,
|
||||
basedir=basedir,
|
||||
valid_backing_file=False)
|
||||
|
|
|
|||
|
|
@ -94,6 +94,8 @@ class SmbFsTestCase(test.TestCase):
|
|||
self._smbfs_driver._alloc_info_file_path = (
|
||||
self._FAKE_ALLOCATION_DATA_PATH)
|
||||
|
||||
self.addCleanup(mock.patch.stopall)
|
||||
|
||||
def _get_fake_allocation_data(self):
|
||||
return {self._FAKE_SHARE_HASH: {
|
||||
'total_allocated': self._FAKE_TOTAL_ALLOCATED}}
|
||||
|
|
@ -242,51 +244,49 @@ class SmbFsTestCase(test.TestCase):
|
|||
fake_config.smbfs_used_ratio = 1.1
|
||||
self._test_setup(config=fake_config)
|
||||
|
||||
def _test_create_volume(self, volume_exists=False, volume_format=None):
|
||||
fake_method = mock.MagicMock()
|
||||
@mock.patch('os.path.exists')
|
||||
def _test_create_volume(self, mock_exists, volume_exists=False,
|
||||
volume_format=None, use_sparsed_file=False):
|
||||
mock.patch.multiple(smbfs.SmbfsDriver,
|
||||
_create_windows_image=mock.DEFAULT,
|
||||
_create_regular_file=mock.DEFAULT,
|
||||
_create_qcow2_file=mock.DEFAULT,
|
||||
_create_sparsed_file=mock.DEFAULT,
|
||||
get_volume_format=mock.DEFAULT,
|
||||
local_path=mock.DEFAULT,
|
||||
_set_rw_permissions_for_all=mock.DEFAULT).start()
|
||||
self._smbfs_driver.configuration = copy.copy(self._FAKE_SMBFS_CONFIG)
|
||||
self._smbfs_driver._set_rw_permissions_for_all = mock.MagicMock()
|
||||
fake_set_permissions = self._smbfs_driver._set_rw_permissions_for_all
|
||||
self._smbfs_driver.get_volume_format = mock.MagicMock()
|
||||
self._smbfs_driver.configuration.smbfs_sparsed_volumes = (
|
||||
use_sparsed_file)
|
||||
|
||||
windows_image_format = False
|
||||
fake_vol_path = self._FAKE_VOLUME_PATH
|
||||
self._smbfs_driver.get_volume_format.return_value = volume_format
|
||||
self._smbfs_driver.local_path.return_value = mock.sentinel.vol_path
|
||||
mock_exists.return_value = volume_exists
|
||||
|
||||
if volume_format:
|
||||
if volume_format in ('vhd', 'vhdx'):
|
||||
windows_image_format = volume_format
|
||||
if volume_format == 'vhd':
|
||||
windows_image_format = 'vpc'
|
||||
method = '_create_windows_image'
|
||||
fake_vol_path += '.' + volume_format
|
||||
else:
|
||||
method = '_create_%s_file' % volume_format
|
||||
if volume_format == 'sparsed':
|
||||
self._smbfs_driver.configuration.smbfs_sparsed_volumes = (
|
||||
True)
|
||||
if volume_exists:
|
||||
self.assertRaises(exception.InvalidVolume,
|
||||
self._smbfs_driver._do_create_volume,
|
||||
self._FAKE_VOLUME)
|
||||
return
|
||||
|
||||
self._smbfs_driver._do_create_volume(self._FAKE_VOLUME)
|
||||
expected_create_args = [mock.sentinel.vol_path,
|
||||
self._FAKE_VOLUME['size']]
|
||||
if volume_format in [self._smbfs_driver._DISK_FORMAT_VHDX,
|
||||
self._smbfs_driver._DISK_FORMAT_VHD]:
|
||||
expected_create_args.append(volume_format)
|
||||
exp_create_method = self._smbfs_driver._create_windows_image
|
||||
else:
|
||||
method = '_create_regular_file'
|
||||
|
||||
setattr(self._smbfs_driver, method, fake_method)
|
||||
|
||||
with mock.patch('os.path.exists', new=lambda x: volume_exists):
|
||||
if volume_exists:
|
||||
self.assertRaises(exception.InvalidVolume,
|
||||
self._smbfs_driver._do_create_volume,
|
||||
self._FAKE_VOLUME)
|
||||
return
|
||||
|
||||
self._smbfs_driver._do_create_volume(self._FAKE_VOLUME)
|
||||
if windows_image_format:
|
||||
fake_method.assert_called_once_with(
|
||||
fake_vol_path,
|
||||
self._FAKE_VOLUME['size'],
|
||||
windows_image_format)
|
||||
if volume_format == self._smbfs_driver._DISK_FORMAT_QCOW2:
|
||||
exp_create_method = self._smbfs_driver._create_qcow2_file
|
||||
elif use_sparsed_file:
|
||||
exp_create_method = self._smbfs_driver._create_sparsed_file
|
||||
else:
|
||||
fake_method.assert_called_once_with(
|
||||
fake_vol_path, self._FAKE_VOLUME['size'])
|
||||
fake_set_permissions.assert_called_once_with(fake_vol_path)
|
||||
exp_create_method = self._smbfs_driver._create_regular_file
|
||||
|
||||
exp_create_method.assert_called_once_with(*expected_create_args)
|
||||
mock_set_permissions = self._smbfs_driver._set_rw_permissions_for_all
|
||||
mock_set_permissions.assert_called_once_with(mock.sentinel.vol_path)
|
||||
|
||||
def test_create_existing_volume(self):
|
||||
self._test_create_volume(volume_exists=True)
|
||||
|
|
@ -298,7 +298,8 @@ class SmbFsTestCase(test.TestCase):
|
|||
self._test_create_volume(volume_format='qcow2')
|
||||
|
||||
def test_create_sparsed(self):
|
||||
self._test_create_volume(volume_format='sparsed')
|
||||
self._test_create_volume(volume_format='raw',
|
||||
use_sparsed_file=True)
|
||||
|
||||
def test_create_regular(self):
|
||||
self._test_create_volume()
|
||||
|
|
@ -392,14 +393,12 @@ class SmbFsTestCase(test.TestCase):
|
|||
@mock.patch.object(smbfs.SmbfsDriver, '_lookup_local_volume_path')
|
||||
@mock.patch.object(smbfs.SmbfsDriver, 'get_volume_format')
|
||||
def _test_get_volume_path(self, mock_get_volume_format, mock_lookup_volume,
|
||||
mock_get_path_template, volume_exists=True,
|
||||
volume_format='raw'):
|
||||
mock_get_path_template, volume_exists=True):
|
||||
drv = self._smbfs_driver
|
||||
mock_get_path_template.return_value = self._FAKE_VOLUME_PATH
|
||||
volume_format = 'raw'
|
||||
|
||||
expected_vol_path = self._FAKE_VOLUME_PATH
|
||||
if volume_format in (drv._DISK_FORMAT_VHD, drv._DISK_FORMAT_VHDX):
|
||||
expected_vol_path += '.' + volume_format
|
||||
expected_vol_path = self._FAKE_VOLUME_PATH + '.' + volume_format
|
||||
|
||||
mock_lookup_volume.return_value = (
|
||||
expected_vol_path if volume_exists else None)
|
||||
|
|
@ -416,12 +415,9 @@ class SmbFsTestCase(test.TestCase):
|
|||
def test_get_existing_volume_path(self):
|
||||
self._test_get_volume_path()
|
||||
|
||||
def test_get_new_raw_volume_path(self):
|
||||
def test_get_new_volume_path(self):
|
||||
self._test_get_volume_path(volume_exists=False)
|
||||
|
||||
def test_get_new_vhd_volume_path(self):
|
||||
self._test_get_volume_path(volume_exists=False, volume_format='vhd')
|
||||
|
||||
@mock.patch.object(smbfs.SmbfsDriver, '_local_volume_dir')
|
||||
def test_get_local_volume_path_template(self, mock_get_local_dir):
|
||||
mock_get_local_dir.return_value = self._FAKE_MNT_POINT
|
||||
|
|
@ -437,8 +433,11 @@ class SmbFsTestCase(test.TestCase):
|
|||
ret_val = self._smbfs_driver._lookup_local_volume_path(
|
||||
self._FAKE_VOLUME_PATH)
|
||||
|
||||
extensions = [''] + [
|
||||
".%s" % ext
|
||||
for ext in self._smbfs_driver._SUPPORTED_IMAGE_FORMATS]
|
||||
possible_paths = [self._FAKE_VOLUME_PATH + ext
|
||||
for ext in ('', '.vhd', '.vhdx')]
|
||||
for ext in extensions]
|
||||
mock_exists.assert_has_calls(
|
||||
[mock.call(path) for path in possible_paths])
|
||||
self.assertEqual(expected_path, ret_val)
|
||||
|
|
@ -495,11 +494,11 @@ class SmbFsTestCase(test.TestCase):
|
|||
self._smbfs_driver._get_mount_point_base = mock.Mock(
|
||||
return_value=self._FAKE_MNT_BASE)
|
||||
self._smbfs_driver.shares = {self._FAKE_SHARE: self._FAKE_SHARE_OPTS}
|
||||
self._smbfs_driver._qemu_img_info = mock.Mock(
|
||||
return_value=mock.Mock(file_format='raw'))
|
||||
self._smbfs_driver.get_volume_format = mock.Mock(
|
||||
return_value=mock.sentinel.format)
|
||||
|
||||
fake_data = {'export': self._FAKE_SHARE,
|
||||
'format': 'raw',
|
||||
'format': mock.sentinel.format,
|
||||
'name': self._FAKE_VOLUME_NAME,
|
||||
'options': self._FAKE_SHARE_OPTS}
|
||||
expected = {
|
||||
|
|
|
|||
|
|
@ -631,6 +631,8 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD):
|
|||
_local_volume_dir(self, volume)
|
||||
"""
|
||||
|
||||
_VALID_IMAGE_EXTENSIONS = []
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
self._remotefsclient = None
|
||||
self.base = None
|
||||
|
|
@ -689,11 +691,18 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD):
|
|||
if info.image:
|
||||
info.image = os.path.basename(info.image)
|
||||
if info.backing_file:
|
||||
if self._VALID_IMAGE_EXTENSIONS:
|
||||
valid_ext = r'(\.(%s))?' % '|'.join(
|
||||
self._VALID_IMAGE_EXTENSIONS)
|
||||
else:
|
||||
valid_ext = ''
|
||||
|
||||
backing_file_template = \
|
||||
"(%(basedir)s/[0-9a-f]+/)?%" \
|
||||
"(volname)s(.(tmp-snap-)?[0-9a-f-]+)?$" % {
|
||||
"(volname)s(.(tmp-snap-)?[0-9a-f-]+)?%(valid_ext)s$" % {
|
||||
'basedir': basedir,
|
||||
'volname': volume_name
|
||||
'volname': volume_name,
|
||||
'valid_ext': valid_ext,
|
||||
}
|
||||
if not re.match(backing_file_template, info.backing_file):
|
||||
msg = _("File %(path)s has invalid backing file "
|
||||
|
|
@ -1326,7 +1335,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD):
|
|||
def _delete_snapshot_online(self, context, snapshot, info):
|
||||
# Update info over the course of this method
|
||||
# active file never changes
|
||||
info_path = self._local_path_volume(snapshot['volume']) + '.info'
|
||||
info_path = self._local_path_volume_info(snapshot['volume'])
|
||||
snap_info = self._read_info_file(info_path)
|
||||
|
||||
if info['active_file'] == info['snapshot_file']:
|
||||
|
|
|
|||
|
|
@ -114,6 +114,10 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
_DISK_FORMAT_RAW = 'raw'
|
||||
_DISK_FORMAT_QCOW2 = 'qcow2'
|
||||
|
||||
_SUPPORTED_IMAGE_FORMATS = [_DISK_FORMAT_RAW, _DISK_FORMAT_QCOW2,
|
||||
_DISK_FORMAT_VHD, _DISK_FORMAT_VHDX]
|
||||
_VALID_IMAGE_EXTENSIONS = _SUPPORTED_IMAGE_FORMATS
|
||||
|
||||
def __init__(self, execute=putils.execute, *args, **kwargs):
|
||||
self._remotefsclient = None
|
||||
super(SmbfsDriver, self).__init__(*args, **kwargs)
|
||||
|
|
@ -143,10 +147,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
"""
|
||||
# Find active image
|
||||
active_file = self.get_active_image_from_info(volume)
|
||||
active_file_path = os.path.join(self._local_volume_dir(volume),
|
||||
active_file)
|
||||
info = self._qemu_img_info(active_file_path, volume['name'])
|
||||
fmt = info.file_format
|
||||
fmt = self.get_volume_format(volume)
|
||||
|
||||
data = {'export': volume['provider_location'],
|
||||
'format': fmt,
|
||||
|
|
@ -251,10 +252,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
# The image does not exist, so retrieve the volume format
|
||||
# in order to build the path.
|
||||
fmt = self.get_volume_format(volume)
|
||||
if fmt in (self._DISK_FORMAT_VHD, self._DISK_FORMAT_VHDX):
|
||||
volume_path = volume_path_template + '.' + fmt
|
||||
else:
|
||||
volume_path = volume_path_template
|
||||
volume_path = volume_path_template + '.' + fmt
|
||||
return volume_path
|
||||
|
||||
def _get_local_volume_path_template(self, volume):
|
||||
|
|
@ -263,7 +261,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
return local_path_template
|
||||
|
||||
def _lookup_local_volume_path(self, volume_path_template):
|
||||
for ext in ['', self._DISK_FORMAT_VHD, self._DISK_FORMAT_VHDX]:
|
||||
for ext in [''] + self._SUPPORTED_IMAGE_FORMATS:
|
||||
volume_path = (volume_path_template + '.' + ext
|
||||
if ext else volume_path_template)
|
||||
if os.path.exists(volume_path):
|
||||
|
|
@ -283,8 +281,12 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
volume_path = self._lookup_local_volume_path(volume_path_template)
|
||||
|
||||
if volume_path:
|
||||
info = self._qemu_img_info(volume_path, volume['name'])
|
||||
volume_format = info.file_format
|
||||
ext = os.path.splitext(volume_path)[1].strip('.').lower()
|
||||
if ext in self._SUPPORTED_IMAGE_FORMATS:
|
||||
volume_format = ext
|
||||
else:
|
||||
info = self._qemu_img_info(volume_path, volume['name'])
|
||||
volume_format = info.file_format
|
||||
else:
|
||||
volume_format = (
|
||||
self._get_volume_format_spec(volume) or
|
||||
|
|
|
|||
Loading…
Reference in New Issue