From 184f0074cc5f4609642b52c98a7ccbfc5f892a33 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Wed, 19 Jan 2022 13:46:56 +0000 Subject: [PATCH] imagebackend: default by_name image_type to config correctly Callers of by_name shouldn't need to pass in the default from CONF. This has implications for the libvirt_imagebackend fixture. The _mock_backend() method in the libvirt_imagebackend fixture is what mocks out Backend.backend(). Before this patch, the defaulting of image_type to CONF.libvirt.images_type was done *inside* Backend.backend() whenever image_type=None was passed in. As of this patch, this is no longer case, and the default value from CONF gets passed in instead. Unit tests were testing the previous behaviour and essentially just asserting that None was passed in. This was never really correct to begin with, so a couple of assertIsNone() assertions need to be updated. Change-Id: Ib95aa440ffff0989c90f09f5b5d4e26b30db9683 --- nova/tests/unit/virt/libvirt/test_driver.py | 4 ++-- nova/virt/libvirt/driver.py | 15 ++++++++++----- nova/virt/libvirt/imagebackend.py | 4 ++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index d701506f0620..7f73e45fd76c 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -24631,7 +24631,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): self.assertEqual('raw', disk.image_type) # Assert that the root rescue disk was created as the default type - self.assertIsNone(disks['disk.rescue'].image_type) + self.assertEqual('default', disks['disk.rescue'].image_type) # We expect the generated domain to contain disk.rescue and # disk, in that order @@ -24703,7 +24703,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin): self.assertEqual('raw', disk.image_type) # Assert that the root rescue disk was created as the default type - self.assertIsNone(disks['disk.rescue'].image_type) + self.assertEqual('default', disks['disk.rescue'].image_type) # We expect the generated domain to contain disk.rescue, disk, and # disk.config.rescue in that order diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 4c584b2c58ad..8d4e31251800 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -4804,8 +4804,7 @@ class LibvirtDriver(driver.ComputeDriver): if size == 0 or suffix == '.rescue': size = None - backend = self.image_backend.by_name(instance, 'disk' + suffix, - CONF.libvirt.images_type) + backend = self.image_backend.by_name(instance, 'disk' + suffix) created_disks = not backend.exists() if instance.task_state == task_states.RESIZE_FINISH: @@ -5357,6 +5356,11 @@ class LibvirtDriver(driver.ComputeDriver): self, instance, name, disk_mapping, flavor, image_type=None, boot_order=None, ): + # NOTE(artom) To pass unit tests, wherein the code here is loaded + # *before* any config with self.flags() is done, we need to have the + # default inline in the method, and not in the kwarg declaration. + if image_type is None: + image_type = CONF.libvirt.images_type disk_unit = None disk = self.image_backend.by_name(instance, name, image_type) if (name == 'disk.config' and image_type == 'rbd' and @@ -5381,7 +5385,9 @@ class LibvirtDriver(driver.ComputeDriver): disk_unit=disk_unit, boot_order=boot_order) return conf - def _get_guest_fs_config(self, instance, name, image_type=None): + def _get_guest_fs_config( + self, instance, name, image_type=CONF.libvirt.images_type + ): disk = self.image_backend.by_name(instance, name, image_type) return disk.libvirt_fs_info("/", "ploop") @@ -10721,8 +10727,7 @@ class LibvirtDriver(driver.ComputeDriver): # Creating backing file follows same way as spawning instances. cache_name = os.path.basename(info['backing_file']) - disk = self.image_backend.by_name(instance, instance_disk, - CONF.libvirt.images_type) + disk = self.image_backend.by_name(instance, instance_disk) if cache_name.startswith('ephemeral'): # The argument 'size' is used by image.cache to # validate disk size retrieved from cache against diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 617adfe03039..155267af2317 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -1311,6 +1311,10 @@ class Backend(object): :return: An Image object for the disk with given name and instance. :rtype: Image """ + # NOTE(artom) To pass functional tests, wherein the code here is loaded + # *before* any config with self.flags() is done, we need to have the + # default inline in the method, and not in the kwarg declaration. + image_type = image_type or CONF.libvirt.images_type backend = self.backend(image_type) return backend(instance=instance, disk_name=name)