CA-384783 Probe for NFS4 when rpcinfo does not include it (xapi-proje…

…ct#655)

Just because the rpcinfo list obtained from rpcbind does not contain
NFS4 does not mean it is not supported. Probe for it if it is not in the
list. Some filers supporting both do not register NFS4 with rpcbind.

Signed-off-by: Tim Smith <[email protected]>

drivers/nfs.py

@@ -81,10 +81,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"])
@@ -339,20 +346,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"]
-    else:
-        util.SMlog("Unable to obtain list of valid nfs versions with NFSv4 pseudo FS mount")
+    # 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")
 
-    raise NfsException("Failed to read supported NFS version from server %s" %
-                           (server))
+    if vers:
+        return vers
+    else:
+        raise NfsException("Failed to read supported NFS version from server %s" % (server))
 
 
 def get_nfs_timeout(other_config):

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.side_effect = ["    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, text=True)
 
     @mock.patch('util.pread', autospec=True)
     def test_check_server_tcp_nfsversion(self, pread):
+        pread.side_effect = ["    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, text=True)
@@ -41,23 +43,28 @@ def test_check_server_service(self, pread, get_supported_nfs_versions, sleep):
         pread.assert_called_with(['/usr/sbin/rpcinfo', '-s', 'aServer'])
         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')
 
@@ -94,6 +101,30 @@ def test_get_supported_nfs_versions(self, pread2):
         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_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, ipv6=False):
         remote = '[remoteserver]' if ipv6 else 'remoteserver'
         transport = 'tcp6' if ipv6 else 'transport'