Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA-398958 Cope with concurrent read-only activations #707

Merged
merged 1 commit into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions drivers/blktap2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion mocks/XenAPI/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class Failure(Exception):
def __init__(self, details):
pass
self.details = details

def xapi_local():
# Mock stub
Expand Down
72 changes: 72 additions & 0 deletions tests/test_blktap2.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import blktap2
import util
import xs_errors
import XenAPI


class BogusException(Exception):
Expand Down Expand Up @@ -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):

Expand Down
Loading