fix(NFSSR): ensure we can attach SR during attach_from_config call

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]>

drivers/FileSR.py

@@ -413,6 +413,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"
@@ -780,11 +783,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")
@@ -1102,12 +1104,15 @@ class SharedFileSR(FileSR):
     """
     FileSR subclass for SRs that use shared network storage
     """
-    NO_HARDLINK_SUPPORT = "no_hardlinks"
 
     def _raise_hardlink_error(self):
         raise OSError(542, "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()
 
@@ -1119,25 +1124,55 @@ def _check_hardlinks(self):
                 self._raise_hardlink_error)
 
             os.link(test_name, link_name)
-            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)
-            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)
-            except XenAPI.Failure:
-                # Might already be set and checking has TOCTOU issues
-                pass
+            # Note: session can be not set during attach/detach_from_config calls.
+            if self.session:
+                try:
+                    self.session.xenapi.message.create(
+                        "sr_does_not_support_hardlinks", 2, "SR", self.uuid,
+                        msg)
+                except XenAPI.Failure:
+                    # Might already be set and checking has TOCTOU issues
+                    pass
         finally:
             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)

tests/mocks/XenAPI/__init__.py

@@ -1,3 +0,0 @@
-class Failure(Exception):
-    def __init__(self, details):
-        pass

tests/test_FileSR.py

@@ -8,7 +8,6 @@
 import stat
 import unittest
 import uuid
-from XenAPI import Failure
 
 
 class FakeFileVDI(FileSR.FileVDI):
@@ -121,14 +120,15 @@ def load(self, sr_uuid):
     def attach(self, sr_uuid):
         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):
         fist_patcher = mock.patch('FileSR.util.FistPoint.is_active',
@@ -182,8 +182,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):
         """
@@ -198,31 +197,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('__builtin__.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_fist_active(self):
         """
         Attach SR with FIST point active to set no hardlinks
@@ -238,7 +215,5 @@ 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)