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

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

[Maximiliano Sandoval]

See the respective commit messages.

This came up in enterprise support in which the showmounts command failed but
rpcbind succeeded (with -T tcp).

This possibility was discussed with Friedrich and Fiona off-list. I hope to get
more feedback here.

This is not yet tested as I need to find a reproducer for a few cases. I Will
update once this is tested.

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

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

-- 
2.47.3

[RFC PATCH storage 1/2] nfs: create a helper to run rpcinfo commands

[Maximiliano Sandoval]

Will be used in the next command as a fallback in the NFSv3 case.

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

diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
index 4cc02c9e..062b3f32 100644
--- a/src/PVE/Storage/NFSPlugin.pm
+++ b/src/PVE/Storage/NFSPlugin.pm
@@ -171,6 +171,20 @@ sub deactivate_storage {
     }
 }
 
+sub tcp_rpcinfo_cmd {
+    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';
+
+    # nfsv4 uses a pseudo-filesystem always beginning with / no exports are listed
+    return ['/usr/sbin/rpcinfo', '-T', $transport, $ip, 'nfs', $version];
+}
+
 sub check_connection {
     my ($class, $storeid, $scfg) = @_;
 
@@ -181,16 +195,7 @@ 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 = tcp_rpcinfo_cmd($server, '4');
     } else {
         $cmd = ['/sbin/showmount', '--no-headers', '--exports', $server];
     }
-- 
2.47.3

[RFC PATCH storage 2/2] nfs: use rpcinfo as a showmounts 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 the
Directory storage instead). We add a fallback check with rpcinfo for
this case.

The rpcinfo check was replaced with showmounts in f8b0d82fe. At the time
the rationale was that rpcinfo did not have support for ipv6
connections, which is not the case at the moment.

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 showmounts command fails.

Potential pitfalls:
- Some NFSv3 servers might only support UDP [RFC 1813]
- The two timeouts add up to 20 seconds which might be too much
- Some NFS servers might not have rpcbind as described in acff89540

[RFC 1813]: https://datatracker.ietf.org/doc/html/rfc1813#section-2.3

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

diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
index 062b3f32..3654e8b3 100644
--- a/src/PVE/Storage/NFSPlugin.pm
+++ b/src/PVE/Storage/NFSPlugin.pm
@@ -193,10 +193,12 @@ sub check_connection {
 
     my $cmd;
 
+    my $is_v3 = defined($opts) && $opts =~ /vers=3.*/;
     my $is_v4 = defined($opts) && $opts =~ /vers=4.*/;
     if ($is_v4) {
         $cmd = tcp_rpcinfo_cmd($server, '4');
     } else {
+        # TODO Consider switching to rpcinfo altogether for Proxmox VE 10.
         $cmd = ['/sbin/showmount', '--no-headers', '--exports', $server];
     }
 
@@ -213,6 +215,12 @@ sub check_connection {
             return 0 if $port == 0;
 
             return PVE::Network::tcp_ping($server, $port, 2);
+        } elsif ($is_v3) {
+            $cmd = tcp_rpcinfo($server, '3');
+            eval {
+                run_command($cmd, timeout => 10, outfunc => sub { }, errfunc => sub { });
+            };
+            return 1 if !$@;
         }
         return 0;
     }
-- 
2.47.3