Calculate count for customized dd blocksize

In previous commit we introduce a configurable blocksize option for
'dd' command, however the 'count' value wasn't updated accordingly.
As a result, count could be too small which is bug.  This patch
introduce a private method to 1) validate custom dd blocksize; 2)
calculate correct count value.

Fix bug: 1192258

Change-Id: Icfd2d80e8f615ed585bd1d40c741a97cf55299d3

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index a2cdfe7bf2cdab83842d1d8e3d0c10e53889071f..face412997693fece2338b3429b39bd1f92bce24 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -22,6 +22,7 @@ Tests for Volume Code.
 
 import datetime
 import os
+import re
 import shutil
 import tempfile
 
@@ -44,12 +45,17 @@ from cinder.tests import fake_flags
 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
 
 
 QUOTAS = quota.QUOTAS
 
 CONF = cfg.CONF
 
+fake_opt = [
+    cfg.StrOpt('fake_opt', default='fake', help='fake opts')
+]
+
 
 class VolumeTestCase(test.TestCase):
     """Test Case for volumes."""
@@ -1336,6 +1342,45 @@ class VolumeDriverTestCase(DriverTestCase):
         self.volume.driver.delete_volume({'name': 'test1', 'size': 1024})
 
 
+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)
+
+
 class ISCSITestCase(DriverTestCase):
     """Test Case for ISCSIDriver"""
     driver_name = "cinder.volume.drivers.lvm.LVMISCSIDriver"

diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index 2f5ee96b192166e653c8a69470a0d02055f44e39..4fba2db4ebaf76325bdacf1b1f320199cf50081e 100644 (file)
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -31,6 +31,8 @@ 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
 
@@ -101,6 +103,30 @@ 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']
@@ -118,10 +144,13 @@ class LVMVolumeDriver(driver.VolumeDriver):
         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' % (size_in_g * 1024),
-                      'bs=%s' % self.configuration.volume_dd_blocksize,
+                      'count=%d' % count,
+                      'bs=%s' % blocksize,
                       *extra_flags, run_as_root=True)
 
     def _volume_not_present(self, volume_name):