From a5da4c353a1df17341c1e745796e00a4ec1afd21 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. Closes-bug: #1454835 Change-Id: I8e06195dc3625ef07bc9858d844541e37a231b73 --- cinder/tests/unit/test_volume.py | 52 +++++++++++++++----------------- cinder/volume/drivers/lvm.py | 6 ++-- 2 files changed, 28 insertions(+), 30 deletions(-) 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) -- 2.45.2