libvirt: Delete duplicate check when live-migrating

A year ago a useless check was added: I7989128d

The above patch was aimed to enable live-migration when
instance is booted from volume and has not local disk
by adding a new check.

However, the same check has been already checked in
_is_shared_block_storage method.

The last part of the _is_shared_block_storage method does
the same that above patch does:
- check whether the instance is booted from volume
- check whether the instance has not a local disk

Also this check calls _is_booted_from_volume incorrectly.
Parameter disk_mapping of _is_booted_from_volume must be a dict, but
this check specifies a string instead.

And finally introduced _has_local_disk method is wrong, because
it does not take into accont disk.ephN names.

This change reverts I7989128d, improves and simplifies related tests.

Closes-Bug: 1598661
Related-Bug: 1469006
Co-Authored-By: Feodor Tersin <ftersin@hotmail.com>
Change-Id: Id59012547c3318d94b65ab9f7c37c33c3a08b0e0

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index de4002eed87ddcf10595600dbfae5dd2f8978866..6c7b75685e48e2907f856344394d495ee040341b 100644 (file)
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -7094,10 +7094,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
     def _mock_can_live_migrate_source(self, block_migration=False,
                                       is_shared_block_storage=False,
                                       is_shared_instance_path=False,
-                                      is_booted_from_volume=False,
-                                      disk_available_mb=1024,
-                                      block_device_info=None,
-                                      block_device_text=None):
+                                      disk_available_mb=1024):
         instance = objects.Instance(**self.test_instance)
         dest_check_data = objects.LibvirtLiveMigrateData(
             filename='file',
@@ -7109,17 +7106,10 @@ class LibvirtConnTestCase(test.NoDBTestCase):
 
         self.mox.StubOutWithMock(drvr, '_is_shared_block_storage')
         drvr._is_shared_block_storage(instance, dest_check_data,
-                block_device_info).AndReturn(is_shared_block_storage)
+                None).AndReturn(is_shared_block_storage)
         self.mox.StubOutWithMock(drvr, '_check_shared_storage_test_file')
         drvr._check_shared_storage_test_file('file', instance).AndReturn(
                 is_shared_instance_path)
-        self.mox.StubOutWithMock(drvr, "get_instance_disk_info")
-        drvr.get_instance_disk_info(instance,
-                                    block_device_info=block_device_info).\
-                                    AndReturn(block_device_text)
-        self.mox.StubOutWithMock(drvr, '_is_booted_from_volume')
-        drvr._is_booted_from_volume(instance, block_device_text).AndReturn(
-            is_booted_from_volume)
 
         return (instance, dest_check_data, drvr)
 
@@ -7137,21 +7127,25 @@ class LibvirtConnTestCase(test.NoDBTestCase):
                                                  dest_check_data)
         self.assertIsInstance(ret, objects.LibvirtLiveMigrateData)
         self.assertIn('is_shared_block_storage', ret)
+        self.assertFalse(ret.is_shared_block_storage)
         self.assertIn('is_shared_instance_path', ret)
+        self.assertFalse(ret.is_shared_instance_path)
 
     def test_check_can_live_migrate_source_shared_block_storage(self):
         instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
                 is_shared_block_storage=True)
         self.mox.ReplayAll()
-        drvr.check_can_live_migrate_source(self.context, instance,
-                                           dest_check_data)
+        ret = drvr.check_can_live_migrate_source(self.context, instance,
+                                                 dest_check_data)
+        self.assertTrue(ret.is_shared_block_storage)
 
     def test_check_can_live_migrate_source_shared_instance_path(self):
         instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
                 is_shared_instance_path=True)
         self.mox.ReplayAll()
-        drvr.check_can_live_migrate_source(self.context, instance,
-                                           dest_check_data)
+        ret = drvr.check_can_live_migrate_source(self.context, instance,
+                                                 dest_check_data)
+        self.assertTrue(ret.is_shared_instance_path)
 
     def test_check_can_live_migrate_source_non_shared_fails(self):
         instance, dest_check_data, drvr = self._mock_can_live_migrate_source()
@@ -7187,53 +7181,31 @@ class LibvirtConnTestCase(test.NoDBTestCase):
                           drvr.check_can_live_migrate_source,
                           self.context, instance, dest_check_data)
 
-    def test_check_can_live_migrate_source_with_dest_not_enough_disk(self):
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
+                'get_instance_disk_info')
+    def test_check_can_live_migrate_source_with_dest_not_enough_disk(
+            self, mock_get_bdi):
+        mock_get_bdi.return_value = '[{"virt_disk_size":2}]'
+
         instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
                 block_migration=True,
                 disk_available_mb=0)
-
-        drvr.get_instance_disk_info(instance,
-                                    block_device_info=None).AndReturn(
-                                        '[{"virt_disk_size":2}]')
-
         self.mox.ReplayAll()
-        self.assertRaises(exception.MigrationError,
-                          drvr.check_can_live_migrate_source,
-                          self.context, instance, dest_check_data)
 
