Move imagecache code from nova.virt.libvirt.utils

The imagecache related code in libvirt.utils uses the base_dir_name
config option.

If we move the imagecache utils into imagecache, then we can move
the base_dir_name option into that module since there are no other
users of it.

Having all the imagecache code in the imagecache module seems like
a sensible thing to do anyway.

blueprint: scope-config-opts
Change-Id: I1e154aa4de1628d40964207bdcbe5b3b55076442

5 files changed, 150 insertions, 163 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index a0835d107..6fcdb222a 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -28,7 +28,6 @@ terminating it.
 **Related Flags**
 
 :instances_path:  Where instances are kept on disk
-:base_dir_name:  Where cached images are stored under instances_path
 
 """
 
@@ -78,11 +77,6 @@ from nova import volume
 
 
 compute_opts = [
-    cfg.StrOpt('base_dir_name',
-               default='_base',
-               help="Where cached images are stored under $instances_path."
-                    "This is NOT the full path - just a folder name."
-                    "For per-compute-host cached images, set to _base_$my_ip"),
     cfg.StrOpt('console_host',
                default=socket.getfqdn(),
                help='Console proxy host to use to connect '
diff --git a/nova/tests/test_imagecache.py b/nova/tests/test_imagecache.py
index 09570204f..2a927b362 100644
--- a/nova/tests/test_imagecache.py
+++ b/nova/tests/test_imagecache.py
@@ -65,7 +65,7 @@ class ImageCacheManagerTestCase(test.TestCase):
 
             csum_input = '{"sha1": "fdghkfhkgjjksfdgjksjkghsdf"}\n'
             fname = os.path.join(tmpdir, 'aaa')
-            info_fname = virtutils.get_info_filename(fname)
+            info_fname = imagecache.get_info_filename(fname)
             f = open(info_fname, 'w')
             f.write(csum_input)
             f.close()
@@ -91,7 +91,8 @@ class ImageCacheManagerTestCase(test.TestCase):
                                                           timestamped=False)
             self.assertEquals(csum_output, 'fdghkfhkgjjksfdgjksjkghsdf')
             self.assertFalse(os.path.exists(old_fname))
-            self.assertTrue(os.path.exists(virtutils.get_info_filename(fname)))
+            info_fname = imagecache.get_info_filename(fname)
+            self.assertTrue(os.path.exists(info_fname))
 
     def test_list_base_images(self):
         listing = ['00000001',
@@ -352,7 +353,7 @@ class ImageCacheManagerTestCase(test.TestCase):
                     'operating system.')
 
         fname = os.path.join(tmpdir, 'aaa')
-        info_fname = virtutils.get_info_filename(fname)
+        info_fname = imagecache.get_info_filename(fname)
 
         with open(fname, 'w') as f:
             f.write(testdata)
@@ -525,7 +526,7 @@ 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)
+            info_fname = imagecache.get_info_filename(fname)
 
             # Files are initially too new to delete
             self.assertTrue(os.path.exists(fname))
@@ -543,7 +544,7 @@ class ImageCacheManagerTestCase(test.TestCase):
             image_cache_manager = imagecache.ImageCacheManager()
             image_cache_manager.originals = [fname]
             image_cache_manager._remove_base_file(fname)
-            info_fname = virtutils.get_info_filename(fname)
+            info_fname = imagecache.get_info_filename(fname)
 
             # Files are initially too new to delete
             self.assertTrue(os.path.exists(fname))
@@ -873,12 +874,13 @@ class ImageCacheManagerTestCase(test.TestCase):
                                                 '%(image)s.info'))
         base_filename = os.path.join(CONF.instances_path, '_base', hashed)
 
-        self.assertFalse(virtutils.is_valid_info_file('banana'))
-        self.assertFalse(virtutils.is_valid_info_file(
+        is_valid_info_file = imagecache.is_valid_info_file
+        self.assertFalse(is_valid_info_file('banana'))
+        self.assertFalse(is_valid_info_file(
                 os.path.join(CONF.instances_path, '_base', '00000001')))
-        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'))
+        self.assertFalse(is_valid_info_file(base_filename))
+        self.assertFalse(is_valid_info_file(base_filename + '.sha1'))
+        self.assertTrue(is_valid_info_file(base_filename + '.info'))
 
     def test_configured_checksum_path(self):
         with utils.tempdir() as tmpdir:
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 768406e9c..c19f72dae 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -73,6 +73,7 @@ CONF = cfg.CONF
 CONF.import_opt('compute_manager', 'nova.config')
 CONF.import_opt('host', 'nova.config')
 CONF.import_opt('my_ip', 'nova.config')
+CONF.import_opt('base_dir_name', 'nova.virt.libvirt.imagecache')
 LOG = logging.getLogger(__name__)
 
 _fake_network_info = fake_network.fake_get_instance_nw_info
diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py
index 753fd966c..db5e6e058 100644
--- a/nova/virt/libvirt/imagecache.py
+++ b/nova/virt/libvirt/imagecache.py
@@ -23,6 +23,7 @@ http://wiki.openstack.org/nova-image-cache-management.
 """
 
 import hashlib
