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=c0f12f28be44a2e05932de1f222698dce221b2b7;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.

(cherry picked from commit a5da4c353a1df17341c1e745796e00a4ec1afd21)
Conflicts:
    cinder/tests/test_volume.py
    (Added mock for create_export.)

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

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 38a2cbc93..2be067959 100644
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -5342,7 +5342,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
@@ -5355,33 +5355,31 @@ 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'), \
+                mock.patch.object(self.volume.driver, 'create_export',
+                                  return_value = 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)
+            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 9b4d0465e..84e6f56e8 100644
--- a/cinder/volume/drivers/lvm.py
+++ b/cinder/volume/drivers/lvm.py
@@ -563,10 +563,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)