Move image checksums into a generic file.

This will allow us to store other information about an image on a
single compute node. This is required for prefetching to be
implemented.

Change-Id: Ic65b04cc1ef2101a1ac588595b943b813abdd50d

3 files changed, 262 insertions, 110 deletions

diff --git a/nova/tests/test_imagecache.py b/nova/tests/test_imagecache.py
index 6de3722eb..410f4b1a8 100644
--- a/nova/tests/test_imagecache.py
+++ b/nova/tests/test_imagecache.py
@@ -53,7 +53,6 @@ class ImageCacheManagerTestCase(test.TestCase):
 
     def test_read_stored_checksum_missing(self):
         self.stubs.Set(os.path, 'exists', lambda x: False)
-
         csum = imagecache.read_stored_checksum('/tmp/foo')
         self.assertEquals(csum, None)
 
@@ -61,30 +60,31 @@ class ImageCacheManagerTestCase(test.TestCase):
         with utils.tempdir() as tmpdir:
             self.flags(instances_path=tmpdir)
             self.flags(image_info_filename_pattern=('$instances_path/'
-                                                    '%(image)s.sha1'))
+                                                    '%(image)s.info'))
 
+            csum_input = '{"sha1": "fdghkfhkgjjksfdgjksjkghsdf"}\n'
             fname = os.path.join(tmpdir, 'aaa')
-
-            csum_input = 'fdghkfhkgjjksfdgjksjkghsdf'
-            f = open('%s.sha1' % fname, 'w')
-            f.write('%s\n' % csum_input)
+            info_fname = virtutils.get_info_filename(fname)
+            f = open(info_fname, 'w')
+            f.write(csum_input)
             f.close()
 
             csum_output = imagecache.read_stored_checksum(fname)
-
-            self.assertEquals(csum_input, csum_output)
+            self.assertEquals(csum_input.rstrip(),
+                              '{"sha1": "%s"}' % csum_output)
 
     def test_list_base_images(self):
         listing = ['00000001',
                    'ephemeral_0_20_None',
-                   'e97222e91fc4241f49a7f520d1dcf446751129b3_sm',
-                   'e09c675c2d1cfac32dae3c2d83689c8c94bc693b_sm',
-                   'e97222e91fc4241f49a7f520d1dcf446751129b3',
-                   '17d1b00b81642842e514494a78e804e9a511637c',
-                   '17d1b00b81642842e514494a78e804e9a511637c_5368709120',
-                   '17d1b00b81642842e514494a78e804e9a511637c_5368709120.sha1',
-                   '17d1b00b81642842e514494a78e804e9a511637c_10737418240',
-                   '00000004']
+                   '17d1b00b81642842e514494a78e804e9a511637c_5368709120.info',
+                    '00000004']
+        images = ['e97222e91fc4241f49a7f520d1dcf446751129b3_sm',
+                  'e09c675c2d1cfac32dae3c2d83689c8c94bc693b_sm',
+                  'e97222e91fc4241f49a7f520d1dcf446751129b3',
+                  '17d1b00b81642842e514494a78e804e9a511637c',
+                  '17d1b00b81642842e514494a78e804e9a511637c_5368709120',
+                  '17d1b00b81642842e514494a78e804e9a511637c_10737418240']
+        listing.extend(images)
 
         self.stubs.Set(os, 'listdir', lambda x: listing)
         self.stubs.Set(os.path, 'isfile', lambda x: True)
@@ -95,7 +95,13 @@ 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), 6)
+        sanitized = []
+        for ent in image_cache_manager.unexplained_images:
+            sanitized.append(ent.replace(base_dir + '/', ''))
+
+        sanitized = sanitized.sort()
+        images = images.sort()
+        self.assertEquals(sanitized, images)
 
         expected = os.path.join(base_dir,
                                 'e97222e91fc4241f49a7f520d1dcf446751129b3')
@@ -296,9 +302,20 @@ class ImageCacheManagerTestCase(test.TestCase):
         finally:
             mylog.logger.removeHandler(handler)
 
-    def test_verify_checksum(self):
+    def _make_checksum(self, tmpdir):
         testdata = ('OpenStack Software delivers a massively scalable cloud '
                     'operating system.')
