Merge "Fix conversion of config disks to qcow2 during resize/migration" into stable/mitaka

2 files changed, 58 insertions, 28 deletions

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 6f3fe95365..2857264170 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -13846,14 +13846,20 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
                                 **inst)
 
     @staticmethod
-    def _disk_info():
+    def _disk_info(type='qcow2', config_disk=False):
         # 10G root and 512M swap disk
-        disk_info = [{'disk_size': 1, 'type': 'qcow2',
+        disk_info = [{'disk_size': 1, 'type': type,
                       'virt_disk_size': 10737418240, 'path': '/test/disk',
                       'backing_file': '/base/disk'},
-                     {'disk_size': 1, 'type': 'qcow2',
+                     {'disk_size': 1, 'type': type,
                       'virt_disk_size': 536870912, 'path': '/test/disk.swap',
                       'backing_file': '/base/swap_512'}]
+
+        if config_disk:
+            disk_info.append({'disk_size': 1, 'type': 'raw',
+                              'virt_disk_size': 1024,
+                              'path': '/test/disk.config'})
+
         return jsonutils.dumps(disk_info)
 
     def test_migrate_disk_and_power_off_exception(self):
@@ -14300,17 +14306,15 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
     def test_disk_size_from_instance_disk_info(self):
         instance_data = {'root_gb': 10, 'ephemeral_gb': 20, 'swap_gb': 30}
         inst = objects.Instance(**instance_data)
-        info = {'path': '/path/disk'}
         self.assertEqual(10 * units.Gi,
-            self.drvr._disk_size_from_instance(inst, info))
+                         self.drvr._disk_size_from_instance(inst, 'disk'))
 
-        info = {'path': '/path/disk.local'}
         self.assertEqual(20 * units.Gi,
-            self.drvr._disk_size_from_instance(inst, info))
+                         self.drvr._disk_size_from_instance(inst,
+                                                            'disk.local'))
 
-        info = {'path': '/path/disk.swap'}
         self.assertEqual(0,
-            self.drvr._disk_size_from_instance(inst, info))
+                         self.drvr._disk_size_from_instance(inst, 'disk.swap'))
 
     @mock.patch('nova.utils.execute')
     def test_disk_raw_to_qcow2(self, mock_execute):
@@ -14446,10 +14450,25 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
         migration.source_node = 'fake-source-node'
         migration.dest_node = 'fake-dest-node'
         image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
-        self.drvr.finish_migration(
-                      context.get_admin_context(), migration, ins_ref,
-                      self._disk_info(), [], image_meta,
-                      resize_instance, None, power_on)
+
+        # Source disks are raw to test conversion
+        disk_info = self._disk_info(type='raw', config_disk=True)
+
+        with mock.patch.object(self.drvr, '_disk_raw_to_qcow2',
+                               autospec=True) as mock_raw_to_qcow2:
+            self.drvr.finish_migration(
+                          context.get_admin_context(), migration, ins_ref,
+                          disk_info, [], image_meta,
+                          resize_instance, None, power_on)
+
+            # Assert that we converted the root and swap disks
+            convert_calls = [mock.call('/test/disk'),
+                             mock.call('/test/disk.swap')]
+            mock_raw_to_qcow2.assert_has_calls(convert_calls, any_order=True)
+
+            # Implicitly assert that we did not convert the config disk
+            self.assertEqual(len(convert_calls), mock_raw_to_qcow2.call_count)
+
         self.assertTrue(self.fake_create_domain_called)
         self.assertEqual(
             resize_instance, self.fake_disk_resize_called)
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 775f5e9601..f48184a26d 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -7251,7 +7251,7 @@ class LibvirtDriver(driver.ComputeDriver):
             raise loopingcall.LoopingCallDone()
 
     @staticmethod
-    def _disk_size_from_instance(instance, info):
+    def _disk_size_from_instance(instance, disk_name):
         """Determines the disk size from instance properties
 
         Returns the disk size by using the disk name to determine whether it
@@ -7260,11 +7260,13 @@ class LibvirtDriver(driver.ComputeDriver):
 
         Returns 0 if the disk name not match (disk, disk.local).
         """
-        fname = os.path.basename(info['path'])
-        if fname == 'disk':
+        if disk_name == 'disk':
             size = instance.root_gb
-        elif fname == 'disk.local':
+        elif disk_name == 'disk.local':
             size = instance.ephemeral_gb
+        # N.B. We don't handle ephemeral disks named disk.ephN here,
+        # which is almost certainly a bug. It's not clear what this function
+        # should return if an instance has multiple ephemeral disks.
         else:
             size = 0
         return size * units.Gi
@@ -7337,16 +7339,22 @@ class LibvirtDriver(driver.ComputeDriver):
                            block_device_info=None, inject_files=False,
                            fallback_from_host=migration.source_compute)
 
-        # resize disks. only "disk" and "disk.local" are necessary.
+        # Resize root disk and a single ephemeral disk called disk.local
+        # Also convert raw disks to qcow2 if migrating to host which uses
+        # qcow2 from host which uses raw.
+        # TODO(mbooth): Handle resize of multiple ephemeral disks, and
+        #               ephemeral disks not called disk.local.
         disk_info = jsonutils.loads(disk_info)
         for info in disk_info:
-            size = self._disk_size_from_instance(instance, info)
+            path = info['path']
+            disk_name = os.path.basename(path)
+
+            size = self._disk_size_from_instance(instance, disk_name)
             if resize_instance:
-                image = imgmodel.LocalFileImage(info['path'],
-                                                info['type'])
+                image = imgmodel.LocalFileImage(path, info['type'])
                 self._disk_resize(image, size)
 
-            # NOTE(mdbooth): The 2 lines below look wrong, but are actually
+            # NOTE(mdbooth): The code below looks wrong, but is actually
             # required to prevent a security hole when migrating from a host
             # with use_cow_images=False to one with use_cow_images=True.
             # Imagebackend uses use_cow_images to select between the
@@ -7372,14 +7380,17 @@ class LibvirtDriver(driver.ComputeDriver):
             # users. It is tightly-coupled to implementation quirks of 2
             # out of 5 backends in imagebackend and defends against a severe
             # security flaw which is not at all obvious without deep analysis,
-            # and is therefore undesirable to developers. It also introduces a
-            # bug, as it converts all disks to qcow2 regardless of their
-            # intended format (config disks are always supposed to be raw). We
-            # should aim to remove it. This will not be possible, though, until
-            # we can represent the storage layout of a specific instance
+            # and is therefore undesirable to developers. We should aim to
+            # remove it. This will not be possible, though, until we can
+            # represent the storage layout of a specific instance
             # independent of the default configuration of the local compute
             # host.
-            if info['type'] == 'raw' and CONF.use_cow_images:
+
+            # Config disks are hard-coded to be raw even when
+            # use_cow_images=True (see _get_disk_config_image_type),so don't
+            # need to be converted.
+            if (disk_name != 'disk.config' and
+                        info['type'] == 'raw' and CONF.use_cow_images):
                 self._disk_raw_to_qcow2(info['path'])
 
         xml = self._get_guest_xml(context, instance, network_info,