From 4a968c98c160cd86c3de32c7455140b5610e02c1 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Mon, 22 Feb 2021 23:01:41 +0000 Subject: [PATCH] libvirt: Create qcow2 disks with the correct size without extending All callers to the create_image method of the Qcow2 imagebackend provide a size and so we should just always create the initial image using that size. The name of the method is also misleading and changed to better reflect that an image is being created here and not copied. Change-Id: If2fa298c2e67266dcaf310e05bda51af4285526f --- nova/tests/unit/virt/libvirt/test_driver.py | 16 +++++++--- .../unit/virt/libvirt/test_imagebackend.py | 30 ++++--------------- nova/virt/libvirt/imagebackend.py | 12 ++------ 3 files changed, 20 insertions(+), 38 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 83facd5f4b62..f857fa3204ca 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -13616,7 +13616,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_utime.assert_called() mock_create_cow_image.assert_called_once_with( - backfile_path, '/fake/instance/dir/disk_path') + backfile_path, '/fake/instance/dir/disk_path', virt_disk_size) @mock.patch('nova.virt.libvirt.utils.create_image', new=mock.NonCallableMock()) @@ -13700,9 +13700,17 @@ class LibvirtConnTestCase(test.NoDBTestCase, # TODO(efried): Should these be disk_info[path]?? mock_create_cow_image.assert_has_calls([ - mock.call(root_backing, CONF.instances_path + '/disk'), - mock.call(ephemeral_backing, - CONF.instances_path + '/disk.local')]) + mock.call( + root_backing, + CONF.instances_path + '/disk', + disk_info_byname['disk']['virt_disk_size'] + ), + mock.call( + ephemeral_backing, + CONF.instances_path + '/disk.local', + disk_info_byname['disk.local']['virt_disk_size'] + ), + ]) def test_create_images_and_backing_disk_info_none(self): instance = objects.Instance(**self.test_instance) diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index e9f2707f735f..decb27f98249 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -524,30 +524,12 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase): @mock.patch.object(imagebackend.utils, 'synchronized') @mock.patch('nova.virt.libvirt.utils.create_cow_image') - @mock.patch.object(imagebackend.disk, 'extend') - @mock.patch('nova.privsep.path.utime') - def test_create_image(self, mock_utime, mock_extend, mock_create, - mock_sync): - mock_sync.side_effect = lambda *a, **kw: self._fake_deco - fn = mock.MagicMock() - image = self.image_class(self.INSTANCE, self.NAME) - - image.create_image(fn, self.TEMPLATE_PATH, None) - - mock_create.assert_called_once_with(self.TEMPLATE_PATH, self.PATH) - fn.assert_called_once_with(target=self.TEMPLATE_PATH) - self.assertTrue(mock_sync.called) - self.assertFalse(mock_extend.called) - mock_utime.assert_called() - - @mock.patch.object(imagebackend.utils, 'synchronized') - @mock.patch('nova.virt.libvirt.utils.create_cow_image') - @mock.patch.object(imagebackend.disk, 'extend') @mock.patch.object(os.path, 'exists', side_effect=[]) @mock.patch.object(imagebackend.Image, 'verify_base_size') @mock.patch('nova.privsep.path.utime') - def test_create_image_with_size(self, mock_utime, mock_verify, mock_exist, - mock_extend, mock_create, mock_sync): + def test_create_image( + self, mock_utime, mock_verify, mock_exist, mock_create, mock_sync + ): mock_sync.side_effect = lambda *a, **kw: self._fake_deco fn = mock.MagicMock() mock_exist.side_effect = [False, True, False, False, False] @@ -561,10 +543,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase): image.create_image(fn, self.TEMPLATE_PATH, self.SIZE) mock_verify.assert_called_once_with(self.TEMPLATE_PATH, self.SIZE) - mock_create.assert_called_once_with(self.TEMPLATE_PATH, self.PATH) - mock_extend.assert_called_once_with( - imgmodel.LocalFileImage(self.PATH, imgmodel.FORMAT_QCOW2), - self.SIZE) + mock_create.assert_called_once_with( + self.TEMPLATE_PATH, self.PATH, self.SIZE) fn.assert_called_once_with(target=self.TEMPLATE_PATH) mock_exist.assert_has_calls(exist_calls) self.assertTrue(mock_sync.called) diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 05b3a8069a90..08bad6948937 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -624,14 +624,8 @@ class Qcow2(Image): filename = self._get_lock_name(base) @utils.synchronized(filename, external=True, lock_path=self.lock_path) - def copy_qcow2_image(base, target, size): - # TODO(pbrady): Consider copying the cow image here - # with preallocation=metadata set for performance reasons. - # This would be keyed on a 'preallocate_images' setting. - libvirt_utils.create_cow_image(base, target) - if size: - image = imgmodel.LocalFileImage(target, imgmodel.FORMAT_QCOW2) - disk.extend(image, size) + def create_qcow2_image(base, target, size): + libvirt_utils.create_cow_image(base, target, size) # Download the unmodified base image unless we already have a copy. if not os.path.exists(base): @@ -670,7 +664,7 @@ class Qcow2(Image): if not os.path.exists(self.path): with fileutils.remove_path_on_error(self.path): - copy_qcow2_image(base, self.path, size) + create_qcow2_image(base, self.path, size) def resize_image(self, size): image = imgmodel.LocalFileImage(self.path, imgmodel.FORMAT_QCOW2)