Merge "libvirt: Fix live block migration"

3 files changed, 79 insertions, 69 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 95ad4b7d0..a9ab17a13 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1938,8 +1938,9 @@ class API(base.Base):
     def live_migrate(self, context, instance, block_migration,
                      disk_over_commit, host):
         """Migrate a server lively to a new host."""
-        LOG.debug(_("Going to try to live migrate instance"),
-                  instance=instance)
+        LOG.debug(_("Going to try to live migrate instance to %s"),
+                  host, instance=instance)
+
         self.scheduler_rpcapi.live_migration(context, block_migration,
                 disk_over_commit, instance, host)
 
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index a1b99388d..ede5afd31 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -1626,14 +1626,11 @@ class LibvirtConnTestCase(test.TestCase):
         conn = libvirt_driver.LibvirtDriver(False)
 
         self.mox.StubOutWithMock(conn, '_get_compute_info')
-        self.mox.StubOutWithMock(conn, 'get_instance_disk_info')
         self.mox.StubOutWithMock(conn, '_create_shared_storage_test_file')
         self.mox.StubOutWithMock(conn, '_compare_cpu')
 
         conn._get_compute_info(self.context, FLAGS.host).AndReturn(
                                               {'disk_available_least': 400})
-        conn.get_instance_disk_info(instance_ref["name"]).AndReturn(
-                                            '[{"virt_disk_size":2}]')
         # _check_cpu_match
         conn._get_compute_info(self.context,
                                src).AndReturn({'cpu_info': "asdf"})
@@ -1645,9 +1642,12 @@ class LibvirtConnTestCase(test.TestCase):
 
         self.mox.ReplayAll()
         return_value = conn.check_can_live_migrate_destination(self.context,
-                                instance_ref, True, False)
+                                instance_ref, True)
         self.assertDictMatch(return_value,
-                             {"filename": "file", "block_migration": True})
+                             {"filename": "file",
+                              'disk_available_mb': 409600,
+                              "disk_over_commit": False,
+                              "block_migration": True})
 
     def test_check_can_live_migrate_dest_all_pass_no_block_migration(self):
         instance_ref = db.instance_create(self.context, self.test_instance)
@@ -1670,28 +1670,12 @@ class LibvirtConnTestCase(test.TestCase):
 
         self.mox.ReplayAll()
         return_value = conn.check_can_live_migrate_destination(self.context,
-                                instance_ref, False, False)
+                                instance_ref, False)
         self.assertDictMatch(return_value,
-                            {"filename": "file", "block_migration": False})
-
-    def test_check_can_live_migrate_dest_fails_not_enough_disk(self):
-        instance_ref = db.instance_create(self.context, self.test_instance)
-        dest = "fake_host_2"
-        src = instance_ref['host']
-        conn = libvirt_driver.LibvirtDriver(False)
-
-        self.mox.StubOutWithMock(conn, '_get_compute_info')
-        self.mox.StubOutWithMock(conn, 'get_instance_disk_info')
-
-        conn._get_compute_info(self.context, FLAGS.host).AndReturn(
-                                              {'disk_available_least': 0})
-        conn.get_instance_disk_info(instance_ref["name"]).AndReturn(
-                                            '[{"virt_disk_size":2}]')
-
-        self.mox.ReplayAll()
-        self.assertRaises(exception.MigrationError,
-                          conn.check_can_live_migrate_destination,
-                          self.context, instance_ref, True, False)
+                            {"filename": "file",
+                             "block_migration": False,
+                             "disk_over_commit": False,
+                             "disk_available_mb": None})
 
     def test_check_can_live_migrate_dest_incompatible_cpu_raises(self):
         instance_ref = db.instance_create(self.context, self.test_instance)
@@ -1709,29 +1693,14 @@ class LibvirtConnTestCase(test.TestCase):
         self.mox.ReplayAll()
         self.assertRaises(exception.InvalidCPUInfo,
                           conn.check_can_live_migrate_destination,
-                          self.context, instance_ref, False, False)
-
-    def test_check_can_live_migrate_dest_fail_space_with_block_migration(self):
-        instance_ref = db.instance_create(self.context, self.test_instance)
-        dest = "fake_host_2"
-        src = instance_ref['host']
-        conn = libvirt_driver.LibvirtDriver(False)
-
-        self.mox.StubOutWithMock(conn, '_get_compute_info')
-        self.mox.StubOutWithMock(conn, 'get_instance_disk_info')
-
-        conn._get_compute_info(self.context, FLAGS.host).AndReturn(
-                                              {'disk_available_least': 0})
-        conn.get_instance_disk_info(instance_ref["name"]).AndReturn(
-                                            '[{"virt_disk_size":2}]')
-
-        self.mox.ReplayAll()
-        self.assertRaises(exception.MigrationError,
-                          conn.check_can_live_migrate_destination,
-                          self.context, instance_ref, True, False)
+                          self.context, instance_ref, False)
 
     def test_check_can_live_migrate_dest_cleanup_works_correctly(self):
