From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id E9FFB1FF13E for ; Fri, 20 Mar 2026 13:54:46 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CECFF17647; Fri, 20 Mar 2026 13:54:59 +0100 (CET) Message-ID: <7c265ae6-1109-4ac6-a859-f51b78638d52@proxmox.com> Date: Fri, 20 Mar 2026 13:54:18 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH storage v2 1/2] nfs: create a helper to run rpcinfo commands To: Maximiliano Sandoval , pve-devel@lists.proxmox.com References: <20260319104241.191910-1-m.sandoval@proxmox.com> <20260319104241.191910-2-m.sandoval@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260319104241.191910-2-m.sandoval@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774011219536 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.061 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.408 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.819 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.903 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: KWIG7NYVKCJBQWJMUUEKTYMKYDH7AKWV X-Message-ID-Hash: KWIG7NYVKCJBQWJMUUEKTYMKYDH7AKWV X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 19.03.26 um 11:42 AM schrieb Maximiliano Sandoval: > Will be used in the next command as a fallback in the NFSv3 case. > > Signed-off-by: Maximiliano Sandoval > --- > 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 { The helper could be private. I wouldn't use tcp as a prefix (if using it, rather later in the name). Other plugins use the plugin name as the prefix for intra-module helpers. Maybe something like nfs_get_rpcinfo_command? > + 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 I feel like putting it on the same line makes it harder to read. There should be a comma or dot after the slash. But actually, the comment does not belong inside the function here, but to the call site. It's the clarification for why rpcinfo is used rather than listing exports. > + 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]; > }