Merge "Fix exceed of dd action when copying with path"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
cef1663e23
@ -23,6 +23,7 @@ import six
|
||||
|
||||
from oslo_concurrency import processutils
|
||||
from oslo_config import cfg
|
||||
from oslo_utils import units
|
||||
|
||||
from cinder import context
|
||||
from cinder import db
|
||||
@ -331,33 +332,27 @@ class NotifyUsageTestCase(test.TestCase):
|
||||
class LVMVolumeDriverTestCase(test.TestCase):
|
||||
def test_convert_blocksize_option(self):
|
||||
# Test valid volume_dd_blocksize
|
||||
bs, count = volume_utils._calculate_count(1024, '10M')
|
||||
bs = volume_utils._check_blocksize('10M')
|
||||
self.assertEqual('10M', bs)
|
||||
self.assertEqual(103, count)
|
||||
|
||||
bs, count = volume_utils._calculate_count(1024, '1xBBB')
|
||||
bs = volume_utils._check_blocksize('1xBBB')
|
||||
self.assertEqual('1M', bs)
|
||||
self.assertEqual(1024, count)
|
||||
|
||||
# Test 'volume_dd_blocksize' with fraction
|
||||
bs, count = volume_utils._calculate_count(1024, '1.3M')
|
||||
bs = volume_utils._check_blocksize('1.3M')
|
||||
self.assertEqual('1M', bs)
|
||||
self.assertEqual(1024, count)
|
||||
|
||||
# Test zero-size 'volume_dd_blocksize'
|
||||
bs, count = volume_utils._calculate_count(1024, '0M')
|
||||
bs = volume_utils._check_blocksize('0M')
|
||||
self.assertEqual('1M', bs)
|
||||
self.assertEqual(1024, count)
|
||||
|
||||
# Test negative 'volume_dd_blocksize'
|
||||
bs, count = volume_utils._calculate_count(1024, '-1M')
|
||||
bs = volume_utils._check_blocksize('-1M')
|
||||
self.assertEqual('1M', bs)
|
||||
self.assertEqual(1024, count)
|
||||
|
||||
# Test non-digital 'volume_dd_blocksize'
|
||||
bs, count = volume_utils._calculate_count(1024, 'ABM')
|
||||
bs = volume_utils._check_blocksize('ABM')
|
||||
self.assertEqual('1M', bs)
|
||||
self.assertEqual(1024, count)
|
||||
|
||||
|
||||
class OdirectSupportTestCase(test.TestCase):
|
||||
@ -450,8 +445,8 @@ class ClearVolumeTestCase(test.TestCase):
|
||||
output = volume_utils.clear_volume(1024, 'volume_path')
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_with(
|
||||
'dd', 'if=/dev/zero', 'of=volume_path', 'count=1', 'bs=1M',
|
||||
'oflag=direct', run_as_root=True)
|
||||
'dd', 'if=/dev/zero', 'of=volume_path', 'count=1048576', 'bs=1M',
|
||||
'iflag=count_bytes', 'oflag=direct', run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.utils.execute')
|
||||
@mock.patch('cinder.volume.utils.CONF')
|
||||
@ -466,8 +461,8 @@ class ClearVolumeTestCase(test.TestCase):
|
||||
output = volume_utils.clear_volume(1024, 'volume_path')
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_with(
|
||||
'dd', 'if=/dev/zero', 'of=volume_path', 'count=1', 'bs=1M',
|
||||
'oflag=direct', run_as_root=True)
|
||||
'dd', 'if=/dev/zero', 'of=volume_path', 'count=1048576', 'bs=1M',
|
||||
'iflag=count_bytes', 'oflag=direct', run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.volume.utils.CONF')
|
||||
def test_clear_volume_invalid_opt(self, mock_conf):
|
||||
@ -480,129 +475,121 @@ class ClearVolumeTestCase(test.TestCase):
|
||||
|
||||
|
||||
class CopyVolumeTestCase(test.TestCase):
|
||||
@mock.patch('cinder.volume.utils._calculate_count',
|
||||
return_value=(1234, 5678))
|
||||
@mock.patch('cinder.volume.utils.check_for_odirect_support',
|
||||
return_value=True)
|
||||
@mock.patch('cinder.utils.execute')
|
||||
@mock.patch('cinder.volume.utils.CONF')
|
||||
def test_copy_volume_dd_iflag_and_oflag(self, mock_conf, mock_exec,
|
||||
mock_support, mock_count):
|
||||
mock_support):
|
||||
fake_throttle = throttling.Throttle(['fake_throttle'])
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=True, execute=utils.execute,
|
||||
ionice=None, throttle=fake_throttle)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('fake_throttle', 'dd',
|
||||
'if=/dev/zero',
|
||||
'of=/dev/null', 'count=5678',
|
||||
'bs=1234', 'iflag=direct',
|
||||
'of=/dev/null',
|
||||
'count=%s' % units.Gi,
|
||||
'bs=3M', 'iflag=count_bytes,direct',
|
||||
'oflag=direct', run_as_root=True)
|
||||
|
||||
mock_exec.reset_mock()
|
||||
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=False, execute=utils.execute,
|
||||
ionice=None, throttle=fake_throttle)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('fake_throttle', 'dd',
|
||||
'if=/dev/zero',
|
||||
'of=/dev/null', 'count=5678',
|
||||
'bs=1234', 'iflag=direct',
|
||||
'of=/dev/null',
|
||||
'count=%s' % units.Gi,
|
||||
'bs=3M', 'iflag=count_bytes,direct',
|
||||
'oflag=direct', run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.volume.utils._calculate_count',
|
||||
return_value=(1234, 5678))
|
||||
@mock.patch('cinder.volume.utils.check_for_odirect_support',
|
||||
return_value=False)
|
||||
@mock.patch('cinder.utils.execute')
|
||||
def test_copy_volume_dd_no_iflag_or_oflag(self, mock_exec,
|
||||
mock_support, mock_count):
|
||||
def test_copy_volume_dd_no_iflag_or_oflag(self, mock_exec, mock_support):
|
||||
fake_throttle = throttling.Throttle(['fake_throttle'])
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=True, execute=utils.execute,
|
||||
ionice=None, throttle=fake_throttle)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('fake_throttle', 'dd',
|
||||
'if=/dev/zero',
|
||||
'of=/dev/null', 'count=5678',
|
||||
'bs=1234', 'conv=fdatasync',
|
||||
'of=/dev/null',
|
||||
'count=%s' % units.Gi,
|
||||
'bs=3M', 'iflag=count_bytes',
|
||||
'conv=fdatasync', run_as_root=True)
|
||||
|
||||
mock_exec.reset_mock()
|
||||
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=False, execute=utils.execute,
|
||||
ionice=None, throttle=fake_throttle)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('fake_throttle', 'dd',
|
||||
'if=/dev/zero',
|
||||
'of=/dev/null',
|
||||
'count=%s' % units.Gi,
|
||||
'bs=3M', 'iflag=count_bytes',
|
||||
run_as_root=True)
|
||||
|
||||
mock_exec.reset_mock()
|
||||
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
sync=False, execute=utils.execute,
|
||||
ionice=None, throttle=fake_throttle)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('fake_throttle', 'dd',
|
||||
'if=/dev/zero',
|
||||
'of=/dev/null', 'count=5678',
|
||||
'bs=1234', run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.volume.utils._calculate_count',
|
||||
return_value=(1234, 5678))
|
||||
@mock.patch('cinder.volume.utils.check_for_odirect_support',
|
||||
return_value=False)
|
||||
@mock.patch('cinder.utils.execute')
|
||||
def test_copy_volume_dd_no_throttle(self, mock_exec, mock_support,
|
||||
mock_count):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
def test_copy_volume_dd_no_throttle(self, mock_exec, mock_support):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=True, execute=utils.execute,
|
||||
ionice=None)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('dd', 'if=/dev/zero', 'of=/dev/null',
|
||||
'count=5678', 'bs=1234',
|
||||
'count=%s' % units.Gi, 'bs=3M',
|
||||
'iflag=count_bytes',
|
||||
'conv=fdatasync', run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.volume.utils._calculate_count',
|
||||
return_value=(1234, 5678))
|
||||
@mock.patch('cinder.volume.utils.check_for_odirect_support',
|
||||
return_value=False)
|
||||
@mock.patch('cinder.utils.execute')
|
||||
def test_copy_volume_dd_with_ionice(self, mock_exec,
|
||||
mock_support, mock_count):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
def test_copy_volume_dd_with_ionice(self, mock_exec, mock_support):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=True, execute=utils.execute,
|
||||
ionice='-c3')
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('ionice', '-c3', 'dd',
|
||||
'if=/dev/zero', 'of=/dev/null',
|
||||
'count=5678', 'bs=1234',
|
||||
'count=%s' % units.Gi, 'bs=3M',
|
||||
'iflag=count_bytes',
|
||||
'conv=fdatasync', run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.volume.utils._calculate_count',
|
||||
return_value=(1234, 5678))
|
||||
@mock.patch('cinder.volume.utils.check_for_odirect_support',
|
||||
return_value=False)
|
||||
@mock.patch('cinder.utils.execute')
|
||||
def test_copy_volume_dd_with_sparse(self, mock_exec,
|
||||
mock_support, mock_count):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
def test_copy_volume_dd_with_sparse(self, mock_exec, mock_support):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=True, execute=utils.execute,
|
||||
sparse=True)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('dd', 'if=/dev/zero', 'of=/dev/null',
|
||||
'count=5678', 'bs=1234',
|
||||
'count=%s' % units.Gi, 'bs=3M',
|
||||
'iflag=count_bytes',
|
||||
'conv=fdatasync,sparse',
|
||||
run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.volume.utils._calculate_count',
|
||||
return_value=(1234, 5678))
|
||||
@mock.patch('cinder.volume.utils.check_for_odirect_support',
|
||||
return_value=True)
|
||||
@mock.patch('cinder.utils.execute')
|
||||
def test_copy_volume_dd_with_sparse_iflag_and_oflag(self, mock_exec,
|
||||
mock_support,
|
||||
mock_count):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, 1,
|
||||
mock_support):
|
||||
output = volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, '3M',
|
||||
sync=True, execute=utils.execute,
|
||||
sparse=True)
|
||||
self.assertIsNone(output)
|
||||
mock_exec.assert_called_once_with('dd', 'if=/dev/zero', 'of=/dev/null',
|
||||
'count=5678', 'bs=1234',
|
||||
'iflag=direct', 'oflag=direct',
|
||||
'conv=sparse', run_as_root=True)
|
||||
'count=%s' % units.Gi, 'bs=3M',
|
||||
'iflag=count_bytes,direct',
|
||||
'oflag=direct', 'conv=sparse',
|
||||
run_as_root=True)
|
||||
|
||||
@mock.patch('cinder.volume.utils._copy_volume_with_file')
|
||||
def test_copy_volume_handles(self, mock_copy):
|
||||
|
||||
@ -17,6 +17,7 @@
|
||||
import mock
|
||||
|
||||
from oslo_concurrency import processutils
|
||||
from oslo_utils import units
|
||||
|
||||
from cinder import context
|
||||
from cinder import exception
|
||||
@ -229,6 +230,8 @@ class HGSTTestCase(test.TestCase):
|
||||
for p in cmdlist:
|
||||
if 'count=' in p:
|
||||
self.dd_count = int(p[6:])
|
||||
elif 'bs=' in p:
|
||||
self.bs = p[3:]
|
||||
return DD_OUTPUT, ''
|
||||
else:
|
||||
return '', ''
|
||||
@ -472,7 +475,8 @@ class HGSTTestCase(test.TestCase):
|
||||
'volume': {'provider_id': 'space10'}}
|
||||
ret = self.driver.create_snapshot(snapshot)
|
||||
# We must copy entier underlying storage, ~12GB, not just 10GB
|
||||
self.assertEqual(11444, self.dd_count)
|
||||
self.assertEqual(11444 * units.Mi, self.dd_count)
|
||||
self.assertEqual('1M', self.bs)
|
||||
# Check space-create command
|
||||
expected = {'redundancy': '0', 'group': 'xanadu',
|
||||
'name': snapshot['display_name'], 'mode': '0777',
|
||||
@ -497,7 +501,8 @@ class HGSTTestCase(test.TestCase):
|
||||
'volume_type_id': type_ref['id'], 'size': 10}
|
||||
pid = self.driver.create_cloned_volume(clone, orig)
|
||||
# We must copy entier underlying storage, ~12GB, not just 10GB
|
||||
self.assertEqual(11444, self.dd_count)
|
||||
self.assertEqual(11444 * units.Mi, self.dd_count)
|
||||
self.assertEqual('1M', self.bs)
|
||||
# Check space-create command
|
||||
expected = {'redundancy': '0', 'group': 'xanadu',
|
||||
'name': 'clone1', 'mode': '0777',
|
||||
@ -537,7 +542,8 @@ class HGSTTestCase(test.TestCase):
|
||||
'volume_type_id': type_ref['id'], 'size': 10}
|
||||
pid = self.driver.create_volume_from_snapshot(volume, snap)
|
||||
# We must copy entier underlying storage, ~12GB, not just 10GB
|
||||
self.assertEqual(11444, self.dd_count)
|
||||
self.assertEqual(11444 * units.Mi, self.dd_count)
|
||||
self.assertEqual('1M', self.bs)
|
||||
# Check space-create command
|
||||
expected = {'redundancy': '0', 'group': 'xanadu',
|
||||
'name': 'volume2', 'mode': '0777',
|
||||
|
||||
@ -338,7 +338,7 @@ def notify_about_group_snapshot_usage(context, group_snapshot, event_suffix,
|
||||
usage_info)
|
||||
|
||||
|
||||
def _calculate_count(size_in_m, blocksize):
|
||||
def _check_blocksize(blocksize):
|
||||
|
||||
# Check if volume_dd_blocksize is valid
|
||||
try:
|
||||
@ -346,7 +346,7 @@ def _calculate_count(size_in_m, blocksize):
|
||||
# cannot be caught by strutils
|
||||
if blocksize.startswith(('-', '0')) or '.' in blocksize:
|
||||
raise ValueError
|
||||
bs = strutils.string_to_bytes('%sB' % blocksize)
|
||||
strutils.string_to_bytes('%sB' % blocksize)
|
||||
except ValueError:
|
||||
LOG.warning(_LW("Incorrect value error: %(blocksize)s, "
|
||||
"it may indicate that \'volume_dd_blocksize\' "
|
||||
@ -355,11 +355,8 @@ def _calculate_count(size_in_m, blocksize):
|
||||
# Fall back to default blocksize
|
||||
CONF.clear_override('volume_dd_blocksize')
|
||||
blocksize = CONF.volume_dd_blocksize
|
||||
bs = strutils.string_to_bytes('%sB' % blocksize)
|
||||
|
||||
count = math.ceil(size_in_m * units.Mi / bs)
|
||||
|
||||
return blocksize, int(count)
|
||||
return blocksize
|
||||
|
||||
|
||||
def check_for_odirect_support(src, dest, flag='oflag=direct'):
|
||||
@ -387,15 +384,16 @@ def _copy_volume_with_path(prefix, srcstr, deststr, size_in_m, blocksize,
|
||||
if ionice:
|
||||
cmd.extend(('ionice', ionice))
|
||||
|
||||
blocksize, count = _calculate_count(size_in_m, blocksize)
|
||||
blocksize = _check_blocksize(blocksize)
|
||||
size_in_bytes = size_in_m * units.Mi
|
||||
|
||||
cmd.extend(('dd', 'if=%s' % srcstr, 'of=%s' % deststr,
|
||||
'count=%d' % count, 'bs=%s' % blocksize))
|
||||
'count=%d' % size_in_bytes, 'bs=%s' % blocksize))
|
||||
|
||||
# Use O_DIRECT to avoid thrashing the system buffer cache
|
||||
odirect = False
|
||||
if check_for_odirect_support(srcstr, deststr, 'iflag=direct'):
|
||||
cmd.append('iflag=direct')
|
||||
odirect = True
|
||||
odirect = check_for_odirect_support(srcstr, deststr, 'iflag=direct')
|
||||
|
||||
cmd.append('iflag=count_bytes,direct' if odirect else 'iflag=count_bytes')
|
||||
|
||||
if check_for_odirect_support(srcstr, deststr, 'oflag=direct'):
|
||||
cmd.append('oflag=direct')
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user