-        dest_check_data = {"filename": "file", "block_migration": True}
+        instance_ref = db.instance_create(self.context, self.test_instance)
+        dest_check_data = {"filename": "file",
+                           "block_migration": True,
+                           "disk_over_commit": False,
+                           "disk_available_mb": 1024}
         conn = libvirt_driver.LibvirtDriver(False)
 
         self.mox.StubOutWithMock(conn, '_cleanup_shared_storage_test_file')
@@ -1743,19 +1712,30 @@ class LibvirtConnTestCase(test.TestCase):
 
     def test_check_can_live_migrate_source_works_correctly(self):
         instance_ref = db.instance_create(self.context, self.test_instance)
-        dest_check_data = {"filename": "file", "block_migration": True}
+        dest_check_data = {"filename": "file",
+                           "block_migration": True,
+                           "disk_over_commit": False,
+                           "disk_available_mb": 1024}
         conn = libvirt_driver.LibvirtDriver(False)
 
         self.mox.StubOutWithMock(conn, "_check_shared_storage_test_file")
         conn._check_shared_storage_test_file("file").AndReturn(False)
 
+        self.mox.StubOutWithMock(conn, "_assert_dest_node_has_enough_disk")
+        conn._assert_dest_node_has_enough_disk(self.context, instance_ref,
+                                        dest_check_data['disk_available_mb'],
+                                               False)
+
         self.mox.ReplayAll()
         conn.check_can_live_migrate_source(self.context, instance_ref,
                                            dest_check_data)
 
     def test_check_can_live_migrate_dest_fail_shared_storage_with_blockm(self):
         instance_ref = db.instance_create(self.context, self.test_instance)
-        dest_check_data = {"filename": "file", "block_migration": True}
+        dest_check_data = {"filename": "file",
+                           "block_migration": True,
+                           "disk_over_commit": False,
+                           'disk_available_mb': 1024}
         conn = libvirt_driver.LibvirtDriver(False)
 
         self.mox.StubOutWithMock(conn, "_check_shared_storage_test_file")
@@ -1768,7 +1748,10 @@ class LibvirtConnTestCase(test.TestCase):
 
     def test_check_can_live_migrate_no_shared_storage_no_blck_mig_raises(self):
         instance_ref = db.instance_create(self.context, self.test_instance)
-        dest_check_data = {"filename": "file", "block_migration": False}
+        dest_check_data = {"filename": "file",
+                           "block_migration": False,
+                           "disk_over_commit": False,
+                           'disk_available_mb': 1024}
         conn = libvirt_driver.LibvirtDriver(False)
 
         self.mox.StubOutWithMock(conn, "_check_shared_storage_test_file")
@@ -1779,6 +1762,28 @@ class LibvirtConnTestCase(test.TestCase):
                           conn.check_can_live_migrate_source,
                           self.context, instance_ref, dest_check_data)
 
+    def test_check_can_live_migrate_source_with_dest_not_enough_disk(self):
+        instance_ref = db.instance_create(self.context, self.test_instance)
+        dest = "fake_host_2"
+        src = instance_ref['host']
+        conn = libvirt_driver.LibvirtDriver(False)
+
+        self.mox.StubOutWithMock(conn, "_check_shared_storage_test_file")
+        conn._check_shared_storage_test_file("file").AndReturn(False)
+
+        self.mox.StubOutWithMock(conn, "get_instance_disk_info")
+        conn.get_instance_disk_info(instance_ref["name"]).AndReturn(
+                                            '[{"virt_disk_size":2}]')
+
+        dest_check_data = {"filename": "file",
+                           "disk_available_mb": 0,
+                           "block_migration": True,
+                           "disk_over_commit": False}
+        self.mox.ReplayAll()
+        self.assertRaises(exception.MigrationError,
+                          conn.check_can_live_migrate_source,
+                          self.context, instance_ref, dest_check_data)
+
     def test_live_migration_raises_exception(self):
         """Confirms recover method is called when exceptions are raised."""
         # Preparing data
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index e2a548014..5e7906b35 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -2214,14 +2214,16 @@ class LibvirtDriver(driver.ComputeDriver):
 
         :param ctxt: security context
         :param instance_ref: nova.db.sqlalchemy.models.Instance
