From db184423e512aff9653d74a3d3bcfc6c80b23403 Mon Sep 17 00:00:00 2001
From: Sven Anderson <sven@redhat.com>
Date: Tue, 10 Nov 2015 14:33:13 +0100
Subject: [PATCH] Fix is_volume_backed_instance() for unset image_ref

is_volume_backed_instance() assumed from old times, that if image_ref
is not set, the instance is always volume backed.  This is not true
anymore, since block device mappings can also reference images as
source.  This fix adapts the test cases and function accordingly.

Change-Id: Ie83d8d8154d372991fb37dae0def37e3f4583090
Closes-Bug: #1501851
---
 nova/compute/api.py                     | 11 +++----
 nova/tests/unit/compute/test_compute.py | 41 ++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 9eda901033..8fe4ad9ea6 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -3251,13 +3251,12 @@ class API(base.Base):
         return bdms.root_bdm()
 
     def is_volume_backed_instance(self, context, instance, bdms=None):
-        if not instance.image_ref:
-            return True
-
         root_bdm = self._get_root_bdm(context, instance, bdms)
-        if not root_bdm:
-            return False
-        return root_bdm.is_volume
+        if root_bdm is not None:
+            return root_bdm.is_volume
+        # in case we hit a very old instance without root bdm, we _assume_ that
+        # instance is backed by a volume, if and only if image_ref is not set
+        return not instance.image_ref
 
     @check_instance_lock
     @check_instance_cell
diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py
index a39771febe..2968386d50 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -8929,24 +8929,30 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertIsNone(
             self.compute_api._volume_size(inst_type, blank_bdm))
 
-    def test_is_volume_backed_instance_no_image(self):
+    def test_is_volume_backed_instance_no_bdm_no_image(self):
         ctxt = self.context
 
         instance = self._create_fake_instance_obj({'image_ref': ''})
         self.assertTrue(
             self.compute_api.is_volume_backed_instance(ctxt, instance, None))
 
-    def test_is_volume_backed_instance_no_bdm(self):
+    def test_is_volume_backed_instance_empty_bdm_with_image(self):
         ctxt = self.context
-        instance = self._create_fake_instance_obj({'root_device_name': 'vda'})
+        instance = self._create_fake_instance_obj({
+            'root_device_name': 'vda',
+            'image_ref': FAKE_IMAGE_REF
+        })
         self.assertFalse(
             self.compute_api.is_volume_backed_instance(
                 ctxt, instance,
                 block_device_obj.block_device_make_list(ctxt, [])))
 
-    def test_is_volume_backed_instance_bdm_volume(self):
+    def test_is_volume_backed_instance_bdm_volume_no_image(self):
         ctxt = self.context
-        instance = self._create_fake_instance_obj({'root_device_name': 'vda'})
+        instance = self._create_fake_instance_obj({
+            'root_device_name': 'vda',
+            'image_ref': ''
+        })
         bdms = block_device_obj.block_device_make_list(ctxt,
                             [fake_block_device.FakeDbBlockDeviceDict(
                                 {'source_type': 'volume',
@@ -8957,9 +8963,14 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertTrue(
             self.compute_api.is_volume_backed_instance(ctxt, instance, bdms))
 
-    def test_is_volume_backed_instance_bdm_local(self):
+    def test_is_volume_backed_instance_bdm_local_no_image(self):
+        # if the root device is local the instance is not volume backed, even
+        # if no image_ref is set.
         ctxt = self.context
-        instance = self._create_fake_instance_obj({'root_device_name': 'vda'})
+        instance = self._create_fake_instance_obj({
+            'root_device_name': 'vda',
+            'image_ref': ''
+        })
         bdms = block_device_obj.block_device_make_list(ctxt,
                [fake_block_device.FakeDbBlockDeviceDict(
                 {'source_type': 'volume',
@@ -8978,6 +8989,22 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertFalse(
             self.compute_api.is_volume_backed_instance(ctxt, instance, bdms))
 
+    def test_is_volume_backed_instance_bdm_volume_with_image(self):
+        ctxt = self.context
+        instance = self._create_fake_instance_obj({
+            'root_device_name': 'vda',
+            'image_ref': FAKE_IMAGE_REF
+        })
+        bdms = block_device_obj.block_device_make_list(ctxt,
+                            [fake_block_device.FakeDbBlockDeviceDict(
+                                {'source_type': 'volume',
+                                 'device_name': '/dev/vda',
+                                 'volume_id': 'fake_volume_id',
+                                 'boot_index': 0,
+                                 'destination_type': 'volume'})])
+        self.assertTrue(
+            self.compute_api.is_volume_backed_instance(ctxt, instance, bdms))
+
     def test_is_volume_backed_instance_bdm_snapshot(self):
         ctxt = self.context
         instance = self._create_fake_instance_obj({'root_device_name': 'vda'})