From ce8fcf83f75e07cfdb314ea131c6a6be67666007 Mon Sep 17 00:00:00 2001 From: Robin Newton Date: Thu, 5 Sep 2024 14:21:04 +0100 Subject: [PATCH] CA-398958 Cope with concurrent read-only activations It should be possible to activate a VDI on two different hosts in a pool simultaneously, provided read-only access is all that it required. So, if one host tries to activate in a brief window when another host has put the transient "activating" key on the VDI, it should be allowed to retry. Signed-off-by: Robin Newton --- drivers/blktap2.py | 10 ++++-- mocks/XenAPI/__init__.py | 2 +- tests/test_blktap2.py | 72 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/drivers/blktap2.py b/drivers/blktap2.py index 5712d8f22..89b124088 100755 --- a/drivers/blktap2.py +++ b/drivers/blktap2.py @@ -1446,8 +1446,14 @@ def _add_tag(self, vdi_uuid, writable): if 'paused' in sm_config: util.SMlog("Paused or host_ref key found [%s]" % sm_config) return False - self._session.xenapi.VDI.add_to_sm_config( - vdi_ref, 'activating', 'True') + try: + self._session.xenapi.VDI.add_to_sm_config( + vdi_ref, 'activating', 'True') + except XenAPI.Failure as e: + if e.details[0] == 'MAP_DUPLICATE_KEY' and not writable: + # Someone else is activating - a retry might succeed + return False + raise host_key = "host_%s" % host_ref assert host_key not in sm_config self._session.xenapi.VDI.add_to_sm_config(vdi_ref, host_key, diff --git a/mocks/XenAPI/__init__.py b/mocks/XenAPI/__init__.py index 8e45a1b46..9dd4441fb 100644 --- a/mocks/XenAPI/__init__.py +++ b/mocks/XenAPI/__init__.py @@ -1,6 +1,6 @@ class Failure(Exception): def __init__(self, details): - pass + self.details = details def xapi_local(): # Mock stub diff --git a/tests/test_blktap2.py b/tests/test_blktap2.py index d0e36daf1..f1f0c9208 100644 --- a/tests/test_blktap2.py +++ b/tests/test_blktap2.py @@ -11,6 +11,7 @@ import blktap2 import util import xs_errors +import XenAPI class BogusException(Exception): @@ -335,6 +336,77 @@ def test_activate_relink_while_tagging( mock.call('vref1', 'activating')], any_order=True) + @mock.patch('blktap2.time.sleep', autospec=True) + @mock.patch('blktap2.util.get_this_host', autospec=True) + @mock.patch('blktap2.VDI._attach', autospec=True) + @mock.patch('blktap2.VDI.PhyLink', autospec=True) + @mock.patch('blktap2.VDI.BackendLink', autospec=True) + @mock.patch('blktap2.VDI.NBDLink', autospec=True) + @mock.patch('blktap2.Tapdisk') + def test_activate_ro_already_activating_retry( + self, mock_tapdisk, mock_nbd_link, mock_backend, + mock_phy, mock_attach, + mock_this_host, mock_sleep): + """ + If we're activating for read-only access, with someone else (let's + say, another host in the pool) also being in the process of + activating, should result in a retry. + """ + mock_this_host.return_value = str(uuid.uuid4()) + + self.mock_session.xenapi.VDI.get_sm_config.return_value = {} + self.mock_session.xenapi.host.get_by_uuid.return_value = 'href1' + self.mock_session.xenapi.VDI.get_by_uuid.return_value = 'vref1' + + self.mock_session.xenapi.VDI.add_to_sm_config.side_effect = [ + XenAPI.Failure(['MAP_DUPLICATE_KEY', 'VDI', 'sm_config', + 'href1', 'activating']), + None, + None + ] + + self.vdi.activate(self.sr_uuid, self.vdi_uuid, False, {}) + + self.mock_session.xenapi.VDI.add_to_sm_config.assert_has_calls( + [mock.call('vref1', 'activating', 'True'), + mock.call('vref1', 'activating', 'True'), + mock.call('vref1', 'host_href1', "RO")]) + self.mock_session.xenapi.VDI.remove_from_sm_config.assert_has_calls( + [mock.call('vref1', 'activating')]) + + @mock.patch('blktap2.time.sleep', autospec=True) + @mock.patch('blktap2.util.get_this_host', autospec=True) + @mock.patch('blktap2.VDI._attach', autospec=True) + @mock.patch('blktap2.VDI.PhyLink', autospec=True) + @mock.patch('blktap2.VDI.BackendLink', autospec=True) + @mock.patch('blktap2.VDI.NBDLink', autospec=True) + @mock.patch('blktap2.Tapdisk') + def test_activate_rw_already_activating_fail( + self, mock_tapdisk, mock_nbd_link, mock_backend, + mock_phy, mock_attach, + mock_this_host, mock_sleep): + """ + If we're activating for read-write access, with someone else (let's + say, another host in the pool) also being in the process of + activating, should result in a failure. + """ + mock_this_host.return_value = str(uuid.uuid4()) + + self.mock_session.xenapi.VDI.get_sm_config.return_value = {} + self.mock_session.xenapi.host.get_by_uuid.return_value = 'href1' + self.mock_session.xenapi.VDI.get_by_uuid.return_value = 'vref1' + + self.mock_session.xenapi.VDI.add_to_sm_config.side_effect = [ + XenAPI.Failure(['MAP_DUPLICATE_KEY', 'VDI', 'sm_config', + 'href1', 'activating']), + ] + + with self.assertRaises(xs_errors.SROSError) as srose: + self.vdi.activate(self.sr_uuid, self.vdi_uuid, True, {}) + + self.assertEqual(46, srose.exception.errno) + self.assertIn( 'MAP_DUPLICATE_KEY', str(srose.exception)) + class TestTapCtl(unittest.TestCase):