From 87883c13750a149ef6e6783a66d1a86472c842d5 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Thu, 20 Sep 2018 14:06:29 +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. NOTE(s10): Another conflict was due to not having change I11e329ac5f5fe4b9819fefbcc32ff1ee504fc58b in Pike. Related-Bug: #1785827 Change-Id: Ibfd0527ed79f60282b542034d7cb97b424becba3 (cherry picked from commit d41ea9d87894111051b591d513cd046e5e357124) (cherry picked from commit d085c8fe536e28ff8a3d757c8407fce37ffc00e4) --- nova/tests/unit/virt/libvirt/test_driver.py | 121 +++++++++----------- nova/virt/disk/api.py | 9 ++ nova/virt/libvirt/driver.py | 5 +- 3 files changed, 63 insertions(+), 72 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 99e73c162dfb..c9d4a1c24fb9 100755 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -4300,7 +4300,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, @@ -8342,12 +8343,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') @@ -8440,7 +8438,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) @@ -8464,22 +8462,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(os.path, 'getsize', mock_getsize), 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): @@ -10915,7 +10909,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" @@ -10929,9 +10924,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: @@ -10942,21 +10941,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) @@ -10971,6 +10955,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': [ {'connection_info': { @@ -11014,7 +11003,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, inst_ref), mock.call({'data': {}}, 'sdb', inst_ref)]) - 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" @@ -11034,9 +11025,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: @@ -11047,21 +11042,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'}, @@ -11081,7 +11061,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 @@ -11095,30 +11081,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) @@ -11130,6 +11108,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 56e62cdeda94..5b0c0876d2ed 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -136,6 +136,15 @@ def resize2fs(image, check_exit_code=False, run_as_root=False): run_as_root=run_as_root) +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 a3cf62c89979..6df469e43edf 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -7373,6 +7373,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): @@ -7380,10 +7381,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