From: Mitsuhiro Tanino <mitsuhiro.tanino@hds.com>
Date: Wed, 13 May 2015 15:59:13 +0000 (-0400)
Subject: LVM: Pass volume size in MiB to copy_volume() during volume migration
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a5da4c353a1df17341c1e745796e00a4ec1afd21;p=openstack-build%2Fcinder-build.git

LVM: Pass volume size in MiB to copy_volume() during volume migration

Currently migrate_volume() in lvm.py does not pass volume size in
MiB but passes volume size in GiB to copy_volume(). The size argument
of copy_volume() requires MiB value. As a result, if the volume size
is 1GiB, copy_volume() copies only 1MiB head of 1GiB volume to the
destination volume, and then the volume data is corrupted.
This patch fixes to pass volume size in MiB.

Closes-bug: #1454835
Change-Id: I8e06195dc3625ef07bc9858d844541e37a231b73
---

diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py
index bfacae8bd..14080e755 100644
--- a/cinder/tests/unit/test_volume.py
+++ b/cinder/tests/unit/test_volume.py
@@ -5515,7 +5515,7 @@ class LVMISCSIVolumeDriverTestCase(DriverTestCase):
         capabilities = {'location_info': 'LVMVolumeDriver:%s:'
                         'cinder-volumes-2:default:0' % hostname}
         host = {'capabilities': capabilities}
-        vol = {'name': 'test', 'id': 1, 'size': 1, 'status': 'available'}
+        vol = {'name': 'testvol', 'id': 1, 'size': 2, 'status': 'available'}
 
         def fake_execute(*args, **kwargs):
             pass
@@ -5528,33 +5528,29 @@ class LVMISCSIVolumeDriverTestCase(DriverTestCase):
         def _fake_get_all_physical_volumes(obj, root_helper, vg_name):
             return [{}]
 
-        self.stubs.Set(brick_lvm.LVM,
-                       'get_all_physical_volumes',
-                       _fake_get_all_physical_volumes)
-
-        self.stubs.Set(self.volume.driver, '_execute', fake_execute)
-
-        self.stubs.Set(volutils, 'copy_volume',
-                       lambda x, y, z, sync=False, execute='foo',
-                       blocksize=mox.IgnoreArg(): None)
-
-        self.stubs.Set(volutils, 'get_all_volume_groups',
-                       get_all_volume_groups)
-
-        self.stubs.Set(self.volume.driver, '_delete_volume',
-                       lambda x: None)
-
-        self.stubs.Set(self.volume.driver, 'create_export',
-                       lambda x, y, vg='vg': None)
-
-        self.volume.driver.vg = fake_lvm.FakeBrickLVM('cinder-volumes',
-                                                      False,
-                                                      None,
-                                                      'default')
-        moved, model_update = self.volume.driver.migrate_volume(self.context,
-                                                                vol, host)
-        self.assertTrue(moved)
-        self.assertIsNone(model_update)
+        with mock.patch.object(brick_lvm.LVM, 'get_all_physical_volumes',
+                               return_value = [{}]), \
+                mock.patch.object(self.volume.driver, '_execute') \
+                as mock_execute, \
+                mock.patch.object(volutils, 'copy_volume') as mock_copy, \
+                mock.patch.object(volutils, 'get_all_volume_groups',
+                                  side_effect = get_all_volume_groups), \
+                mock.patch.object(self.volume.driver, '_delete_volume'):
+
+            self.volume.driver.vg = fake_lvm.FakeBrickLVM('cinder-volumes',
+                                                          False,
+                                                          None,
+                                                          'default')
+            moved, model_update = \
+                self.volume.driver.migrate_volume(self.context, vol, host)
+            self.assertTrue(moved)
+            self.assertIsNone(model_update)
+            mock_copy.assert_called_once_with(
+                '/dev/mapper/cinder--volumes-testvol',
+                '/dev/mapper/cinder--volumes--2-testvol',
+                2048,
+                '1M',
+                execute=mock_execute)
 
     @staticmethod
     def _get_manage_existing_lvs(name):
diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py
index 855a2df6d..838e1e3be 100644
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -561,10 +561,12 @@ class LVMVolumeDriver(driver.VolumeDriver):
                                 lvm_type,
                                 lvm_mirrors,
                                 dest_vg_ref)
-
+            # copy_volume expects sizes in MiB, we store integer GiB
+            # be sure to convert before passing in
+            size_in_mb = int(volume['size']) * units.Ki
             volutils.copy_volume(self.local_path(volume),
                                  self.local_path(volume, vg=dest_vg),
-                                 volume['size'],
+                                 size_in_mb,
                                  self.configuration.volume_dd_blocksize,
                                  execute=self._execute)
             self._delete_volume(volume)