Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 20 Jan 2022 22:01:38 +0100
From:   Petr Vorel <pvorel@suse.cz>
To:     Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
Cc:     ltp@lists.linux.it, kernel@openvz.org, linux-nfs@vger.kernel.org,
        Steve Dickson <SteveD@redhat.com>, NeilBrown <neilb@suse.de>
Subject: Re: [PATCH] rpc_lib.sh: fix portmapper detection in case of socket
 activation
Message-ID: <YenNsuS1gcA9tDe3@pevik>
Reply-To: Petr Vorel <pvorel@suse.cz>
References: <20220120143727.27057-1-nikita.yushchenko@virtuozzo.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220120143727.27057-1-nikita.yushchenko@virtuozzo.com>
Precedence: bulk

Hi Nikita,

[ Cc: Steve as user-space maintainer, also Neil and whole linux-nfs ]

> On systemd-based linux hosts, rpcbind service is typically started via
> socket activation, when the first client connects. If no client has
> connected before LTP rpc test starts, rpcbind process will not be
> running at the time of check_portmap_rpcbind() execution, causing
> check_portmap_rpcbind() to report TCONF error.

> Fix that by adding a quiet invocation of 'rpcinfo' before checking for
> rpcbind.

Looks reasonable, but I'd prefer to have confirmation from NFS experts.

> For portmap, similar step is likely not needed, because portmap is used
> only on old systemd and those don't use systemd.

> Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
> ---
>  testcases/network/rpc/basic_tests/rpc_lib.sh | 6 ++++++
>  1 file changed, 6 insertions(+)

> diff --git a/testcases/network/rpc/basic_tests/rpc_lib.sh b/testcases/network/rpc/basic_tests/rpc_lib.sh
> index c7c868709..e882e41b3 100644
> --- a/testcases/network/rpc/basic_tests/rpc_lib.sh
> +++ b/testcases/network/rpc/basic_tests/rpc_lib.sh
> @@ -8,6 +8,12 @@ check_portmap_rpcbind()
>  	if pgrep portmap > /dev/null; then
>  		PORTMAPPER="portmap"
>  	else
> +		# In case of systemd socket activation, rpcbind could be
> +		# not started until somebody tries to connect to it's socket.
> +		#
> +		# To handle that case properly, run a client now.
> +		rpcinfo >/dev/null 2>&1
nit: Shouldn't we keep stderr? In LTP we put required commands into
$TST_NEEDS_CMDS. It'd be better not require rpcinfo (not a hard dependency),
and thus it'd be better to see "command not found" when rpcinfo missing and test
fails.

Kind regards,
Petr

> +
>  		pgrep rpcbind > /dev/null && PORTMAPPER="rpcbind" || \
>  			tst_brk TCONF "portmap or rpcbind is not running"
>  	fi