Skip to content

Commit

Permalink
fix(NFSSR): ensure we can attach SR during attach_from_config call
Browse files Browse the repository at this point in the history
We can get a trace like that if the SR is not attached:
```
2170:Oct 10 16:02:59 xcp4 SM: [2564] ***** NFSFileVDI.attach_from_config: EXCEPTION <type 'exceptions.AttributeError'>, 'NoneType' object has no attribute 'xenapi'
2329-Oct 10 16:02:59 xcp4 SM: [2564]   File "/opt/xensource/sm/NFSSR", line 296, in attach_from_config
2427-Oct 10 16:02:59 xcp4 SM: [2564]     self.sr.attach(sr_uuid)
2487-Oct 10 16:02:59 xcp4 SM: [2564]   File "/opt/xensource/sm/NFSSR", line 148, in attach
2573-Oct 10 16:02:59 xcp4 SM: [2564]     self._check_hardlinks()
2633-Oct 10 16:02:59 xcp4 SM: [2564]   File "/opt/xensource/sm/FileSR.py", line 1122, in _check_hardlinks
2734-Oct 10 16:02:59 xcp4 SM: [2564]     self.session.xenapi.SR.remove_from_sm_config(
2816-Oct 10 16:02:59 xcp4 SM: [2564]
```

Because the session is not set during this call.
So instead of using the XenAPI to store hardlink support, use a file on the storage itself.

Signed-off-by: Ronan Abhamon <[email protected]>
  • Loading branch information
Wescoeur committed Apr 10, 2024
1 parent ae021b6 commit 681198c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 59 deletions.
53 changes: 43 additions & 10 deletions drivers/FileSR.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,9 @@ def _checkmount(self):
(util.ismount(mount_path) or \
util.pathexists(self.remotepath) and self._isbind()))

# Override in SharedFileSR.
def _check_hardlinks(self):
return True

class FileVDI(VDI.VDI):
PARAM_VHD = "vhd"
Expand Down Expand Up @@ -762,11 +765,10 @@ def _unlink(self, path):
os.unlink(path)

def _create_new_parent(self, src, newsrc):
sr_sm_config = self.session.xenapi.SR.get_sm_config(self.sr.sr_ref)
if SharedFileSR.NO_HARDLINK_SUPPORT in sr_sm_config:
self._rename(src, newsrc)
else:
if self.sr._check_hardlinks():
self._link(src, newsrc)
else:
self._rename(src, newsrc)

def __fist_enospace(self):
raise util.CommandException(28, "vhd-util snapshot", reason="No space")
Expand Down Expand Up @@ -1072,7 +1074,6 @@ class SharedFileSR(FileSR):
"""
FileSR subclass for SRs that use shared network storage
"""
NO_HARDLINK_SUPPORT = "no_hardlinks"

