[PATCH storage v6 0/2] nfs: use rpcinfo as a showmount fallback for nfs3

[PATCH storage v6 0/2] nfs: use rpcinfo as a showmount fallback for nfs3

[Maximiliano Sandoval]

See the respective commit messages.

This came up in enterprise support. In this case the NFS appliance only
supported NFSv3 and the showmount command failed, but rpcbind
succeeded (with -T tcp).

This possibility was discussed with Friedrich and Fiona off-list.

Tested:
 - Created a NFS share on host "B"
 - On host "A" a NFS storage was added (using B's share) with --options vers=3
 - Ran `ss -tulpn4 |grep mountd` on host B to see ports used by showmount
 - The ports listed were blocked on host "A", so that showmount fails:

   /sbin/showmount --no-headers --exports $B
   rpc mount export: RPC: Unable to send; errno = Operation not permitted

 - Ran `pvesm status` on A and checked that the NFS storage is available
 - Created a VM on A with a disk image in the the NFS storage

The later two operations would have failed without the changes here.

Differences from v5:
 - Move note regarding timeout to fallback

Differences from v4:
 - Use proper FIXME: MAJOR VERSION comment
 - Simplify the FIXME comment
 - Move the NFSv4-only note to the fallback branch

Differences from v3:
 - Fix some typos
 - Turn TODO note into FIXME
 - Moved the pitfalls mentioned in the commit message to the FIXME

Differences from v2:
 - add PVE 10 prefix to TODO note
 - rename the rpcinfo helper
 - make the rpcinfo helper private
 - Always run the fallback branch when v4 != 4.
 - Leave nfsv4+rpcinfo comment on call-site

Differences from v1:
 - the name of the helper function was fixed

Maximiliano Sandoval (2):
  nfs: create a helper to get rpcinfo commands
  nfs: use rpcinfo as a showmount fallback for nfs3

 src/PVE/Storage/NFSPlugin.pm | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

-- 
2.47.3

[PATCH storage v6 1/2] nfs: create a helper to get rpcinfo commands

[Maximiliano Sandoval]

Will also be used in the next commit as a fallback in the case where opts vers=4
is not used.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/PVE/Storage/NFSPlugin.pm | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
index 4cc02c9e..56dbd9ec 100644
--- a/src/PVE/Storage/NFSPlugin.pm
+++ b/src/PVE/Storage/NFSPlugin.pm
@@ -171,6 +171,19 @@ sub deactivate_storage {
     }
 }
 
+my $nfs_get_rpcinfo_command = sub {
+    my ($server, $version) = @_;
+
+    my $ip = PVE::JSONSchema::pve_verify_ip($server, 1);
+    if (!defined($ip)) {
+        $ip = PVE::Network::get_ip_from_hostname($server);
+    }
+
+    my $transport = PVE::JSONSchema::pve_verify_ipv4($ip, 1) ? 'tcp' : 'tcp6';
+
+    return ['/usr/sbin/rpcinfo', '-T', $transport, $ip, 'nfs', $version];
+};
+
 sub check_connection {
     my ($class, $storeid, $scfg) = @_;
 
@@ -181,16 +194,9 @@ sub check_connection {
 
     my $is_v4 = defined($opts) && $opts =~ /vers=4.*/;
     if ($is_v4) {
-        my $ip = PVE::JSONSchema::pve_verify_ip($server, 1);
-        if (!defined($ip)) {
-            $ip = PVE::Network::get_ip_from_hostname($server);
-        }
-
-        my $transport = PVE::JSONSchema::pve_verify_ipv4($ip, 1) ? 'tcp' : 'tcp6';
-
         # nfsv4 uses a pseudo-filesystem always beginning with /
         # no exports are listed
-        $cmd = ['/usr/sbin/rpcinfo', '-T', $transport, $ip, 'nfs', '4'];
+        $cmd = $nfs_get_rpcinfo_command->($server, '4');
     } else {
         $cmd = ['/sbin/showmount', '--no-headers', '--exports', $server];
     }
-- 
2.47.3

[PATCH storage v6 2/2] nfs: use rpcinfo as a showmount fallback for nfs3

[Maximiliano Sandoval]

The check_connection call must succeed in order for the NFS storage to
be activated. In a setup in which the NFS server does not list exports
and which only supports NFSv3 this would result in the storage not being
usable (not without manually mounting the NFS share and using a
Directory storage instead). We add a fallback check with rpcinfo for
this case.

The rpcinfo check was replaced with showmount in f8b0d82f. At the time
the rationale was that rpcinfo did not have support for ipv6
connections, which does not hold anymore.

In order to not break compatibility with setups that already activate
correctly, we add a rpcinfo check as a fallback for NFSv3-only setups
only after the showmount command fails.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/PVE/Storage/NFSPlugin.pm | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
index 56dbd9ec..123f14a3 100644
--- a/src/PVE/Storage/NFSPlugin.pm
+++ b/src/PVE/Storage/NFSPlugin.pm
@@ -198,6 +198,8 @@ sub check_connection {
         # no exports are listed
         $cmd = $nfs_get_rpcinfo_command->($server, '4');
     } else {
+        # FIXME: MAJOR VERSION: Consider switching to rpcinfo. Note that some NFSv3 servers might
+        # only support UDP (cf. RFC 1813, Section 2.3)
         $cmd = ['/sbin/showmount', '--no-headers', '--exports', $server];
     }
 
@@ -214,6 +216,15 @@ sub check_connection {
             return 0 if $port == 0;
 
             return PVE::Network::tcp_ping($server, $port, 2);
+        } else {
+            # NOTE: Some NFSv4-only servers might not support rpcbind as described at commit acff8954
+            # NOTE: For a NFSv3-only server the two timeouts add up to a worst-case scenario of 20
+            # seconds
+            $cmd = $nfs_get_rpcinfo_command->($server, '3');
+            eval {
+                run_command($cmd, timeout => 10, outfunc => sub { }, errfunc => sub { });
+            };
+            return 1 if !$@;
         }
         return 0;
     }
-- 
2.47.3