From f0c9c85760605f0b7460d9bc08ac79487703a6fd Mon Sep 17 00:00:00 2001 From: Rafi Khardalian Date: Tue, 5 Feb 2013 09:40:15 +0000 Subject: Recache or rebuild missing images on hard_reboot The primary purpose of this change is to provide the ability to re-cache missing backing files on hard_reboot. The old pre_block_migration function was already performing a very similar operation. That function has been refactored to be idempotent and renamed to _create_images_and_backing. The pre_block_migration function is a wrapper, with some additional checking, around the renamed function. Image backend was also adjusted to look for either a missing backing file or disk image, recaching or creating accordingly. It should also be idempotent, never clobbering existing images. Change-Id: Icf4c488d6db59e732b463d08d0606b428ee1e7b9 --- nova/compute/manager.py | 3 +- nova/tests/compute/test_compute.py | 5 +-- nova/tests/test_hypervapi.py | 3 +- nova/tests/test_imagebackend.py | 9 +++--- nova/tests/test_libvirt.py | 4 +-- nova/tests/test_virt_drivers.py | 10 +++++- nova/tests/test_vmwareapi.py | 9 ++++-- nova/tests/test_xenapi.py | 10 +++--- nova/virt/baremetal/driver.py | 2 +- nova/virt/driver.py | 3 +- nova/virt/fake.py | 2 +- nova/virt/hyperv/driver.py | 2 +- nova/virt/libvirt/driver.py | 64 ++++++++++++++++++++++++-------------- nova/virt/libvirt/imagebackend.py | 23 ++++++++------ nova/virt/powervm/driver.py | 2 +- nova/virt/vmwareapi/driver.py | 2 +- nova/virt/xenapi/driver.py | 2 +- 17 files changed, 94 insertions(+), 61 deletions(-) mode change 100644 => 100755 nova/compute/manager.py mode change 100644 => 100755 nova/virt/baremetal/driver.py mode change 100644 => 100755 nova/virt/driver.py mode change 100644 => 100755 nova/virt/fake.py mode change 100644 => 100755 nova/virt/hyperv/driver.py mode change 100644 => 100755 nova/virt/libvirt/driver.py mode change 100644 => 100755 nova/virt/libvirt/imagebackend.py mode change 100644 => 100755 nova/virt/powervm/driver.py mode change 100644 => 100755 nova/virt/vmwareapi/driver.py mode change 100644 => 100755 nova/virt/xenapi/driver.py diff --git a/nova/compute/manager.py b/nova/compute/manager.py old mode 100644 new mode 100755 index 605ebab6c..572868c58 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1479,7 +1479,8 @@ class ComputeManager(manager.SchedulerDependentManager): context=context, instance=instance) try: - self.driver.reboot(instance, self._legacy_nw_info(network_info), + self.driver.reboot(context, instance, + self._legacy_nw_info(network_info), reboot_type, block_device_info) except Exception, exc: LOG.error(_('Cannot reboot instance: %(exc)s'), locals(), diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 1584cde34..442832553 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -1126,8 +1126,9 @@ class ComputeTestCase(BaseTestCase): # this is called with the wrong args, so we have to hack # around it. reboot_call_info = {} - expected_call_info = {'args': (updated_instance1, expected_nw_info, - reboot_type, fake_block_dev_info), + expected_call_info = {'args': (econtext, updated_instance1, + expected_nw_info, reboot_type, + fake_block_dev_info), 'kwargs': {}} def fake_reboot(*args, **kwargs): diff --git a/nova/tests/test_hypervapi.py b/nova/tests/test_hypervapi.py index 3b624e6d1..0c2f90a4d 100644 --- a/nova/tests/test_hypervapi.py +++ b/nova/tests/test_hypervapi.py @@ -486,7 +486,8 @@ class HyperVAPITestCase(test.TestCase): constants.HYPERV_VM_STATE_REBOOT) self._mox.ReplayAll() - self._conn.reboot(self._instance_data, network_info, None) + self._conn.reboot(self._context, self._instance_data, network_info, + None) self._mox.VerifyAll() def test_destroy(self): diff --git a/nova/tests/test_imagebackend.py b/nova/tests/test_imagebackend.py index 495e7c947..76fd1d5b6 100644 --- a/nova/tests/test_imagebackend.py +++ b/nova/tests/test_imagebackend.py @@ -55,8 +55,8 @@ class _ImageTestCase(object): def test_cache(self): self.mox.StubOutWithMock(os.path, 'exists') - os.path.exists(self.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) fn = self.mox.CreateMockAnything() fn(target=self.TEMPLATE_PATH) @@ -72,7 +72,9 @@ class _ImageTestCase(object): def test_cache_image_exists(self): self.mox.StubOutWithMock(os.path, 'exists') + os.path.exists(self.TEMPLATE_DIR).AndReturn(True) os.path.exists(self.PATH).AndReturn(True) + os.path.exists(self.TEMPLATE_PATH).AndReturn(True) self.mox.ReplayAll() image = self.image_class(self.INSTANCE, self.NAME) @@ -82,8 +84,8 @@ class _ImageTestCase(object): def test_cache_base_dir_exists(self): self.mox.StubOutWithMock(os.path, 'exists') - os.path.exists(self.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) fn = self.mox.CreateMockAnything() fn(target=self.TEMPLATE_PATH) @@ -98,8 +100,8 @@ class _ImageTestCase(object): def test_cache_template_exists(self): self.mox.StubOutWithMock(os.path, 'exists') - os.path.exists(self.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) fn = self.mox.CreateMockAnything() self.mox.ReplayAll() @@ -195,7 +197,6 @@ 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') imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH, self.PATH) imagebackend.disk.extend(self.PATH, self.SIZE) diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 52e308b85..3e0e51f58 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -4206,7 +4206,7 @@ class LibvirtDriverTestCase(test.TestCase): self.counter = 0 - def fake_get_instance_disk_info(instance): + def fake_get_instance_disk_info(instance, xml=None): return '[]' def fake_destroy(instance): @@ -4251,7 +4251,7 @@ class LibvirtDriverTestCase(test.TestCase): 'disk_size':'83886080'}] disk_info_text = jsonutils.dumps(disk_info) - def fake_get_instance_disk_info(instance): + def fake_get_instance_disk_info(instance, xml=None): return disk_info_text def fake_destroy(instance): diff --git a/nova/tests/test_virt_drivers.py b/nova/tests/test_virt_drivers.py index 29c7f9959..67452bc24 100644 --- a/nova/tests/test_virt_drivers.py +++ b/nova/tests/test_virt_drivers.py @@ -104,6 +104,13 @@ class _FakeDriverBackendTestCase(object): def fake_make_drive(_self, _path): pass + def fake_get_instance_disk_info(_self, instance, xml=None): + return '[]' + + self.stubs.Set(nova.virt.libvirt.driver.LibvirtDriver, + 'get_instance_disk_info', + fake_get_instance_disk_info) + self.stubs.Set(nova.virt.libvirt.driver.disk, 'extend', fake_extend) @@ -227,7 +234,8 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase): def test_reboot(self): reboot_type = "SOFT" instance_ref, network_info = self._get_running_instance() - self.connection.reboot(instance_ref, network_info, reboot_type) + self.connection.reboot(self.ctxt, instance_ref, network_info, + reboot_type) @catch_notimplementederror def test_get_host_ip_addr(self): diff --git a/nova/tests/test_vmwareapi.py b/nova/tests/test_vmwareapi.py index 34f03a555..22544fd2d 100644 --- a/nova/tests/test_vmwareapi.py +++ b/nova/tests/test_vmwareapi.py @@ -199,14 +199,16 @@ class VMwareAPIVMTestCase(test.TestCase): info = self.conn.get_info({'name': 1}) self._check_vm_info(info, power_state.RUNNING) reboot_type = "SOFT" - self.conn.reboot(self.instance, self.network_info, reboot_type) + self.conn.reboot(self.context, self.instance, self.network_info, + reboot_type) info = self.conn.get_info({'name': 1}) self._check_vm_info(info, power_state.RUNNING) def test_reboot_non_existent(self): self._create_instance_in_the_db() self.assertRaises(exception.InstanceNotFound, self.conn.reboot, - self.instance, self.network_info, 'SOFT') + self.context, self.instance, self.network_info, + 'SOFT') def test_reboot_not_poweredon(self): self._create_vm() @@ -216,7 +218,8 @@ class VMwareAPIVMTestCase(test.TestCase): info = self.conn.get_info({'name': 1}) self._check_vm_info(info, power_state.SUSPENDED) self.assertRaises(exception.InstanceRebootFailure, self.conn.reboot, - self.instance, self.network_info, 'SOFT') + self.context, self.instance, self.network_info, + 'SOFT') def test_suspend(self): self._create_vm() diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index c480d5c5f..c6359b5d8 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -954,12 +954,12 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): def test_reboot_hard(self): instance = self._create_instance() conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) - conn.reboot(instance, None, "HARD") + conn.reboot(self.context, instance, None, "HARD") def test_reboot_soft(self): instance = self._create_instance() conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) - conn.reboot(instance, None, "SOFT") + conn.reboot(self.context, instance, None, "SOFT") def test_reboot_halted(self): session = xenapi_conn.XenAPISession('test_url', 'root', 'test_pass', @@ -967,7 +967,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): instance = self._create_instance(spawn=False) conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) xenapi_fake.create_vm(instance['name'], 'Halted') - conn.reboot(instance, None, "SOFT") + conn.reboot(self.context, instance, None, "SOFT") vm_ref = vm_utils.lookup(session, instance['name']) vm = xenapi_fake.get_record('VM', vm_ref) self.assertEquals(vm['power_state'], 'Running') @@ -976,8 +976,8 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): instance = self._create_instance(spawn=False) conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) xenapi_fake.create_vm(instance['name'], 'Unknown') - self.assertRaises(xenapi_fake.Failure, conn.reboot, instance, - None, "SOFT") + self.assertRaises(xenapi_fake.Failure, conn.reboot, self.context, + instance, None, "SOFT") def _test_maintenance_mode(self, find_host, find_aggregate): real_call_xenapi = self.conn._session.call_xenapi diff --git a/nova/virt/baremetal/driver.py b/nova/virt/baremetal/driver.py old mode 100644 new mode 100755 index 43af951fd..9160485a6 --- a/nova/virt/baremetal/driver.py +++ b/nova/virt/baremetal/driver.py @@ -271,7 +271,7 @@ class BareMetalDriver(driver.ComputeDriver): LOG.warning(_("Failed to update state record for " "baremetal node %s") % instance['uuid']) - def reboot(self, instance, network_info, reboot_type, + def reboot(self, context, instance, network_info, reboot_type, block_device_info=None): node = _get_baremetal_node_by_instance_uuid(instance['uuid']) ctx = nova_context.get_admin_context() diff --git a/nova/virt/driver.py b/nova/virt/driver.py old mode 100644 new mode 100755 index 747b60714..ba0dfbafe --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -239,7 +239,7 @@ class ComputeDriver(object): # TODO(Vek): Need to pass context in for access to auth_token raise NotImplementedError() - def reboot(self, instance, network_info, reboot_type, + def reboot(self, context, instance, network_info, reboot_type, block_device_info=None): """Reboot the specified instance. @@ -254,7 +254,6 @@ class ComputeDriver(object): :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info` :param reboot_type: Either a HARD or SOFT reboot """ - # TODO(Vek): Need to pass context in for access to auth_token raise NotImplementedError() def get_console_pool_info(self, console_type): diff --git a/nova/virt/fake.py b/nova/virt/fake.py old mode 100644 new mode 100755 index 5a5bb7b13..30a5fc758 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -129,7 +129,7 @@ class FakeDriver(driver.ComputeDriver): raise exception.InstanceNotRunning(instance_id=instance['uuid']) update_task_state(task_state=task_states.IMAGE_UPLOADING) - def reboot(self, instance, network_info, reboot_type, + def reboot(self, context, instance, network_info, reboot_type, block_device_info=None): pass diff --git a/nova/virt/hyperv/driver.py b/nova/virt/hyperv/driver.py old mode 100644 new mode 100755 index e8bf8c416..4af3b8b05 --- a/nova/virt/hyperv/driver.py +++ b/nova/virt/hyperv/driver.py @@ -52,7 +52,7 @@ class HyperVDriver(driver.ComputeDriver): self._vmops.spawn(context, instance, image_meta, injected_files, admin_password, network_info, block_device_info) - def reboot(self, instance, network_info, reboot_type, + def reboot(self, context, instance, network_info, reboot_type, block_device_info=None): self._vmops.reboot(instance, network_info, reboot_type) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py old mode 100644 new mode 100755 index fdf37fe31..0d0ff31a3 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -955,7 +955,7 @@ class LibvirtDriver(driver.ComputeDriver): libvirt_utils.extract_snapshot(disk_delta, 'qcow2', None, out_path, image_format) - def reboot(self, instance, network_info, reboot_type='SOFT', + def reboot(self, context, instance, network_info, reboot_type='SOFT', block_device_info=None): """Reboot a virtual machine, given an instance reference.""" if reboot_type == 'SOFT': @@ -967,7 +967,8 @@ class LibvirtDriver(driver.ComputeDriver): else: LOG.warn(_("Failed to soft reboot instance."), instance=instance) - return self._hard_reboot(instance, network_info, block_device_info) + return self._hard_reboot(context, instance, network_info, + block_device_info) def _soft_reboot(self, instance): """Attempt to shutdown and restart the instance gracefully. @@ -1013,7 +1014,8 @@ class LibvirtDriver(driver.ComputeDriver): greenthread.sleep(1) return False - def _hard_reboot(self, instance, network_info, block_device_info=None): + def _hard_reboot(self, context, instance, network_info, + block_device_info=None): """Reboot a virtual machine, given an instance reference. Performs a Libvirt reset (if supported) on the domain. @@ -1033,6 +1035,13 @@ class LibvirtDriver(driver.ComputeDriver): xml = self.to_xml(instance, network_info, disk_info, block_device_info=block_device_info, write_to_disk=True) + + # NOTE (rmk): Re-populate any missing backing files. + disk_info_json = self.get_instance_disk_info(instance['name'], xml) + self._create_images_and_backing(context, instance, disk_info_json) + + # Initialize all the necessary networking, block devices and + # start the instance. self._create_domain_and_network(xml, instance, network_info, block_device_info) @@ -1101,7 +1110,7 @@ class LibvirtDriver(driver.ComputeDriver): # Instance is not up and could be in an unknown state. # Be as absolute as possible about getting it back into # a known and running state. - self._hard_reboot(instance, network_info, block_device_info) + self._hard_reboot(context, instance, network_info, block_device_info) def rescue(self, context, instance, network_info, image_meta, rescue_password): @@ -2832,8 +2841,17 @@ class LibvirtDriver(driver.ComputeDriver): greenthread.sleep(1) def pre_block_migration(self, ctxt, instance, disk_info_json): - """Preparation block migration. + """Preparation for block migration.""" + # NOTE (rmk): When preparing for a block migration, the instance dir + # should not exist on the destination hypervisor. + instance_dir = libvirt_utils.get_instance_path(instance) + if os.path.exists(instance_dir): + raise exception.DestinationDiskExists(path=instance_dir) + os.mkdir(instance_dir) + self._create_images_and_backing(ctxt, instance, disk_info_json) + def _create_images_and_backing(self, ctxt, instance, disk_info_json): + """ :params ctxt: security context :params instance: nova.db.sqlalchemy.models.Instance object @@ -2843,19 +2861,14 @@ class LibvirtDriver(driver.ComputeDriver): """ disk_info = jsonutils.loads(disk_info_json) - - # make instance directory instance_dir = libvirt_utils.get_instance_path(instance) - if os.path.exists(instance_dir): - raise exception.DestinationDiskExists(path=instance_dir) - os.mkdir(instance_dir) for info in disk_info: base = os.path.basename(info['path']) # Get image type and create empty disk image, and # create backing file in case of qcow2. instance_disk = os.path.join(instance_dir, base) - if not info['backing_file']: + if not info['backing_file'] and not os.path.exists(instance_disk): libvirt_utils.create_image(info['type'], instance_disk, info['disk_size']) else: @@ -2908,10 +2921,9 @@ class LibvirtDriver(driver.ComputeDriver): dom = self._lookup_by_name(instance_ref["name"]) self._conn.defineXML(dom.XMLDesc(0)) - def get_instance_disk_info(self, instance_name): + def get_instance_disk_info(self, instance_name, xml=None): """Preparation block migration. - :params ctxt: security context :params instance_ref: nova.db.sqlalchemy.models.Instance object instance object that is migrated. @@ -2924,18 +2936,22 @@ class LibvirtDriver(driver.ComputeDriver): 'disk_size':'83886080'},...]" """ - disk_info = [] + # NOTE (rmk): Passing the domain XML into this function is optional. + # When it is not passed, we attempt to extract it from + # the pre-existing definition. + if xml is None: + try: + virt_dom = self._lookup_by_name(instance_name) + xml = virt_dom.XMLDesc(0) + except libvirt.libvirtError as ex: + error_code = ex.get_error_code() + msg = _("Error from libvirt while getting description of " + "%(instance_name)s: [Error Code %(error_code)s] " + "%(ex)s") % locals() + LOG.warn(msg) + raise exception.InstanceNotFound(instance_id=instance_name) - virt_dom = self._lookup_by_name(instance_name) - try: - xml = virt_dom.XMLDesc(0) - except libvirt.libvirtError as ex: - error_code = ex.get_error_code() - msg = _("Error from libvirt while getting description of " - "%(instance_name)s: [Error Code %(error_code)s] " - "%(ex)s") % locals() - LOG.warn(msg) - raise exception.InstanceNotFound(instance_id=instance_name) + disk_info = [] doc = etree.fromstring(xml) disk_nodes = doc.findall('.//devices/disk') path_nodes = doc.findall('.//devices/disk/source') diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py old mode 100644 new mode 100755 index 0815c142f..0d5e8cb66 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -120,12 +120,12 @@ class Image(object): if not os.path.exists(target): fetch_func(target=target, *args, **kwargs) - if not os.path.exists(self.path): - base_dir = os.path.join(CONF.instances_path, CONF.base_dir_name) - if not os.path.exists(base_dir): - fileutils.ensure_tree(base_dir) - base = os.path.join(base_dir, filename) + base_dir = os.path.join(CONF.instances_path, CONF.base_dir_name) + if not os.path.exists(base_dir): + fileutils.ensure_tree(base_dir) + base = os.path.join(base_dir, filename) + if not os.path.exists(self.path) or not os.path.exists(base): self.create_image(call_if_not_exists, base, size, *args, **kwargs) @@ -160,8 +160,9 @@ class Raw(Image): prepare_template(target=self.path, *args, **kwargs) else: prepare_template(target=base, *args, **kwargs) - with utils.remove_path_on_error(self.path): - copy_raw_image(base, self.path, size) + if not os.path.exists(self.path): + with utils.remove_path_on_error(self.path): + copy_raw_image(base, self.path, size) def snapshot(self, name): return snapshots.RawSnapshot(self.path, name) @@ -183,9 +184,11 @@ class Qcow2(Image): if size: disk.extend(target, size) - prepare_template(target=base, *args, **kwargs) - with utils.remove_path_on_error(self.path): - copy_qcow2_image(base, self.path, size) + if not os.path.exists(base): + prepare_template(target=base, *args, **kwargs) + if not os.path.exists(self.path): + with utils.remove_path_on_error(self.path): + copy_qcow2_image(base, self.path, size) def snapshot(self, name): return snapshots.Qcow2Snapshot(self.path, name) diff --git a/nova/virt/powervm/driver.py b/nova/virt/powervm/driver.py old mode 100644 new mode 100755 index 6679c38f9..2cd7b2ac7 --- a/nova/virt/powervm/driver.py +++ b/nova/virt/powervm/driver.py @@ -98,7 +98,7 @@ class PowerVMDriver(driver.ComputeDriver): """Destroy (shutdown and delete) the specified instance.""" self._powervm.destroy(instance['name'], destroy_disks) - def reboot(self, instance, network_info, reboot_type, + def reboot(self, context, instance, network_info, reboot_type, block_device_info=None): """Reboot the specified instance. diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py old mode 100644 new mode 100755 index 3e9269530..19f984c7d --- a/nova/virt/vmwareapi/driver.py +++ b/nova/virt/vmwareapi/driver.py @@ -180,7 +180,7 @@ class VMwareESXDriver(driver.ComputeDriver): """Create snapshot from a running VM instance.""" self._vmops.snapshot(context, instance, name, update_task_state) - def reboot(self, instance, network_info, reboot_type, + def reboot(self, context, instance, network_info, reboot_type, block_device_info=None): """Reboot VM instance.""" self._vmops.reboot(instance, network_info) diff --git a/nova/virt/xenapi/driver.py b/nova/virt/xenapi/driver.py old mode 100644 new mode 100755 index 46b759f43..c1a578f3b --- a/nova/virt/xenapi/driver.py +++ b/nova/virt/xenapi/driver.py @@ -193,7 +193,7 @@ class XenAPIDriver(driver.ComputeDriver): """Create snapshot from a running VM instance.""" self._vmops.snapshot(context, instance, image_id, update_task_state) - def reboot(self, instance, network_info, reboot_type, + def reboot(self, context, instance, network_info, reboot_type, block_device_info=None): """Reboot VM instance.""" self._vmops.reboot(instance, reboot_type) -- cgit