From 0717d064a50ae489a2db75d665c811e9e12c37f7 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 2 Oct 2015 13:00:43 -0700 Subject: [PATCH] libvirt: check if ImageMeta.disk_format is set before accessing it Commit 4f43bbafa4bfc2ab149d5db4eb55320c1602658b changed blockinfo.get_root_info to access the disk_format attribute on the passed in image_meta (ImageMeta object) like an object, but if that attribute is not set you'll get an error. image_meta can be an empty object in the case of evacuate where image_ref is passed as None to the compute manager's rebuild_instance method in the case of evacuate, so image_meta defaults to {}. So when the ImageMeta object is created in LibvirtDriver.spawn, there is nothing in it. We have to check if the attribute is set before accessing it using obj_attr_is_set because the ImageMeta class does not include the NovaObjectDictCompat mixin, so we can't use image_meta.get('disk_format'). Closes-Bug: #1501831 Change-Id: I95e543415b881603bb997ae0e159017c03e58a53 --- .../tests/unit/virt/libvirt/test_blockinfo.py | 20 +++++++++++++++++++ nova/virt/libvirt/blockinfo.py | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/nova/tests/unit/virt/libvirt/test_blockinfo.py b/nova/tests/unit/virt/libvirt/test_blockinfo.py index 0116620fb66e..54540d33123f 100644 --- a/nova/tests/unit/virt/libvirt/test_blockinfo.py +++ b/nova/tests/unit/virt/libvirt/test_blockinfo.py @@ -868,6 +868,26 @@ class LibvirtBlockInfoTest(test.NoDBTestCase): 'virtio', 'ide', root_device_name='/dev/vda') mock_get_bus.assert_called_once_with('kvm', '/dev/vda') + @mock.patch('nova.virt.libvirt.blockinfo.find_disk_dev_for_disk_bus', + return_value='vda') + @mock.patch('nova.virt.libvirt.blockinfo.get_disk_bus_for_disk_dev', + return_value='virtio') + def test_get_root_info_no_bdm_empty_image_meta(self, mock_get_bus, + mock_find_dev): + # The evacuate operation passes image_ref=None to the compute node for + # rebuild which then defaults image_meta to {}, so we don't have any + # attributes in the ImageMeta object passed to get_root_info and we + # need to make sure we don't try lazy-loading anything. + instance = objects.Instance(**self.test_instance) + image_meta = objects.ImageMeta.from_dict({}) + blockinfo.get_root_info(instance, 'kvm', image_meta, None, + 'virtio', 'ide') + mock_find_dev.assert_called_once_with({}, 'virtio') + + blockinfo.get_root_info(instance, 'kvm', image_meta, None, + 'virtio', 'ide', root_device_name='/dev/vda') + mock_get_bus.assert_called_once_with('kvm', '/dev/vda') + @mock.patch('nova.virt.libvirt.blockinfo.get_info_from_bdm') def test_get_root_info_bdm(self, mock_get_info): instance = objects.Instance(**self.test_instance) diff --git a/nova/virt/libvirt/blockinfo.py b/nova/virt/libvirt/blockinfo.py index 28390c658f87..f0012e6d8a73 100644 --- a/nova/virt/libvirt/blockinfo.py +++ b/nova/virt/libvirt/blockinfo.py @@ -429,7 +429,11 @@ def get_root_info(instance, virt_type, image_meta, root_bdm, root_bdm.get('source_type') == 'image' and root_bdm.get('destination_type') == 'local')) if no_root_bdm: - if image_meta.disk_format == 'iso': + # NOTE(mriedem): In case the image_meta object was constructed from + # an empty dict, like in the case of evacuate, we have to first check + # if disk_format is set on the ImageMeta object. + if (image_meta.obj_attr_is_set('disk_format') and + image_meta.disk_format == 'iso'): root_device_bus = cdrom_bus root_device_type = 'cdrom' else: