From: Avishay Traeger Date: Wed, 17 Jul 2013 12:35:03 +0000 (+0300) Subject: Move copy_volume function to volume/utils.py. X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=30397d8a0a22f3ecad0001ac50a084cc41f4a43d;p=openstack-build%2Fcinder-build.git Move copy_volume function to volume/utils.py. There are several copy-pastes of running dd, so moved the most correct one (LVM) to volume/utils.py, and also updated BlockDeviceDriver to use it. Other drivers (e.g., NFS, GPFS, Scality) should be updated as well. Volume migration for detached volumes is a future use case. Change-Id: I56c64c94eccf0a311e6f3d611738ad0403351971 --- diff --git a/cinder/tests/test_block_device.py b/cinder/tests/test_block_device.py index ebbc8a101..cd928b2c9 100644 --- a/cinder/tests/test_block_device.py +++ b/cinder/tests/test_block_device.py @@ -24,6 +24,7 @@ from cinder.image import image_utils import cinder.test from cinder.volume.driver import ISCSIDriver from cinder.volume.drivers.block_device import BlockDeviceDriver +from cinder.volume import utils as volutils class TestBlockDeviceDriver(cinder.test.TestCase): @@ -131,12 +132,13 @@ class TestBlockDeviceDriver(cinder.test.TestCase): self.mox.StubOutWithMock(self.drv, 'find_appropriate_size_device') dev = self.drv.find_appropriate_size_device(TEST_SRC['size']).\ AndReturn('/dev/loop2') - self.mox.StubOutWithMock(self.drv, '_copy_volume') + self.mox.StubOutWithMock(volutils, 'copy_volume') self.mox.StubOutWithMock(self.drv, 'local_path') self.mox.StubOutWithMock(self.drv, '_get_device_size') self.drv.local_path(TEST_SRC).AndReturn('/dev/loop1') self.drv._get_device_size('/dev/loop2').AndReturn(1) - self.drv._copy_volume('/dev/loop1', dev, 2048) + volutils.copy_volume('/dev/loop1', dev, 2048, + execute=self.drv._execute) self.mox.ReplayAll() self.assertEquals(self.drv.create_cloned_volume(TEST_VOLUME, TEST_SRC), {'provider_location': 'None:3260,' diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 7204749b1..0dc07b3dd 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -46,6 +46,7 @@ from cinder.tests.image import fake as fake_image from cinder.volume import configuration as conf from cinder.volume import driver from cinder.volume.drivers import lvm +from cinder.volume import utils as volutils QUOTAS = quota.QUOTAS @@ -1504,46 +1505,13 @@ class LVMVolumeDriverTestCase(DriverTestCase): """Test case for VolumeDriver""" driver_name = "cinder.volume.drivers.lvm.LVMVolumeDriver" - def test_convert_blocksize_option(self): - # Test invalid volume_dd_blocksize - configuration = conf.Configuration(fake_opt, 'fake_group') - lvm_driver = lvm.LVMVolumeDriver(configuration=configuration) - - # Test valid volume_dd_blocksize - bs, count = lvm_driver._calculate_count('10M', 1) - self.assertEquals(bs, '10M') - self.assertEquals(count, 103) - - bs, count = lvm_driver._calculate_count('1xBBB', 1) - self.assertEquals(bs, '1M') - self.assertEquals(count, 1024) - - # Test volume_dd_blocksize with fraction - bs, count = lvm_driver._calculate_count('1.3M', 1) - self.assertEquals(bs, '1M') - self.assertEquals(count, 1024) - - # Test zero-size volume_dd_blocksize - bs, count = lvm_driver._calculate_count('0M', 1) - self.assertEquals(bs, '1M') - self.assertEquals(count, 1024) - - # Test negative volume_dd_blocksize - bs, count = lvm_driver._calculate_count('-1M', 1) - self.assertEquals(bs, '1M') - self.assertEquals(count, 1024) - - # Test non-digital volume_dd_blocksize - bs, count = lvm_driver._calculate_count('ABM', 1) - self.assertEquals(bs, '1M') - self.assertEquals(count, 1024) - def test_clear_volume(self): configuration = conf.Configuration(fake_opt, 'fake_group') configuration.volume_clear = 'zero' configuration.volume_clear_size = 0 lvm_driver = lvm.LVMVolumeDriver(configuration=configuration) - self.stubs.Set(lvm_driver, '_copy_volume', lambda *a, **kw: True) + self.stubs.Set(volutils, 'copy_volume', + lambda x, y, z, sync=False, execute='foo': True) fake_volume = {'name': 'test1', 'volume_name': 'test1', diff --git a/cinder/tests/test_volume_utils.py b/cinder/tests/test_volume_utils.py index 929397e5e..32584eaaf 100644 --- a/cinder/tests/test_volume_utils.py +++ b/cinder/tests/test_volume_utils.py @@ -118,3 +118,41 @@ class UsageInfoTestCase(test.TestCase): self.MULTI_AT_BACKEND) self.assertEquals(volume_utils.get_host_from_queue(fullname), self.HOSTIP) + + +class LVMVolumeDriverTestCase(test.TestCase): + def test_convert_blocksize_option(self): + # Test valid volume_dd_blocksize + CONF.set_override('volume_dd_blocksize', '10M') + bs, count = volume_utils._calculate_count(1024) + self.assertEquals(bs, '10M') + self.assertEquals(count, 103) + + CONF.set_override('volume_dd_blocksize', '1xBBB') + bs, count = volume_utils._calculate_count(1024) + self.assertEquals(bs, '1M') + self.assertEquals(count, 1024) + + # Test 'volume_dd_blocksize' with fraction + CONF.set_override('volume_dd_blocksize', '1.3M') + bs, count = volume_utils._calculate_count(1024) + self.assertEquals(bs, '1M') + self.assertEquals(count, 1024) + + # Test zero-size 'volume_dd_blocksize' + CONF.set_override('volume_dd_blocksize', '0M') + bs, count = volume_utils._calculate_count(1024) + self.assertEquals(bs, '1M') + self.assertEquals(count, 1024) + + # Test negative 'volume_dd_blocksize' + CONF.set_override('volume_dd_blocksize', '-1M') + bs, count = volume_utils._calculate_count(1024) + self.assertEquals(bs, '1M') + self.assertEquals(count, 1024) + + # Test non-digital 'volume_dd_blocksize' + CONF.set_override('volume_dd_blocksize', 'ABM') + bs, count = volume_utils._calculate_count(1024) + self.assertEquals(bs, '1M') + self.assertEquals(count, 1024) diff --git a/cinder/volume/drivers/block_device.py b/cinder/volume/drivers/block_device.py index 834b4f6b3..130c642df 100644 --- a/cinder/volume/drivers/block_device.py +++ b/cinder/volume/drivers/block_device.py @@ -25,6 +25,7 @@ from cinder.image import image_utils from cinder.openstack.common import log as logging from cinder import utils from cinder.volume import driver +from cinder.volume import utils as volutils LOG = logging.getLogger(__name__) @@ -274,8 +275,8 @@ class BlockDeviceDriver(driver.ISCSIDriver): if self.configuration.volume_clear == 'zero': if clear_size == 0: - return self._copy_volume('/dev/zero', vol_path, size_in_m, - clearing=True) + return volutils.copy_volume('/dev/zero', vol_path, size_in_m, + sync=True, execute=self._execute) else: clear_cmd = ['shred', '-n0', '-z', '-s%dMiB' % clear_size] elif self.configuration.volume_clear == 'shred': @@ -290,27 +291,6 @@ class BlockDeviceDriver(driver.ISCSIDriver): clear_cmd.append(vol_path) self._execute(*clear_cmd, run_as_root=True) - def _copy_volume(self, srcstr, deststr, size_in_m=None, clearing=False): - # Use O_DIRECT to avoid thrashing the system buffer cache - extra_flags = ['iflag=direct', 'oflag=direct'] - - # Check whether O_DIRECT is supported - try: - self._execute('dd', 'count=0', 'if=%s' % srcstr, 'of=%s' % deststr, - *extra_flags, run_as_root=True) - except exception.ProcessExecutionError: - extra_flags = [] - - # If the volume is being unprovisioned then - # request the data is persisted before returning, - # so that it's not discarded from the cache. - if clearing and not extra_flags: - extra_flags.append('conv=fdatasync') - # Perform the copy - self._execute('dd', 'if=%s' % srcstr, 'of=%s' % deststr, - 'count=%d' % size_in_m, 'bs=1M', - *extra_flags, run_as_root=True) - def copy_image_to_volume(self, context, volume, image_service, image_id): """Fetch the image from image_service and write it to the volume.""" image_utils.fetch_to_raw(context, @@ -328,8 +308,9 @@ class BlockDeviceDriver(driver.ISCSIDriver): def create_cloned_volume(self, volume, src_vref): LOG.info(_('Creating clone of volume: %s') % src_vref['id']) device = self.find_appropriate_size_device(src_vref['size']) - self._copy_volume(self.local_path(src_vref), device, - self._get_device_size(device) * 2048) + volutils.copy_volume(self.local_path(src_vref), device, + self._get_device_size(device) * 2048, + execute=self._execute) return { 'provider_location': self._iscsi_location(None, None, None, None, device), diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 34d4c08ae..74a731c85 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -31,10 +31,9 @@ from cinder import exception from cinder.image import image_utils from cinder.openstack.common import fileutils from cinder.openstack.common import log as logging -from cinder.openstack.common import strutils -from cinder import units from cinder import utils from cinder.volume import driver +from cinder.volume import utils as volutils LOG = logging.getLogger(__name__) @@ -49,9 +48,6 @@ volume_opts = [ cfg.IntOpt('volume_clear_size', default=0, help='Size in MiB to wipe at start of old volumes. 0 => all'), - cfg.StrOpt('volume_dd_blocksize', - default='1M', - help='The default block size used when clearing volumes'), cfg.StrOpt('pool_size', default=None, help='Size of thin provisioning pool ' @@ -103,56 +99,6 @@ class LVMVolumeDriver(driver.VolumeDriver): self._try_execute(*cmd, run_as_root=True, no_retry_list=no_retry_list) - def _calculate_count(self, blocksize, size_in_g): - # Check if volume_dd_blocksize is valid - try: - # Rule out zero-sized/negative dd blocksize which - # cannot be caught by strutils - if blocksize.startswith(('-', '0')): - raise ValueError - bs = strutils.to_bytes(blocksize) - except (ValueError, TypeError): - msg = (_("Incorrect value error: %(blocksize)s, " - "it may indicate that \'volume_dd_blocksize\' " - "was configured incorrectly. Fall back to default.") - % {'blocksize': blocksize}) - LOG.warn(msg) - # Fall back to default blocksize - CONF.clear_override('volume_dd_blocksize', - self.configuration.config_group) - blocksize = self.configuration.volume_dd_blocksize - bs = strutils.to_bytes(blocksize) - - count = math.ceil(size_in_g * units.GiB / float(bs)) - - return blocksize, int(count) - - def _copy_volume(self, srcstr, deststr, size_in_g, clearing=False): - # Use O_DIRECT to avoid thrashing the system buffer cache - extra_flags = ['iflag=direct', 'oflag=direct'] - - # Check whether O_DIRECT is supported - try: - self._execute('dd', 'count=0', 'if=%s' % srcstr, 'of=%s' % deststr, - *extra_flags, run_as_root=True) - except exception.ProcessExecutionError: - extra_flags = [] - - # If the volume is being unprovisioned then - # request the data is persisted before returning, - # so that it's not discarded from the cache. - if clearing and not extra_flags: - extra_flags.append('conv=fdatasync') - - blocksize = self.configuration.volume_dd_blocksize - blocksize, count = self._calculate_count(blocksize, size_in_g) - - # Perform the copy - self._execute('dd', 'if=%s' % srcstr, 'of=%s' % deststr, - 'count=%d' % count, - 'bs=%s' % blocksize, - *extra_flags, run_as_root=True) - def _volume_not_present(self, volume_name): path_name = '%s/%s' % (self.configuration.volume_group, volume_name) try: @@ -196,8 +142,10 @@ class LVMVolumeDriver(driver.VolumeDriver): def create_volume_from_snapshot(self, volume, snapshot): """Creates a volume from a snapshot.""" self._create_volume(volume['name'], self._sizestr(volume['size'])) - self._copy_volume(self.local_path(snapshot), self.local_path(volume), - snapshot['volume_size']) + volutils.copy_volume(self.local_path(snapshot), + self.local_path(volume), + snapshot['volume_size'] * 1024, + execute=self._execute) def delete_volume(self, volume): """Deletes a logical volume.""" @@ -238,9 +186,10 @@ class LVMVolumeDriver(driver.VolumeDriver): if self.configuration.volume_clear == 'zero': if size_in_m == 0: - return self._copy_volume('/dev/zero', - vol_path, size_in_g, - clearing=True) + return volutils.copy_volume('/dev/zero', + vol_path, size_in_g * 1024, + sync=True, + execute=self._execute) else: clear_cmd = ['shred', '-n0', '-z', '-s%dMiB' % size_in_m] elif self.configuration.volume_clear == 'shred': @@ -309,9 +258,10 @@ class LVMVolumeDriver(driver.VolumeDriver): self.create_snapshot(temp_snapshot) self._create_volume(volume['name'], self._sizestr(volume['size'])) try: - self._copy_volume(self.local_path(temp_snapshot), - self.local_path(volume), - src_vref['size']) + volutils.copy_volume(self.local_path(temp_snapshot), + self.local_path(volume), + src_vref['size'] * 1024, + execute=self._execute) finally: self.delete_snapshot(temp_snapshot) diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py index e865c8718..5ace4c811 100644 --- a/cinder/volume/utils.py +++ b/cinder/volume/utils.py @@ -17,18 +17,31 @@ """Volume-related Utilities and helpers.""" +import math import os import stat from oslo.config import cfg +from cinder import exception from cinder.openstack.common import log as logging from cinder.openstack.common.notifier import api as notifier_api +from cinder.openstack.common import strutils from cinder.openstack.common import timeutils +from cinder import units from cinder import utils +volume_opts = [ + cfg.StrOpt('volume_dd_blocksize', + default='1M', + help='The default block size used when copying/clearing ' + 'volumes'), +] + CONF = cfg.CONF +CONF.register_opts(volume_opts) + LOG = logging.getLogger(__name__) @@ -132,3 +145,55 @@ def notify_about_snapshot_usage(context, snapshot, event_suffix, def is_block(path): mode = os.stat(path).st_mode return stat.S_ISBLK(mode) + + +def _calculate_count(size_in_m): + blocksize = CONF.volume_dd_blocksize + # Check if volume_dd_blocksize is valid + try: + # Rule out zero-sized/negative dd blocksize which + # cannot be caught by strutils + if blocksize.startswith(('-', '0')): + raise ValueError + bs = strutils.to_bytes(blocksize) + except (ValueError, TypeError): + msg = (_("Incorrect value error: %(blocksize)s, " + "it may indicate that \'volume_dd_blocksize\' " + "was configured incorrectly. Fall back to default.") + % {'blocksize': blocksize}) + LOG.warn(msg) + # Fall back to default blocksize + CONF.clear_override('volume_dd_blocksize') + blocksize = CONF.volume_dd_blocksize + bs = strutils.to_bytes(blocksize) + + count = math.ceil(size_in_m * units.MiB / float(bs)) + + return blocksize, int(count) + + +def copy_volume(srcstr, deststr, size_in_m, sync=False, + execute=utils.execute): + # Use O_DIRECT to avoid thrashing the system buffer cache + extra_flags = ['iflag=direct', 'oflag=direct'] + + # Check whether O_DIRECT is supported + try: + execute('dd', 'count=0', 'if=%s' % srcstr, 'of=%s' % deststr, + *extra_flags, run_as_root=True) + except exception.ProcessExecutionError: + extra_flags = [] + + # If the volume is being unprovisioned then + # request the data is persisted before returning, + # so that it's not discarded from the cache. + if sync and not extra_flags: + extra_flags.append('conv=fdatasync') + + blocksize, count = _calculate_count(size_in_m) + + # Perform the copy + execute('dd', 'if=%s' % srcstr, 'of=%s' % deststr, + 'count=%d' % count, + 'bs=%s' % blocksize, + *extra_flags, run_as_root=True)