From e3ef5214c664e44c125fd80a25418141146ff381 Mon Sep 17 00:00:00 2001 From: Mate Lakat Date: Wed, 10 Jul 2013 18:51:17 +0100 Subject: [PATCH] xenapi: remove pv detection It was assumed that pygrub is there in domU, so that the RAW and unknown disks were attached to domU and investigated with pygrub to find out if PV could be used. This patch removes this functionality. Also some cleanup is made around the ImageType mess. From now RAW and unknown disks will not be investigated for PV-ness and will boot as HVM. Fixes bug 1196570 Change-Id: If84b93cee36938257b7ad0fa44b3b22878520506 --- nova/tests/virt/xenapi/stubs.py | 6 --- nova/tests/virt/xenapi/test_xenapi.py | 18 ++----- nova/virt/xenapi/vm_utils.py | 69 ++++++++------------------- 3 files changed, 24 insertions(+), 69 deletions(-) diff --git a/nova/tests/virt/xenapi/stubs.py b/nova/tests/virt/xenapi/stubs.py index d83a45fe8775..56a397a8a298 100644 --- a/nova/tests/virt/xenapi/stubs.py +++ b/nova/tests/virt/xenapi/stubs.py @@ -83,12 +83,6 @@ def stubout_stream_disk(stubs): stubs.Set(vm_utils, '_stream_disk', fake_stream_disk) -def stubout_is_vdi_pv(stubs): - def f(_1): - return False - stubs.Set(vm_utils, '_is_vdi_pv', f) - - def stubout_determine_is_pv_objectstore(stubs): """Assumes VMs stu have PV kernels.""" diff --git a/nova/tests/virt/xenapi/test_xenapi.py b/nova/tests/virt/xenapi/test_xenapi.py index 950f3bcfecc0..9d24a9ef9b20 100644 --- a/nova/tests/virt/xenapi/test_xenapi.py +++ b/nova/tests/virt/xenapi/test_xenapi.py @@ -147,7 +147,7 @@ def get_fake_device_info(): return fake -def stub_vm_utils_with_vdi_attached_here(function, should_return=True): +def stub_vm_utils_with_vdi_attached_here(function): """ vm_utils.with_vdi_attached_here needs to be stubbed out because it calls down to the filesystem to attach a vdi. This provides a @@ -163,19 +163,13 @@ def stub_vm_utils_with_vdi_attached_here(function, should_return=True): def fake_image_download(*args, **kwargs): pass - def fake_is_vdi_pv(*args, **kwargs): - return should_return - orig_vdi_attached_here = vm_utils.vdi_attached_here orig_image_download = fake_image._FakeImageService.download - orig_is_vdi_pv = vm_utils._is_vdi_pv try: vm_utils.vdi_attached_here = fake_vdi_attached_here fake_image._FakeImageService.download = fake_image_download - vm_utils._is_vdi_pv = fake_is_vdi_pv return function(self, *args, **kwargs) finally: - vm_utils._is_vdi_pv = orig_is_vdi_pv fake_image._FakeImageService.download = orig_image_download vm_utils.vdi_attached_here = orig_vdi_attached_here @@ -328,7 +322,6 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): xenapi_fake.create_network('fake', CONF.flat_network_bridge) stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests) stubs.stubout_get_this_vm_uuid(self.stubs) - stubs.stubout_is_vdi_pv(self.stubs) stubs.stub_out_vm_methods(self.stubs) fake_processutils.stub_out_processutils_execute(self.stubs) self.user_id = 'fake' @@ -810,10 +803,9 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): # No additional VMs should be found. self.assertEqual(start_vms, end_vms) - @stub_vm_utils_with_vdi_attached_here def test_spawn_raw_glance(self): self._test_spawn(IMAGE_RAW, None, None) - self.check_vm_params_for_linux() + self.check_vm_params_for_windows() def test_spawn_vhd_glance_linux(self): self._test_spawn(IMAGE_VHD, None, None, @@ -1886,9 +1878,8 @@ class XenAPIDetermineIsPVTestCase(test.TestCase): def test_linux_vhd(self): self.assert_pv_status(vm_utils.ImageType.DISK_VHD, 'linux', True) - @stub_vm_utils_with_vdi_attached_here def test_raw(self): - self.assert_pv_status(vm_utils.ImageType.DISK_RAW, 'linux', True) + self.assert_pv_status(vm_utils.ImageType.DISK_RAW, 'linux', False) def test_disk(self): self.assert_pv_status(vm_utils.ImageType.DISK, None, True) @@ -1896,9 +1887,8 @@ class XenAPIDetermineIsPVTestCase(test.TestCase): def test_iso(self): self.assert_pv_status(vm_utils.ImageType.DISK_ISO, None, False) - @stub_vm_utils_with_vdi_attached_here def test_none(self): - self.assert_pv_status(None, None, True) + self.assert_pv_status(None, None, False) class CompareVersionTestCase(test.TestCase): diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index b18b868fae5f..640bdd5dcf8d 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -1436,24 +1436,27 @@ def determine_disk_image_type(image_meta): disk_format = image_meta['disk_format'] disk_format_map = { - 'ami': 'DISK', - 'aki': 'KERNEL', - 'ari': 'RAMDISK', - 'raw': 'DISK_RAW', - 'vhd': 'DISK_VHD', - 'iso': 'DISK_ISO', + 'ami': ImageType.DISK, + 'aki': ImageType.KERNEL, + 'ari': ImageType.RAMDISK, + 'raw': ImageType.DISK_RAW, + 'vhd': ImageType.DISK_VHD, + 'iso': ImageType.DISK_ISO, } try: - image_type_str = disk_format_map[disk_format] + image_type = disk_format_map[disk_format] except KeyError: raise exception.InvalidDiskFormat(disk_format=disk_format) - image_type = getattr(ImageType, image_type_str) - image_ref = image_meta['id'] + + params = { + 'image_type_str': ImageType.to_string(image_type), + 'image_ref': image_ref + } LOG.debug(_("Detected %(image_type_str)s format for image %(image_ref)s"), - {'image_type_str': image_type_str, 'image_ref': image_ref}) + params) return image_type @@ -1463,21 +1466,15 @@ def determine_is_pv(session, vdi_ref, disk_image_type, os_type): Determine whether the VM will use a paravirtualized kernel or if it will use hardware virtualization. - 1. Glance (VHD): then we use `os_type`, raise if not set + 1. Glance (VHD): if `os_type` is windows, HVM, otherwise PV - 2. Glance (DISK_RAW): use Pygrub to figure out if pv kernel is - available + 2. Glance (DISK_RAW): HVM - 3. Glance (DISK): pv is assumed + 3. Glance (DISK): PV - 4. Glance (DISK_ISO): no pv is assumed + 4. Glance (DISK_ISO): HVM - 5. Boot From Volume - without image metadata (None): attempt to - use Pygrub to figure out if the volume stores a PV VM or a - HVM one. Log a warning, because there may be cases where the - volume is RAW (in which case using pygrub is fine) and cases - where the content of the volume is VHD, and pygrub might not - work as expected. + 5. Boot From Volume - without image metadata (None): use HVM NOTE: if disk_image_type is not specified, instances launched from remote volumes will have to include kernel and ramdisk because external kernel and ramdisk will not be fetched. @@ -1492,8 +1489,7 @@ def determine_is_pv(session, vdi_ref, disk_image_type, os_type): is_pv = True elif disk_image_type == ImageType.DISK_RAW: # 2. RAW - with vdi_attached_here(session, vdi_ref, read_only=True) as dev: - is_pv = _is_vdi_pv(dev) + is_pv = False elif disk_image_type == ImageType.DISK: # 3. Disk is_pv = True @@ -1501,11 +1497,7 @@ def determine_is_pv(session, vdi_ref, disk_image_type, os_type): # 4. ISO is_pv = False elif not disk_image_type: - LOG.warning(_("Image format is None: trying to determine PV status " - "using pygrub; if instance with vdi %s does not boot " - "correctly, try with image metadata.") % vdi_ref) - with vdi_attached_here(session, vdi_ref, read_only=True) as dev: - is_pv = _is_vdi_pv(dev) + is_pv = False else: raise exception.NovaException(_("Unknown image format %s") % disk_image_type) @@ -2018,27 +2010,6 @@ def _get_this_vm_ref(session): return session.call_xenapi("VM.get_by_uuid", get_this_vm_uuid()) -def _is_vdi_pv(dev): - LOG.debug(_("Running pygrub against %s"), dev) - dev_path = utils.make_dev_path(dev) - try: - out, err = utils.execute('pygrub', '-qn', dev_path, run_as_root=True) - for line in out: - # try to find kernel string - m = re.search('(?<=kernel:)/.*(?:>)', line) - if m and m.group(0).find('xen') != -1: - LOG.debug(_("Found Xen kernel %s") % m.group(0)) - return True - LOG.debug(_("No Xen kernel found. Booting HVM.")) - except processutils.ProcessExecutionError: - LOG.exception(_("Error while executing pygrub! Please, ensure the " - "binary is installed correctly, and available in your " - "PATH; on some Linux distros, pygrub may be installed " - "in /usr/lib/xen-X.Y/bin/pygrub. Attempting to boot " - "in HVM mode.")) - return False - - def _get_partitions(dev): """Return partition information (num, size, type) for a device.""" dev_path = utils.make_dev_path(dev)