-    def test_check_can_live_migrate_source_booted_from_volume(self):
-        instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
-                is_booted_from_volume=True,
-                block_device_text='[]')
-        self.mox.ReplayAll()
-        drvr.check_can_live_migrate_source(self.context, instance,
-                                           dest_check_data)
-
-    def test_check_can_live_migrate_source_booted_from_volume_with_swap(self):
-        instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
-                is_booted_from_volume=True,
-                block_device_text='[{"path":"disk.swap"}]')
-        self.mox.ReplayAll()
-        self.assertRaises(exception.InvalidSharedStorage,
+        self.assertRaises(exception.MigrationError,
                           drvr.check_can_live_migrate_source,
                           self.context, instance, dest_check_data)
+        mock_get_bdi.assert_called_once_with(instance, block_device_info=None)
 
     @mock.patch.object(host.Host, 'has_min_version', return_value=False)
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_assert_dest_node_has_enough_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_has_local_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_is_booted_from_volume')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                'get_instance_disk_info')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_is_shared_block_storage', return_value=False)
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_check_shared_storage_test_file', return_value=False)
     def test_check_can_live_migrate_source_block_migration_with_bdm_error(
-            self, mock_check, mock_shared_block, mock_get_bdi,
-            mock_booted_from_volume, mock_has_local, mock_enough,
+            self, mock_check, mock_shared_block, mock_enough,
             mock_min_version):
 
         bdi = {'block_device_mapping': ['bdm']}
@@ -7253,19 +7225,12 @@ class LibvirtConnTestCase(test.NoDBTestCase):
     @mock.patch.object(host.Host, 'has_min_version', return_value=True)
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_assert_dest_node_has_enough_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_has_local_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_is_booted_from_volume')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                'get_instance_disk_info')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_is_shared_block_storage', return_value=False)
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_check_shared_storage_test_file', return_value=False)
     def test_check_can_live_migrate_source_bm_with_bdm_tunnelled_error(
-            self, mock_check, mock_shared_block, mock_get_bdi,
-            mock_booted_from_volume, mock_has_local, mock_enough,
+            self, mock_check, mock_shared_block, mock_enough,
             mock_min_version):
 
         self.flags(live_migration_tunnelled=True,
@@ -7288,20 +7253,13 @@ class LibvirtConnTestCase(test.NoDBTestCase):
     @mock.patch.object(host.Host, 'has_min_version', return_value=True)
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_assert_dest_node_has_enough_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_has_local_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_is_booted_from_volume')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                'get_instance_disk_info')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_is_shared_block_storage')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_check_shared_storage_test_file')
     def _test_check_can_live_migrate_source_block_migration_none(
             self, block_migrate, is_shared_instance_path, is_share_block,
-            mock_check, mock_shared_block, mock_get_bdi,
-            mock_booted_from_volume, mock_has_local, mock_enough, mock_verson):
+            mock_check, mock_shared_block, mock_enough, mock_verson):
 
         mock_check.return_value = is_shared_instance_path
         mock_shared_block.return_value = is_share_block
@@ -7338,20 +7296,12 @@ class LibvirtConnTestCase(test.NoDBTestCase):
                 '_assert_dest_node_has_enough_disk')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_assert_dest_node_has_enough_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_has_local_disk')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                '_is_booted_from_volume')
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
-                'get_instance_disk_info')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_is_shared_block_storage')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
                 '_check_shared_storage_test_file')
     def test_check_can_live_migration_source_disk_over_commit_none(self,
-            mock_check, mock_shared_block, mock_get_bdi,
-            mock_booted_from_volume, mock_has_local,
-            mock_enough, mock_disk_check):
+            mock_check, mock_shared_block, mock_enough, mock_disk_check):
 
         mock_check.return_value = False
         mock_shared_block.return_value = False

diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 5c386c55c9547ab4134901264acb84790cfddcb9..e82374797bb9894b78d2105730b191c31ad21f06 100644 (file)
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -2938,20 +2938,6 @@ class LibvirtDriver(driver.ComputeDriver):
         return ((not bool(instance.get('image_ref')))
                 or 'disk' not in disk_mapping)
 
-    @staticmethod
-    def _has_local_disk(instance, disk_mapping):
-        """Determines whether the VM has a local disk
-
-        Determines whether the disk mapping indicates that the VM
-        has a local disk (e.g. ephemeral, swap disk and config-drive).
-        """
-        if disk_mapping:
-            if ('disk.local' in disk_mapping or
-                'disk.swap' in disk_mapping or
-                'disk.config' in disk_mapping):
-                return True
-        return False
-
     def _inject_data(self, injection_image, instance, network_info,
                      admin_pass, files):
         """Injects data in a disk image
@@ -5564,12 +5550,6 @@ class LibvirtDriver(driver.ComputeDriver):
             self._is_shared_block_storage(instance, dest_check_data,
                                           block_device_info))
 
-        disk_info_text = self.get_instance_disk_info(
-            instance, block_device_info=block_device_info)
-        booted_from_volume = self._is_booted_from_volume(instance,
-                                                         disk_info_text)
-        has_local_disk = self._has_local_disk(instance, disk_info_text)
-
         if 'block_migration' not in dest_check_data:
             dest_check_data.block_migration = (
                 not dest_check_data.is_on_shared_storage())
@@ -5621,8 +5601,7 @@ class LibvirtDriver(driver.ComputeDriver):
                     LOG.error(msg, instance=instance)
                     raise exception.MigrationPreCheckError(reason=msg)
         elif not (dest_check_data.is_shared_block_storage or
-                  dest_check_data.is_shared_instance_path or
-                  (booted_from_volume and not has_local_disk)):
+                  dest_check_data.is_shared_instance_path):
             reason = _("Live migration can not be used "
                        "without shared storage except "
                        "a booted from volume VM which "