From 33d7f56cc424071a90f78ebd21913ce2bc9e6c0d Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Wed, 21 Nov 2012 22:44:32 +0000 Subject: 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 --- nova/compute/manager.py | 6 -- nova/tests/test_imagecache.py | 22 +++--- nova/tests/test_libvirt.py | 1 + nova/virt/libvirt/imagecache.py | 145 +++++++++++++++++++++++++++++++++++++--- nova/virt/libvirt/utils.py | 139 +------------------------------------- 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) -- cgit