diff options
| -rw-r--r-- | nova/tests/fake_libvirt_utils.py | 6 | ||||
| -rw-r--r-- | nova/tests/test_imagebackend.py | 34 | ||||
| -rw-r--r-- | nova/tests/test_imagecache.py | 13 | ||||
| -rw-r--r-- | nova/tests/test_libvirt.py | 30 | ||||
| -rw-r--r-- | nova/tests/test_virt_drivers.py | 5 | ||||
| -rwxr-xr-x | nova/virt/libvirt/driver.py | 3 | ||||
| -rw-r--r-- | nova/virt/libvirt/imagecache.py | 7 | ||||
| -rwxr-xr-x | nova/virt/libvirt/utils.py | 16 |
8 files changed, 77 insertions, 37 deletions
diff --git a/nova/tests/fake_libvirt_utils.py b/nova/tests/fake_libvirt_utils.py index 285a4b7e3..9dc1db76c 100644 --- a/nova/tests/fake_libvirt_utils.py +++ b/nova/tests/fake_libvirt_utils.py @@ -18,6 +18,7 @@ import os import StringIO from nova.openstack.common import cfg +from nova.virt.libvirt import utils as libvirt_utils CONF = cfg.CONF @@ -141,9 +142,8 @@ def fetch_image(context, target, image_id, user_id, project_id): pass -def get_instance_path(instance): - # TODO(mikal): we should really just call the real one here - return os.path.join(CONF.instances_path, instance['name']) +def get_instance_path(instance, forceold=False): + return libvirt_utils.get_instance_path(instance, forceold=forceold) def pick_disk_driver_name(is_block_dev=False): diff --git a/nova/tests/test_imagebackend.py b/nova/tests/test_imagebackend.py index 87e51819d..bd88fcd60 100644 --- a/nova/tests/test_imagebackend.py +++ b/nova/tests/test_imagebackend.py @@ -19,16 +19,16 @@ import fixtures import os from nova.openstack.common import cfg +from nova.openstack.common import uuidutils from nova import test from nova.tests import fake_libvirt_utils from nova.virt.libvirt import imagebackend -from nova.virt.libvirt import utils as libvirt_utils CONF = cfg.CONF class _ImageTestCase(object): - INSTANCES_PATH = '/fake' + INSTANCES_PATH = '/instances_path' def mock_create_image(self, image): def create_image(fn, base, size, *args, **kwargs): @@ -39,14 +39,19 @@ class _ImageTestCase(object): super(_ImageTestCase, self).setUp() self.flags(disable_process_locking=True, instances_path=self.INSTANCES_PATH) - self.INSTANCE = {'name': 'instance'} + self.INSTANCE = {'name': 'instance', + 'uuid': uuidutils.generate_uuid()} self.NAME = 'fake.vm' self.TEMPLATE = 'template' + self.OLD_STYLE_INSTANCE_PATH = \ + fake_libvirt_utils.get_instance_path(self.INSTANCE, forceold=True) self.PATH = os.path.join( - libvirt_utils.get_instance_path(self.INSTANCE), self.NAME) - self.TEMPLATE_DIR = os.path.join(CONF.instances_path, - '_base') + fake_libvirt_utils.get_instance_path(self.INSTANCE), self.NAME) + + # TODO(mikal): rename template_dir to base_dir and template_path + # to cached_image_path. This will be less confusing. + self.TEMPLATE_DIR = os.path.join(CONF.instances_path, '_base') self.TEMPLATE_PATH = os.path.join(self.TEMPLATE_DIR, 'template') self.useFixture(fixtures.MonkeyPatch( @@ -55,6 +60,8 @@ class _ImageTestCase(object): def test_cache(self): self.mox.StubOutWithMock(os.path, 'exists') + if self.OLD_STYLE_INSTANCE_PATH: + os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False) os.path.exists(self.TEMPLATE_DIR).AndReturn(False) os.path.exists(self.PATH).AndReturn(False) os.path.exists(self.TEMPLATE_PATH).AndReturn(False) @@ -72,6 +79,8 @@ class _ImageTestCase(object): def test_cache_image_exists(self): self.mox.StubOutWithMock(os.path, 'exists') + if self.OLD_STYLE_INSTANCE_PATH: + os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False) os.path.exists(self.TEMPLATE_DIR).AndReturn(True) os.path.exists(self.PATH).AndReturn(True) os.path.exists(self.TEMPLATE_PATH).AndReturn(True) @@ -84,6 +93,8 @@ class _ImageTestCase(object): def test_cache_base_dir_exists(self): self.mox.StubOutWithMock(os.path, 'exists') + if self.OLD_STYLE_INSTANCE_PATH: + os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False) os.path.exists(self.TEMPLATE_DIR).AndReturn(True) os.path.exists(self.PATH).AndReturn(False) os.path.exists(self.TEMPLATE_PATH).AndReturn(False) @@ -100,6 +111,8 @@ class _ImageTestCase(object): def test_cache_template_exists(self): self.mox.StubOutWithMock(os.path, 'exists') + if self.OLD_STYLE_INSTANCE_PATH: + os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False) os.path.exists(self.TEMPLATE_DIR).AndReturn(True) os.path.exists(self.PATH).AndReturn(False) os.path.exists(self.TEMPLATE_PATH).AndReturn(True) @@ -197,6 +210,11 @@ class Qcow2TestCase(_ImageTestCase, test.TestCase): def test_create_image_with_size(self): fn = self.prepare_mocks() fn(target=self.TEMPLATE_PATH) + self.mox.StubOutWithMock(os.path, 'exists') + if self.OLD_STYLE_INSTANCE_PATH: + os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False) + os.path.exists(self.TEMPLATE_PATH).AndReturn(False) + os.path.exists(self.PATH).AndReturn(False) imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH, self.PATH) imagebackend.disk.extend(self.PATH, self.SIZE) @@ -218,6 +236,7 @@ class LvmTestCase(_ImageTestCase, test.TestCase): super(LvmTestCase, self).setUp() self.flags(libvirt_images_volume_group=self.VG) self.LV = '%s_%s' % (self.INSTANCE['name'], self.NAME) + self.OLD_STYLE_INSTANCE_PATH = None self.PATH = os.path.join('/dev', self.VG, self.LV) self.disk = imagebackend.disk @@ -344,7 +363,8 @@ class LvmTestCase(_ImageTestCase, test.TestCase): class BackendTestCase(test.TestCase): - INSTANCE = {'name': 'fake-instance'} + INSTANCE = {'name': 'fake-instance', + 'uuid': uuidutils.generate_uuid()} NAME = 'fake-name.suffix' def get_image(self, use_cow, image_type): diff --git a/nova/tests/test_imagecache.py b/nova/tests/test_imagecache.py index 611519514..389612c64 100644 --- a/nova/tests/test_imagecache.py +++ b/nova/tests/test_imagecache.py @@ -47,10 +47,10 @@ class ImageCacheManagerTestCase(test.TestCase): def setUp(self): super(ImageCacheManagerTestCase, self).setUp() - self.stock_instance_names = {'instance-00000001': '123', - 'instance-00000002': '456', - 'instance-00000003': '789', - 'banana-42-hamster': '444'} + self.stock_instance_names = set(['instance-00000001', + 'instance-00000002', + 'instance-00000003', + 'banana-42-hamster']) def test_read_stored_checksum_missing(self): self.stubs.Set(os.path, 'exists', lambda x: False) @@ -181,6 +181,9 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertTrue(image_cache_manager.used_images['2'] == (1, 1, ['inst-2', 'inst-3'])) + self.assertTrue('inst-1' in image_cache_manager.instance_names) + self.assertTrue('123' in image_cache_manager.instance_names) + self.assertEqual(len(image_cache_manager.image_popularity), 2) self.assertEqual(image_cache_manager.image_popularity['1'], 1) self.assertEqual(image_cache_manager.image_popularity['2'], 2) @@ -200,7 +203,7 @@ class ImageCacheManagerTestCase(test.TestCase): self.assertTrue(image_cache_manager.used_images['1'] == (1, 0, ['inst-1'])) self.assertTrue(image_cache_manager.instance_names == - set(['inst-1', 'inst-1_resize'])) + set(['inst-1', '123', 'inst-1_resize', '123_resize'])) self.assertEqual(len(image_cache_manager.image_popularity), 1) self.assertEqual(image_cache_manager.image_popularity['1'], 1) diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 8a2130c80..4dfc3c488 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -43,6 +43,7 @@ from nova.openstack.common import fileutils from nova.openstack.common import importutils from nova.openstack.common import jsonutils from nova.openstack.common import log as logging +from nova.openstack.common import uuidutils from nova import test from nova.tests import fake_libvirt_utils from nova.tests import fake_network @@ -135,7 +136,8 @@ class FakeVirtDomain(object): class CacheConcurrencyTestCase(test.TestCase): def setUp(self): super(CacheConcurrencyTestCase, self).setUp() - self.flags(instances_path='nova.compute.manager') + + self.flags(instances_path=self.useFixture(fixtures.TempDir()).path) # utils.synchronized() will create the lock_path for us if it # doesn't already exist. It will also delete it when it's done, @@ -165,19 +167,18 @@ class CacheConcurrencyTestCase(test.TestCase): fake_libvirt_utils)) def tearDown(self): - # Make sure the lock_path for this test is cleaned up - if os.path.exists(self.lock_path): - shutil.rmtree(self.lock_path) - super(CacheConcurrencyTestCase, self).tearDown() def test_same_fname_concurrency(self): # Ensures that the same fname cache runs at a sequentially. + uuid = uuidutils.generate_uuid() + backend = imagebackend.Backend(False) wait1 = eventlet.event.Event() done1 = eventlet.event.Event() sig1 = eventlet.event.Event() - thr1 = eventlet.spawn(backend.image({'name': 'instance'}, + thr1 = eventlet.spawn(backend.image({'name': 'instance', + 'uuid': uuid}, 'name').cache, _concurrency, 'fname', None, signal=sig1, wait=wait1, done=done1) @@ -188,7 +189,8 @@ class CacheConcurrencyTestCase(test.TestCase): wait2 = eventlet.event.Event() done2 = eventlet.event.Event() sig2 = eventlet.event.Event() - thr2 = eventlet.spawn(backend.image({'name': 'instance'}, + thr2 = eventlet.spawn(backend.image({'name': 'instance', + 'uuid': uuid}, 'name').cache, _concurrency, 'fname', None, signal=sig2, wait=wait2, done=done2) @@ -209,11 +211,14 @@ class CacheConcurrencyTestCase(test.TestCase): def test_different_fname_concurrency(self): # Ensures that two different fname caches are concurrent. + uuid = uuidutils.generate_uuid() + backend = imagebackend.Backend(False) wait1 = eventlet.event.Event() done1 = eventlet.event.Event() sig1 = eventlet.event.Event() - thr1 = eventlet.spawn(backend.image({'name': 'instance'}, + thr1 = eventlet.spawn(backend.image({'name': 'instance', + 'uuid': uuid}, 'name').cache, _concurrency, 'fname2', None, signal=sig1, wait=wait1, done=done1) @@ -224,7 +229,8 @@ class CacheConcurrencyTestCase(test.TestCase): wait2 = eventlet.event.Event() done2 = eventlet.event.Event() sig2 = eventlet.event.Event() - thr2 = eventlet.spawn(backend.image({'name': 'instance'}, + thr2 = eventlet.spawn(backend.image({'name': 'instance', + 'uuid': uuid}, 'name').cache, _concurrency, 'fname1', None, signal=sig2, wait=wait2, done=done2) @@ -2353,8 +2359,8 @@ class LibvirtConnTestCase(test.TestCase): ret = conn.pre_live_migration(c, inst_ref, vol, nw_info, migrate_data) self.assertEqual(ret, None) - self.assertTrue(os.path.exists('%s/%s/' % - (tmpdir, inst_ref['name']))) + self.assertTrue(os.path.exists('%s/%s/' % (tmpdir, + inst_ref['uuid']))) db.instance_destroy(self.context, inst_ref['uuid']) def test_pre_block_migration_works_correctly(self): @@ -2390,7 +2396,7 @@ class LibvirtConnTestCase(test.TestCase): dummyjson) self.assertTrue(os.path.exists('%s/%s/' % - (tmpdir, instance_ref['name']))) + (tmpdir, instance_ref['uuid']))) db.instance_destroy(self.context, instance_ref['uuid']) diff --git a/nova/tests/test_virt_drivers.py b/nova/tests/test_virt_drivers.py index a94fdc3c5..5a46beffb 100644 --- a/nova/tests/test_virt_drivers.py +++ b/nova/tests/test_virt_drivers.py @@ -184,11 +184,16 @@ class VirtDriverLoaderTestCase(_FakeDriverBackendTestCase, test.TestCase): class _VirtDriverTestCase(_FakeDriverBackendTestCase): def setUp(self): super(_VirtDriverTestCase, self).setUp() + + self.flags(instances_path=self.useFixture(fixtures.TempDir()).path) self.connection = importutils.import_object(self.driver_module, fake.FakeVirtAPI()) self.ctxt = test_utils.get_test_admin_context() self.image_service = fake_image.FakeImageService() + def tearDown(self): + super(_VirtDriverTestCase, self).tearDown() + def _get_running_instance(self): instance_ref = test_utils.get_test_instance() network_info = test_utils.get_test_network_info() diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 386fe836c..dd07be83c 100755 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1977,8 +1977,7 @@ class LibvirtDriver(driver.ComputeDriver): xml = conf.to_xml() if write_to_disk: - instance_dir = os.path.join(CONF.instances_path, - instance["name"]) + instance_dir = libvirt_utils.get_instance_path(instance) xml_path = os.path.join(instance_dir, 'libvirt.xml') libvirt_utils.write_to_file(xml_path, xml) diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py index ea7bded95..adc7691c6 100644 --- a/nova/virt/libvirt/imagecache.py +++ b/nova/virt/libvirt/imagecache.py @@ -220,7 +220,7 @@ class ImageCacheManager(object): self.used_images = {} self.image_popularity = {} - self.instance_names = {} + self.instance_names = set() self.active_base_files = [] self.corrupt_base_files = [] @@ -263,7 +263,11 @@ class ImageCacheManager(object): self.instance_names = set() for instance in all_instances: + # NOTE(mikal): "instance name" here means "the name of a directory + # which might contain an instance" and therefore needs to include + # historical permutations as well as the current one. self.instance_names.add(instance['name']) + self.instance_names.add(instance['uuid']) resize_states = [task_states.RESIZE_PREP, task_states.RESIZE_MIGRATING, @@ -272,6 +276,7 @@ class ImageCacheManager(object): if instance['task_state'] in resize_states or \ instance['vm_state'] == vm_states.RESIZED: self.instance_names.add(instance['name'] + '_resize') + self.instance_names.add(instance['uuid'] + '_resize') image_ref_str = str(instance['image_ref']) local, remote, insts = self.used_images.get(image_ref_str, diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index b8e0cafec..4f2b55670 100755 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -518,17 +518,19 @@ def fetch_image(context, target, image_id, user_id, project_id): images.fetch_to_raw(context, image_id, target, user_id, project_id) -def get_instance_path(instance): +def get_instance_path(instance, forceold=False): """Determine the correct path for instance storage. - This used to be calculated all over the place. This method centralizes - this into one location, which will make it easier to change the - algorithm used to name instance storage directories. + This method determines the directory name for instance storage, while + handling the fact that we changed the naming style to something more + unique in the grizzly release. :param instance: the instance we want a path for + :param forceold: force the use of the pre-grizzly format :returns: a path to store information about that instance """ - # TODO(mikal): we should use UUID instead of name, as name isn't - # nessesarily unique - return os.path.join(CONF.instances_path, instance['name']) + pre_grizzly_name = os.path.join(CONF.instances_path, instance['name']) + if forceold or os.path.exists(pre_grizzly_name): + return pre_grizzly_name + return os.path.join(CONF.instances_path, instance['uuid']) |