From 4989069571f1b0721bd8cb1376cefcf73af7d159 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Wed, 9 Dec 2015 15:36:32 +0000 Subject: [PATCH] All the things Change-Id: I94c1c0d26215c061f71c3f95e1a6bf3a58fa19ea --- nova/tests/unit/virt/libvirt/fake_libvirt_utils.py | 10 +++-- nova/tests/unit/virt/libvirt/test_driver.py | 16 ++++---- nova/tests/unit/virt/libvirt/test_utils.py | 46 +++------------------- nova/virt/libvirt/driver.py | 26 +++++++++--- nova/virt/libvirt/utils.py | 31 ++++++++++++--- 5 files changed, 65 insertions(+), 64 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py index a313c52..352cf65 100644 --- a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py +++ b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py @@ -40,7 +40,9 @@ def get_disk_backing_file(path): return disk_backing_files.get(path, None) -def get_disk_type(path): +def get_disk_type_from_path(path): + if disk_type in ('raw', 'qcow2'): + return None return disk_type @@ -99,11 +101,11 @@ def file_open(path, mode=None): def find_disk(virt_dom): if disk_type == 'lvm': - return "/dev/nova-vg/lv" + return ("/dev/nova-vg/lv", "raw") elif disk_type in ['raw', 'qcow2']: - return "filename" + return ("filename", disk_type) else: - return "unknown_type_disk" + return ("unknown_type_disk", None) def load_file(path): diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 48493ef..ef631ee 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -14694,11 +14694,11 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): mock.Mock(return_value=True)) @mock.patch("nova.virt.libvirt.guest.Guest.is_active", mock.Mock(return_value=False)) - @mock.patch('nova.virt.libvirt.utils.get_disk_type', - return_value="fake_fmt") + @mock.patch('nova.virt.images.qemu_img_info', + return_value=mock.Mock(file_format="fake_fmt")) @mock.patch('nova.utils.execute') def test_volume_snapshot_delete_when_dom_not_running(self, mock_execute, - mock_get_disk_type): + mock_qemu_img_info): """Deleting newest snapshot of a file-based image when the domain is not running should trigger a blockRebase using qemu-img not libvirt. In this test, we rebase the image with another image as backing file. @@ -14714,7 +14714,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): self.volume_uuid, snapshot_id, self.delete_info_1) - mock_get_disk_type.assert_called_once_with("snap.img") + mock_qemu_img_info.assert_called_once_with("snap.img") mock_execute.assert_called_once_with('qemu-img', 'rebase', '-b', 'snap.img', '-F', 'fake_fmt', 'disk1_file') @@ -14723,11 +14723,11 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): mock.Mock(return_value=True)) @mock.patch("nova.virt.libvirt.guest.Guest.is_active", mock.Mock(return_value=False)) - @mock.patch('nova.virt.libvirt.utils.get_disk_type', - return_value="fake_fmt") + @mock.patch('nova.virt.images.qemu_img_info', + return_value=mock.Mock(file_format="fake_fmt")) @mock.patch('nova.utils.execute') def test_volume_snapshot_delete_when_dom_not_running_and_no_rebase_base( - self, mock_execute, mock_get_disk_type): + self, mock_execute, mock_qemu_img_info): """Deleting newest snapshot of a file-based image when the domain is not running should trigger a blockRebase using qemu-img not libvirt. In this test, the image is rebased onto no backing file (i.e. @@ -14744,7 +14744,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): self.volume_uuid, snapshot_id, self.delete_info_3) - self.assertEqual(0, mock_get_disk_type.call_count) + self.assertEqual(0, mock_qemu_img_info.call_count) mock_execute.assert_called_once_with('qemu-img', 'rebase', '-b', '', 'disk1_file') diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 623f169..157dbf7 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -40,24 +40,6 @@ CONF = cfg.CONF class LibvirtUtilsTestCase(test.NoDBTestCase): - @mock.patch('os.path.exists', return_value=True) - @mock.patch('nova.utils.execute') - def test_get_disk_type(self, mock_execute, mock_exists): - path = "disk.config" - example_output = """image: disk.config -file format: raw -virtual size: 64M (67108864 bytes) -cluster_size: 65536 -disk size: 96K -blah BLAH: bb -""" - mock_execute.return_value = (example_output, '') - disk_type = libvirt_utils.get_disk_type(path) - mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) - mock_exists.assert_called_once_with(path) - self.assertEqual('raw', disk_type) - @mock.patch('nova.utils.execute') def test_copy_image_local(self, mock_execute): libvirt_utils.copy_image('src', 'dest') @@ -78,43 +60,27 @@ blah BLAH: bb on_completion=None, on_execute=None, compression=True) @mock.patch('os.path.exists', return_value=True) - def test_disk_type(self, mock_exists): + def test_disk_type_from_path(self, mock_exists): # Seems like lvm detection # if its in /dev ?? for p in ['/dev/b', '/dev/blah/blah']: - d_type = libvirt_utils.get_disk_type(p) + d_type = libvirt_utils.get_disk_type_from_path(p) self.assertEqual('lvm', d_type) # Try rbd detection - d_type = libvirt_utils.get_disk_type('rbd:pool/instance') + d_type = libvirt_utils.get_disk_type_from_path('rbd:pool/instance') self.assertEqual('rbd', d_type) # Try the other types - template_output = """image: %(path)s -file format: %(format)s -virtual size: 64M (67108864 bytes) -cluster_size: 65536 -disk size: 96K -""" path = '/myhome/disk.config' - for f in ['raw', 'qcow2']: - output = template_output % ({ - 'format': f, - 'path': path, - }) - with mock.patch('nova.utils.execute', - return_value=(output, '')) as mock_execute: - d_type = libvirt_utils.get_disk_type(path) - mock_execute.assert_called_once_with( - 'env', 'LC_ALL=C', 'LANG=C', - 'qemu-img', 'info', path) - self.assertEqual(f, d_type) + d_type = libvirt_utils.get_disk_type_from_path(path) + self.assertIsNone(d_type) @mock.patch('os.path.exists', return_value=True) @mock.patch('os.path.isdir', return_value=True) def test_disk_type_ploop(self, mock_isdir, mock_exists): path = '/some/path' - d_type = libvirt_utils.get_disk_type(path) + d_type = libvirt_utils.get_disk_type_from_path(path) mock_isdir.assert_called_once_with(path) mock_exists.assert_called_once_with("%s/DiskDescriptor.xml" % path) self.assertEqual('ploop', d_type) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index d551cf2..cd8dae3 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -91,6 +91,7 @@ from nova.virt import driver from nova.virt import firewall from nova.virt import hardware from nova.virt.image import model as imgmodel +from nova.virt import images from nova.virt.libvirt import blockinfo from nova.virt.libvirt import config as vconfig from nova.virt.libvirt import firewall as libvirt_firewall @@ -1356,10 +1357,23 @@ class LibvirtDriver(driver.ComputeDriver): snapshot = self._image_api.get(context, image_id) - disk_path = libvirt_utils.find_disk(virt_dom) - source_format = libvirt_utils.get_disk_type(disk_path) + # source_format is an on-disk format + # source_type is a backend type + disk_path, source_format = libvirt_utils.find_disk(virt_dom) + source_type = libvirt_utils.get_disk_type_from_path(disk_path) - image_format = CONF.libvirt.snapshot_image_format or source_format + # We won't have source_type for raw or qcow2 disks, because we can't + # determine that from the path. We should have it from the libvirt + # xml, though. + if source_type is None: + source_type = source_format + # For lxc instances we won't have it either from libvirt xml + # (because we just gave libvirt the mounted filesystem), or the path, + # so source_type is still going to be None. In this case, + # snapshot_backend is going to default to CONF.libvirt.images_type + # below, which is still safe. + + image_format = CONF.libvirt.snapshot_image_format or source_type # NOTE(bfilippov): save lvm and rbd as raw if image_format == 'lvm' or image_format == 'rbd': @@ -1385,7 +1399,7 @@ class LibvirtDriver(driver.ComputeDriver): if (self._host.has_min_version(MIN_LIBVIRT_LIVESNAPSHOT_VERSION, MIN_QEMU_LIVESNAPSHOT_VERSION, host.HV_DRIVER_QEMU) - and source_format not in ('lvm', 'rbd') + and source_type not in ('lvm', 'rbd') and not CONF.ephemeral_storage_encryption.enabled and not CONF.workarounds.disable_libvirt_livesnapshot): live_snapshot = True @@ -1420,7 +1434,7 @@ class LibvirtDriver(driver.ComputeDriver): snapshot_backend = self.image_backend.snapshot(instance, disk_path, - image_type=source_format) + image_type=source_type) if live_snapshot: LOG.info(_LI("Beginning live snapshot process"), @@ -1854,7 +1868,7 @@ class LibvirtDriver(driver.ComputeDriver): # explicitly set the backing file format to avoid any security # concerns related to file format auto detection. backing_file = rebase_base - b_file_fmt = libvirt_utils.get_disk_type(backing_file) + b_file_fmt = images.qemu_img_info(backing_file).file_format qemu_img_extra_arg = ['-F', b_file_fmt] qemu_img_extra_arg.append(active_disk_object.source_path) diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index c2b7790..d1d121b 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -338,16 +338,26 @@ def find_disk(virt_dom): xml_desc = virt_dom.XMLDesc(0) domain = etree.fromstring(xml_desc) os_type = domain.find('os/type').text + driver = None if CONF.libvirt.virt_type == 'lxc': - source = domain.find('devices/filesystem/source') + filesystem = domain.find('devices/filesystem') + driver = filesystem.find('driver') + + source = filesystem.find('source') disk_path = source.get('dir') disk_path = disk_path[0:disk_path.rfind('rootfs')] disk_path = os.path.join(disk_path, 'disk') elif CONF.libvirt.virt_type == 'parallels' and os_type == vm_mode.EXE: - source = domain.find('devices/filesystem/source') + filesystem = domain.find('devices/filesystem') + driver = filesystem.find('driver') + + source = filesystem.find('source') disk_path = source.get('file') else: - source = domain.find('devices/disk/source') + disk = domain.find('devices/disk') + driver = disk.find('driver') + + source = disk.find('source') disk_path = source.get('file') or source.get('dev') if not disk_path and CONF.libvirt.images_type == 'rbd': disk_path = source.get('name') @@ -358,10 +368,18 @@ def find_disk(virt_dom): raise RuntimeError(_("Can't retrieve root device path " "from instance libvirt configuration")) - return disk_path + if driver is not None: + format = driver.get('type') + # This is a legacy quirk of libvirt/xen. Everything else should + # report the on-disk format in type. + if format == 'aio': + format = 'raw' + else: + format = None + return (disk_path, format) -def get_disk_type(path): +def get_disk_type_from_path(path): """Retrieve disk type (raw, qcow2, lvm, ploop) for given file.""" if path.startswith('/dev'): return 'lvm' @@ -371,7 +389,8 @@ def get_disk_type(path): os.path.exists(os.path.join(path, "DiskDescriptor.xml"))): return 'ploop' - return images.qemu_img_info(path).file_format + # We can't reliably determine the type from this path + return None def get_fs_info(path): -- 2.5.0