From caca71376fbc2c1195cda71608c8d2a70ef9a678 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Mon, 16 Apr 2018 18:08:56 +0100 Subject: [PATCH] libvirt: Report the allocated size of preallocated file based disks At present the Libvirt driver can preallocate file based disks using the fallocate command and importantly the `-n` option. This option allocates blocks on the filesystem past the initial EOF of a given file: ``` $ touch test.img ; fallocate -n -l $(( 1024 * 1024 )) test.img $ ll -lah test.img -rw-rw-r--. 1 stack stack 0 Apr 16 13:28 test.img $ du -h test.img 1.0M test.img ``` This results in a miscalculation of the total disk overcommit for file based (excluding ploop) disks as os.path.getsize is currently used to determine the allocated size of these disks: ``` >>> import os >>> os.path.getsize('test.img') 0 ``` Using the above example the disk overcommit would be reported as 1.0M as the disk appears empty yet will report a potential (virtual) size of 1.0M. However as the required blocks have already been allocated on the filesystem the host will report disk_available_least as missing an additional 1.0M, essentially doubling the allocation for each disk. To correct this the allocated size of file based (excluding ploop) disks is reported using `disk_size` from the `qemu-img info` command. This should ensure blocks allocated past the EOF of the file are taken into account and correctly reported as allocated. A future change should ultimately remove the use of the `-n` option with fallocate, however as this would not help disks that have already been allocated this has not been included in this change to simplify backports. Conflicts: nova/tests/unit/virt/libvirt/test_driver.py NOTE(lyarwood): I11e329ac5f5fe4b9819fefbcc32ff1ee504fc58b made get_domain private in Queens. Change-Id: If642e51a4e186833349a8e30b04224a3687f5594 Closes-bug: #1764489 (cherry picked from commit 23bd8f62634707fc9896a38ff4dae606c89c6c4b) (cherry picked from commit 2d50f6e7854543849c4cb7641ae6b88fe04cb6f6) (cherry picked from commit d88b75e81eabfbd463007f6a4f27e6966a466530) --- nova/tests/unit/virt/libvirt/test_driver.py | 40 ++++++++++++++------- nova/virt/disk/api.py | 10 ++++++ nova/virt/libvirt/driver.py | 2 +- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index db1a692801f1..998fb0cd5d77 100755 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -4025,8 +4025,9 @@ class LibvirtConnTestCase(test.NoDBTestCase): @mock.patch.object(dmcrypt, 'delete_volume') @mock.patch.object(conn._host, 'get_domain', return_value=dom) - def detach_encrypted_volumes(block_device_info, mock_get_domain, - mock_delete_volume): + @mock.patch.object(libvirt_driver.disk_api, 'get_allocated_disk_size') + def detach_encrypted_volumes(block_device_info, mock_get_alloc_size, + mock_get_domain, mock_delete_volume): conn._detach_encrypted_volumes(instance, block_device_info) mock_get_domain.assert_called_once_with(instance) @@ -7689,6 +7690,8 @@ class LibvirtConnTestCase(test.NoDBTestCase): is_shared_instance_path=False) with test.nested( mock.patch.object(os.path, 'getsize', mock_getsize), + mock.patch.object(libvirt_driver.disk_api, + 'get_allocated_disk_size', mock_getsize), mock.patch.object(host.Host, 'get_domain', mock_lookup)): self.assertFalse(drvr._is_shared_block_storage( instance, data, @@ -10149,9 +10152,14 @@ class LibvirtConnTestCase(test.NoDBTestCase): fake_libvirt_utils.disk_sizes['/test/disk.local'] = 20 * units.Gi fake_libvirt_utils.disk_backing_files['/test/disk.local'] = 'file' - self.mox.StubOutWithMock(os.path, "getsize") - os.path.getsize('/test/disk').AndReturn((10737418240)) - os.path.getsize('/test/disk.local').AndReturn((3328599655)) + self.mox.StubOutWithMock(libvirt_driver.disk_api, + 'get_allocated_disk_size') + path = '/test/disk' + size = 10737418240 + libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) + path = '/test/disk.local' + size = 3328599655 + libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) ret = ("image: /test/disk.local\n" "file format: qcow2\n" @@ -10258,9 +10266,14 @@ class LibvirtConnTestCase(test.NoDBTestCase): fake_libvirt_utils.disk_sizes['/test/disk.local'] = 20 * units.Gi fake_libvirt_utils.disk_backing_files['/test/disk.local'] = 'file' - self.mox.StubOutWithMock(os.path, "getsize") - os.path.getsize('/test/disk').AndReturn((10737418240)) - os.path.getsize('/test/disk.local').AndReturn((3328599655)) + self.mox.StubOutWithMock(libvirt_driver.disk_api, + 'get_allocated_disk_size') + path = '/test/disk' + size = 10737418240 + libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) + path = '/test/disk.local' + size = 3328599655 + libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) ret = ("image: /test/disk.local\n" "file format: qcow2\n" @@ -10324,8 +10337,11 @@ class LibvirtConnTestCase(test.NoDBTestCase): fake_libvirt_utils.disk_sizes['/test/disk'] = 10 * units.Gi - self.mox.StubOutWithMock(os.path, "getsize") - os.path.getsize('/test/disk').AndReturn((10737418240)) + self.mox.StubOutWithMock(libvirt_driver.disk_api, + "get_allocated_disk_size") + path = '/test/disk' + size = 10737418240 + libvirt_driver.disk_api.get_allocated_disk_size(path).AndReturn((size)) self.mox.ReplayAll() drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -10334,8 +10350,8 @@ class LibvirtConnTestCase(test.NoDBTestCase): info = jsonutils.loads(info) self.assertEqual(1, len(info)) self.assertEqual(info[0]['type'], 'raw') - self.assertEqual(info[0]['path'], '/test/disk') - self.assertEqual(info[0]['disk_size'], 10737418240) + self.assertEqual(info[0]['path'], path) + self.assertEqual(info[0]['disk_size'], size) self.assertEqual(info[0]['backing_file'], "") self.assertEqual(info[0]['over_committed_disk_size'], 0) diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index 94933cf10742..acfcffed992d 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -148,6 +148,16 @@ def get_disk_size(path): return images.qemu_img_info(path).virtual_size +def get_allocated_disk_size(path): + """Get the allocated size of a disk image + + :param path: Path to the disk image + :returns: Size (in bytes) of the given disk image as allocated on the + filesystem + """ + return images.qemu_img_info(path).disk_size + + def extend(image, size): """Increase image to size. diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 2c56ea0269f4..ec1d1917eaea 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -7112,7 +7112,7 @@ class LibvirtDriver(driver.ComputeDriver): fp = os.path.join(dirpath, f) dk_size += os.path.getsize(fp) else: - dk_size = int(os.path.getsize(path)) + dk_size = disk_api.get_allocated_disk_size(path) elif disk_type == 'block' and block_device_info: dk_size = lvm.get_volume_size(path) else: