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

@@ -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(

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):