+
+        fname = os.path.join(tmpdir, 'aaa')
+        info_fname = virtutils.get_info_filename(fname)
+
+        f = open(fname, 'w')
+        f.write(testdata)
+        f.close()
+
+        return fname, info_fname, testdata
+
+    def test_verify_checksum(self):
         img = {'container_format': 'ami', 'id': '42'}
 
         self.flags(checksum_base_images=True)
@@ -307,45 +324,117 @@ class ImageCacheManagerTestCase(test.TestCase):
             with utils.tempdir() as tmpdir:
                 self.flags(instances_path=tmpdir)
                 self.flags(image_info_filename_pattern=('$instances_path/'
-                                                        '%(image)s.sha1'))
-
-                fname = os.path.join(tmpdir, 'aaa')
-
-                f = open(fname, 'w')
-                f.write(testdata)
-                f.close()
+                                                        '%(image)s.info'))
+                fname, info_fname, testdata = self._make_checksum(tmpdir)
 
                 # Checksum is valid
-                f = open('%s.sha1' % fname, 'w')
+                f = open(info_fname, 'w')
                 csum = hashlib.sha1()
                 csum.update(testdata)
-                f.write(csum.hexdigest())
+                f.write('{"sha1": "%s"}\n' % csum.hexdigest())
                 f.close()
 
                 image_cache_manager = imagecache.ImageCacheManager()
                 res = image_cache_manager._verify_checksum(img, fname)
                 self.assertTrue(res)
 
-                # Checksum is invalid
-                f = open('%s.sha1' % fname, 'w')
+    def test_verify_checksum_invalid_json(self):
+        img = {'container_format': 'ami', 'id': '42'}
+
+        self.flags(checksum_base_images=True)
+
+        with self._intercept_log_messages() as stream:
+            with utils.tempdir() as tmpdir:
+                self.flags(instances_path=tmpdir)
+                self.flags(image_info_filename_pattern=('$instances_path/'
+                                                        '%(image)s.info'))
+
+                fname, info_fname, testdata = self._make_checksum(tmpdir)
+
+                # Checksum is invalid, and not json
+                f = open(info_fname, 'w')
                 f.write('banana')
                 f.close()
 
                 image_cache_manager = imagecache.ImageCacheManager()
+                res = image_cache_manager._verify_checksum(
+                    img, fname, create_if_missing=False)
+                self.assertFalse(res)
+                log = stream.getvalue()
+
+                # NOTE(mikal): this is a skip not a fail because the file is
+                # present, but is not in valid json format and therefore is
+                # skipped.
+                self.assertNotEqual(log.find('image verification skipped'), -1)
+
+    def test_verify_checksum_invalid_repaired(self):
+        img = {'container_format': 'ami', 'id': '42'}
+
+        self.flags(checksum_base_images=True)
+
+        with utils.tempdir() as tmpdir:
+            self.flags(instances_path=tmpdir)
+            self.flags(image_info_filename_pattern=('$instances_path/'
+                                                    '%(image)s.info'))
+
+            fname, info_fname, testdata = self._make_checksum(tmpdir)
+
+            # Checksum is invalid, and not json
+            f = open(info_fname, 'w')
+            f.write('banana')
+            f.close()
+
+            image_cache_manager = imagecache.ImageCacheManager()
+            res = image_cache_manager._verify_checksum(
+                img, fname, create_if_missing=True)
+            self.assertTrue(res == None)
+
+    def test_verify_checksum_invalid(self):
+        img = {'container_format': 'ami', 'id': '42'}
+
+        self.flags(checksum_base_images=True)
+
+        with self._intercept_log_messages() as stream:
+            with utils.tempdir() as tmpdir:
+                self.flags(instances_path=tmpdir)
+                self.flags(image_info_filename_pattern=('$instances_path/'
+                                                        '%(image)s.info'))
+
+                fname, info_fname, testdata = self._make_checksum(tmpdir)
+
+                # Checksum is invalid, but is in valid json
+                f = open(info_fname, 'w')
+                f.write('{"sha1": "banana"}')
+                f.close()
+
+                image_cache_manager = imagecache.ImageCacheManager()
                 res = image_cache_manager._verify_checksum(img, fname)
                 self.assertFalse(res)
                 log = stream.getvalue()