+import json
 import os
 import re
 import time
@@ -30,6 +31,8 @@ import time
 from nova.compute import task_states
 from nova.compute import vm_states
 from nova.openstack.common import cfg
+from nova.openstack.common import fileutils
+from nova.openstack.common import jsonutils
 from nova.openstack.common import lockutils
 from nova.openstack.common import log as logging
 from nova import utils
@@ -39,6 +42,15 @@ from nova.virt.libvirt import utils as virtutils
 LOG = logging.getLogger(__name__)
 
 imagecache_opts = [
+    cfg.StrOpt('base_dir_name',
+               default='_base',
+               help="Where cached images are stored under $instances_path."
+                    "This is NOT the full path - just a folder name."
+                    "For per-compute-host cached images, set to _base_$my_ip"),
+    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'),
     cfg.BoolOpt('remove_unused_base_images',
                 default=True,
                 help='Should unused base images be removed?'),
@@ -62,7 +74,124 @@ CONF = cfg.CONF
 CONF.register_opts(imagecache_opts)
 CONF.import_opt('host', 'nova.config')
 CONF.import_opt('instances_path', 'nova.compute.manager')
-CONF.import_opt('base_dir_name', 'nova.compute.manager')
+
+
+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 (CONF.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 = (CONF.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_possible_json(serialized, info_file):
+    try:
+        d = jsonutils.loads(serialized)
+
+    except ValueError, e:
+        LOG.error(_('Error reading image info file %(filename)s: '
+                    '%(error)s'),
+                  {'filename': info_file,
+                   'error': e})
+        d = {}
+
+    return d
+
+
+def read_stored_info(target, field=None, timestamped=False):
+    """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(target)
+    if not os.path.exists(info_file):
+        # NOTE(mikal): Special case to handle essex checksums being converted.
+        # There is an assumption here that target is a base image filename.
+        old_filename = target + '.sha1'
+        if field == 'sha1' and os.path.exists(old_filename):
+            hash_file = open(old_filename)
+            hash_value = hash_file.read()
+            hash_file.close()
+
+            write_stored_info(target, field=field, value=hash_value)
+            os.remove(old_filename)
+            d = {field: hash_value}
+
+        else:
+            d = {}
+
+    else:
+        lock_name = 'info-%s' % os.path.split(target)[-1]
+        lock_path = os.path.join(CONF.instances_path, 'locks')
+
+        @lockutils.synchronized(lock_name, 'nova-', external=True,
+                                lock_path=lock_path)
+        def read_file(info_file):
+            LOG.debug(_('Reading image info file: %s'), info_file)
+            with open(info_file, 'r') as f:
+                return f.read().rstrip()
+
+        serialized = read_file(info_file)
+        d = _read_possible_json(serialized, info_file)
+
+    if field:
+        if timestamped:
+            return (d.get(field, None), d.get('%s-timestamp' % field, None))
+        else:
+            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)
+    LOG.info(_('Writing stored info to %s'), info_file)
+    fileutils.ensure_tree(os.path.dirname(info_file))
+
+    lock_name = 'info-%s' % os.path.split(target)[-1]
+    lock_path = os.path.join(CONF.instances_path, 'locks')
+
+    @lockutils.synchronized(lock_name, 'nova-', external=True,
+                            lock_path=lock_path)
+    def write_file(info_file, field, value):
+        d = {}
+
+        if os.path.exists(info_file):
+            with open(info_file, 'r') as f:
+                d = _read_possible_json(f.read(), info_file)
+
+        d[field] = value
+        d['%s-timestamp' % field] = time.time()
+
+        with open(info_file, 'w') as f:
+            f.write(json.dumps(d))
+
+    write_file(info_file, field, value)
 
 
 def read_stored_checksum(target, timestamped=True):
@@ -70,8 +199,7 @@ def read_stored_checksum(target, timestamped=True):
 
     Returns the checksum (as hex) or None.
     """
-    return virtutils.read_stored_info(target, field='sha1',
-                                      timestamped=timestamped)
+    return read_stored_info(target, field='sha1', timestamped=timestamped)
 
 
 def write_stored_checksum(target):
@@ -79,7 +207,7 @@ def write_stored_checksum(target):
 
     with open(target, 'r') as img_file:
         checksum = utils.hash_file(img_file)
-    virtutils.write_stored_info(target, field='sha1', value=checksum)
+    write_stored_info(target, field='sha1', value=checksum)
 
 
 class ImageCacheManager(object):
@@ -125,8 +253,7 @@ class ImageCacheManager(object):
 
             elif (len(ent) > digest_size + 2 and
                   ent[digest_size] == '_' and
-                  not virtutils.is_valid_info_file(os.path.join(base_dir,
-                                                                ent))):
+                  not is_valid_info_file(os.path.join(base_dir, ent))):
                 self._store_image(base_dir, ent, original=False)
 
     def _list_running_instances(self, context, all_instances):
@@ -251,8 +378,8 @@ class ImageCacheManager(object):
                 # NOTE(mikal): If there is no timestamp, then the checksum was
                 # performed by a previous version of the code.
                 if not stored_timestamp:
-                    virtutils.write_stored_info(base_file, field='sha1',
-                                                value=stored_checksum)
+                    write_stored_info(base_file, field='sha1',
+                                      value=stored_checksum)
 
                 with open(base_file, 'r') as f:
                     current_checksum = utils.hash_file(f)
@@ -310,7 +437,7 @@ class ImageCacheManager(object):
             LOG.info(_('Removing base file: %s'), base_file)
             try:
                 os.remove(base_file)
-                signature = virtutils.get_info_filename(base_file)
+                signature = 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 d4ce445bc..523d4def3 100644
--- a/nova/virt/libvirt/utils.py
+++ b/nova/virt/libvirt/utils.py
@@ -19,38 +19,19 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import hashlib
-import json
 import os
-import re
 import time
 
 from lxml import etree
 
 from nova import exception
 from nova.openstack.common import cfg
-from nova.openstack.common import fileutils
-from nova.openstack.common import jsonutils
-from nova.openstack.common import lockutils
 from nova.openstack.common import log as logging
 from nova import utils
 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')
-    ]
-
 CONF = cfg.CONF
-CONF.register_opts(util_opts)
-CONF.import_opt('instances_path', 'nova.compute.manager')
-CONF.import_opt('base_dir_name', 'nova.compute.manager')
+LOG = logging.getLogger(__name__)
 
 
 def execute(*args, **kwargs):
@@ -441,121 +422,3 @@ 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 (CONF.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 = (CONF.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_possible_json(serialized, info_file):
-    try:
-        d = jsonutils.loads(serialized)
-
-    except ValueError, e:
-        LOG.error(_('Error reading image info file %(filename)s: '
-                    '%(error)s'),
-                  {'filename': info_file,
-                   'error': e})
-        d = {}
-
-    return d
-
-
-def read_stored_info(target, field=None, timestamped=False):
-    """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(target)
-    if not os.path.exists(info_file):
-        # NOTE(mikal): Special case to handle essex checksums being converted.
-        # There is an assumption here that target is a base image filename.
-        old_filename = target + '.sha1'
-        if field == 'sha1' and os.path.exists(old_filename):
-            hash_file = open(old_filename)
-            hash_value = hash_file.read()
-            hash_file.close()
-
-            write_stored_info(target, field=field, value=hash_value)
-            os.remove(old_filename)
-            d = {field: hash_value}
-
-        else:
-            d = {}
-
-    else:
-        lock_name = 'info-%s' % os.path.split(target)[-1]
-        lock_path = os.path.join(CONF.instances_path, 'locks')
-
-        @lockutils.synchronized(lock_name, 'nova-', external=True,
-                                lock_path=lock_path)
-        def read_file(info_file):
-            LOG.debug(_('Reading image info file: %s'), info_file)
-            with open(info_file, 'r') as f:
-                return f.read().rstrip()
-
-        serialized = read_file(info_file)
-        d = _read_possible_json(serialized, info_file)
-
-    if field:
-        if timestamped:
-            return (d.get(field, None), d.get('%s-timestamp' % field, None))
-        else:
-            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)
-    LOG.info(_('Writing stored info to %s'), info_file)
-    fileutils.ensure_tree(os.path.dirname(info_file))
-
-    lock_name = 'info-%s' % os.path.split(target)[-1]
-    lock_path = os.path.join(CONF.instances_path, 'locks')
-
-    @lockutils.synchronized(lock_name, 'nova-', external=True,
-                            lock_path=lock_path)
-    def write_file(info_file, field, value):
-        d = {}
-
-        if os.path.exists(info_file):
-            with open(info_file, 'r') as f:
-                d = _read_possible_json(f.read(), info_file)
-
-        d[field] = value
-        d['%s-timestamp' % field] = time.time()
-
-        with open(info_file, 'w') as f:
-            f.write(json.dumps(d))
-
-    write_file(info_file, field, value)