From ed54ff8c9d59169308d5a39e7e24c66c7be2f440 Mon Sep 17 00:00:00 2001 From: Michael Still Date: Mon, 6 Feb 2012 15:02:33 +1100 Subject: Handle refactoring of libvirt image caching. This patch handles the refactored image caching for libvirt. The new scheme keeps multiple copies of an image: : the image from glance _: the resized image The resized image is then copied or CoW'd across to the instance disk. We also want to keep original images from glance longer than we keep the resized images, as they're smaller and this reduces the load on glance. Therefore a new flag to handle the differing rules has been added. Change-Id: If6d9471b3d67cb8fac3f168b1b4a3cd57b9cc9a7 --- nova/tests/test_imagecache.py | 90 ++++++++++++++++++++++--- nova/virt/libvirt/imagecache.py | 141 ++++++++++++++++++++++++---------------- 2 files changed, 167 insertions(+), 64 deletions(-) diff --git a/nova/tests/test_imagecache.py b/nova/tests/test_imagecache.py index dee937bd2..632d15a86 100644 --- a/nova/tests/test_imagecache.py +++ b/nova/tests/test_imagecache.py @@ -69,6 +69,9 @@ class ImageCacheManagerTestCase(test.TestCase): 'e97222e91fc4241f49a7f520d1dcf446751129b3_sm', 'e09c675c2d1cfac32dae3c2d83689c8c94bc693b_sm', 'e97222e91fc4241f49a7f520d1dcf446751129b3', + '17d1b00b81642842e514494a78e804e9a511637c', + '17d1b00b81642842e514494a78e804e9a511637c_5368709120', + '17d1b00b81642842e514494a78e804e9a511637c_10737418240', '00000004'] self.stubs.Set(os, 'listdir', lambda x: listing) @@ -78,18 +81,34 @@ class ImageCacheManagerTestCase(test.TestCase): image_cache_manager = imagecache.ImageCacheManager() image_cache_manager._list_base_images(base_dir) - self.assertEquals(len(image_cache_manager.unexplained_images), 3) + self.assertEquals(len(image_cache_manager.unexplained_images), 6) expected = os.path.join(base_dir, 'e97222e91fc4241f49a7f520d1dcf446751129b3') self.assertTrue(expected in image_cache_manager.unexplained_images) + expected = os.path.join(base_dir, + '17d1b00b81642842e514494a78e804e9a511637c_' + '10737418240') + self.assertTrue(expected in image_cache_manager.unexplained_images) + unexpected = os.path.join(base_dir, '00000004') self.assertFalse(unexpected in image_cache_manager.unexplained_images) for ent in image_cache_manager.unexplained_images: self.assertTrue(ent.startswith(base_dir)) + self.assertEquals(len(image_cache_manager.originals), 2) + + expected = os.path.join(base_dir, + '17d1b00b81642842e514494a78e804e9a511637c') + self.assertTrue(expected in image_cache_manager.originals) + + unexpected = os.path.join(base_dir, + '17d1b00b81642842e514494a78e804e9a511637c_' + '10737418240') + self.assertFalse(unexpected in image_cache_manager.originals) + def test_list_running_instances(self): self.stubs.Set(db, 'instance_get_all', lambda x: [{'image_ref': 'image-1', @@ -117,7 +136,7 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertEqual(image_cache_manager.image_popularity['image-1'], 1) self.assertEqual(image_cache_manager.image_popularity['image-2'], 2) - def test_list_backing_images(self): + def test_list_backing_images_small(self): self.stubs.Set(os, 'listdir', lambda x: ['_base', 'instance-00000001', 'instance-00000002', 'instance-00000003']) @@ -137,6 +156,28 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertEquals(inuse_images, [found]) self.assertEquals(len(image_cache_manager.unexplained_images), 0) + def test_list_backing_images_resized(self): + self.stubs.Set(os, 'listdir', + lambda x: ['_base', 'instance-00000001', + 'instance-00000002', 'instance-00000003']) + self.stubs.Set(os.path, 'exists', + lambda x: x.find('instance-') != -1) + self.stubs.Set(virtutils, 'get_disk_backing_file', + lambda x: ('e97222e91fc4241f49a7f520d1dcf446751129b3_' + '10737418240')) + + found = os.path.join(FLAGS.instances_path, '_base', + 'e97222e91fc4241f49a7f520d1dcf446751129b3_' + '10737418240') + + image_cache_manager = imagecache.ImageCacheManager() + image_cache_manager.unexplained_images = [found] + + inuse_images = image_cache_manager._list_backing_images() + + self.assertEquals(inuse_images, [found]) + self.assertEquals(len(image_cache_manager.unexplained_images), 0) + def test_find_base_file_nothing(self): self.stubs.Set(os.path, 'exists', lambda x: False) @@ -148,28 +189,61 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertEqual(0, len(res)) def test_find_base_file_small(self): + fingerprint = '968dd6cc49e01aaa044ed11c0cce733e0fa44a6a' self.stubs.Set(os.path, 'exists', - lambda x: x.endswith('549867354867_sm')) + lambda x: x.endswith('%s_sm' % fingerprint)) base_dir = '/var/lib/nova/instances/_base' - fingerprint = '549867354867' image_cache_manager = imagecache.ImageCacheManager() res = list(image_cache_manager._find_base_file(base_dir, fingerprint)) base_file = os.path.join(base_dir, fingerprint + '_sm') - self.assertTrue(res == [(base_file, True)]) + self.assertTrue(res == [(base_file, True, False)]) + + def test_find_base_file_resized(self): + fingerprint = '968dd6cc49e01aaa044ed11c0cce733e0fa44a6a' + listing = ['00000001', + 'ephemeral_0_20_None', + '968dd6cc49e01aaa044ed11c0cce733e0fa44a6a_10737418240', + '00000004'] + + self.stubs.Set(os, 'listdir', lambda x: listing) + self.stubs.Set(os.path, 'exists', + lambda x: x.endswith('%s_10737418240' % fingerprint)) + self.stubs.Set(os.path, 'isfile', lambda x: True) - def test_find_base_file_both(self): + base_dir = '/var/lib/nova/instances/_base' + image_cache_manager = imagecache.ImageCacheManager() + image_cache_manager._list_base_images(base_dir) + res = list(image_cache_manager._find_base_file(base_dir, fingerprint)) + + base_file = os.path.join(base_dir, fingerprint + '_10737418240') + self.assertTrue(res == [(base_file, False, True)]) + + def test_find_base_file_all(self): + fingerprint = '968dd6cc49e01aaa044ed11c0cce733e0fa44a6a' + listing = ['00000001', + 'ephemeral_0_20_None', + '968dd6cc49e01aaa044ed11c0cce733e0fa44a6a_sm', + '968dd6cc49e01aaa044ed11c0cce733e0fa44a6a_10737418240', + '00000004'] + + self.stubs.Set(os, 'listdir', lambda x: listing) self.stubs.Set(os.path, 'exists', lambda x: True) + self.stubs.Set(os.path, 'isfile', lambda x: True) base_dir = '/var/lib/nova/instances/_base' - fingerprint = '549867354867' image_cache_manager = imagecache.ImageCacheManager() + image_cache_manager._list_base_images(base_dir) res = list(image_cache_manager._find_base_file(base_dir, fingerprint)) base_file1 = os.path.join(base_dir, fingerprint) base_file2 = os.path.join(base_dir, fingerprint + '_sm') - self.assertTrue(res == [(base_file1, False), (base_file2, True)]) + base_file3 = os.path.join(base_dir, fingerprint + '_10737418240') + print res + self.assertTrue(res == [(base_file1, False, False), + (base_file2, True, False), + (base_file3, False, True)]) def test_verify_checksum(self): testdata = ('OpenStack Software delivers a massively scalable cloud ' diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py index 04b35f1ef..0285da378 100644 --- a/nova/virt/libvirt/imagecache.py +++ b/nova/virt/libvirt/imagecache.py @@ -26,6 +26,7 @@ http://wiki.openstack.org/nova-image-cache-management. import datetime import hashlib import os +import re import sys import time @@ -46,10 +47,14 @@ imagecache_opts = [ cfg.BoolOpt('remove_unused_base_images', default=False, help='Should unused base images be removed?'), - cfg.IntOpt('remove_unused_minimum_age_seconds', + cfg.IntOpt('remove_unused_resized_minimum_age_seconds', default=3600, - help='Unused base images younger than this will not be ' + help='Unused resized base images younger than this will not be ' 'removed'), + cfg.IntOpt('remove_unused_original_minimum_age_seconds', + default=(24 * 3600), + help='Unused unresized base images younger than this will not ' + 'be removed'), ] flags.DECLARE('instances_path', 'nova.compute.manager') @@ -75,23 +80,33 @@ def read_stored_checksum(base_file): class ImageCacheManager(object): def __init__(self): self.unexplained_images = [] + self.originals = [] + + def _store_image(self, base_dir, ent, original=False): + """Store a base image for later examination.""" + entpath = os.path.join(base_dir, ent) + if os.path.isfile(entpath): + self.unexplained_images.append(entpath) + if original: + self.originals.append(entpath) 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 (for example kernels) so we only grab the ones which + are the right length to be disk images. + Note that this does not return a value. It instead populates a class variable with a list of images that we need to try and explain. """ - # Determine what images we have on disk. There will be other files in - # this directory (for example kernels) so we only grab the ones which - # are the right length to be disk images. - self.unexplained_images = [] digest_size = hashlib.sha1().digestsize * 2 for ent in os.listdir(base_dir): - if len(ent) == digest_size or len(ent) == digest_size + 3: - entpath = os.path.join(base_dir, ent) - if os.path.isfile(entpath): - self.unexplained_images.append(entpath) + if len(ent) == digest_size: + self._store_image(base_dir, ent, original=True) + + elif len(ent) > digest_size + 2 and ent[digest_size] == '_': + self._store_image(base_dir, ent, original=False) def _list_running_instances(self, context): """List running instances (on all compute nodes).""" @@ -144,19 +159,29 @@ class ImageCacheManager(object): def _find_base_file(self, base_dir, fingerprint): """Find the base file matching this fingerprint. - Yields the name of the base file, and a boolean which is True if - the image is "small". Note that is is possible for more than one - yield to result from this check. + Yields the name of the base file, a boolean which is True if the image + is "small", and a boolean which indicates if this is a resized image. + Note that is is possible for more than one yield to result from this + check. If no base file is found, then nothing is yielded. """ + # The original file from glance base_file = os.path.join(base_dir, fingerprint) if os.path.exists(base_file): - yield base_file, False + yield base_file, False, False + # An older naming style which can be removed sometime after Folsom base_file = os.path.join(base_dir, fingerprint + '_sm') if os.path.exists(base_file): - yield base_file, True + yield base_file, True, False + + # Resized images + resize_re = re.compile('.*/%s_[0-9]+$' % fingerprint) + for img in self.unexplained_images: + m = resize_re.match(img) + if m: + yield img, False, True def _verify_checksum(self, img, base_file): """Compare the checksum stored on disk with the current file. @@ -200,7 +225,11 @@ class ImageCacheManager(object): mtime = os.path.getmtime(base_file) age = time.time() - mtime - if age < FLAGS.remove_unused_minimum_age_seconds: + maxage = FLAGS.remove_unused_resized_minimum_age_seconds + if base_file in self.originals: + maxage = FLAGS.remove_unused_original_minimum_age_seconds + + if age < maxage: LOG.info(_('Base file too young to remove: %s'), base_file) else: @@ -216,31 +245,21 @@ class ImageCacheManager(object): {'base_file': base_file, 'error': e}) - def _handle_base_image(self, img, base_file, image_small): + def _handle_base_image(self, img, base_file): """Handle the checks for a single base image.""" # TODO(mikal): Write a unit test for this method image_bad = False image_in_use = False - if base_file: - LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): ' - 'checking'), - {'container_format': img['container_format'], - 'id': img['id'], - 'base_file': base_file}) - - if base_file in self.unexplained_images: - self.unexplained_images.remove(base_file) - self._verify_checksum(img, base_file) + LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): checking'), + {'container_format': img['container_format'], + 'id': img['id'], + 'base_file': base_file}) - else: - LOG.debug(_('%(container_format)s-%(id)s (%(base_file)s): ' - 'base file absent'), - {'container_format': img['container_format'], - 'id': img['id'], - 'base_file': base_file}) - base_file = None + if base_file in self.unexplained_images: + self.unexplained_images.remove(base_file) + self._verify_checksum(img, base_file) instances = [] if str(img['id']) in self.used_images: @@ -299,6 +318,23 @@ class ImageCacheManager(object): def verify_base_images(self, context): """Verify that base images are in a reasonable state.""" # TODO(mikal): Write a unit test for this method + # TODO(mikal): Handle "generated" images + + # The new scheme for base images is as follows -- an image is streamed + # from the image service to _base (filename is the sha1 hash of the + # image id). If CoW is enabled, that file is then resized to be the + # correct size for the instance (filename is the same as the original, + # but with an underscore and the resized size in bytes). This second + # file is then CoW'd to the instance disk. If CoW is disabled, the + # resize occurs as part of the copy from the cache to the instance + # directory. Files ending in _sm are no longer created, but may remain + # from previous versions. + + self.unexplained_images = [] + self.active_base_files = [] + self.corrupt_base_files = [] + self.removable_base_files = [] + self.originals = [] base_dir = os.path.join(FLAGS.instances_path, '_base') if not os.path.exists(base_dir): @@ -310,34 +346,27 @@ class ImageCacheManager(object): self._list_base_images(base_dir) self._list_running_instances(context) - # Determine what images are in glance + # Determine what images are in glance. GlanceImageService.detail uses + # _fetch_images which handles pagination for us image_service = image.get_default_image_service() - - self.active_base_files = [] - self.corrupt_base_files = [] - self.removable_base_files = [] - - # GlanceImageService.detail uses _fetch_images which handles pagination - # for us for img in image_service.detail(context): if img['container_format'] != 'ami': continue fingerprint = hashlib.sha1(str(img['id'])).hexdigest() - for base_file, image_small in self._find_base_file(base_dir, - fingerprint): - self._handle_base_image(img, base_file, image_small) - - # Elements remaining in unexplained_images are not currently in - # glance. That doesn't mean that they're really not in use though - # (consider images which have been removed from glance but are still - # used by instances). So, we check the backing file for any running - # instances as well. - if self.unexplained_images: - inuse_backing_images = self._list_backing_images() - if inuse_backing_images: - for backing_path in inuse_backing_images: - self.active_base_files.append(backing_path) + for result in self._find_base_file(base_dir, fingerprint): + base_file, image_small, image_resized = res + self._handle_base_image(img, base_file) + + if not image_small and not image_resized: + self.originals.append(base_file) + + # Elements remaining in unexplained_images are not directly from + # glance. That might mean they're from a image that was removed from + # glance, but it might also mean that they're a resized image. + inuse_backing_images = self._list_backing_images() + for backing_path in inuse_backing_images: + self.active_base_files.append(backing_path) # Anything left is an unknown base image for img in self.unexplained_images: -- cgit