Merge "Handle refactoring of libvirt image caching."

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 1b8e1f531..8a1691c89 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: