]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
LVM: Pass volume size in MiB to copy_volume() during volume migration
authorMitsuhiro Tanino <mitsuhiro.tanino@hds.com>
Wed, 13 May 2015 15:59:13 +0000 (11:59 -0400)
committerEric Harney <eharney@redhat.com>
Thu, 14 May 2015 20:09:34 +0000 (16:09 -0400)
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

cinder/tests/test_volume.py
cinder/volume/drivers/lvm.py

index 38a2cbc93a0245c0cf28dba6df7b6defde8f6802..2be0679595278a71e374ed0b40a05399fa95320c 100644 (file)
@@ -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):
index 9b4d0465ee0f36914945e457a77505670463de3a..84e6f56e87e5436bbca97862bf8c4a0aea9f851d 100644 (file)
@@ -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)