From 8ea98c56b647526aae7a786531e934eeee7a90a2 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Thu, 17 May 2018 09:47:58 +0100 Subject: [PATCH] libvirt: Skip fetching the virtual size of block devices In this latest episode of `Which CI job has lyarwood broken today?!` we find that I464bc2b88123a012cd12213beac4b572c3c20a56 introduced a regression in the nova-lvm experimental job as n-cpu attempted to run qemu-img info against block devices as an unprivileged user. For the time being we should skip any attempt to use this command against block devices until the disk_api layer can make privileged calls using privsep. Closes-bug: #1771700 Change-Id: I9653f81ec716f80eb638810f65e2d3cdfeedaa22 (cherry picked from commit fda48219a378d09a9a363078ba161d7f54e32c0a) --- nova/tests/unit/virt/libvirt/test_driver.py | 37 +++++++++++++++++++++ nova/virt/libvirt/driver.py | 14 ++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index b8b4600d865e..9bf18ace9cc8 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -14241,6 +14241,43 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertRaises(exception.DiskNotFound, drvr._get_disk_over_committed_size_total) + @mock.patch('nova.virt.libvirt.storage.lvm.get_volume_size') + @mock.patch('nova.virt.disk.api.get_disk_size', + new_callable=mock.NonCallableMock) + def test_get_instance_disk_info_from_config_block_devices(self, + mock_disk_api, mock_get_volume_size): + """Test that for block devices the actual and virtual sizes are + reported as the same and that the disk_api is not used. + """ + c = context.get_admin_context() + instance = objects.Instance(root_device_name='/dev/vda', + **self.test_instance) + bdms = objects.BlockDeviceMappingList(objects=[ + fake_block_device.fake_bdm_object(c, { + 'device_name': '/dev/mapper/vg-lv', + 'source_type': 'image', + 'destination_type': 'local' + }), + + ]) + block_device_info = driver.get_block_device_info(instance, bdms) + + config = vconfig.LibvirtConfigGuest() + disk_config = vconfig.LibvirtConfigGuestDisk() + disk_config.source_type = "block" + disk_config.source_path = mock.sentinel.volume_path + config.devices.append(disk_config) + + mock_get_volume_size.return_value = mock.sentinel.volume_size + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + disk_info = drvr._get_instance_disk_info_from_config(config, + block_device_info) + + mock_get_volume_size.assert_called_once_with(mock.sentinel.volume_path) + self.assertEqual(disk_info[0]['disk_size'], + disk_info[0]['virt_disk_size']) + def test_cpu_info(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index bedad3895667..ca4414fb70df 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -7916,17 +7916,25 @@ class LibvirtDriver(driver.ComputeDriver): dk_size += os.path.getsize(fp) else: dk_size = disk_api.get_allocated_disk_size(path) + + # NOTE(lyarwood): Fetch the virtual size for all file disks. + virt_size = disk_api.get_disk_size(path) + elif disk_type == 'block' and block_device_info: + # FIXME(lyarwood): There's no reason to use a separate call + # here, once disk_api uses privsep this should be removed along + # with the surrounding conditionals to simplify this mess. dk_size = lvm.get_volume_size(path) + # NOTE(lyarwood): As above, we should be using disk_api to + # fetch the virt-size but can't as it currently runs qemu-img + # as an unprivileged user, causing a failure for block devices. + virt_size = dk_size else: LOG.debug('skipping disk %(path)s (%(target)s) - unable to ' 'determine if volume', {'path': path, 'target': target}) continue - # NOTE(lyarwood): Always fetch the virtual size for all disk types. - virt_size = disk_api.get_disk_size(path) - if driver_type in ("qcow2", "ploop"): backing_file = libvirt_utils.get_disk_backing_file(path) over_commit_size = int(virt_size) - dk_size