From 12b8bd380e22a40e10e9d2758f5a0d594517de51 Mon Sep 17 00:00:00 2001 From: Michael Still Date: Sun, 13 Jan 2013 20:17:05 +1100 Subject: [PATCH] Centralize instance directory logic. This is the first step to being able to change the name of the instance directories to something more unique than instance name. Sneaks up on bug 1023865. Change-Id: I1340a6b8644680325cba1e90a208e4be33ace6b4 --- nova/tests/fake_imagebackend.py | 2 +- nova/tests/fake_libvirt_utils.py | 11 +++ nova/tests/test_imagebackend.py | 11 +-- nova/tests/test_libvirt.py | 16 ++-- nova/virt/libvirt/driver.py | 117 +++++++++++++----------------- nova/virt/libvirt/imagebackend.py | 12 +-- nova/virt/libvirt/utils.py | 17 +++++ 7 files changed, 102 insertions(+), 84 deletions(-) diff --git a/nova/tests/fake_imagebackend.py b/nova/tests/fake_imagebackend.py index 978c879fdf41..c284a5042cc4 100644 --- a/nova/tests/fake_imagebackend.py +++ b/nova/tests/fake_imagebackend.py @@ -28,7 +28,7 @@ class Backend(object): def image(self, instance, name, image_type=''): class FakeImage(imagebackend.Image): def __init__(self, instance, name): - self.path = os.path.join(instance, name) + self.path = os.path.join(instance['name'], name) def create_image(self, prepare_template, base, size, *args, **kwargs): diff --git a/nova/tests/fake_libvirt_utils.py b/nova/tests/fake_libvirt_utils.py index bb789b74a7cd..b3d842468c6f 100644 --- a/nova/tests/fake_libvirt_utils.py +++ b/nova/tests/fake_libvirt_utils.py @@ -17,6 +17,12 @@ import os import StringIO +from nova.openstack.common import cfg + + +CONF = cfg.CONF +CONF.import_opt('instances_path', 'nova.compute.manager') + files = {'console.log': True} disk_sizes = {} @@ -133,3 +139,8 @@ def get_fs_info(path): 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']) diff --git a/nova/tests/test_imagebackend.py b/nova/tests/test_imagebackend.py index 82b5eb475c1a..a9865cb44513 100644 --- a/nova/tests/test_imagebackend.py +++ b/nova/tests/test_imagebackend.py @@ -22,6 +22,7 @@ from nova.openstack.common import cfg 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 @@ -38,12 +39,12 @@ class _ImageTestCase(object): super(_ImageTestCase, self).setUp() self.flags(disable_process_locking=True, instances_path=self.INSTANCES_PATH) - self.INSTANCE = 'instance' + self.INSTANCE = {'name': 'instance'} self.NAME = 'fake.vm' self.TEMPLATE = 'template' - self.PATH = os.path.join(CONF.instances_path, self.INSTANCE, - self.NAME) + self.PATH = os.path.join( + libvirt_utils.get_instance_path(self.INSTANCE), self.NAME) self.TEMPLATE_DIR = os.path.join(CONF.instances_path, '_base') self.TEMPLATE_PATH = os.path.join(self.TEMPLATE_DIR, 'template') @@ -215,7 +216,7 @@ class LvmTestCase(_ImageTestCase, test.TestCase): self.image_class = imagebackend.Lvm super(LvmTestCase, self).setUp() self.flags(libvirt_images_volume_group=self.VG) - self.LV = '%s_%s' % (self.INSTANCE, self.NAME) + self.LV = '%s_%s' % (self.INSTANCE['name'], self.NAME) self.PATH = os.path.join('/dev', self.VG, self.LV) self.disk = imagebackend.disk @@ -342,7 +343,7 @@ class LvmTestCase(_ImageTestCase, test.TestCase): class BackendTestCase(test.TestCase): - INSTANCE = 'fake-instance' + INSTANCE = {'name': 'fake-instance'} NAME = 'fake-name.suffix' def get_image(self, use_cow, image_type): diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 53bb1b9844cc..cd6ac05365de 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -478,7 +478,8 @@ class CacheConcurrencyTestCase(test.TestCase): wait1 = eventlet.event.Event() done1 = eventlet.event.Event() sig1 = eventlet.event.Event() - thr1 = eventlet.spawn(backend.image('instance', 'name').cache, + thr1 = eventlet.spawn(backend.image({'name': 'instance'}, + 'name').cache, _concurrency, 'fname', None, signal=sig1, wait=wait1, done=done1) eventlet.sleep(0) @@ -488,7 +489,8 @@ class CacheConcurrencyTestCase(test.TestCase): wait2 = eventlet.event.Event() done2 = eventlet.event.Event() sig2 = eventlet.event.Event() - thr2 = eventlet.spawn(backend.image('instance', 'name').cache, + thr2 = eventlet.spawn(backend.image({'name': 'instance'}, + 'name').cache, _concurrency, 'fname', None, signal=sig2, wait=wait2, done=done2) @@ -512,7 +514,8 @@ class CacheConcurrencyTestCase(test.TestCase): wait1 = eventlet.event.Event() done1 = eventlet.event.Event() sig1 = eventlet.event.Event() - thr1 = eventlet.spawn(backend.image('instance', 'name').cache, + thr1 = eventlet.spawn(backend.image({'name': 'instance'}, + 'name').cache, _concurrency, 'fname2', None, signal=sig1, wait=wait1, done=done1) eventlet.sleep(0) @@ -522,7 +525,8 @@ class CacheConcurrencyTestCase(test.TestCase): wait2 = eventlet.event.Event() done2 = eventlet.event.Event() sig2 = eventlet.event.Event() - thr2 = eventlet.spawn(backend.image('instance', 'name').cache, + thr2 = eventlet.spawn(backend.image({'name': 'instance'}, + 'name').cache, _concurrency, 'fname1', None, signal=sig2, wait=wait2, done=done2) eventlet.sleep(0) @@ -4447,7 +4451,7 @@ class LibvirtDriverTestCase(test.TestCase): block_device_info=None): pass - def fake_create_domain(xml, inst_name=''): + def fake_create_domain(xml, instance=None): return None def fake_enable_hairpin(instance): @@ -4493,7 +4497,7 @@ class LibvirtDriverTestCase(test.TestCase): def fake_plug_vifs(instance, network_info): pass - def fake_create_domain(xml, inst_name=''): + def fake_create_domain(xml, instance=None): return None def fake_enable_hairpin(instance): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 83d2c96bd6a8..6eb70f0ae965 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -586,7 +586,7 @@ class LibvirtDriver(driver.ComputeDriver): mount_device) if destroy_disks: - target = os.path.join(CONF.instances_path, instance['name']) + target = libvirt_utils.get_instance_path(instance) LOG.info(_('Deleting instance files %(target)s') % locals(), instance=instance) if os.path.exists(target): @@ -642,8 +642,7 @@ class LibvirtDriver(driver.ComputeDriver): } def _cleanup_resize(self, instance, network_info): - target = os.path.join(CONF.instances_path, - instance['name'] + "_resize") + target = libvirt_utils.get_instance_path(instance) + "_resize" if os.path.exists(target): shutil.rmtree(target) @@ -990,11 +989,9 @@ class LibvirtDriver(driver.ComputeDriver): data recovery. """ - + instance_dir = libvirt_utils.get_instance_path(instance) unrescue_xml = self._get_domain_xml(instance, network_info) - unrescue_xml_path = os.path.join(CONF.instances_path, - instance['name'], - 'unrescue.xml') + unrescue_xml_path = os.path.join(instance_dir, 'unrescue.xml') libvirt_utils.write_to_file(unrescue_xml_path, unrescue_xml) rescue_images = { @@ -1014,16 +1011,14 @@ class LibvirtDriver(driver.ComputeDriver): def unrescue(self, instance, network_info): """Reboot the VM which is being rescued back into primary images. """ - unrescue_xml_path = os.path.join(CONF.instances_path, - instance['name'], - 'unrescue.xml') + instance_dir = libvirt_utils.get_instance_path(instance) + unrescue_xml_path = os.path.join(instance_dir, 'unrescue.xml') xml = libvirt_utils.load_file(unrescue_xml_path) virt_dom = self._lookup_by_name(instance['name']) self._destroy(instance) self._create_domain(xml, virt_dom) libvirt_utils.file_delete(unrescue_xml_path) - rescue_files = os.path.join(CONF.instances_path, instance['name'], - "*.rescue") + rescue_files = os.path.join(instance_dir, "*.rescue") for rescue_file in glob.iglob(rescue_files): libvirt_utils.file_delete(rescue_file) @@ -1134,9 +1129,9 @@ class LibvirtDriver(driver.ComputeDriver): msg = _("Guest does not have a console available") raise exception.NovaException(msg) - self._chown_console_log_for_instance(instance['name']) + self._chown_console_log_for_instance(instance) data = self._flush_libvirt_console(pty) - console_log = self._get_console_log_path(instance['name']) + console_log = self._get_console_log_path(instance) fpath = self._append_to_file(data, console_log) with libvirt_utils.file_open(fpath, 'rb') as fp: @@ -1227,11 +1222,12 @@ class LibvirtDriver(driver.ComputeDriver): utils.mkfs('swap', target) @staticmethod - def _get_console_log_path(instance_name): - return os.path.join(CONF.instances_path, instance_name, 'console.log') + def _get_console_log_path(instance): + return os.path.join(libvirt_utils.get_instance_path(instance), + 'console.log') - def _chown_console_log_for_instance(self, instance_name): - console_log = self._get_console_log_path(instance_name) + def _chown_console_log_for_instance(self, instance): + console_log = self._get_console_log_path(instance) if os.path.exists(console_log): libvirt_utils.chown(console_log, os.getuid()) @@ -1243,12 +1239,11 @@ class LibvirtDriver(driver.ComputeDriver): # syntactic nicety def basepath(fname='', suffix=suffix): - return os.path.join(CONF.instances_path, - instance['name'], + return os.path.join(libvirt_utils.get_instance_path(instance), fname + suffix) def image(fname, image_type=CONF.libvirt_images_type): - return self.image_backend.image(instance['name'], + return self.image_backend.image(instance, fname + suffix, image_type) def raw(fname): @@ -1261,11 +1256,11 @@ class LibvirtDriver(driver.ComputeDriver): libvirt_utils.write_to_file(basepath('libvirt.xml'), libvirt_xml) # NOTE(dprince): for rescue console.log may already exist... chown it. - self._chown_console_log_for_instance(instance['name']) + self._chown_console_log_for_instance(instance) # NOTE(vish): No need add the suffix to console.log libvirt_utils.write_to_file( - self._get_console_log_path(instance['name']), '', 007) + self._get_console_log_path(instance), '', 007) if not disk_images: disk_images = {'image_id': instance['image_ref'], @@ -1537,9 +1532,8 @@ class LibvirtDriver(driver.ComputeDriver): if CONF.libvirt_type == "lxc": fs = vconfig.LibvirtConfigGuestFilesys() fs.source_type = "mount" - fs.source_dir = os.path.join(CONF.instances_path, - instance['name'], - 'rootfs') + fs.source_dir = os.path.join( + libvirt_utils.get_instance_path(instance), 'rootfs') devices.append(fs) else: if image_meta and image_meta.get('disk_format') == 'iso': @@ -1557,8 +1551,7 @@ class LibvirtDriver(driver.ComputeDriver): def disk_info(name, disk_dev, disk_bus=default_disk_bus, device_type="disk"): - image = self.image_backend.image(instance['name'], - name) + image = self.image_backend.image(instance, name) return image.libvirt_info(disk_bus, disk_dev, device_type, @@ -1645,9 +1638,8 @@ class LibvirtDriver(driver.ComputeDriver): diskconfig.source_type = "file" diskconfig.driver_format = "raw" diskconfig.driver_cache = self.disk_cachemode - diskconfig.source_path = os.path.join(CONF.instances_path, - instance['name'], - "disk.config") + diskconfig.source_path = os.path.join( + libvirt_utils.get_instance_path(instane), "disk.config") diskconfig.target_dev = self.default_last_device diskconfig.target_bus = default_disk_bus devices.append(diskconfig) @@ -1675,6 +1667,7 @@ class LibvirtDriver(driver.ComputeDriver): 'kernel_id' if a kernel is needed for the rescue image. """ inst_type = instance['instance_type'] + inst_path = libvirt_utils.get_instance_path(instance) guest = vconfig.LibvirtConfigGuest() guest.virt_type = CONF.libvirt_type @@ -1733,9 +1726,7 @@ class LibvirtDriver(driver.ComputeDriver): if rescue: if rescue.get('kernel_id'): - guest.os_kernel = os.path.join(CONF.instances_path, - instance['name'], - "kernel.rescue") + guest.os_kernel = os.path.join(inst_path, "kernel.rescue") if CONF.libvirt_type == "xen": guest.os_cmdline = "ro" else: @@ -1743,22 +1734,16 @@ class LibvirtDriver(driver.ComputeDriver): (root_device_name or "/dev/vda",)) if rescue.get('ramdisk_id'): - guest.os_initrd = os.path.join(CONF.instances_path, - instance['name'], - "ramdisk.rescue") + guest.os_initrd = os.path.join(inst_path, "ramdisk.rescue") elif instance['kernel_id']: - guest.os_kernel = os.path.join(CONF.instances_path, - instance['name'], - "kernel") + guest.os_kernel = os.path.join(inst_path, "kernel") if CONF.libvirt_type == "xen": guest.os_cmdline = "ro" else: guest.os_cmdline = ("root=%s console=ttyS0" % (root_device_name or "/dev/vda",)) if instance['ramdisk_id']: - guest.os_initrd = os.path.join(CONF.instances_path, - instance['name'], - "ramdisk") + guest.os_initrd = os.path.join(inst_path, "ramdisk") else: guest.os_boot_dev = "hd" @@ -1806,8 +1791,7 @@ class LibvirtDriver(driver.ComputeDriver): # to configure two separate consoles. consolelog = vconfig.LibvirtConfigGuestSerial() consolelog.type = "file" - consolelog.source_path = self._get_console_log_path( - instance['name']) + consolelog.source_path = self._get_console_log_path(instance) guest.add_device(consolelog) consolepty = vconfig.LibvirtConfigGuestSerial() @@ -1877,18 +1861,23 @@ class LibvirtDriver(driver.ComputeDriver): 'cpu_time': cpu_time} def _create_domain(self, xml=None, domain=None, - inst_name='', launch_flags=0): + instance=None, launch_flags=0): """Create a domain. Either domain or xml must be passed in. If both are passed, then the domain definition is overwritten from the xml. """ + inst_path = None + if instance: + inst_path = libvirt_utils.get_instance_path(instance) + if CONF.libvirt_type == 'lxc': - container_dir = os.path.join(CONF.instances_path, - inst_name, - 'rootfs') + if not inst_path: + inst_path = None + + container_dir = os.path.join(inst_path, 'rootfs') fileutils.ensure_tree(container_dir) - image = self.image_backend.image(inst_name, 'disk') + image = self.image_backend.image(instance, 'disk') disk.setup_container(image.path, container_dir=container_dir, use_cow=CONF.use_cow_images) @@ -1902,9 +1891,7 @@ class LibvirtDriver(driver.ComputeDriver): # namespace and so there is no need to keep the container rootfs # mounted in the host namespace if CONF.libvirt_type == 'lxc': - container_dir = os.path.join(CONF.instances_path, - inst_name, - 'rootfs') + container_dir = os.path.join(inst_path, 'rootfs') disk.teardown_container(container_dir=container_dir) return domain @@ -1926,7 +1913,7 @@ class LibvirtDriver(driver.ComputeDriver): self.plug_vifs(instance, network_info) self.firewall_driver.setup_basic_filtering(instance, network_info) self.firewall_driver.prepare_instance_filter(instance, network_info) - domain = self._create_domain(xml, inst_name=instance['name']) + domain = self._create_domain(xml, instance=instance) self.firewall_driver.apply_instance_filter(instance, network_info) return domain @@ -2615,7 +2602,7 @@ class LibvirtDriver(driver.ComputeDriver): def _fetch_instance_kernel_ramdisk(self, context, instance): """Download kernel and ramdisk for instance in instance directory.""" - instance_dir = os.path.join(CONF.instances_path, instance['name']) + instance_dir = libvirt_utils.get_instance_path(instance) if instance['kernel_id']: libvirt_utils.fetch_image(context, os.path.join(instance_dir, 'kernel'), @@ -2645,14 +2632,13 @@ class LibvirtDriver(driver.ComputeDriver): if is_volume_backed and not (is_block_migration or is_shared_storage): # Create the instance directory on destination compute node. - instance_dir = os.path.join(CONF.instances_path, - instance_ref['name']) + instance_dir = libvirt_utils.get_instance_path(instance_ref) if os.path.exists(instance_dir): raise exception.DestinationDiskExists(path=instance_dir) os.mkdir(instance_dir) # Touch the console.log file, required by libvirt. - console_file = self._get_console_log_path(instance_ref['name']) + console_file = self._get_console_log_path(instance_ref) libvirt_utils.file_open(console_file, 'a').close() # if image has kernel and ramdisk, just download @@ -2701,7 +2687,7 @@ class LibvirtDriver(driver.ComputeDriver): disk_info = jsonutils.loads(disk_info_json) # make instance directory - instance_dir = os.path.join(CONF.instances_path, instance['name']) + instance_dir = libvirt_utils.get_instance_path(instance) if os.path.exists(instance_dir): raise exception.DestinationDiskExists(path=instance_dir) os.mkdir(instance_dir) @@ -2720,7 +2706,7 @@ class LibvirtDriver(driver.ComputeDriver): # Remove any size tags which the cache manages cache_name = cache_name.split('_')[0] - image = self.image_backend.image(instance['name'], + image = self.image_backend.image(instance, instance_disk, CONF.libvirt_images_type) image.cache(fetch_func=libvirt_utils.fetch_image, @@ -2751,8 +2737,7 @@ class LibvirtDriver(driver.ComputeDriver): # Define migrated instance, otherwise, suspend/destroy does not work. dom_list = self._conn.listDefinedDomains() if instance_ref["name"] not in dom_list: - instance_dir = os.path.join(CONF.instances_path, - instance_ref["name"]) + instance_dir = libvirt_utils.get_instance_path(instance_ref) xml_path = os.path.join(instance_dir, 'libvirt.xml') # In case of block migration, destination does not have # libvirt.xml @@ -2915,7 +2900,7 @@ class LibvirtDriver(driver.ComputeDriver): # rename instance dir to +_resize at first for using # shared storage for instance dir (eg. NFS). same_host = (dest == self.get_host_ip_addr()) - inst_base = "%s/%s" % (CONF.instances_path, instance['name']) + inst_base = libvirt_utils.get_instance_path(instance) inst_base_resize = inst_base + "_resize" try: utils.execute('mv', inst_base, inst_base_resize) @@ -3016,7 +3001,7 @@ class LibvirtDriver(driver.ComputeDriver): LOG.debug(_("Starting finish_revert_migration"), instance=instance) - inst_base = "%s/%s" % (CONF.instances_path, instance['name']) + inst_base = libvirt_utils.get_instance_path(instance) inst_base_resize = inst_base + "_resize" utils.execute('mv', inst_base_resize, inst_base) @@ -3122,12 +3107,10 @@ class LibvirtDriver(driver.ComputeDriver): def instance_on_disk(self, instance): # ensure directories exist and are writable - instance_path = os.path.join(CONF.instances_path, instance["name"]) - + instance_path = libvirt_utils.get_instance_path(instance) LOG.debug(_('Checking instance files accessability' '%(instance_path)s') % locals()) - return os.access(instance_path, os.W_OK) diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index f4c41f53918d..d272e408cf64 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -142,8 +142,9 @@ class Raw(Image): def __init__(self, instance=None, name=None, path=None): super(Raw, self).__init__("file", "raw", is_block_dev=False) - self.path = path or os.path.join(CONF.instances_path, - instance, name) + self.path = (path or + os.path.join(libvirt_utils.get_instance_path(instance), + name)) def create_image(self, prepare_template, base, size, *args, **kwargs): @lockutils.synchronized(base, 'nova-', external=True, @@ -170,8 +171,9 @@ class Qcow2(Image): def __init__(self, instance=None, name=None, path=None): super(Qcow2, self).__init__("file", "qcow2", is_block_dev=False) - self.path = path or os.path.join(CONF.instances_path, - instance, name) + self.path = (path or + os.path.join(libvirt_utils.get_instance_path(instance), + name)) def create_image(self, prepare_template, base, size, *args, **kwargs): @lockutils.synchronized(base, 'nova-', external=True, @@ -208,7 +210,7 @@ class Lvm(Image): ' libvirt_images_volume_group' ' flag to use LVM images.')) self.vg = CONF.libvirt_images_volume_group - self.lv = '%s_%s' % (self.escape(instance), + self.lv = '%s_%s' % (self.escape(instance['name']), self.escape(name)) self.path = os.path.join('/dev', self.vg, self.lv) diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index 9c8d192c724c..4b3517da73a0 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -30,6 +30,7 @@ from nova import utils from nova.virt import images CONF = cfg.CONF +CONF.import_opt('instances_path', 'nova.compute.manager') LOG = logging.getLogger(__name__) @@ -498,3 +499,19 @@ 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_instance_path(instance): + """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. + + :param instance: the instance we want a path for + + :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'])