libvirt: Fix live block migration

Current check_can_live_migrate_destination trys to get instance info
by call self._conn.lookupByName(instance_name) on the destination
host before migrating. Because the instance is not on the destination
host yet, it failes with InstanceNotFound.

This fix gets the available disk size of the destination side.
On the source side, checks whether the above space is enough before
migrating.

Fixes bug 1044237

Change-Id: I20f64e1f85828049b697a4b1173bac8e5779d45a

nova/compute/api.py

@@ -1938,8 +1938,9 @@ def is_volume_backed_instance(self, context, instance, bdms):
     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)
 

nova/tests/test_libvirt.py

@@ -1626,14 +1626,11 @@ def test_check_can_live_migrate_dest_all_pass_with_block_migration(self):
         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 @@ def test_check_can_live_migrate_dest_all_pass_with_block_migration(self):
 
         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 @@ def test_check_can_live_migrate_dest_all_pass_no_block_migration(self):
 
         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 @@ def test_check_can_live_migrate_dest_incompatible_cpu_raises(self):
         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 @@ def test_check_can_live_migrate_dest_cleanup_works_correctly(self):
 
     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 @@ def test_check_can_live_migrate_dest_fail_shared_storage_with_blockm(self):
 
     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 @@ def test_check_can_live_migrate_no_shared_storage_no_blck_mig_raises(self):
                           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

nova/virt/libvirt/driver.py

@@ -2214,14 +2214,16 @@ def check_can_live_migrate_destination(self, ctxt, instance_ref,
 
         :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 @@ def check_can_live_migrate_destination(self, ctxt, instance_ref,
         # 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 @@ def check_can_live_migrate_source(self, ctxt, instance_ref,
                 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 @@ def _get_compute_info(self, context, host):
         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 @@ def _assert_compute_node_has_enough_disk(self, context, instance_ref,
         # 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 @@ def _assert_compute_node_has_enough_disk(self, context, instance_ref,
         # 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):