Merge "libvirt: Improve _is_booted_from_volume implementation"

nova/tests/unit/virt/libvirt/test_driver.py

@@ -309,7 +309,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,
@@ -13676,7 +13677,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')])
@@ -15347,22 +15347,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,
+        info = {
-                                          'mount_device': '/dev/vdd',
+            'block_device_mapping': [
-                                          'connection_info': None},
+                {'boot_index': None,
-                                         {'boot_index': 0,
+                 'mount_device': '/dev/vdd',
-                                          'mount_device': '/dev/vda',
+                 'connection_info': mock.sentinel.conn_info_vdd},
-                                          'connection_info': None}]}
+                {'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')
@@ -15533,6 +15563,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(
@@ -16155,14 +16205,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.assertTrue(func(instance, disk_mapping))
+        self.assertFalse(func(bdi))
-        disk_mapping['disk'] = 'map'
-        self.assertTrue(func(instance, disk_mapping))
 
-        instance['image_ref'] = 'uuid'
+        bdm.append({'boot_index': -1})
-        self.assertFalse(func(instance, disk_mapping))
+        self.assertFalse(func(bdi))
+
+        bdm.append({'boot_index': None})
+        self.assertFalse(func(bdi))
+
+        bdm.append({'boot_index': 1})
+        self.assertFalse(func(bdi))
+
+        bdm.append({'boot_index': 0})
+        self.assertTrue(func(bdi))
 
     @mock.patch('nova.virt.libvirt.driver.imagebackend')
     @mock.patch(

nova/virt/libvirt/driver.py

@@ -2939,14 +2939,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')))
+        block_device_mapping = driver.block_device_info_get_mapping(
-                or 'disk' not in disk_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):
@@ -3011,8 +3012,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(
+        booted_from_volume = self._is_booted_from_volume(block_device_info)
-            instance, disk_mapping)
 
         def image(fname, image_type=CONF.libvirt.images_type):
             return self.image_backend.by_name(instance,
@@ -4838,13 +4838,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 {}
+        block_device_mapping = driver.block_device_info_get_mapping(
-        disk_mapping = disk_info.get('mapping', {})
+            block_device_info)
-
+        root_disk = block_device.get_root_bdm(block_device_mapping)
-        if self._is_booted_from_volume(instance, disk_mapping):
+        if root_disk:
-            block_device_mapping = driver.block_device_info_get_mapping(
-                                                            block_device_info)
-            root_disk = block_device.get_root_bdm(block_device_mapping)
             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)
@@ -7177,17 +7174,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(
+        booted_from_volume = self._is_booted_from_volume(block_device_info)
-            instance, block_device_info=block_device_info)
+
-        booted_from_volume = self._is_booted_from_volume(instance,
-                                                         disk_info_text)
         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")
@@ -7221,6 +7214,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