diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index cd375585fce2..33c06df4f77b 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/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 \ diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 6e8a4f0dacde..b8d496527d74 100644 --- a/nova/virt/libvirt/driver.py +++ b/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)