From c859509f8c435a73f4dc524d8d5f00c80d8c8526 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Thu, 21 Jan 2016 18:56:36 +0000 Subject: [PATCH] Rename Image.check_image_exists to Image.exists() Apart from being shorter, this is also another step away from the overloading of the term 'image' in the image backend. An 'image' in this code refers either to the thing which glance stores, or an instance's disk. Image in the latter context has come about because that's what libvirt calls it. However, as it's overloaded in nova we should never have used it. At some point I'd like to rename these all to 'disk'. This patch also renames 2 variables called 'backend' to 'root_disk', to more accurately reflect their meaning. Changing the name of check_image_exists() made these uses harder to read when the meaning of the variable wasn't clear. Change-Id: Ic15e8412971b2a3f8e87ec68605c4753e23f94b5 --- nova/tests/unit/virt/libvirt/test_driver.py | 4 +- .../unit/virt/libvirt/test_imagebackend.py | 48 +++++++++---------- nova/virt/libvirt/driver.py | 18 +++---- nova/virt/libvirt/imagebackend.py | 14 +++--- 4 files changed, 42 insertions(+), 42 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 1d03674aa377..a49e08300fdd 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -14765,7 +14765,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) drvr.image_backend = mock.Mock() drvr.image_backend.image.return_value = drvr.image_backend - drvr.image_backend.check_image_exists.return_value = False + drvr.image_backend.exists.return_value = False ins_ref = self._create_instance() with test.nested( @@ -14876,7 +14876,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) drvr.image_backend = mock.Mock() drvr.image_backend.image.return_value = drvr.image_backend - drvr.image_backend.check_image_exists.return_value = False + drvr.image_backend.exists.return_value = False with test.nested( mock.patch.object(os.path, 'exists'), diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index bce695ff6656..86ae48c32ded 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -128,7 +128,7 @@ class _ImageTestCase(object): def fake_fetch(target, *args, **kwargs): return - self.stubs.Set(image, 'check_image_exists', lambda: True) + self.stubs.Set(image, 'exists', lambda: True) self.stubs.Set(image, '_can_fallocate', lambda: True) self.stubs.Set(image, 'get_disk_size', lambda _: self.SIZE) self.stub_out('os.path.exists', lambda _: True) @@ -766,7 +766,7 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase): return self.stub_out('os.path.exists', lambda _: True) - self.stubs.Set(image, 'check_image_exists', lambda: True) + self.stubs.Set(image, 'exists', lambda: True) self.stubs.Set(image, 'get_disk_size', lambda _: self.SIZE) image.cache(fake_fetch, self.TEMPLATE_PATH, self.SIZE) @@ -1139,7 +1139,7 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase): return self.stub_out('os.path.exists', lambda _: True) - self.stubs.Set(image, 'check_image_exists', lambda: True) + self.stubs.Set(image, 'exists', lambda: True) self.stubs.Set(image, 'get_disk_size', lambda _: self.SIZE) image.cache(fake_fetch, self.TEMPLATE_PATH, self.SIZE) @@ -1176,9 +1176,9 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): image = self.image_class(self.INSTANCE, self.NAME) self.mox.StubOutWithMock(os.path, 'exists') - self.mox.StubOutWithMock(image, 'check_image_exists') + self.mox.StubOutWithMock(image, 'exists') os.path.exists(self.TEMPLATE_DIR).AndReturn(False) - image.check_image_exists().AndReturn(False) + image.exists().AndReturn(False) os.path.exists(self.TEMPLATE_PATH).AndReturn(False) fn = self.mox.CreateMockAnything() fn(target=self.TEMPLATE_PATH) @@ -1196,9 +1196,9 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): image = self.image_class(self.INSTANCE, self.NAME) self.mox.StubOutWithMock(os.path, 'exists') - self.mox.StubOutWithMock(image, 'check_image_exists') + self.mox.StubOutWithMock(image, 'exists') os.path.exists(self.TEMPLATE_DIR).AndReturn(True) - image.check_image_exists().AndReturn(False) + image.exists().AndReturn(False) os.path.exists(self.TEMPLATE_PATH).AndReturn(False) fn = self.mox.CreateMockAnything() fn(target=self.TEMPLATE_PATH) @@ -1214,9 +1214,9 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): image = self.image_class(self.INSTANCE, self.NAME) self.mox.StubOutWithMock(os.path, 'exists') - self.mox.StubOutWithMock(image, 'check_image_exists') + self.mox.StubOutWithMock(image, 'exists') os.path.exists(self.TEMPLATE_DIR).AndReturn(True) - image.check_image_exists().AndReturn(True) + image.exists().AndReturn(True) os.path.exists(self.TEMPLATE_PATH).AndReturn(True) self.mox.ReplayAll() @@ -1228,9 +1228,9 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): image = self.image_class(self.INSTANCE, self.NAME) self.mox.StubOutWithMock(os.path, 'exists') - self.mox.StubOutWithMock(image, 'check_image_exists') + self.mox.StubOutWithMock(image, 'exists') os.path.exists(self.TEMPLATE_DIR).AndReturn(True) - image.check_image_exists().AndReturn(False) + image.exists().AndReturn(False) os.path.exists(self.TEMPLATE_PATH).AndReturn(True) self.mox.ReplayAll() @@ -1249,9 +1249,9 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): fake_processutils.stub_out_processutils_execute(self.stubs) image = self.image_class(self.INSTANCE, self.NAME) - self.mox.StubOutWithMock(image, 'check_image_exists') - image.check_image_exists().AndReturn(False) - image.check_image_exists().AndReturn(False) + self.mox.StubOutWithMock(image, 'exists') + image.exists().AndReturn(False) + image.exists().AndReturn(False) self.mox.ReplayAll() image.create_image(fn, self.TEMPLATE_PATH, None) @@ -1275,9 +1275,9 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): fake_processutils.stub_out_processutils_execute(self.stubs) image = self.image_class(self.INSTANCE, self.NAME) - self.mox.StubOutWithMock(image, 'check_image_exists') - image.check_image_exists().AndReturn(False) - image.check_image_exists().AndReturn(False) + self.mox.StubOutWithMock(image, 'exists') + image.exists().AndReturn(False) + image.exists().AndReturn(False) rbd_name = "%s_%s" % (self.INSTANCE['uuid'], self.NAME) cmd = ('rbd', 'import', '--pool', self.POOL, self.TEMPLATE_PATH, rbd_name, '--image-format=2', '--id', self.USER, @@ -1301,11 +1301,11 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): rbd_utils.rbd.RBD_FEATURE_LAYERING = 1 image = self.image_class(self.INSTANCE, self.NAME) - self.mox.StubOutWithMock(image, 'check_image_exists') - image.check_image_exists().AndReturn(True) + self.mox.StubOutWithMock(image, 'exists') + image.exists().AndReturn(True) self.mox.StubOutWithMock(image, 'get_disk_size') image.get_disk_size(self.TEMPLATE_PATH).AndReturn(self.SIZE) - image.check_image_exists().AndReturn(True) + image.exists().AndReturn(True) rbd_name = "%s_%s" % (self.INSTANCE['uuid'], self.NAME) image.get_disk_size(rbd_name).AndReturn(self.SIZE) @@ -1327,7 +1327,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): return self.stub_out('os.path.exists', lambda _: True) - self.stubs.Set(image, 'check_image_exists', lambda: True) + self.stubs.Set(image, 'exists', lambda: True) self.stubs.Set(image, 'get_disk_size', lambda _: self.SIZE) image.cache(fake_fetch, self.TEMPLATE_PATH, self.SIZE) @@ -1411,7 +1411,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): def test_import_file(self): image = self.image_class(self.INSTANCE, self.NAME) - @mock.patch.object(image, 'check_image_exists') + @mock.patch.object(image, 'exists') @mock.patch.object(image.driver, 'remove_image') @mock.patch.object(image.driver, 'import_image') def _test(mock_import, mock_remove, mock_exists): @@ -1428,7 +1428,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): def test_import_file_not_found(self): image = self.image_class(self.INSTANCE, self.NAME) - @mock.patch.object(image, 'check_image_exists') + @mock.patch.object(image, 'exists') @mock.patch.object(image.driver, 'remove_image') @mock.patch.object(image.driver, 'import_image') def _test(mock_import, mock_remove, mock_exists): @@ -1637,7 +1637,7 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase): return self.stub_out('os.path.exists', lambda _: True) - self.stubs.Set(image, 'check_image_exists', lambda: True) + self.stubs.Set(image, 'exists', lambda: True) self.stubs.Set(image, 'get_disk_size', lambda _: self.SIZE) image.cache(fake_fetch, self.TEMPLATE_PATH, self.SIZE) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 3fb0ca1d6d37..6f4c5efdf6ab 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1052,7 +1052,7 @@ class LibvirtDriver(driver.ComputeDriver): utils.execute('rm', '-rf', target, delay_on_retry=True, attempts=5) - backend = self.image_backend.image(instance, 'disk') + root_disk = self.image_backend.image(instance, 'disk') # TODO(nic): Set ignore_errors=False in a future release. # It is set to True here to avoid any upgrade issues surrounding # instances being in pending resize state when the software is updated; @@ -1060,9 +1060,9 @@ class LibvirtDriver(driver.ComputeDriver): # reasonably assumed that no such instances exist in the wild # anymore, it should be set back to False (the default) so it will # throw errors, like it should. - if backend.check_image_exists(): - backend.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME, - ignore_errors=True) + if root_disk.exists(): + root_disk.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME, + ignore_errors=True) if instance.host != CONF.host: self._undefine_domain(instance) @@ -7166,7 +7166,7 @@ class LibvirtDriver(driver.ComputeDriver): self._cleanup_failed_migration(inst_base) utils.execute('mv', inst_base_resize, inst_base) - backend = self.image_backend.image(instance, 'disk') + root_disk = self.image_backend.image(instance, 'disk') # Once we rollback, the snapshot is no longer needed, so remove it # TODO(nic): Remove the try/except/finally in a future release # To avoid any upgrade issues surrounding instances being in pending @@ -7176,15 +7176,15 @@ class LibvirtDriver(driver.ComputeDriver): # anymore, the try/except/finally should be removed, # and ignore_errors should be set back to False (the default) so # that problems throw errors, like they should. - if backend.check_image_exists(): + if root_disk.exists(): try: - backend.rollback_to_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME) + root_disk.rollback_to_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME) except exception.SnapshotNotFound: LOG.warning(_LW("Failed to rollback snapshot (%s)"), libvirt_utils.RESIZE_SNAPSHOT_NAME) finally: - backend.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME, - ignore_errors=True) + root_disk.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME, + ignore_errors=True) disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type, instance, diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 42c94581c51c..88d3fcaaf5b1 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -180,7 +180,7 @@ class Image(object): return info - def check_image_exists(self): + def exists(self): return os.path.exists(self.path) def cache(self, fetch_func, filename, size=None, *args, **kwargs): @@ -208,7 +208,7 @@ class Image(object): fileutils.ensure_tree(base_dir) base = os.path.join(base_dir, filename) - if not self.check_image_exists() or not os.path.exists(base): + if not self.exists() or not os.path.exists(base): self.create_image(fetch_func_sync, base, size, *args, **kwargs) @@ -491,7 +491,7 @@ class Flat(Image): generating = 'image_id' not in kwargs if generating: - if not self.check_image_exists(): + if not self.exists(): # Generating image in place prepare_template(target=self.path, *args, **kwargs) else: @@ -823,7 +823,7 @@ class Rbd(Image): def _can_fallocate(self): return False - def check_image_exists(self): + def exists(self): return self.driver.exists(self.rbd_name) def get_disk_size(self, name): @@ -836,12 +836,12 @@ class Rbd(Image): def create_image(self, prepare_template, base, size, *args, **kwargs): - if not self.check_image_exists(): + if not self.exists(): prepare_template(target=base, max_size=size, *args, **kwargs) # prepare_template() may have cloned the image into a new rbd # image already instead of downloading it locally - if not self.check_image_exists(): + if not self.exists(): self.driver.import_image(base, self.rbd_name) self.verify_base_size(base, size) @@ -896,7 +896,7 @@ class Rbd(Image): def import_file(self, instance, local_file, remote_name): name = '%s_%s' % (instance.uuid, remote_name) - if self.check_image_exists(): + if self.exists(): self.driver.remove_image(name) self.driver.import_image(local_file, name)