-        :param dest: destination host
         :param block_migration: if true, prepare for block migration
         :param disk_over_commit: if true, allow disk over commit
         """
+        disk_available_mb = None
         if block_migration:
-            self._assert_compute_node_has_enough_disk(ctxt,
-                                                     instance_ref,
-                                                     disk_over_commit)
+            disk_available_gb = self._get_compute_info(ctxt,
+                                    FLAGS.host)['disk_available_least']
+            disk_available_mb = \
+                    (disk_available_gb * 1024) - FLAGS.reserved_host_disk_mb
+
         # Compare CPU
         src = instance_ref['host']
         source_cpu_info = self._get_compute_info(ctxt, src)['cpu_info']
@@ -2230,14 +2232,16 @@ class LibvirtDriver(driver.ComputeDriver):
         # Create file on storage, to be checked on source host
         filename = self._create_shared_storage_test_file()
 
-        return {"filename": filename, "block_migration": block_migration}
+        return {"filename": filename,
+                "block_migration": block_migration,
+                "disk_over_commit": disk_over_commit,
+                "disk_available_mb": disk_available_mb}
 
     def check_can_live_migrate_destination_cleanup(self, ctxt,
                                                    dest_check_data):
         """Do required cleanup on dest host after check_can_live_migrate calls
 
         :param ctxt: security context
-        :param disk_over_commit: if true, allow disk over commit
         """
         filename = dest_check_data["filename"]
         self._cleanup_shared_storage_test_file(filename)
@@ -2266,6 +2270,9 @@ class LibvirtDriver(driver.ComputeDriver):
                 reason = _("Block migration can not be used "
                            "with shared storage.")
                 raise exception.InvalidLocalStorage(reason=reason, path=source)
+            self._assert_dest_node_has_enough_disk(ctxt, instance_ref,
+                                    dest_check_data['disk_available_mb'],
+                                    dest_check_data['disk_over_commit'])
 
         elif not shared:
             reason = _("Live migration can not be used "
@@ -2277,9 +2284,9 @@ class LibvirtDriver(driver.ComputeDriver):
         compute_node_ref = db.service_get_all_compute_by_host(context, host)
         return compute_node_ref[0]['compute_node'][0]
 
-    def _assert_compute_node_has_enough_disk(self, context, instance_ref,
-                                             disk_over_commit):
-        """Checks if host has enough disk for block migration."""
+    def _assert_dest_node_has_enough_disk(self, context, instance_ref,
+                                             available_mb, disk_over_commit):
+        """Checks if destination has enough disk for block migration."""
         # Libvirt supports qcow2 disk format,which is usually compressed
         # on compute nodes.
         # Real disk image (compressed) may enlarged to "virtual disk size",
@@ -2290,11 +2297,7 @@ class LibvirtDriver(driver.ComputeDriver):
         # if disk_over_commit is True,
         #  otherwise virtual disk size < available disk size.
 
-        # Getting total available disk of host
-        dest = FLAGS.host
-        available_gb = self._get_compute_info(context,
-                                              dest)['disk_available_least']
-        available = available_gb * (1024 ** 3)
+        available = available_mb * (1024 ** 2)
 
         ret = self.get_instance_disk_info(instance_ref['name'])
         disk_infos = jsonutils.loads(ret)
@@ -2310,9 +2313,10 @@ class LibvirtDriver(driver.ComputeDriver):
         # Check that available disk > necessary disk
         if (available - necessary) < 0:
             instance_uuid = instance_ref['uuid']
-            reason = _("Unable to migrate %(instance_uuid)s to %(dest)s: "
-                       "Lack of disk(host:%(available)s "
-                       "<= instance:%(necessary)s)")
+            reason = _("Unable to migrate %(instance_uuid)s: "
+                       "Disk of instance is too large(available"
+                       " on destination host:%(available)s "
+                       "< need:%(necessary)s)")
             raise exception.MigrationError(reason=reason % locals())
 
     def _compare_cpu(self, cpu_info):