From 379273925517e8b9bf971e018a1dc41f809c4814 Mon Sep 17 00:00:00 2001 From: lirenke Date: Tue, 4 Mar 2014 04:59:07 -0800 Subject: [PATCH] Can't force-create snapshot by an non-exist error volume If we create an LVM volume whose size larger than rest space in VG, the volume would not exist with error status. But then we still can force create a snapshot by this volume, and the snapshot's status is available. In the code,I found in create_lv_snapshot, it return False when fail to get the volume. However, raising an exception is only way to be handle outside. So, we should raise exception instead of return False. Closes-Bug: #1283338 Change-Id: I80256f19d66da460c95ff23834abb79a557763bf --- cinder/brick/local_dev/lvm.py | 5 +++-- cinder/tests/brick/test_brick_lvm.py | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/cinder/brick/local_dev/lvm.py b/cinder/brick/local_dev/lvm.py index 0c8c10f813a..095d79e29e5 100644 --- a/cinder/brick/local_dev/lvm.py +++ b/cinder/brick/local_dev/lvm.py @@ -490,8 +490,9 @@ class LVM(executor.Executor): """ source_lvref = self.get_volume(source_lv_name) if source_lvref is None: - LOG.error(_("Unable to find LV: %s") % source_lv_name) - return False + LOG.error(_("Trying to create snapshot by non-existent LV: %s") + % source_lv_name) + raise exception.VolumeDeviceNotFound(device=source_lv_name) cmd = ['lvcreate', '--name', name, '--snapshot', '%s/%s' % (self.vg_name, source_lv_name)] if lv_type != 'thin': diff --git a/cinder/tests/brick/test_brick_lvm.py b/cinder/tests/brick/test_brick_lvm.py index a213b96037f..95b37ae1203 100644 --- a/cinder/tests/brick/test_brick_lvm.py +++ b/cinder/tests/brick/test_brick_lvm.py @@ -15,7 +15,7 @@ import mox - +from cinder.brick import exception from cinder.brick.local_dev import lvm as brick from cinder.openstack.common import log as logging from cinder.openstack.common import processutils @@ -109,11 +109,27 @@ class BrickLvmTestCase(test.TestCase): pass elif 'lvcreate, -T, -V, ' in cmd_string: pass + elif 'lvcreate, --name, ' in cmd_string: + pass else: raise AssertionError('unexpected command called: %s' % cmd_string) return (data, "") + def test_create_lv_snapshot(self): + self.assertEqual(self.vg.create_lv_snapshot('snapshot-1', 'fake-1'), + None) + + self._mox.StubOutWithMock(self.vg, 'get_volume') + self.vg.get_volume('fake-non-existent').AndReturn(None) + self._mox.ReplayAll() + try: + self.vg.create_lv_snapshot('snapshot-1', 'fake-non-existent') + except exception.VolumeDeviceNotFound as e: + self.assertEqual(e.kwargs['device'], 'fake-non-existent') + else: + self.fail("Exception not raised") + def test_vg_exists(self): self.assertEqual(self.vg._vg_exists(), True)