From cb15d34cc1caf980c636eace5cae0be2b1b5108f Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Thu, 26 May 2016 10:06:54 +0100 Subject: [PATCH] Rename ImageCacheManager._list_base_images to _scan_base_images The method's return value is never used, only its side-effects are used. We rename it to reflect its actual usage, and remove the unused return value to avoid confusion. NOTE(mdbooth): The use of global state is a large part of what makes ImageCacheManager hard to read. I don't mean to imply with this change that's what should be done here, only to make it clearer to the reader that it is what's being done currently. Change-Id: Ic82f6d4f00fd1226673750538019339937a623bc --- nova/tests/unit/virt/libvirt/test_imagecache.py | 6 +++--- nova/tests/unit/virt/test_imagecache.py | 5 ++--- nova/virt/imagecache.py | 12 ++++-------- nova/virt/libvirt/imagecache.py | 15 +++++---------- 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_imagecache.py b/nova/tests/unit/virt/libvirt/test_imagecache.py index 87535e6ccce4..5009d8c2bc73 100644 --- a/nova/tests/unit/virt/libvirt/test_imagecache.py +++ b/nova/tests/unit/virt/libvirt/test_imagecache.py @@ -98,7 +98,7 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): self.flags(instances_path='/var/lib/nova/instances') image_cache_manager = imagecache.ImageCacheManager() - image_cache_manager._list_base_images(base_dir) + image_cache_manager._scan_base_images(base_dir) sanitized = [] for ent in image_cache_manager.unexplained_images: @@ -256,7 +256,7 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): base_dir = '/var/lib/nova/instances/_base' image_cache_manager = imagecache.ImageCacheManager() - image_cache_manager._list_base_images(base_dir) + image_cache_manager._scan_base_images(base_dir) res = list(image_cache_manager._find_base_file(base_dir, fingerprint)) base_file = os.path.join(base_dir, fingerprint + '_10737418240') @@ -276,7 +276,7 @@ class ImageCacheManagerTestCase(test.NoDBTestCase): base_dir = '/var/lib/nova/instances/_base' image_cache_manager = imagecache.ImageCacheManager() - image_cache_manager._list_base_images(base_dir) + image_cache_manager._scan_base_images(base_dir) res = list(image_cache_manager._find_base_file(base_dir, fingerprint)) base_file1 = os.path.join(base_dir, fingerprint) diff --git a/nova/tests/unit/virt/test_imagecache.py b/nova/tests/unit/virt/test_imagecache.py index 497611fce0e0..0d560fd53a60 100644 --- a/nova/tests/unit/virt/test_imagecache.py +++ b/nova/tests/unit/virt/test_imagecache.py @@ -63,9 +63,8 @@ class ImageCacheManagerTests(test.NoDBTestCase): cache_manager.update, None, []) self.assertRaises(NotImplementedError, cache_manager._get_base) - base_images = cache_manager._list_base_images(None) - self.assertEqual([], base_images['unexplained_images']) - self.assertEqual([], base_images['originals']) + self.assertRaises(NotImplementedError, + cache_manager._scan_base_images, None) self.assertRaises(NotImplementedError, cache_manager._age_and_verify_cached_images, None, [], None) diff --git a/nova/virt/imagecache.py b/nova/virt/imagecache.py index 801aa17f357c..841e043e4e98 100644 --- a/nova/virt/imagecache.py +++ b/nova/virt/imagecache.py @@ -88,15 +88,11 @@ class ImageCacheManager(object): 'instance_names': instance_names, 'used_swap_images': used_swap_images} - def _list_base_images(self, base_dir): - """Return a list of the images present in _base. - - This method returns a dictionary with the following keys: - - unexplained_images - - originals + def _scan_base_images(self, base_dir): + """Scan base images present in base_dir and populate internal + state. """ - return {'unexplained_images': [], - 'originals': []} + raise NotImplementedError() def _age_and_verify_cached_images(self, context, all_instances, base_dir): """Ages and verifies cached images.""" diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py index dcbf61f8242c..1967987531ec 100644 --- a/nova/virt/libvirt/imagecache.py +++ b/nova/virt/libvirt/imagecache.py @@ -116,12 +116,10 @@ class ImageCacheManager(imagecache.ImageCacheManager): LOG.debug('Adding %s into backend swap images', ent) self.back_swap_images.add(ent) - def _list_base_images(self, base_dir): - """Return a list of the images present in _base. - - Determine what images we have on disk. There will be other files in - this directory so we only grab the ones which are the right length - to be disk images. + def _scan_base_images(self, base_dir): + """Scan base images in base_dir and call _store_image or + _store_swap_image on each as appropriate. These methods populate + self.unexplained_images, self.originals, and self.back_swap_images. """ digest_size = hashlib.sha1().digestsize * 2 @@ -157,9 +155,6 @@ class ImageCacheManager(imagecache.ImageCacheManager): else: self._store_swap_image(ent) - return {'unexplained_images': self.unexplained_images, - 'originals': self.originals} - def _list_backing_images(self): """List the backing images currently in use.""" inuse_images = [] @@ -452,7 +447,7 @@ class ImageCacheManager(imagecache.ImageCacheManager): # reset the local statistics self._reset_state() # read the cached images - self._list_base_images(base_dir) + self._scan_base_images(base_dir) # read running instances data running = self._list_running_instances(context, all_instances) self.used_images = running['used_images']