From 42cae28241cd0c213201d036bfbe13fb118e4bee Mon Sep 17 00:00:00 2001
From: Cyril Roelandt <cyril.roelandt@enovance.com>
Date: Mon, 18 Aug 2014 17:45:35 +0000
Subject: [PATCH] libvirt: Make sure volumes are well detected during block
 migration

Current implementation of live migration in libvirt incorrectly includes
block devices on shared storage (e.g., NFS) when computing destination
storage requirements. Since these volumes are already on shared storage
they do not need to be migrated. As a result, migration fails if the
amount of free space on the shared drive is less than the size of the
volume to be migrated. The problem is addressed by adding a
block_device_info parameter to check_can_live_migrate_source() to allow
volumes to be filtered correctly when computing migration space
requirements.

This only fixes the issue on libvirt: it is unclear whether other
implementations suffer from the same issue.

Thanks to Florent Flament for spotting and fixing an issue while trying out
this patch.

Co-Authored-By: Florent Flament <florent.flament@cloudwatt.com>
Change-Id: Iac7d2cd2a70800fd89864463ca45c030c47411b0
Closes-Bug: #1356552
(cherry picked from commit 671aa9f8b7ca5274696f83bde0d4822ee431b837)
---
 nova/compute/manager.py                |  5 ++++-
 nova/tests/compute/test_compute_mgr.py |  8 +++++++-
 nova/tests/virt/libvirt/test_driver.py |  7 ++++---
 nova/virt/driver.py                    |  3 ++-
 nova/virt/fake.py                      |  2 +-
 nova/virt/hyperv/driver.py             |  2 +-
 nova/virt/libvirt/driver.py            | 13 +++++++++----
 nova/virt/xenapi/driver.py             |  3 ++-
 8 files changed, 30 insertions(+), 13 deletions(-)

Index: nova-2014.2.3.dev5/nova/compute/manager.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/compute/manager.py
+++ nova-2014.2.3.dev5/nova/compute/manager.py
@@ -4883,8 +4883,11 @@ class ComputeManager(manager.Manager):
         is_volume_backed = self.compute_api.is_volume_backed_instance(ctxt,
                                                                       instance)
         dest_check_data['is_volume_backed'] = is_volume_backed
+        block_device_info = self._get_instance_block_device_info(
+                            ctxt, instance, refresh_conn_info=True)
         return self.driver.check_can_live_migrate_source(ctxt, instance,
-                                                         dest_check_data)
+                                                         dest_check_data,
+                                                         block_device_info)
 
     @object_compat
     @wrap_exception()
Index: nova-2014.2.3.dev5/nova/tests/compute/test_compute_mgr.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/tests/compute/test_compute_mgr.py
+++ nova-2014.2.3.dev5/nova/tests/compute/test_compute_mgr.py
@@ -1229,13 +1229,19 @@ class ComputeManagerUnitTestCase(test.No
 
         self.mox.StubOutWithMock(self.compute.compute_api,
                                  'is_volume_backed_instance')
+        self.mox.StubOutWithMock(self.compute,
+                                 '_get_instance_block_device_info')
         self.mox.StubOutWithMock(self.compute.driver,
                                  'check_can_live_migrate_source')
 
         self.compute.compute_api.is_volume_backed_instance(
                 self.context, instance).AndReturn(is_volume_backed)
+        self.compute._get_instance_block_device_info(
+                self.context, instance, refresh_conn_info=True
+                ).AndReturn({'block_device_mapping': 'fake'})
         self.compute.driver.check_can_live_migrate_source(
-                self.context, instance, expected_dest_check_data)
+                self.context, instance, expected_dest_check_data,
+                {'block_device_mapping': 'fake'})
 
         self.mox.ReplayAll()
 
Index: nova-2014.2.3.dev5/nova/tests/virt/libvirt/test_driver.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/tests/virt/libvirt/test_driver.py
+++ nova-2014.2.3.dev5/nova/tests/virt/libvirt/test_driver.py
@@ -5323,7 +5323,7 @@ class LibvirtConnTestCase(test.TestCase)
         self.mox.StubOutWithMock(conn, "_assert_dest_node_has_enough_disk")
         conn._assert_dest_node_has_enough_disk(
             self.context, instance, dest_check_data['disk_available_mb'],
-            False)
+            False, None)
 
         self.mox.ReplayAll()
         ret = conn.check_can_live_migrate_source(self.context, instance,
@@ -5386,8 +5386,9 @@ class LibvirtConnTestCase(test.TestCase)
                 disk_available_mb=0)
 
         self.mox.StubOutWithMock(conn, "get_instance_disk_info")
