From 2397c6b902174ef39bff5f9194a06f82c6746267 Mon Sep 17 00:00:00 2001 From: Michael Still Date: Sat, 29 Dec 2012 19:22:54 +1100 Subject: Report failures to mount in localfs correctly. The wrong exception type was being thrown, which meant that the virt disk api though the disk could be resized when it couldn't. I've added two unit tests to cover regressions as well, but this code needs more unit testing in general. Resolves bug 1094373. Change-Id: I9c974e138ff90e8b7a5a40f5b31dcdb25a59622d --- nova/tests/virt/disk/test_api.py | 63 ++++++++++++++++++++++++++++++++++++++++ nova/tests/virt/disk/test_nbd.py | 24 +++++++++++++-- nova/virt/disk/api.py | 19 +++++++++--- nova/virt/disk/vfs/localfs.py | 3 +- 4 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 nova/tests/virt/disk/test_api.py diff --git a/nova/tests/virt/disk/test_api.py b/nova/tests/virt/disk/test_api.py new file mode 100644 index 000000000..95368a2c2 --- /dev/null +++ b/nova/tests/virt/disk/test_api.py @@ -0,0 +1,63 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2012 OpenStack LLC +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +import os +import tempfile + +import fixtures + +from nova.openstack.common import importutils +from nova import test +from nova.virt.disk import api + + +class APITestCase(test.TestCase): + + def test_can_resize_need_fs_type_specified(self): + # NOTE(mikal): Bug 1094373 saw a regression where we failed to + # treat a failure to mount as a failure to be able to resize the + # filesystem + def _fake_get_disk_size(path): + return 10 + self.useFixture(fixtures.MonkeyPatch( + 'nova.virt.disk.api.get_disk_size', _fake_get_disk_size)) + + def fake_trycmd(*args, **kwargs): + return '', 'broken' + self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', fake_trycmd)) + + def fake_returns_true(*args, **kwargs): + return True + self.useFixture(fixtures.MonkeyPatch( + 'nova.virt.disk.mount.nbd.NbdMount.get_dev', + fake_returns_true)) + self.useFixture(fixtures.MonkeyPatch( + 'nova.virt.disk.mount.nbd.NbdMount.map_dev', + fake_returns_true)) + + # Force the use of localfs, which is what was used during the failure + # reported in the bug + def fake_import_fails(*args, **kwargs): + raise Exception('Failed') + self.useFixture(fixtures.MonkeyPatch( + 'nova.openstack.common.importutils.import_module', + fake_import_fails)) + + imgfile = tempfile.NamedTemporaryFile() + self.addCleanup(imgfile.close) + self.assertFalse(api.can_resize_fs(imgfile, 100, use_cow=True)) diff --git a/nova/tests/virt/disk/test_nbd.py b/nova/tests/virt/disk/test_nbd.py index 16003c9ac..59b0784d9 100644 --- a/nova/tests/virt/disk/test_nbd.py +++ b/nova/tests/virt/disk/test_nbd.py @@ -16,11 +16,12 @@ # under the License. -import fixtures import os +import tempfile -from nova import test +import fixtures +from nova import test from nova.virt.disk.mount import nbd ORIG_EXISTS = os.path.exists @@ -270,3 +271,22 @@ class NbdTestCase(test.TestCase): # No error logged, device consumed self.assertFalse(n.get_dev()) + + def test_do_mount_need_to_specify_fs_type(self): + # NOTE(mikal): Bug 1094373 saw a regression where we failed to + # communicate a failed mount properly. + def fake_trycmd(*args, **kwargs): + return '', 'broken' + self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', fake_trycmd)) + + imgfile = tempfile.NamedTemporaryFile() + self.addCleanup(imgfile.close) + tempdir = self.useFixture(fixtures.TempDir()).path + mount = nbd.NbdMount(imgfile.name, tempdir) + + def fake_returns_true(*args, **kwargs): + return True + mount.get_dev = fake_returns_true + mount.map_dev = fake_returns_true + + self.assertFalse(mount.do_mount()) diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index 758299f16..f663515cd 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -127,20 +127,32 @@ def can_resize_fs(image, size, use_cow=False): # Check that we're increasing the size virt_size = get_disk_size(image) if virt_size >= size: + LOG.debug(_('Cannot resize filesystem %s to a smaller size.'), + image) return False # Check the image is unpartitioned if use_cow: # Try to mount an unpartitioned qcow2 image + LOG.debug(_('Checking if we can resize the COW image %s.'), image) try: inject_data(image, use_cow=True) - except exception.NovaException: + except exception.NovaException, e: + LOG.debug(_('File injection failed for image %(image)s with ' + 'error %(error)s. Cannot resize.'), + {'image': image, + 'error': e}) return False else: # For raw, we can directly inspect the file system + LOG.debug(_('Checking if we can resize the non-COW image %s.'), image) try: utils.execute('e2label', image) - except exception.ProcessExecutionError: + except exception.ProcessExecutionError, e: + LOG.debug(_('Unable to determine label for image %(image)s with ' + 'error %(errror)s. Cannot resize.'), + {'image': image, + 'error': e}) return False return True @@ -252,8 +264,7 @@ class _DiskImage(object): # Public module functions -def inject_data(image, - key=None, net=None, metadata=None, admin_password=None, +def inject_data(image, key=None, net=None, metadata=None, admin_password=None, files=None, partition=None, use_cow=False): """Injects a ssh key and optionally net data into a disk image. diff --git a/nova/virt/disk/vfs/localfs.py b/nova/virt/disk/vfs/localfs.py index 3686994fa..9efa6798b 100644 --- a/nova/virt/disk/vfs/localfs.py +++ b/nova/virt/disk/vfs/localfs.py @@ -74,8 +74,7 @@ class VFSLocalFS(vfs.VFS): self.imgdir, self.partition) if not mount.do_mount(): - raise Exception(_("Failed to mount image: %s") % - mount.error) + raise exception.NovaException(mount.error) self.mount = mount except Exception, e: LOG.debug(_("Failed to mount image %(ex)s)") % -- cgit