libvirt: Improve _is_booted_from_volume implementation

Currently this method cannot be used widely due to its parameters. It
requires device_info - the dict which can not be easily obtained in many
cases. Since it is often needed to figure out if an instance is booted
from volume, and the method name is "appropriate", this sometimes leads
to errors (when string result of get_instance_disk_info is passed as an
argument to _is_booted_from_volume; see also Id5901254).

This patch makes _is_booted_from_volume to use standard
block_device_info structure, which is accessible in almost any driver
method.

Closes-bug: 1596957
Closes-bug: 1587802
Change-Id: Ie424d172ac9d6aeb42d83e512f2a18713134be3b

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 6c7b75685e48e2907f856344394d495ee040341b..b0b47db9980c97f6ead8d762dd49c766bae88d34 100644 (file)
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -310,7 +310,8 @@ def fake_disk_info_byname(instance, type='qcow2'):
     disk_info = OrderedDict()
 
     # root disk
-    if instance.image_ref is not None:
+    if (instance.image_ref is not None and
+            instance.image_ref != uuids.fake_volume_backed_image_ref):
         cache_name = imagecache.get_cache_fname(instance.image_ref)
         disk_info['disk'] = {
             'type': type,
@@ -13635,7 +13636,6 @@ class LibvirtConnTestCase(test.NoDBTestCase):
         self.assertEqual('/dev/nbd0', inst_sys_meta['rootfs_device_name'])
         self.assertFalse(mock_instance.called)
         mock_get_inst_path.assert_has_calls([mock.call(mock_instance)])
-        mock_is_booted_from_volume.assert_called_once_with(mock_instance, {})
         mock_ensure_tree.assert_has_calls([mock.call('/tmp/rootfs')])
         drvr.image_backend.by_name.assert_has_calls([mock.call(mock_instance,
                                                     'disk')])
@@ -15306,22 +15306,52 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
     def test_migrate_disk_and_power_off_boot_from_volume(self,
                                                          disconnect_volume):
-        info = {'block_device_mapping': [{'boot_index': None,
-                                          'mount_device': '/dev/vdd',
-                                          'connection_info': None},
-                                         {'boot_index': 0,
-                                          'mount_device': '/dev/vda',
-                                          'connection_info': None}]}
+        info = {
+            'block_device_mapping': [
+                {'boot_index': None,
+                 'mount_device': '/dev/vdd',
+                 'connection_info': mock.sentinel.conn_info_vdd},
+                {'boot_index': 0,
+                 'mount_device': '/dev/vda',
+                 'connection_info': mock.sentinel.conn_info_vda}]}
         flavor = {'root_gb': 1, 'ephemeral_gb': 0}
         flavor_obj = objects.Flavor(**flavor)
         # Note(Mike_D): The size of instance's ephemeral_gb is 0 gb.
         self._test_migrate_disk_and_power_off(
             flavor_obj, block_device_info=info,
             params_for_instance={'image_ref': None,
-                                 'flavor': {'root_gb': 1,
+                                 'root_gb': 10,
+                                 'ephemeral_gb': 0,
+                                 'flavor': {'root_gb': 10,
                                             'ephemeral_gb': 0}})
         disconnect_volume.assert_called_with(
-            info['block_device_mapping'][1]['connection_info'], 'vda')
+            mock.sentinel.conn_info_vda, 'vda')
+
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
+    def test_migrate_disk_and_power_off_boot_from_volume_backed_snapshot(
+            self, disconnect_volume):
+        # Such instance has not empty image_ref, but must be considered as
+        # booted from volume.
+        info = {
+            'block_device_mapping': [
+                {'boot_index': None,
+                 'mount_device': '/dev/vdd',
+                 'connection_info': mock.sentinel.conn_info_vdd},
+                {'boot_index': 0,
+                 'mount_device': '/dev/vda',
+                 'connection_info': mock.sentinel.conn_info_vda}]}
+        flavor = {'root_gb': 1, 'ephemeral_gb': 0}
+        flavor_obj = objects.Flavor(**flavor)
+        self._test_migrate_disk_and_power_off(
+            flavor_obj, block_device_info=info,
+            params_for_instance={
+                'image_ref': uuids.fake_volume_backed_image_ref,
+                'root_gb': 10,
+                'ephemeral_gb': 0,
+                'flavor': {'root_gb': 10,
+                           'ephemeral_gb': 0}})
+        disconnect_volume.assert_called_with(
+            mock.sentinel.conn_info_vda, 'vda')
 
     @mock.patch('nova.utils.execute')
     @mock.patch('nova.virt.libvirt.utils.copy_image')
@@ -15492,6 +15522,26 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
             self.drvr.migrate_disk_and_power_off,
             'ctx', instance, '10.0.0.1', flavor_obj, None)
 
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
+                '.get_instance_disk_info')
+    def test_migrate_disk_and_power_off_resize_error_rbd(self,
+                                                         mock_get_disk_info):
+        # Check error on resize root disk down for rbd.
+        # The difference is that get_instance_disk_info always returns
+        # an emply list for rbd.
+        # Ephemeral size is not changed in this case (otherwise other check
+        # will raise the same error).
+        self.flags(images_type='rbd', group='libvirt')
+        instance = self._create_instance()
+        flavor = {'root_gb': 5, 'ephemeral_gb': 20}
+        flavor_obj = objects.Flavor(**flavor)
+        mock_get_disk_info.return_value = []
+
+        self.assertRaises(
+            exception.InstanceFaultRollback,
+            self.drvr.migrate_disk_and_power_off,
+            'ctx', instance, '10.0.0.1', flavor_obj, None)
+
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
                 '.get_instance_disk_info')
     def test_migrate_disk_and_power_off_resize_error_default_ephemeral(
@@ -16114,14 +16164,22 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
 
     def test_is_booted_from_volume(self):
         func = libvirt_driver.LibvirtDriver._is_booted_from_volume
-        instance, disk_mapping = {}, {}
+        bdm = []
+        bdi = {'block_device_mapping': bdm}
+
+        self.assertFalse(func(bdi))
+
+        bdm.append({'boot_index': -1})
+        self.assertFalse(func(bdi))
+
+        bdm.append({'boot_index': None})
+        self.assertFalse(func(bdi))
 
-        self.assertTrue(func(instance, disk_mapping))
-        disk_mapping['disk'] = 'map'
-        self.assertTrue(func(instance, disk_mapping))
+        bdm.append({'boot_index': 1})
+        self.assertFalse(func(bdi))
 
-        instance['image_ref'] = 'uuid'
-        self.assertFalse(func(instance, disk_mapping))
+        bdm.append({'boot_index': 0})
+        self.assertTrue(func(bdi))
 
     @mock.patch('nova.virt.libvirt.driver.imagebackend')
     @mock.patch(

diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index e82374797bb9894b78d2105730b191c31ad21f06..44eaebcd8ccd5725756d25232928ae4da31ad3be 100644 (file)
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -2929,14 +2929,15 @@ class LibvirtDriver(driver.ComputeDriver):
         return 'rbd' if CONF.libvirt.images_type == 'rbd' else 'raw'
 
     @staticmethod
-    def _is_booted_from_volume(instance, disk_mapping):
+    def _is_booted_from_volume(block_device_info):
         """Determines whether the VM is booting from volume
 
-        Determines whether the disk mapping indicates that the VM
+        Determines whether the block device info indicates that the VM
         is booting from a volume.
         """
-        return ((not bool(instance.get('image_ref')))
-                or 'disk' not in disk_mapping)
+        block_device_mapping = driver.block_device_info_get_mapping(
+            block_device_info)
+        return bool(block_device.get_root_bdm(block_device_mapping))
 
     def _inject_data(self, injection_image, instance, network_info,
                      admin_pass, files):
@@ -3001,8 +3002,7 @@ class LibvirtDriver(driver.ComputeDriver):
                       admin_pass=None, inject_files=True,
                       fallback_from_host=None,
                       ignore_bdi_for_swap=False):
-        booted_from_volume = self._is_booted_from_volume(
-            instance, disk_mapping)
+        booted_from_volume = self._is_booted_from_volume(block_device_info)
 
         def image(fname, image_type=CONF.libvirt.images_type):
             return self.image_backend.by_name(instance,
@@ -4755,13 +4755,10 @@ class LibvirtDriver(driver.ComputeDriver):
     def _create_domain_setup_lxc(self, instance, image_meta,
                                  block_device_info, disk_info):
         inst_path = libvirt_utils.get_instance_path(instance)
-        disk_info = disk_info or {}
-        disk_mapping = disk_info.get('mapping', {})
-
-        if self._is_booted_from_volume(instance, disk_mapping):
-            block_device_mapping = driver.block_device_info_get_mapping(
-                                                            block_device_info)
-            root_disk = block_device.get_root_bdm(block_device_mapping)
+        block_device_mapping = driver.block_device_info_get_mapping(
+            block_device_info)
+        root_disk = block_device.get_root_bdm(block_device_mapping)
+        if root_disk:
             disk_info = blockinfo.get_info_from_bdm(
                 instance, CONF.libvirt.virt_type, image_meta, root_disk)
             self._connect_volume(root_disk['connection_info'], disk_info)
@@ -7094,17 +7091,13 @@ class LibvirtDriver(driver.ComputeDriver):
         # Checks if the migration needs a disk resize down.
         root_down = flavor.root_gb < instance.flavor.root_gb
         ephemeral_down = flavor.ephemeral_gb < eph_size
-        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)
+        booted_from_volume = self._is_booted_from_volume(block_device_info)
+
         if (root_down and not booted_from_volume) or ephemeral_down:
             reason = _("Unable to resize disk down.")
             raise exception.InstanceFaultRollback(
                 exception.ResizeError(reason=reason))
 
-        disk_info = jsonutils.loads(disk_info_text)
-
         # NOTE(dgenin): Migration is not implemented for LVM backed instances.
         if CONF.libvirt.images_type == 'lvm' and not booted_from_volume:
             reason = _("Migration is not supported for LVM backed instances")
@@ -7138,6 +7131,10 @@ class LibvirtDriver(driver.ComputeDriver):
             disk_dev = vol['mount_device'].rpartition("/")[2]
             self._disconnect_volume(connection_info, disk_dev)
 
+        disk_info_text = self.get_instance_disk_info(
+            instance, block_device_info=block_device_info)
+        disk_info = jsonutils.loads(disk_info_text)
+
         try:
             utils.execute('mv', inst_base, inst_base_resize)
             # if we are migrating the instance with shared storage then