-        conn.get_instance_disk_info(instance["name"]).AndReturn(
-                                            '[{"virt_disk_size":2}]')
+        conn.get_instance_disk_info(instance["name"],
+                                    block_device_info=None).AndReturn(
+                                        '[{"virt_disk_size":2}]')
 
         self.mox.ReplayAll()
         self.assertRaises(exception.MigrationError,
Index: nova-2014.2.3.dev5/nova/virt/driver.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/virt/driver.py
+++ nova-2014.2.3.dev5/nova/virt/driver.py
@@ -808,7 +808,7 @@ class ComputeDriver(object):
         raise NotImplementedError()
 
     def check_can_live_migrate_source(self, context, instance,
-                                      dest_check_data):
+                                      dest_check_data, block_device_info=None):
         """Check if it is possible to execute live migration.
 
         This checks if the live migration can succeed, based on the
@@ -817,6 +817,7 @@ class ComputeDriver(object):
         :param context: security context
         :param instance: nova.db.sqlalchemy.models.Instance
         :param dest_check_data: result of check_can_live_migrate_destination
+        :param block_device_info: result of _get_instance_block_device_info
         :returns: a dict containing migration info (hypervisor-dependent)
         """
         raise NotImplementedError()
Index: nova-2014.2.3.dev5/nova/virt/fake.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/virt/fake.py
+++ nova-2014.2.3.dev5/nova/virt/fake.py
@@ -426,7 +426,7 @@ class FakeDriver(driver.ComputeDriver):
         return {}
 
     def check_can_live_migrate_source(self, ctxt, instance_ref,
-                                      dest_check_data):
+                                      dest_check_data, block_device_info=None):
         return
 
     def finish_migration(self, context, migration, instance, disk_info,
Index: nova-2014.2.3.dev5/nova/virt/hyperv/driver.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/virt/hyperv/driver.py
+++ nova-2014.2.3.dev5/nova/virt/hyperv/driver.py
@@ -169,7 +169,7 @@ class HyperVDriver(driver.ComputeDriver)
             context, dest_check_data)
 
     def check_can_live_migrate_source(self, context, instance,
-                                      dest_check_data):
+                                      dest_check_data, block_device_info=None):
         return self._livemigrationops.check_can_live_migrate_source(
             context, instance, dest_check_data)
 
Index: nova-2014.2.3.dev5/nova/virt/libvirt/driver.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/virt/libvirt/driver.py
+++ nova-2014.2.3.dev5/nova/virt/libvirt/driver.py
@@ -5070,7 +5070,8 @@ class LibvirtDriver(driver.ComputeDriver
         self._cleanup_shared_storage_test_file(filename)
 
     def check_can_live_migrate_source(self, context, instance,
-                                      dest_check_data):
+                                      dest_check_data,
+                                      block_device_info=None):
         """Check if it is possible to execute live migration.
 
         This checks if the live migration can succeed, based on the
@@ -5079,6 +5080,7 @@ class LibvirtDriver(driver.ComputeDriver
         :param context: security context
         :param instance: nova.db.sqlalchemy.models.Instance
         :param dest_check_data: result of check_can_live_migrate_destination
+        :param block_device_info: result of _get_instance_block_device_info
         :returns: a dict containing migration info
         """
         # Checking shared storage connectivity
@@ -5100,7 +5102,8 @@ class LibvirtDriver(driver.ComputeDriver
                 raise exception.InvalidLocalStorage(reason=reason, path=source)
             self._assert_dest_node_has_enough_disk(context, instance,
                                     dest_check_data['disk_available_mb'],
-                                    dest_check_data['disk_over_commit'])
+                                    dest_check_data['disk_over_commit'],
+                                    block_device_info)
 
         elif not (dest_check_data['is_shared_block_storage'] or
                   dest_check_data['is_shared_instance_path']):
@@ -5148,7 +5151,8 @@ class LibvirtDriver(driver.ComputeDriver
         return False
 
     def _assert_dest_node_has_enough_disk(self, context, instance,
-                                             available_mb, disk_over_commit):
+                                             available_mb, disk_over_commit,
+                                             block_device_info=None):
         """Checks if destination has enough disk for block migration."""
         # Libvirt supports qcow2 disk format,which is usually compressed
         # on compute nodes.
@@ -5164,7 +5168,8 @@ class LibvirtDriver(driver.ComputeDriver
         if available_mb:
             available = available_mb * units.Mi
 
-        ret = self.get_instance_disk_info(instance['name'])
+        ret = self.get_instance_disk_info(instance['name'],
+                                          block_device_info=block_device_info)
         disk_infos = jsonutils.loads(ret)
 
         necessary = 0
Index: nova-2014.2.3.dev5/nova/virt/xenapi/driver.py
===================================================================
--- nova-2014.2.3.dev5.orig/nova/virt/xenapi/driver.py
+++ nova-2014.2.3.dev5/nova/virt/xenapi/driver.py
@@ -502,7 +502,7 @@ class XenAPIDriver(driver.ComputeDriver)
         pass
 
     def check_can_live_migrate_source(self, context, instance,
-                                      dest_check_data):
+                                      dest_check_data, block_device_info=None):
         """Check if it is possible to execute live migration.
 
         This checks if the live migration can succeed, based on the
@@ -512,6 +512,7 @@ class XenAPIDriver(driver.ComputeDriver)
         :param instance: nova.db.sqlalchemy.models.Instance
         :param dest_check_data: result of check_can_live_migrate_destination
                                 includes the block_migration flag
+        :param block_device_info: result of _get_instance_block_device_info
         """
         return self._vmops.check_can_live_migrate_source(context, instance,
                                                          dest_check_data)