def _check_writable(self):
"""
Expand All @@ -1092,6 +1093,10 @@ def _raise_hardlink_error(self):
raise OSError(524, "Unknown error 524")

def _check_hardlinks(self):
hardlink_conf = self._read_hardlink_conf()
if hardlink_conf is not None:
return hardlink_conf

test_name = os.path.join(self.path, str(uuid4()))
open(test_name, 'ab').close()

Expand All @@ -1103,17 +1108,17 @@ def _check_hardlinks(self):
self._raise_hardlink_error)

os.link(test_name, link_name)
if self.session:
self.session.xenapi.SR.remove_from_sm_config(
self.sr_ref, SharedFileSR.NO_HARDLINK_SUPPORT)
self._write_hardlink_conf(supported=True)
return True
except OSError:
self._write_hardlink_conf(supported=False)

msg = "File system for SR %s does not support hardlinks, crash " \
"consistency of snapshots cannot be assured" % self.uuid
util.SMlog(msg, priority=util.LOG_WARNING)
# Note: session can be not set during attach/detach_from_config calls.
if self.session:
try:
self.session.xenapi.SR.add_to_sm_config(
self.sr_ref, SharedFileSR.NO_HARDLINK_SUPPORT, 'True')
self.session.xenapi.message.create(
"sr_does_not_support_hardlinks", 2, "SR", self.uuid,
msg)
Expand All @@ -1124,6 +1129,34 @@ def _check_hardlinks(self):
util.force_unlink(link_name)
util.force_unlink(test_name)

return False

def _get_hardlink_conf_path(self):
return os.path.join(self.path, 'sm-hardlink.conf')

def _read_hardlink_conf(self):
try:
with open(self._get_hardlink_conf_path(), 'r') as f:
try:
return bool(int(f.read()))
except Exception as e:
# If we can't read, assume the file is empty and test for hardlink support.
return None
except IOError as e:
if e.errno == errno.ENOENT:
# If the config file doesn't exist, assume we want to support hardlinks.
return None
util.SMlog('Failed to read hardlink conf: {}'.format(e))
# Can be caused by a concurrent access, not a major issue.
return None

def _write_hardlink_conf(self, supported):
try:
with open(self._get_hardlink_conf_path(), 'w') as f:
f.write('1' if supported else '0')
except Exception as e:
# Can be caused by a concurrent access, not a major issue.
util.SMlog('Failed to write hardlink conf: {}'.format(e))

if __name__ == '__main__':
SRCommand.run(FileSR, DRIVER_INFO)
Expand Down
53 changes: 4 additions & 49 deletions tests/test_FileSR.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import testlib
import util
import vhdutil
from XenAPI import Failure


class FakeFileVDI(FileSR.FileVDI):
Expand Down Expand Up @@ -193,6 +192,7 @@ def test_clone_no_links_success(
vdi_uuid = str(uuid.uuid4())
sr = mock.MagicMock()
sr.path = "sr_path"
sr._check_hardlinks.return_value = False
vdi = FakeFileVDI(sr, vdi_uuid)
vdi.sr = sr

Expand All @@ -205,9 +205,6 @@ def test_clone_no_links_success(

mock_query_p_uuid.side_effect = [new_vdi_uuid, new_vdi_uuid, grandp_uuid]

sr.session.xenapi.SR.get_sm_config.return_value = {
"no_hardlinks": "True"}

# Act
clone_xml = vdi.clone(sr_uuid, vdi_uuid)

Expand Down Expand Up @@ -432,14 +429,15 @@ def attach(self, sr_uuid):
self._check_writable()
self._check_hardlinks()

def _read_hardlink_conf(self):
return None

class TestShareFileSR(unittest.TestCase):
"""
Tests for common Shared File SR operations
"""
TEST_SR_REF = "test_sr_ref"
ERROR_524 = "Unknown error 524"
NO_HARDLINKS = "no_hardlinks"

def setUp(self):
util_patcher = mock.patch('FileSR.util', autospec=True)
Expand Down Expand Up @@ -495,8 +493,7 @@ def test_attach_success(self):
test_sr.attach(self.sr_uuid)

# Assert
self.mock_session.xenapi.SR.remove_from_sm_config.assert_called_with(
TestShareFileSR.TEST_SR_REF, TestShareFileSR.NO_HARDLINKS)
self.assertEqual(0, self.mock_session.xenapi.message.create.call_count)

def test_attach_link_fail(self):
"""
Expand All @@ -511,49 +508,9 @@ def test_attach_link_fail(self):
test_sr.attach(self.sr_uuid)

# Assert
self.mock_session.xenapi.SR.add_to_sm_config.assert_called_with(
TestShareFileSR.TEST_SR_REF, TestShareFileSR.NO_HARDLINKS, 'True')
self.mock_session.xenapi.message.create.assert_called_with(
'sr_does_not_support_hardlinks', 2, "SR", self.sr_uuid, mock.ANY)

def test_attach_link_fail_already_set(self):
"""
Attach SR on FS with no hardlinks with config set
"""
test_sr = self.create_test_sr()

self.mock_link.side_effect = OSError(524, TestShareFileSR.ERROR_524)
self.mock_session.xenapi.SR.add_to_sm_config.side_effect = Failure(
['MAP_DUPLICATE_KEY', 'SR', 'sm_config',
'OpaqueRef:be8cc595-4924-4946-9082-59aef531daae',
TestShareFileSR.NO_HARDLINKS])

# Act
with mock.patch('FileSR.open'):
test_sr.attach(self.sr_uuid)

# Assert
self.mock_session.xenapi.SR.add_to_sm_config.assert_called_with(
TestShareFileSR.TEST_SR_REF, TestShareFileSR.NO_HARDLINKS, 'True')

def test_attach_success_no_session(self):
test_sr = self.create_sessionless_test_sr()

with mock.patch('FileSR.open'):
test_sr.attach(self.sr_uuid)

self.mock_session.xenapi.SR.remove_from_sm_config.assert_not_called()

def test_attach_link_fail_no_session(self):
test_sr = self.create_sessionless_test_sr()
self.mock_link.side_effect = OSError(524, TestShareFileSR.ERROR_524)

with mock.patch('FileSR.open'):
test_sr.attach(self.sr_uuid)

self.mock_session.xenapi.SR.add_to_sm_config.assert_not_called()
self.mock_session.xenapi.message.create.assert_not_called()

def test_attach_fist_active(self):
"""
Attach SR with FIST point active to set no hardlinks
Expand All @@ -567,8 +524,6 @@ def test_attach_fist_active(self):
test_sr.attach(self.sr_uuid)

# Assert
self.mock_session.xenapi.SR.add_to_sm_config.assert_called_with(
TestShareFileSR.TEST_SR_REF, TestShareFileSR.NO_HARDLINKS, 'True')
self.mock_session.xenapi.message.create.assert_called_with(
'sr_does_not_support_hardlinks', 2, "SR", self.sr_uuid, mock.ANY)

Expand Down

0 comments on commit 681198c

Please sign in to comment.