+
                 self.assertNotEqual(log.find('image verification failed'), -1)
 
+    def test_verify_checksum_file_missing(self):
+        img = {'container_format': 'ami', 'id': '42'}
+
+        self.flags(checksum_base_images=True)
+
+        with self._intercept_log_messages() as stream:
+            with utils.tempdir() as tmpdir:
+                self.flags(instances_path=tmpdir)
+                self.flags(image_info_filename_pattern=('$instances_path/'
+                                                        '%(image)s.info'))
+
+                fname, info_fname, testdata = self._make_checksum(tmpdir)
+
                 # Checksum file missing
-                os.remove('%s.sha1' % fname)
                 image_cache_manager = imagecache.ImageCacheManager()
                 res = image_cache_manager._verify_checksum(img, fname)
                 self.assertEquals(res, None)
 
                 # Checksum requests for a file with no checksum now have the
                 # side effect of creating the checksum
-                self.assertTrue(os.path.exists('%s.sha1' % fname))
+                self.assertTrue(os.path.exists(info_fname))
 
     @contextlib.contextmanager
     def _make_base_file(self, checksum=True):
@@ -354,8 +443,7 @@ class ImageCacheManagerTestCase(test.TestCase):
         with utils.tempdir() as tmpdir:
             self.flags(instances_path=tmpdir)
             self.flags(image_info_filename_pattern=('$instances_path/'
-                                                    '%(image)s.sha1'))
-
+                                                    '%(image)s.info'))
             fname = os.path.join(tmpdir, 'aaa')
 
             base_file = open(fname, 'w')
@@ -364,9 +452,7 @@ class ImageCacheManagerTestCase(test.TestCase):
             base_file = open(fname, 'r')
 
             if checksum:
-                checksum_file = open('%s.sha1' % fname, 'w')
-                checksum_file.write(utils.hash_file(base_file))
-                checksum_file.close()
+                imagecache.write_stored_checksum(fname)
 
             base_file.close()
             yield fname
@@ -375,46 +461,52 @@ class ImageCacheManagerTestCase(test.TestCase):
         with self._make_base_file() as fname:
             image_cache_manager = imagecache.ImageCacheManager()
             image_cache_manager._remove_base_file(fname)
+            info_fname = virtutils.get_info_filename(fname)
 
             # Files are initially too new to delete
             self.assertTrue(os.path.exists(fname))
-            self.assertTrue(os.path.exists('%s.sha1' % fname))
+            self.assertTrue(os.path.exists(info_fname))
 
             # Old files get cleaned up though
             os.utime(fname, (-1, time.time() - 3601))
             image_cache_manager._remove_base_file(fname)
 
             self.assertFalse(os.path.exists(fname))
-            self.assertFalse(os.path.exists('%s.sha1' % fname))
+            self.assertFalse(os.path.exists(info_fname))
 
     def test_remove_base_file_original(self):
         with self._make_base_file() as fname:
             image_cache_manager = imagecache.ImageCacheManager()
             image_cache_manager.originals = [fname]
             image_cache_manager._remove_base_file(fname)
+            info_fname = virtutils.get_info_filename(fname)
 
             # Files are initially too new to delete
             self.assertTrue(os.path.exists(fname))
-            self.assertTrue(os.path.exists('%s.sha1' % fname))
+            self.assertTrue(os.path.exists(info_fname))
 
             # This file should stay longer than a resized image
             os.utime(fname, (-1, time.time() - 3601))
             image_cache_manager._remove_base_file(fname)
 
             self.assertTrue(os.path.exists(fname))
-            self.assertTrue(os.path.exists('%s.sha1' % fname))
+            self.assertTrue(os.path.exists(info_fname))
 
             # Originals don't stay forever though
             os.utime(fname, (-1, time.time() - 3600 * 25))
             image_cache_manager._remove_base_file(fname)
 
             self.assertFalse(os.path.exists(fname))
