From d085c8fe536e28ff8a3d757c8407fce37ffc00e4 Mon Sep 17 00:00:00 2001 From: Vlad Gusev Date: Tue, 18 Sep 2018 17:53:16 +0300 Subject: [PATCH] libvirt: Reduce calls to qemu-img during update_available_resource I464bc2b88123a012cd12213beac4b572c3c20a56 introduced a second call to ``qemu-img`` that can easily be collapsed into one with the addition of a new call within the disk_api. Conflicts: nova/tests/unit/virt/libvirt/test_driver.py NOTE(s10): The conflict was due to not having change Ic853743573aa0b74d5d2c5b8b47252b875d5f7ef in Queens. Related-Bug: #1785827 Change-Id: Ibfd0527ed79f60282b542034d7cb97b424becba3 (cherry picked from commit d41ea9d87894111051b591d513cd046e5e357124) --- nova/tests/unit/virt/libvirt/test_driver.py | 120 +++++++++----------- nova/virt/disk/api.py | 9 ++ nova/virt/libvirt/driver.py | 5 +- 3 files changed, 63 insertions(+), 71 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 907a83c87b9e..49b4baf91ee5 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -4350,7 +4350,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, [mock.call(host='127.0.0.1', port=10000), mock.call(host='127.0.0.1', port=10001)]) - @mock.patch('nova.virt.disk.api.get_disk_size', return_value=0) + @mock.patch('nova.virt.disk.api.get_disk_info', + return_value=mock.Mock(disk_size=0)) @mock.patch('nova.virt.libvirt.storage.lvm.get_volume_size', return_value='fake-size') def test_detach_encrypted_volumes(self, mock_get_volume_size, @@ -8844,12 +8845,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, return mock_virDomain mock_lookup.side_effect = mock_lookup_side_effect - mock_getsize = mock.Mock() - mock_getsize.return_value = "10737418240" - mock_get_virtual_size = mock.Mock() - mock_get_virtual_size.return_value = "10737418240" - - return (mock_getsize, mock_get_virtual_size, mock_lookup) + mock_qemu_img_info = mock.Mock(disk_size=10737418240, + virtual_size=10737418240) + return (mock_qemu_img_info, mock_lookup) def test_is_shared_block_storage_rbd(self): self.flags(images_type='rbd', group='libvirt') @@ -8942,7 +8940,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, {'connection_info': 'info', 'mount_device': '/dev/vda'}]} instance = objects.Instance(**self.test_instance) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - (mock_getsize, mock_get_virtual_size, mock_lookup) =\ + (mock_qemu_img_info, mock_lookup) =\ self._is_shared_block_storage_test_create_mocks(disks) data = objects.LibvirtLiveMigrateData(is_volume_backed=True, is_shared_instance_path=False) @@ -8966,21 +8964,18 @@ class LibvirtConnTestCase(test.NoDBTestCase, {'connection_info': 'info', 'mount_device': '/dev/vda'}]} instance = objects.Instance(**self.test_instance) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - (mock_getsize, mock_get_virtual_size, mock_lookup) =\ + (mock_qemu_img_info, mock_lookup) =\ self._is_shared_block_storage_test_create_mocks(disks) data = objects.LibvirtLiveMigrateData(is_volume_backed=True, is_shared_instance_path=False) with test.nested( mock.patch.object(libvirt_driver.disk_api, - 'get_allocated_disk_size', mock_getsize), - mock.patch.object(libvirt_driver.disk_api, - 'get_disk_size', mock_get_virtual_size), + 'get_disk_info', mock_qemu_img_info), mock.patch.object(host.Host, '_get_domain', mock_lookup)): self.assertFalse(drvr._is_shared_block_storage( instance, data, block_device_info = bdi)) - mock_getsize.assert_called_once_with('/instance/disk.local') - mock_get_virtual_size.assert_called_once_with('/instance/disk.local') + mock_qemu_img_info.assert_called_once_with('/instance/disk.local') mock_lookup.assert_called_once_with(instance) def test_is_shared_block_storage_nfs(self): @@ -11490,7 +11485,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, migrate_data=migrate_data) self.assertEqual(['cmt'], res.supported_perf_events) - def test_get_instance_disk_info_works_correctly(self): + @mock.patch('nova.virt.disk.api.get_disk_info') + def test_get_instance_disk_info_works_correctly(self, mock_qemu_img_info): # Test data instance = objects.Instance(**self.test_instance) dummyxml = ("instance-0000000a" @@ -11504,9 +11500,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, "") # Preparing mocks - vdmock = self.mox.CreateMock(fakelibvirt.virDomain) - self.mox.StubOutWithMock(vdmock, "XMLDesc") - vdmock.XMLDesc(0).AndReturn(dummyxml) + vdmock = mock.Mock(autospec=fakelibvirt.virDomain) + vdmock.XMLDesc.return_value = dummyxml + + mock_qemu_img_info.side_effect = [ + mock.Mock(disk_size=10737418240, virtual_size=10737418240), + mock.Mock(disk_size=3328599655, virtual_size=21474836480) + ] def fake_lookup(_uuid): if _uuid == instance.uuid: @@ -11517,21 +11517,6 @@ class LibvirtConnTestCase(test.NoDBTestCase, fake_libvirt_utils.disk_sizes['/test/disk.local'] = 20 * units.Gi fake_libvirt_utils.disk_backing_files['/test/disk.local'] = 'file' - self.mox.StubOutWithMock(libvirt_driver.disk_api, - 'get_allocated_disk_size') - self.mox.StubOutWithMock(libvirt_driver.disk_api, 'get_disk_size') - - path = '/test/disk' - size = 10737418240 - libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) - libvirt_driver.disk_api.get_disk_size(path).AndReturn((size)) - path = '/test/disk.local' - size = 3328599655 - vsize = 21474836480 - libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) - libvirt_driver.disk_api.get_disk_size(path).AndReturn((vsize)) - - self.mox.ReplayAll() drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) info = drvr.get_instance_disk_info(instance) info = jsonutils.loads(info) @@ -11546,6 +11531,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(info[1]['backing_file'], "file") self.assertEqual(info[1]['over_committed_disk_size'], 18146236825) + vdmock.XMLDesc.assert_called_once_with(0) + mock_qemu_img_info.assert_has_calls([mock.call('/test/disk'), + mock.call('/test/disk.local')]) + self.assertEqual(2, mock_qemu_img_info.call_count) + def test_post_live_migration(self): vol = {'block_device_mapping': [ {'attachment_id': None, @@ -11631,7 +11621,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance) _test() - def test_get_instance_disk_info_excludes_volumes(self): + @mock.patch('nova.virt.disk.api.get_disk_info') + def test_get_instance_disk_info_excludes_volumes( + self, mock_qemu_img_info): # Test data instance = objects.Instance(**self.test_instance) dummyxml = ("instance-0000000a" @@ -11651,9 +11643,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, "") # Preparing mocks - vdmock = self.mox.CreateMock(fakelibvirt.virDomain) - self.mox.StubOutWithMock(vdmock, "XMLDesc") - vdmock.XMLDesc(0).AndReturn(dummyxml) + vdmock = mock.Mock(autospec=fakelibvirt.virDomain) + vdmock.XMLDesc.return_value = dummyxml + + mock_qemu_img_info.side_effect = [ + mock.Mock(disk_size=10737418240, virtual_size=10737418240), + mock.Mock(disk_size=3328599655, virtual_size=21474836480) + ] def fake_lookup(_uuid): if _uuid == instance.uuid: @@ -11664,21 +11660,6 @@ class LibvirtConnTestCase(test.NoDBTestCase, fake_libvirt_utils.disk_sizes['/test/disk.local'] = 20 * units.Gi fake_libvirt_utils.disk_backing_files['/test/disk.local'] = 'file' - self.mox.StubOutWithMock(libvirt_driver.disk_api, - 'get_allocated_disk_size') - self.mox.StubOutWithMock(libvirt_driver.disk_api, 'get_disk_size') - - path = '/test/disk' - size = 10737418240 - libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) - libvirt_driver.disk_api.get_disk_size(path).AndReturn((size)) - path = '/test/disk.local' - size = 3328599655 - vsize = 21474836480 - libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) - libvirt_driver.disk_api.get_disk_size(path).AndReturn((vsize)) - - self.mox.ReplayAll() conn_info = {'driver_volume_type': 'fake'} info = {'block_device_mapping': [ {'connection_info': conn_info, 'mount_device': '/dev/vdc'}, @@ -11698,7 +11679,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(info[1]['backing_file'], "file") self.assertEqual(info[1]['over_committed_disk_size'], 18146236825) - def test_get_instance_disk_info_no_bdinfo_passed(self): + vdmock.XMLDesc.assert_called_once_with(0) + mock_qemu_img_info.assert_has_calls([mock.call('/test/disk'), + mock.call('/test/disk.local')]) + self.assertEqual(2, mock_qemu_img_info.call_count) + + @mock.patch('nova.virt.disk.api.get_disk_info') + def test_get_instance_disk_info_no_bdinfo_passed(self, mock_qemu_img_info): # NOTE(ndipanov): _get_disk_overcomitted_size_total calls this method # without access to Nova's block device information. We want to make # sure that we guess volumes mostly correctly in that case as well @@ -11712,30 +11699,22 @@ class LibvirtConnTestCase(test.NoDBTestCase, "" "" "") + path = '/test/disk' + size = 10737418240 # Preparing mocks - vdmock = self.mox.CreateMock(fakelibvirt.virDomain) - self.mox.StubOutWithMock(vdmock, "XMLDesc") - vdmock.XMLDesc(0).AndReturn(dummyxml) + vdmock = mock.Mock(autospec=fakelibvirt.virDomain) + vdmock.XMLDesc.return_value = dummyxml + + mock_qemu_img_info.return_value = mock.Mock(disk_size=10737418240, + virtual_size=10737418240) def fake_lookup(_uuid): if _uuid == instance.uuid: return vdmock self.create_fake_libvirt_mock(lookupByUUIDString=fake_lookup) + fake_libvirt_utils.disk_sizes[path] = 10 * units.Gi - fake_libvirt_utils.disk_sizes['/test/disk'] = 10 * units.Gi - - self.mox.StubOutWithMock(libvirt_driver.disk_api, - "get_allocated_disk_size") - self.mox.StubOutWithMock(libvirt_driver.disk_api, - "get_disk_size") - - path = '/test/disk' - size = 10737418240 - libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) - libvirt_driver.disk_api.get_disk_size(path).AndReturn((size)) - - self.mox.ReplayAll() drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) info = drvr.get_instance_disk_info(instance) @@ -11747,6 +11726,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(info[0]['backing_file'], "") self.assertEqual(info[0]['over_committed_disk_size'], 0) + vdmock.XMLDesc.assert_called_once_with(0) + mock_qemu_img_info.assert_called_once_with(path) + def test_spawn_with_network_info(self): def fake_getLibVersion(): return fakelibvirt.FAKE_LIBVIRT_VERSION diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index cf1e13c0aa3c..075c7e765960 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -127,6 +127,15 @@ def resize2fs(image, check_exit_code=False, run_as_root=False): nova.privsep.fs.unprivileged_resize2fs(image, check_exit_code) +def get_disk_info(path): + """Get QEMU info of a disk image + + :param path: Path to the disk image + :returns: oslo_utils.imageutils.QemuImgInfo object for the image. + """ + return images.qemu_img_info(path) + + def get_disk_size(path): """Get the (virtual) size of a disk image diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index c8ac4e23ecca..25d528208dd5 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -7857,6 +7857,7 @@ class LibvirtDriver(driver.ComputeDriver): # get the real disk size or # raise a localized error if image is unavailable if disk_type == 'file': + qemu_img_info = disk_api.get_disk_info(path) if driver_type == 'ploop': dk_size = 0 for dirpath, dirnames, filenames in os.walk(path): @@ -7864,10 +7865,10 @@ class LibvirtDriver(driver.ComputeDriver): fp = os.path.join(dirpath, f) dk_size += os.path.getsize(fp) else: - dk_size = disk_api.get_allocated_disk_size(path) + dk_size = qemu_img_info.disk_size # NOTE(lyarwood): Fetch the virtual size for all file disks. - virt_size = disk_api.get_disk_size(path) + virt_size = qemu_img_info.virtual_size elif disk_type == 'block' and block_device_info: # FIXME(lyarwood): There's no reason to use a separate call