Backport probe for NFS4 when rpcinfo does not include it

See: xapi-project#655

Signed-off-by: Benjamin Reis <benjamin.reis@vates.tech>

drivers/nfs.py

@@ -83,10 +83,14 @@ def check_server_service(server, transport):
     NFS_SERVICE_RETRY * NFS_SERVICE_WAIT
     """
 
-    sv = get_supported_nfs_versions(server, transport)
-    # Services are not present in NFS4 only, this doesn't mean there's no NFS
-    if sv == ['4']:
-        return True
+    try:
+        sv = get_supported_nfs_versions(server, transport)
+        # Services are not present in NFS4 only, this doesn't mean there's no NFS
+        if "4" in sv:
+            return True
+    except NfsException:
+        # Server failed to give us supported versions
+        pass
 
     retries = 0
     errlist = [errno.EPERM, errno.EPIPE, errno.EIO]
@@ -309,6 +313,9 @@ def scan_srlist(path, transport, dconf):
 def _get_supported_nfs_version_rpcinfo(server):
     """ Return list of supported nfs versions.
         Using NFS3 services.
+        *Might* return "4" in the list of supported NFS versions, but might not:
+        There is no requirement for NFS4 to register with rpcbind, even though it can, so
+        a server which supports NFS4 might still only return ["3"] from here.
     """
 
     valid_versions = set(["3", "4"])
@@ -340,20 +347,27 @@ def _is_nfs4_supported(server, transport):
 
 
 def get_supported_nfs_versions(server, transport):
-    """Return list of supported nfs versions."""
+    """
+    Return list of supported nfs versions.
+    First check list from rpcinfo and if that does not contain NFS4, probe for it and
+    add it to the list if available.
+    """
+    vers = []
     try:
-        return _get_supported_nfs_version_rpcinfo(server)
+        vers = _get_supported_nfs_version_rpcinfo(server)
     except Exception:
         util.SMlog("Unable to obtain list of valid nfs versions with %s, trying NFSv4" % RPCINFO_BIN)
 
-    # NFSv4 only
-    if _is_nfs4_supported(server, transport):
-        return ["4"]
+    # Test for NFS4 if the rpcinfo query did not find it (NFS4 does not *have* to register with rpcbind)
+    if "4" not in vers:
+        if _is_nfs4_supported(server, transport):
+            vers.append("4")
+
+    if vers:
+        return vers
     else:
-        util.SMlog("Unable to obtain list of valid nfs versions with NFSv4 pseudo FS mount")
+        raise NfsException("Failed to read supported NFS version from server %s" % (server))
 
-    raise NfsException("Failed to read supported NFS version from server %s" %
-                           (server))
 
 def get_nfs_timeout(other_config):
     nfs_timeout = 200

tests/test_nfs.py

@@ -9,12 +9,14 @@ class Test_nfs(unittest.TestCase):
 
     @mock.patch('util.pread', autospec=True)
     def test_check_server_tcp(self, pread):
+        pread.return_value = "    100003  4,3,2     udp6,tcp6,udp,tcp                nfs         superuser"
         nfs.check_server_tcp('aServer', 'tcp')
 
         pread.assert_called_once_with(['/usr/sbin/rpcinfo', '-s', 'aServer'], quiet=False)
 
     @mock.patch('util.pread', autospec=True)
     def test_check_server_tcp_nfsversion(self, pread):
+        pread.return_value = "    100003  4,3,2     udp6,tcp6,udp,tcp                nfs         superuser"
         nfs.check_server_tcp('aServer', 'tcp', 'aNfsversion')
 
         pread.assert_called_once_with(['/usr/sbin/rpcinfo', '-s', 'aServer'], quiet=False)
@@ -42,23 +44,28 @@ def test_check_server_service(self, pread, get_supported_nfs_versions, sleep):
         # Mock==1.0.1 (all that is available in Epel) doesn't suport this
         #sleep.assert_not_called()
 
+    @mock.patch('nfs._is_nfs4_supported', autospec=True)
     @mock.patch('time.sleep', autospec=True)
     # Can't use autospec due to http://bugs.python.org/issue17826
     @mock.patch('util.pread')
-    def test_check_server_service_with_retries(self, pread, sleep):
+    def test_check_server_service_with_retries(self, pread, sleep, nfs4sup):
         pread.side_effect=["",
                            "",
-                           "    100003  4,3,2     udp6,tcp6,udp,tcp                nfs         superuser"]
+                           "    100003  3,2     udp6,tcp6,udp,tcp                nfs         superuser"]
+        nfs4sup.return_value = False
+
         service_found = nfs.check_server_service('aServer', 'tcp')
 
         self.assertTrue(service_found)
         self.assertEqual(len(pread.mock_calls), 3)
         pread.assert_called_with(['/usr/sbin/rpcinfo', '-s', 'aServer'])
 
+    @mock.patch('nfs._is_nfs4_supported', autospec=True)
     @mock.patch('time.sleep', autospec=True)
     @mock.patch('util.pread', autospec=True)
-    def test_check_server_service_not_available(self, pread, sleep):
+    def test_check_server_service_not_available(self, pread, sleep, nfs4sup):
         pread.return_value=""
+        nfs4sup.return_value = False
 
         service_found = nfs.check_server_service('aServer', 'tcp')
 
@@ -86,6 +93,30 @@ def test_check_server_service_first_call_exception(self, pread, get_supported_nf
         self.assertTrue(service_found)
         self.assertEqual(len(pread.mock_calls), 2)
 
+    @mock.patch('nfs._is_nfs4_supported', autospec=True)
+    @mock.patch('util.pread2')
+    def test_get_supported_nfs_versions_rpc_nov4(self, pread2, nfs4sup):
+        pread2.side_effect = ["    100003  3,2     udp6,tcp6,udp,tcp                nfs         superuser"]
+        nfs4sup.return_value = True
+
+        versions = nfs.get_supported_nfs_versions('aServer', 'tcp')
+
+        self.assertEqual(versions, ['3', '4'])
+        self.assertEqual(len(pread2.mock_calls), 1)
+        pread2.assert_called_with(['/usr/sbin/rpcinfo', '-s', 'aServer'])
+
+    @mock.patch('nfs._is_nfs4_supported', autospec=True)
+    @mock.patch('util.pread2')
+    def test_get_supported_nfs_versions_nov4(self, pread2, nfs4sup):
+        pread2.side_effect = ["    100003  3,2     udp6,tcp6,udp,tcp                nfs         superuser"]
+        nfs4sup.return_value = False
+
+        versions = nfs.get_supported_nfs_versions('aServer', 'tcp')
+
+        self.assertEqual(versions, ['3'])
+        self.assertEqual(len(pread2.mock_calls), 1)
+        pread2.assert_called_with(['/usr/sbin/rpcinfo', '-s', 'aServer'])
+
     def get_soft_mount_pread(self, binary, vers):
         return ([binary, 'remoteserver:remotepath', 'mountpoint', '-o',
                  'soft,proto=transport,vers=%s,acdirmin=0,acdirmax=0' % vers])