Check for multiattach before removing connections

With the addition of multiattach we need to ensure that we
don't make brick calls to remove connections on detach volume
if that volume is attached to another Instance on the same
node.

This patch adds a new helper method (_should_disconnect_target)
to the virt driver that will inform the caller if the specified
volume is attached multiple times to the current host.

The general strategy for this call is to fetch a current reference
of the specified volume and then:
1. Check if that volume has >1 active attachments
2. Fetch the attachments for the volume and extract the server_uuids
   for each of the attachments.
3. Check the server_uuids against a list of all known server_uuids
   on the current host.  Increment a connection_count for each item
   found.

If the connection_count is >1 we return `False` indicating that the
volume is being used by more than one attachment on the host and
we therefore should NOT destroy the connection.

*NOTE*
This scenario is very different than the `shared_targets`
case (for which we supply a property on the Volume object).  The
`shared_targets` scenario is specifically for Volume backends that
present >1 Volumes using a single Target.  This mechanism is meant
to provide a signal to consumers that locking is required for the
creation and deletion of initiator/target sessions.

Closes-Bug: #1752115

Change-Id: Idc5cecffa9129d600c36e332c97f01f1e5ff1f9f
(cherry picked from commit 139426d514)

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

@@ -6745,6 +6745,84 @@ class LibvirtConnTestCase(test.NoDBTestCase,
         mock_encryptor.detach_volume.called_once_with(self.context,
                                                       **encryption)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_detach_encryptor')
+    @mock.patch('nova.objects.InstanceList.get_uuids_by_host')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver')
+    @mock.patch('nova.volume.cinder.API.get')
+    def test_disconnect_multiattach_single_connection(
+            self, mock_volume_get, mock_get_volume_driver,
+            mock_get_instances, mock_detach_encryptor):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        attachments = (
+            [('70ab645f-6ffc-406a-b3d2-5007a0c01b82',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'9402c249-99df-4f72-89e7-fd611493ee5d'}),
+             ('00803490-f768-4049-aa7d-151f54e6311e',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'd6128a7b-19c8-4a3e-8036-011396df95ac'})])
+
+        mock_volume_get.return_value = (
+            {'attachments': OrderedDict(attachments), 'multiattach': True,
+             'id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'})
+
+        fake_connection_info = {
+            'multiattach': True,
+            'volume_id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'}
+        fake_instance_1 = fake_instance.fake_instance_obj(
+            self.context,
+            host='fake-host-1')
+
+        mock_get_instances.return_value = (
+            ['00803490-f768-4049-aa7d-151f54e6311e'])
+        drvr._disconnect_volume(
+            self.context, fake_connection_info, fake_instance_1)
+        mock_volume_driver.disconnect_volume.assert_called_once_with(
+            fake_connection_info, fake_instance_1)
+
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_detach_encryptor')
+    @mock.patch('nova.objects.InstanceList.get_uuids_by_host')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver')
+    @mock.patch('nova.volume.cinder.API.get')
+    def test_disconnect_multiattach_multi_connection(
+            self, mock_volume_get, mock_get_volume_driver,
+            mock_get_instances, mock_detach_encryptor):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        attachments = (
+            [('70ab645f-6ffc-406a-b3d2-5007a0c01b82',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'9402c249-99df-4f72-89e7-fd611493ee5d'}),
+             ('00803490-f768-4049-aa7d-151f54e6311e',
+              {'mountpoint': u'/dev/vdb',
+               'attachment_id': u'd6128a7b-19c8-4a3e-8036-011396df95ac'})])
+
+        mock_volume_get.return_value = (
+            {'attachments': OrderedDict(attachments), 'multiattach': True,
+             'id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'})
+
+        fake_connection_info = {
+            'multiattach': True,
+            'volume_id': 'd30559cf-f092-4693-8589-0d0a1e7d9b1f'}
+        fake_instance_1 = fake_instance.fake_instance_obj(
+            self.context,
+            host='fake-host-1')
+
+        mock_get_instances.return_value = (
+            ['00803490-f768-4049-aa7d-151f54e6311e',
+             '70ab645f-6ffc-406a-b3d2-5007a0c01b82'])
+        drvr._disconnect_volume(
+            self.context, fake_connection_info, fake_instance_1)
+        mock_volume_driver.disconnect_volume.assert_not_called()
+
     def test_attach_invalid_volume_type(self):
         self.create_fake_libvirt_mock()
         libvirt_driver.LibvirtDriver._conn.lookupByUUIDString \

nova/virt/libvirt/driver.py

@@ -1237,11 +1237,53 @@ class LibvirtDriver(driver.ComputeDriver):
         self._attach_encryptor(context, connection_info, encryption,
                                allow_native_luks)
 
+    def _should_disconnect_target(self, context, connection_info, instance):
+        connection_count = 0
+
+        # NOTE(jdg): Multiattach is a special case (not to be confused
+        # with shared_targets). With multiattach we may have a single volume
+        # attached multiple times to *this* compute node (ie Server-1 and
+        # Server-2).  So, if we receive a call to delete the attachment for
+        # Server-1 we need to take special care to make sure that the Volume
+        # isn't also attached to another Server on this Node.  Otherwise we
+        # will indiscriminantly delete the connection for all Server and that's
+        # no good.  So check if it's attached multiple times on this node
+        # if it is we skip the call to brick to delete the connection.
+        if connection_info.get('multiattach', False):
+            volume = self._volume_api.get(
+                context,
+                driver_block_device.get_volume_id(connection_info))
+            attachments = volume.get('attachments', {})
+            if len(attachments) > 1:
+                # First we get a list of all Server UUID's associated with
+                # this Host (Compute Node).  We're going to use this to
+                # determine if the Volume being detached is also in-use by
+                # another Server on this Host, ie just check to see if more
+                # than one attachment.server_id for this volume is in our
+                # list of Server UUID's for this Host
+                servers_this_host = objects.InstanceList.get_uuids_by_host(
+                    context, instance.host)
+
+                # NOTE(jdg): nova.volume.cinder translates the
+                # volume['attachments'] response into a dict which includes
+                # the Server UUID as the key, so we're using that
+                # here to check against our server_this_host list
+                for server_id, data in attachments.items():
+                    if server_id in servers_this_host:
+                        connection_count += 1
+        return (False if connection_count > 1 else True)
+
     def _disconnect_volume(self, context, connection_info, instance,
                            encryption=None):
         self._detach_encryptor(context, connection_info, encryption=encryption)
-        vol_driver = self._get_volume_driver(connection_info)
-        vol_driver.disconnect_volume(connection_info, instance)
+        if self._should_disconnect_target(context, connection_info, instance):
+            vol_driver = self._get_volume_driver(connection_info)
+            vol_driver.disconnect_volume(connection_info, instance)
+        else:
+            LOG.info("Detected multiple connections on this host for volume: "
+                     "%s, skipping target disconnect.",
+                     driver_block_device.get_volume_id(connection_info),
+                     instance=instance)
 
     def _extend_volume(self, connection_info, instance):
         vol_driver = self._get_volume_driver(connection_info)