-            self.assertFalse(os.path.exists('%s.sha1' % fname))
+            self.assertFalse(os.path.exists(info_fname))
 
     def test_remove_base_file_dne(self):
         # This test is solely to execute the "does not exist" code path. We
         # don't expect the method being tested to do anything in this case.
         with utils.tempdir() as tmpdir:
+            self.flags(instances_path=tmpdir)
+            self.flags(image_info_filename_pattern=('$instances_path/'
+                                                    '%(image)s.info'))
+
             fname = os.path.join(tmpdir, 'aaa')
             image_cache_manager = imagecache.ImageCacheManager()
             image_cache_manager._remove_base_file(fname)
@@ -422,6 +514,10 @@ class ImageCacheManagerTestCase(test.TestCase):
     def test_remove_base_file_oserror(self):
         with self._intercept_log_messages() as stream:
             with utils.tempdir() as tmpdir:
+                self.flags(instances_path=tmpdir)
+                self.flags(image_info_filename_pattern=('$instances_path/'
+                                                        '%(image)s.info'))
+
                 fname = os.path.join(tmpdir, 'aaa')
 
                 os.mkdir(fname)
@@ -500,6 +596,10 @@ class ImageCacheManagerTestCase(test.TestCase):
         img = '123'
 
         with utils.tempdir() as tmpdir:
+            self.flags(instances_path=tmpdir)
+            self.flags(image_info_filename_pattern=('$instances_path/'
+                                                    '%(image)s.info'))
+
             fname = os.path.join(tmpdir, 'aaa')
 
             image_cache_manager = imagecache.ImageCacheManager()
@@ -565,13 +665,13 @@ class ImageCacheManagerTestCase(test.TestCase):
                         '/instance_path/instance-1/disk',
                         '/instance_path/instance-2/disk',
                         '/instance_path/instance-3/disk',
-                        '/instance_path/_base/%s.sha1' % hashed_42]:
+                        '/instance_path/_base/%s.info' % hashed_42]:
                 return True
 
             for p in base_file_list:
                 if path == fq_path(p):
                     return True
-                if path == fq_path(p) + '.sha1':
+                if path == fq_path(p) + '.info':
                     return False
 
             if path in ['/instance_path/_base/%s_sm' % hashed_1,
@@ -702,13 +802,12 @@ class ImageCacheManagerTestCase(test.TestCase):
                                                 '%(image)s.info'))
         base_filename = os.path.join(FLAGS.instances_path, '_base', hashed)
 
