From cd862fe8ef52719422940e05955ffffb6b073b96 Mon Sep 17 00:00:00 2001
From: Lee Yarwood <lyarwood@redhat.com>
Date: Thu, 17 May 2018 09:47:58 +0100
Subject: [PATCH] libvirt: Skip fetching the virtual size of block devices

In this latest episode of `Which CI job has lyarwood broken today?!` we
find that I464bc2b88123a012cd12213beac4b572c3c20a56 introduced a
regression in the nova-lvm experimental job as n-cpu attempted to run
qemu-img info against block devices as an unprivileged user.

For the time being we should skip any attempt to use this command
against block devices until the disk_api layer can make privileged
calls using privsep.

Conflicts:
        nova/virt/libvirt/driver.py
        nova/tests/unit/virt/libvirt/test_driver.py

NOTE(lyarwood): Conflicts due to the substantial refactoring of
_get_instance_disk_info via I9616a602ee0605f7f1dc1f47b6416f01895e025b,
for this change the test has been extended to provide valid XML via the
config classes.

Closes-bug: #1771700
Change-Id: I9653f81ec716f80eb638810f65e2d3cdfeedaa22
(cherry picked from commit fda48219a378d09a9a363078ba161d7f54e32c0a)
(cherry picked from commit 8ea98c56b647526aae7a786531e934eeee7a90a2)
(cherry picked from commit 43cac615f6a0a4399c7bf3dda6c2595749f27ace)
---
 nova/tests/unit/virt/libvirt/test_driver.py | 46 +++++++++++++++++++++
 nova/virt/libvirt/driver.py                 | 14 +++++--
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index f34f03bf8d20..2245d5396666 100755
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -12982,6 +12982,52 @@ class LibvirtConnTestCase(test.NoDBTestCase):
         self.assertRaises(exception.DiskNotFound,
                           drvr._get_disk_over_committed_size_total)
 
+    @mock.patch('nova.virt.libvirt.storage.lvm.get_volume_size')
+    @mock.patch('nova.virt.disk.api.get_disk_size',
+                new_callable=mock.NonCallableMock)
+    def test_get_instance_disk_info_block_devices(self,
+            mock_disk_api, mock_get_volume_size):
+        """Test that for block devices the actual and virtual sizes are
+        reported as the same and that the disk_api is not used.
+        """
+        c = context.get_admin_context()
+        instance = objects.Instance(root_device_name='/dev/vda',
+                                    **self.test_instance)
+        bdms = objects.BlockDeviceMappingList(objects=[
+            fake_block_device.fake_bdm_object(c, {
+                'device_name': '/dev/mapper/vg-lv',
+                'source_type': 'image',
+                'destination_type': 'local'
+            }),
+
+        ])
+        block_device_info = driver.get_block_device_info(instance, bdms)
+
+        config = vconfig.LibvirtConfigGuest()
+        config.virt_type = 'kvm'
+        disk_config = vconfig.LibvirtConfigGuestDisk()
+        disk_config.driver_name = 'qemu'
+        disk_config.driver_format = 'raw'
+        disk_config.driver_cache = 'none'
+        disk_config.source_type = 'block'
+        disk_config.source_path = '/dev/sda'
+        disk_config.bus = 'scsi'
+        disk_config.target_dev = '/dev/xda'
+        disk_config.target_bus = 'scsi'
+        disk_config.serial = uuids.serial
+
+        config.devices.append(disk_config)
+
+        mock_get_volume_size.return_value = mock.sentinel.volume_size
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        disk_info = drvr._get_instance_disk_info(instance.name,
+                         config.to_xml(), block_device_info=block_device_info)
+
+        mock_get_volume_size.assert_called_once_with('/dev/sda')
+        self.assertEqual(disk_info[0]['disk_size'],
+                         disk_info[0]['virt_disk_size'])
+
     def test_cpu_info(self):
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
 
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 3ca158aaa6f6..b0e4395b2dd8 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -7186,17 +7186,25 @@ class LibvirtDriver(driver.ComputeDriver):
                             dk_size += os.path.getsize(fp)
                 else:
                     dk_size = disk_api.get_allocated_disk_size(path)
+
+                # NOTE(lyarwood): Fetch the virtual size for all file disks.
+                virt_size = disk_api.get_disk_size(path)
+
             elif disk_type == 'block' and block_device_info:
+                # FIXME(lyarwood): There's no reason to use a separate call
+                # here, once disk_api uses privsep this should be removed along
+                # with the surrounding conditionals to simplify this mess.
                 dk_size = lvm.get_volume_size(path)
+                # NOTE(lyarwood): As above, we should be using disk_api to
+                # fetch the virt-size but can't as it currently runs qemu-img
+                # as an unprivileged user, causing a failure for block devices.
+                virt_size = dk_size
             else:
                 LOG.debug('skipping disk %(path)s (%(target)s) - unable to '
                           'determine if volume',
                           {'path': path, 'target': target})
                 continue
 
-            # NOTE(lyarwood): Always fetch the virtual size for all disk types.
-            virt_size = disk_api.get_disk_size(path)
-
             disk_type = driver_nodes[cnt].get('type')
 
             if disk_type in ("qcow2", "ploop"):