Preserve multiattach flag when refreshing connection_info

When we attach a multiattach-capable volume, we do something
dirty and stash a "multiattach" boolean flag in the
BlockDeviceMapping.connection_info dict. This is used by the
virt driver to determine how to connect the volume (for the
libvirt driver, it sets the "shareable" element on the disk
config xml).

When resizing an instance, ComputeManager._finish_resize on
the destination host refreshes the block device mapping list
along with the connection_info for each BDM. Because of this,
it would overwrite the BDM.connection_info along with the stashed
"multiattach" flag which is later used to connect the volumes
on the destination host via the virt driver.finish_migration
method. This leads to failures with multiattach volumes because
the disk config is wrong.

To fix this, when refreshing BDM connection_info, we preserve
the multiattach flag in the connection_info, similar to the
serial (volume ID) and multipath_id.

Interestingly enough, the nova-multiattach job does not fail
the volume multiattach resize test with libvirt 1.3.1 and qemu
2.5. This failure was only noticed once the nova-multiattach
job was tested with libvirt 4.0.0 and qemu 2.11.1. So maybe there
was something in the older package versions that masked this
obvious bug in the nova code.

Change-Id: Iaee13478212cc04e6d1a1249f33822369d94d41d
Closes-Bug: #1757190
(cherry picked from commit 5f3cca205581d45d92714ce1a909d4394b7812ff)

2 files changed, 43 insertions, 0 deletions

diff --git a/nova/tests/unit/virt/test_block_device.py b/nova/tests/unit/virt/test_block_device.py
index e8f5d7042d..c669542aa7 100644
--- a/nova/tests/unit/virt/test_block_device.py
+++ b/nova/tests/unit/virt/test_block_device.py
@@ -1275,6 +1275,39 @@ class TestDriverBlockDeviceNewFlow(TestDriverBlockDevice):
                           test_bdm.attach, self.context, instance,
                           self.volume_api, self.virt_driver)
 
+    @mock.patch('nova.objects.BlockDeviceMapping.save')
+    def test_refresh_connection_preserve_multiattach(self, mock_bdm_save):
+        """Tests that we've already attached a multiattach-capable volume
+        and when refreshing the connection_info from the attachment record,
+        the multiattach flag in the bdm.connection_info is preserved.
+        """
+        test_bdm = self.driver_classes['volume'](self.volume_bdm)
+        test_bdm['connection_info']['multiattach'] = True
+        volume_api = mock.Mock()
+        volume_api.attachment_get.return_value = {
+            'connection_info': {
+                'data': {
+                    'some': 'goodies'
+                }
+            }
+        }
+
+        test_bdm.refresh_connection_info(
+            self.context, mock.sentinel.instance,
+            volume_api, mock.sentinel.virt_driver)
+        volume_api.attachment_get.assert_called_once_with(
+            self.context, self.attachment_id)
+        mock_bdm_save.assert_called_once_with()
+        expected_connection_info = {
+            'data': {
+                'some': 'goodies'
+            },
+            'serial': self.volume_bdm.volume_id,
+            'multiattach': True
+        }
+        self.assertDictEqual(expected_connection_info,
+                             test_bdm['connection_info'])
+
 
 class TestGetVolumeId(test.NoDBTestCase):
 
diff --git a/nova/virt/block_device.py b/nova/virt/block_device.py
index 61f633935e..eec003b79c 100644
--- a/nova/virt/block_device.py
+++ b/nova/virt/block_device.py
@@ -631,7 +631,17 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
         else:
             attachment_ref = volume_api.attachment_get(context,
                                                        self['attachment_id'])
+            # The _volume_attach method stashes a 'multiattach' flag in the
+            # BlockDeviceMapping.connection_info which is not persisted back
+            # in cinder so before we overwrite the BDM.connection_info (via
+            # the update_db decorator on this method), we need to make sure
+            # and preserve the multiattach flag if it's set. Note that this
+            # is safe to do across refreshes because the multiattach capability
+            # of a volume cannot be changed while the volume is in-use.
+            multiattach = self['connection_info'].get('multiattach', False)
             connection_info = attachment_ref['connection_info']
+            if multiattach:
+                connection_info['multiattach'] = True
 
         if 'serial' not in connection_info:
             connection_info['serial'] = self.volume_id