-        self.assertFalse(imagecache.is_valid_info_file('banana'))
-        self.assertFalse(imagecache.is_valid_info_file(
+        self.assertFalse(virtutils.is_valid_info_file('banana'))
+        self.assertFalse(virtutils.is_valid_info_file(
                 os.path.join(FLAGS.instances_path, '_base', '00000001')))
-        self.assertFalse(imagecache.is_valid_info_file(base_filename))
-        self.assertFalse(imagecache.is_valid_info_file(base_filename +
-                                                       '.sha1'))
-        self.assertTrue(imagecache.is_valid_info_file(base_filename + '.info'))
+        self.assertFalse(virtutils.is_valid_info_file(base_filename))
+        self.assertFalse(virtutils.is_valid_info_file(base_filename + '.sha1'))
+        self.assertTrue(virtutils.is_valid_info_file(base_filename + '.info'))
 
     def test_configured_checksum_path(self):
         with utils.tempdir() as tmpdir:
@@ -742,8 +841,8 @@ class ImageCacheManagerTestCase(test.TestCase):
             touch(base_filename)
             touch(base_filename + '.info')
             os.utime(base_filename + '.info', (old, old))
-            touch(base_filename + '.sha1')
-            os.utime(base_filename + '.sha1', (old, old))
+            touch(base_filename + '.info')
+            os.utime(base_filename + '.info', (old, old))
 
             image_cache_manager = imagecache.ImageCacheManager()
             image_cache_manager.verify_base_images(None)
diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py
index 5c8eba66f..3a92b6253 100644
--- a/nova/virt/libvirt/imagecache.py
+++ b/nova/virt/libvirt/imagecache.py
@@ -55,10 +55,6 @@ imagecache_opts = [
     cfg.BoolOpt('checksum_base_images',
                 default=False,
                 help='Write a checksum for files in _base to disk'),
-    cfg.StrOpt('image_info_filename_pattern',
-               default='$instances_path/$base_dir_name/%(image)s.sha1',
-               help='Allows image information files to be stored in '
-                    'non-standard locations'),
     ]
 
 flags.DECLARE('instances_path', 'nova.compute.manager')
@@ -67,65 +63,23 @@ FLAGS = flags.FLAGS
 FLAGS.register_opts(imagecache_opts)
 
 
-def get_info_filename(base_path):
-    """Construct a filename for storing addtional information about a base
-    image.
-
-    Returns a filename.
-    """
-
-    base_file = os.path.basename(base_path)
-    return (FLAGS.image_info_filename_pattern
-            % {'image': base_file})
-
-
-def is_valid_info_file(path):
-    """Test if a given path matches the pattern for info files."""
-
-    digest_size = hashlib.sha1().digestsize * 2
-    regexp = (FLAGS.image_info_filename_pattern
-              % {'instances_path': FLAGS.instances_path,
-                 'image': ('([0-9a-f]{%(digest_size)d}|'
-                           '[0-9a-f]{%(digest_size)d}_sm|'
-                           '[0-9a-f]{%(digest_size)d}_[0-9]+)'
-                           % {'digest_size': digest_size})})
-    m = re.match(regexp, path)
-    if m:
-        return True
-    return False
-
-
-def read_stored_checksum(base_path):
-    """Read the checksum which was created at image fetch time.
+def read_stored_checksum(target):
+    """Read the checksum.
 
     Returns the checksum (as hex) or None.
     """
-
-    checksum_file = get_info_filename(base_path)
-    if not os.path.exists(checksum_file):
-        return None
-
-    f = open(checksum_file, 'r')
-    stored_checksum = f.read().rstrip()
-    f.close()
-    return stored_checksum
+    return virtutils.read_stored_info(target, field='sha1')
 
 
 def write_stored_checksum(target):
     """Write a checksum to disk for a file in _base."""
 
-    checksum_filename = get_info_filename(target)
-    if os.path.exists(target) and not os.path.exists(checksum_filename):
-        # NOTE(mikal): Create the checksum file first to exclude possible
-        # overlap in checksum operations. An empty checksum file is ignored if
-        # encountered during verification.
-        sum_file = open(checksum_filename, 'w')
+    if not read_stored_checksum(target):
         img_file = open(target, 'r')
         checksum = utils.hash_file(img_file)
         img_file.close()
 
-        sum_file.write(checksum)
-        sum_file.close()
+        virtutils.write_stored_info(target, field='sha1', value=checksum)
 
 
 class ImageCacheManager(object):
@@ -170,7 +124,8 @@ class ImageCacheManager(object):
 
             elif (len(ent) > digest_size + 2 and
                   ent[digest_size] == '_' and
-                  not is_valid_info_file(os.path.join(base_dir, ent))):
+                  not virtutils.is_valid_info_file(os.path.join(base_dir,
+                                                                ent))):
                 self._store_image(base_dir, ent, original=False)
 
     def _list_running_instances(self, context):
@@ -254,7 +209,7 @@ class ImageCacheManager(object):
             if m:
                 yield img, False, True
 
-    def _verify_checksum(self, img_id, base_file):
+    def _verify_checksum(self, img_id, base_file, create_if_missing=True):
         """Compare the checksum stored on disk with the current file.
 
         Note that if the checksum fails to verify this is logged, but no actual
@@ -279,15 +234,15 @@ class ImageCacheManager(object):
                 return True
 
         else:
-            LOG.debug(_('%(id)s (%(base_file)s): image verification skipped, '
-                        'no hash stored'),
-                      {'id': img_id,
-                       'base_file': base_file})
+            LOG.info(_('%(id)s (%(base_file)s): image verification skipped, '
+                       'no hash stored'),
+                     {'id': img_id,
+                      'base_file': base_file})
 
             # NOTE(mikal): If the checksum file is missing, then we should
             # create one. We don't create checksums when we download images
             # from glance because that would delay VM startup.
-            if FLAGS.checksum_base_images:
+            if FLAGS.checksum_base_images and create_if_missing:
                 write_stored_checksum(base_file)
 
             return None
@@ -316,7 +271,7 @@ class ImageCacheManager(object):
             LOG.info(_('Removing base file: %s'), base_file)
             try:
                 os.remove(base_file)
-                signature = get_info_filename(base_file)
+                signature = virtutils.get_info_filename(base_file)
                 if os.path.exists(signature):
                     os.remove(signature)
             except OSError, e:
diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py
index db7e11b26..f4c6d20a5 100644
--- a/nova/virt/libvirt/utils.py
+++ b/nova/virt/libvirt/utils.py
@@ -19,16 +19,34 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import hashlib
+import json
 import os
 import random
+import re
 
 from nova import exception
 from nova import flags
+from nova import log as logging
 from nova import utils
+from nova.openstack.common import cfg
 from nova.virt import images
 
 
+LOG = logging.getLogger(__name__)
+
+
+util_opts = [
+    cfg.StrOpt('image_info_filename_pattern',
+               default='$instances_path/$base_dir_name/%(image)s.info',
+               help='Allows image information files to be stored in '
+                    'non-standard locations')
+    ]
+
+flags.DECLARE('instances_path', 'nova.compute.manager')
+flags.DECLARE('base_dir_name', 'nova.compute.manager')
 FLAGS = flags.FLAGS
+FLAGS.register_opts(util_opts)
 
 
 def execute(*args, **kwargs):
@@ -286,3 +304,83 @@ def get_fs_info(path):
 def fetch_image(context, target, image_id, user_id, project_id):
     """Grab image"""
     images.fetch_to_raw(context, image_id, target, user_id, project_id)
+
+
+def get_info_filename(base_path):
+    """Construct a filename for storing addtional information about a base
+    image.
+
+    Returns a filename.
+    """
+
+    base_file = os.path.basename(base_path)
+    return (FLAGS.image_info_filename_pattern
+            % {'image': base_file})
+
+
+def is_valid_info_file(path):
+    """Test if a given path matches the pattern for info files."""
+
+    digest_size = hashlib.sha1().digestsize * 2
+    regexp = (FLAGS.image_info_filename_pattern
+              % {'image': ('([0-9a-f]{%(digest_size)d}|'
+                           '[0-9a-f]{%(digest_size)d}_sm|'
+                           '[0-9a-f]{%(digest_size)d}_[0-9]+)'
+                           % {'digest_size': digest_size})})
+    m = re.match(regexp, path)
+    if m:
+        return True
+    return False
+
+
+def read_stored_info(base_path, field=None):
+    """Read information about an image.
+
+    Returns an empty dictionary if there is no info, just the field value if
+    a field is requested, or the entire dictionary otherwise.
+    """
+
+    info_file = get_info_filename(base_path)
+    if not os.path.exists(info_file):
+        d = {}
+
+    else:
+        LOG.info(_('Reading image info file: %s'), info_file)
+        f = open(info_file, 'r')
+        serialized = f.read().rstrip()
+        f.close()
+        LOG.info(_('Read: %s'), serialized)
+
+        try:
+            d = json.loads(serialized)
+
+        except ValueError, e:
+            LOG.error(_('Error reading image info file %(filename)s: '
+                        '%(error)s'),
+                      {'filename': info_file,
+                       'error': e})
+            d = {}
+
+    if field:
+        return d.get(field, None)
+    return d
+
+
+def write_stored_info(target, field=None, value=None):
+    """Write information about an image."""
+
+    if not field:
+        return
+
+    info_file = get_info_filename(target)
+    ensure_tree(os.path.dirname(info_file))
+
+    d = read_stored_info(info_file)
+    d[field] = value
+    serialized = json.dumps(d)
+
+    LOG.info(_('Writing image info file: %s'), info_file)
+    LOG.info(_('Wrote: %s'), serialized)
+    f = open(info_file, 'w')
+    f.write(serialized)
+    f.close()