From e6f4503fe3e20c3fb8afef0aa944d4994665786b Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Wed, 24 Jul 2024 09:01:31 -0700 Subject: [PATCH] Remove AMI snapshot format special case Note that this includes seemingly-unrelated test changes because we were actually skipping the snapshot_running test for libvirt, which has been a bug for years. In that test case, when we went to look for image_meta.disk_format, that attribute was not set on the o.vo object, which raised a NotImplementedError. That error is also checked by the test to skip the test for drivers that do not support snapshot, which meant that for libvirt, we haven't been running that case beyond the point at which we create snapshot metadata and trip that exception. Thus, once removing that, there are other mocks not in place that are required for the test to actually run. So, this adds mocks for qemu_img_info() calls that actually try to read the file on disk, as well as the privsep chown() that attempts to run after. Change-Id: Ie731045629f0899840a4680d21793a16ade9b98e (cherry picked from commit d5a631ba7791b37e49213707e4ea650a56d2ed9e) (cherry picked from commit 8c5929ff5156d5409d41872f1b8ee0abb04f35a8) (cherry picked from commit d2d3b2c9e87fe2247a34a776310221c8b12be515) (cherry picked from commit 77dfa4f6f3c39048b5d3bb9eb2b14dd6998b406b) --- nova/tests/unit/virt/libvirt/test_driver.py | 11 +++++++---- nova/tests/unit/virt/test_virt_drivers.py | 5 +++++ nova/virt/libvirt/driver.py | 6 +----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index a34a20619f0..47612a6db8c 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -9242,7 +9242,7 @@ def test_unquiesce(self, mock_has_min_version): def test_create_snapshot_metadata(self): base = objects.ImageMeta.from_dict( - {'disk_format': 'raw'}) + {'disk_format': 'qcow2'}) instance_data = {'kernel_id': 'kernel', 'project_id': 'prj_id', 'ramdisk_id': 'ram_id', @@ -9274,10 +9274,12 @@ def test_create_snapshot_metadata(self): {'disk_format': 'ami', 'container_format': 'test_container'}) expected['properties']['os_type'] = instance['os_type'] - expected['disk_format'] = base.disk_format + # The disk_format of the snapshot should be the *actual* format of the + # thing we upload, regardless of what type of image we booted from. + expected['disk_format'] = img_fmt expected['container_format'] = base.container_format ret = drvr._create_snapshot_metadata(base, instance, img_fmt, snp_name) - self.assertEqual(ret, expected) + self.assertEqual(expected, ret) def test_get_volume_driver(self): conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -28778,7 +28780,8 @@ def test_ami(self): utils.get_system_metadata_from_image( {'disk_format': 'ami'}) - self._test_snapshot(disk_format='ami') + # If we're uploading a qcow2, we must set the disk_format as such + self._test_snapshot(disk_format='qcow2') @mock.patch('nova.virt.libvirt.utils.get_disk_type_from_path', new=mock.Mock(return_value=None)) diff --git a/nova/tests/unit/virt/test_virt_drivers.py b/nova/tests/unit/virt/test_virt_drivers.py index ed9f1e3822d..802ea4f027f 100644 --- a/nova/tests/unit/virt/test_virt_drivers.py +++ b/nova/tests/unit/virt/test_virt_drivers.py @@ -838,6 +838,11 @@ def setUp(self): # since we don't care about it. self.stub_out('os_vif.unplug', lambda a, kw: None) self.stub_out('nova.compute.utils.get_machine_ips', lambda: []) + self.stub_out('nova.virt.libvirt.utils.get_disk_size', + lambda *a, **k: 123456) + self.stub_out('nova.virt.libvirt.utils.get_disk_backing_file', + lambda *a, **k: None) + self.stub_out('nova.privsep.path.chown', lambda *a, **k: None) def test_init_host_image_type_rbd_force_raw_images_true(self): CONF.set_override('images_type', 'rbd', group='libvirt') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index e8bfffc3fa9..b88103862d6 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -2938,11 +2938,7 @@ def _create_snapshot_metadata(self, image_meta, instance, if instance.os_type: metadata['properties']['os_type'] = instance.os_type - # NOTE(vish): glance forces ami disk format to be ami - if image_meta.disk_format == 'ami': - metadata['disk_format'] = 'ami' - else: - metadata['disk_format'] = img_fmt + metadata['disk_format'] = img_fmt if image_meta.obj_attr_is_set("container_format"): metadata['container_format'] = image_meta.container_format