From d0ff6363d477da3be179e77d9958e6c8f09b459d Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 4 Jan 2016 11:21:01 -0800 Subject: [PATCH] libvirt: only get instance.flavor if needed in get_disk_mapping When attaching a volume, the compute API gets the instance with the default extra fields, which does not include flavor. So get_disk_mapping is lazy-loading the flavor in this case just to get the swap information from the flavor, which is only done if it's not already in the block_device_info dict passed in. So this change simply moves the instance.get_flavor() call to where it's needed to avoid a potentially unnecessary round-trip to the database to lazy-load the instance.flavor attribute. Change-Id: I316c3a917fcd8ac543c7990657fdfeb96f9a2eb8 Closes-Bug: #1530952 --- .../tests/unit/virt/libvirt/test_blockinfo.py | 23 +++++++++++++------ nova/virt/libvirt/blockinfo.py | 4 +--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_blockinfo.py b/nova/tests/unit/virt/libvirt/test_blockinfo.py index d1c0a7cdfaa3..4e7736724d03 100644 --- a/nova/tests/unit/virt/libvirt/test_blockinfo.py +++ b/nova/tests/unit/virt/libvirt/test_blockinfo.py @@ -165,9 +165,14 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): instance_ref = objects.Instance(**self.test_instance) image_meta = objects.ImageMeta.from_dict(self.test_image_meta) - mapping = blockinfo.get_disk_mapping("kvm", instance_ref, - "virtio", "ide", - image_meta) + with mock.patch.object(instance_ref, 'get_flavor', + return_value=instance_ref.flavor) as get_flavor: + mapping = blockinfo.get_disk_mapping("kvm", instance_ref, + "virtio", "ide", + image_meta) + # Since there was no block_device_info passed to get_disk_mapping we + # expect to get the swap info from the flavor in the instance. + get_flavor.assert_called_once_with() expect = { 'disk': {'bus': 'virtio', 'dev': 'vda', @@ -657,10 +662,14 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): 'disk_bus': 'virtio', 'delete_on_termination': True} - blockinfo.get_disk_mapping("kvm", instance_ref, - "virtio", "ide", - image_meta, - block_device_info) + with mock.patch.object(instance_ref, 'get_flavor') as get_flavor_mock: + blockinfo.get_disk_mapping("kvm", instance_ref, + "virtio", "ide", + image_meta, + block_device_info) + # we should have gotten the swap info from block_device_info rather + # than the flavor information on the instance + self.assertFalse(get_flavor_mock.called) self.assertEqual(expected_swap, block_device_info['swap']) self.assertEqual(expected_ephemeral, diff --git a/nova/virt/libvirt/blockinfo.py b/nova/virt/libvirt/blockinfo.py index feaa352c620f..ab93fcad141d 100644 --- a/nova/virt/libvirt/blockinfo.py +++ b/nova/virt/libvirt/blockinfo.py @@ -520,8 +520,6 @@ def get_disk_mapping(virt_type, instance, return mapping - inst_type = instance.get_flavor() - pre_assigned_device_names = \ [block_device.strip_dev(get_device_name(bdm)) for bdm in itertools.chain( driver.block_device_info_get_ephemerals(block_device_info), @@ -577,7 +575,7 @@ def get_disk_mapping(virt_type, instance, swap, mapping, disk_bus) mapping['disk.swap'] = swap_info update_bdm(swap, swap_info) - elif inst_type['swap'] > 0: + elif instance.get_flavor()['swap'] > 0: swap_info = get_next_disk_info(mapping, disk_bus, assigned_devices=pre_assigned_device_names) if not block_device.volume_in_mapping(swap_info['dev'],