openstack/nova
Code Issues Proposed changes

Rename Image.check_image_exists to Image.exists()

Browse Source 
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
This commit is contained in:
Matthew Booth
2016-01-21 18:56:36 +00:00
parent f07306ee3d
commit c859509f8c
4 changed files with 42 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -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'),

48
nova/tests/unit/virt/libvirt/test_imagebackend.py
View File

@ -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)

18
nova/virt/libvirt/driver.py
View File

@ -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,

14
nova/virt/libvirt/imagebackend.py
View File

@ -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)