From 27b15516ae2ab1a83cad2c41fdbe09c08cc75d88 Mon Sep 17 00:00:00 2001
From: Matthew Booth <mbooth@redhat.com>
Date: Wed, 15 Jun 2016 14:16:36 +0100
Subject: [PATCH] Rename compute manager _check_dev_name to
 _add_missing_dev_names

'check' is a misleading name for this function, as it modifies its
input.

We tweak the loop to be slightly more efficient and readable.

We also save the bdm after updating it. This may have previously been
a bug.

Change-Id: Idadb6192eff68c21cff3387147b98319b6203505
---
 nova/compute/manager.py                 | 14 ++++++++------
 nova/tests/unit/compute/test_compute.py | 22 ++++++++++++++++------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index c49771989448..b8e81a2270e1 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -1549,20 +1549,22 @@ class ComputeManager(manager.Manager):
                 'swap': swap,
                 'block_device_mapping': mapping})
 
-    def _check_dev_name(self, bdms, instance):
-        bdms_no_device_name = [x for x in bdms if x.device_name is None]
-        for bdm in bdms_no_device_name:
+    def _add_missing_dev_names(self, bdms, instance):
+        for bdm in bdms:
+            if bdm.device_name is not None:
+                continue
+
             device_name = self._get_device_name_for_instance(instance,
-                                                             bdms,
-                                                             bdm)
+                                                             bdms, bdm)
             values = {'device_name': device_name}
             bdm.update(values)
+            bdm.save()
 
     def _prep_block_device(self, context, instance, bdms,
                            do_check_attach=True):
         """Set up the block device for an instance with error logging."""
         try:
-            self._check_dev_name(bdms, instance)
+            self._add_missing_dev_names(bdms, instance)
             block_device_info = driver.get_block_device_info(instance, bdms)
             mapping = driver.block_device_info_get_mapping(block_device_info)
             driver_block_device.attach_block_devices(
diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py
index 25e7925a1ea1..fea45c0ed74a 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -1259,8 +1259,14 @@ class ComputeVolumeTestCase(BaseTestCase):
 
         manager = compute_manager.ComputeManager()
         manager.use_legacy_block_device_info = False
-        block_device_info = manager._prep_block_device(self.context, instance,
-                                                       bdms)
+        mock_bdm_saves = [mock.patch.object(bdm, 'save') for bdm in bdms]
+        with test.nested(*mock_bdm_saves):
+            block_device_info = manager._prep_block_device(self.context,
+                                                           instance, bdms)
+
+            for bdm in bdms:
+                bdm.save.assert_called_once_with()
+                self.assertIsNotNone(bdm.device_name)
 
         convert_swap.assert_called_once_with(bdms)
         convert_ephemerals.assert_called_once_with(bdms)
@@ -8894,7 +8900,7 @@ class ComputeAPITestCase(BaseTestCase):
                 None,
                 '/invalid')
 
-    def test_check_dev_name_assign_dev_name(self):
+    def test_add_missing_dev_names_assign_dev_name(self):
         instance = self._create_fake_instance_obj()
         bdms = [objects.BlockDeviceMapping(
                 **fake_block_device.FakeDbBlockDeviceDict(
@@ -8908,12 +8914,16 @@ class ComputeAPITestCase(BaseTestCase):
                  'disk_bus': None,
                  'device_type': None
                  }))]
-        self.compute._check_dev_name(bdms, instance)
+        with mock.patch.object(objects.BlockDeviceMapping,
+                               'save') as mock_save:
+            self.compute._add_missing_dev_names(bdms, instance)
+            mock_save.assert_called_once_with()
         self.assertIsNotNone(bdms[0].device_name)
 
     @mock.patch.object(compute_manager.ComputeManager,
                        '_get_device_name_for_instance')
-    def test_check_dev_name_skip_bdms_with_dev_name(self, mock_get_dev_name):
+    def test_add_missing_dev_names_skip_bdms_with_dev_name(self,
+                                                       mock_get_dev_name):
         instance = self._create_fake_instance_obj()
         bdms = [objects.BlockDeviceMapping(
                 **fake_block_device.FakeDbBlockDeviceDict(
@@ -8927,7 +8937,7 @@ class ComputeAPITestCase(BaseTestCase):
                  'disk_bus': None,
                  'device_type': None
                  }))]
-        self.compute._check_dev_name(bdms, instance)
+        self.compute._add_missing_dev_names(bdms, instance)
         self.assertFalse(mock_get_dev_name.called)
 
     def test_no_attach_volume_in_rescue_state(self):