From c0f12f28be44a2e05932de1f222698dce221b2b7 Mon Sep 17 00:00:00 2001 From: Mitsuhiro Tanino Date: Wed, 13 May 2015 11:59:13 -0400 Subject: [PATCH] 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 --- cinder/tests/test_volume.py | 54 +++++++++++++++++------------------- cinder/volume/drivers/lvm.py | 6 ++-- 2 files changed, 30 insertions(+), 30 deletions(-) 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) -- 2.45.2