Received-SPF: pass (google.com: domain of linux-nfs+bounces-2434-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Feedback-ID: i197146af:Fastmail
Date: Thu, 21 Mar 2024 16:39:52 +0100
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Chuck Lever <chuck.lever@oracle.com>, rsbecker@nexbridge.com,
	Jeff King <peff@peff.net>, linux-nfs@vger.kernel.org
Subject: [PATCH] reftable: fix tests being broken by NFS' delete-after-close
 semantics
Message-ID: <8ac5e94a3930cdd2aee9ea86acda3155674b635c.1711035529.git.ps@pks.im>
References: <ZfBwZTL9zqDsac5m@manet.1015granger.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="lVOhhQ7Lw62NesZC"
Content-Disposition: inline
In-Reply-To: <ZfBwZTL9zqDsac5m@manet.1015granger.net>


--lVOhhQ7Lw62NesZC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

It was reported that the reftable unit tests in t0032 fail with the
following assertion when running on top of NFS:

    running test_reftable_stack_compaction_concurrent_clean
    reftable/stack_test.c: 1063: failed assertion count_dir_entries(dir) =
=3D=3D 2
    Aborted

Setting a breakpoint immediately before the assertion in fact shows the
following list of files:

    ./stack_test-1027.QJBpnd
    ./stack_test-1027.QJBpnd/0x000000000001-0x000000000003-dad7ac80.ref
    ./stack_test-1027.QJBpnd/.nfs000000000001729f00001e11
    ./stack_test-1027.QJBpnd/tables.list

Note the weird ".nfs*" file? This file is maintained by NFS clients in
order to emulate delete-after-last-close semantics that we rely on in
the reftable code [1]. Instead of unlinking the file right away and
keeping it open in the client, the NFS client will rename it to ".nfs*"
and then delete that temporary file when the last reference to it gets
dropped. Quoting the NFS FAQ:

    > D2. What is a "silly rename"? Why do these .nfsXXXXX files keep
    > showing up?
    >
    > A. Unix applications often open a scratch file and then unlink it.
    > They do this so that the file is not visible in the file system name
    > space to any other applications, and so that the system will
    > automatically clean up (delete) the file when the application exits.
    > This is known as "delete on last close", and is a tradition among
    > Unix applications.
    >
    > Because of the design of the NFS protocol, there is no way for a
    > file to be deleted from the name space but still remain in use by an
    > application. Thus NFS clients have to emulate this using what
    > already exists in the protocol. If an open file is unlinked, an NFS
    > client renames it to a special name that looks like ".nfsXXXXX".
    > This "hides" the file while it remains in use. This is known as a
    > "silly rename." Note that NFS servers have nothing to do with this
    > behavior.

This of course throws off the assertion that we got exactly two files in
that directory.

The test in question triggers this behaviour by holding two open file
descriptors to the "tables.list" file. One of the references is because
we are about to append to the stack, whereas the other reference is
because we want to compact it. As the compaction has just finished we
already rewrote "tables.list" to point to the new contents, but the
other file descriptor pointing to the old version is still open. Thus we
trigger the delete-after-last-close emulation.

Furthermore, it was reported that this behaviour only triggers with
4f36b8597c (reftable/stack: fix race in up-to-date check, 2024-01-18).
This is expected as well because it is the first point in time where we
actually keep the "tables.list" file descriptor open for the stat cache.

Fix this bug by skipping over any files that start with a leading dot
when counting files. While we could explicitly check for a prefix of
".nfs", other network file systems like SMB for example do the same
trickery but with a ".smb" prefix. In any case though, this loosening of
the assertion should be fine given that the reftable library would never
write files with leading dots by itself.

[1]: https://nfs.sourceforge.net/#faq_d2

Reported-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack_test.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 509f486623..621578fda8 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -38,7 +38,17 @@ static int count_dir_entries(const char *dirname)
 		return 0;
=20
 	while ((d =3D readdir(dir))) {
-		if (!strcmp(d->d_name, "..") || !strcmp(d->d_name, "."))
+		/*
+		 * Besides skipping over "." and "..", we also need to
+		 * skip over other files that have a leading ".". This
+		 * is due to behaviour of NFS, which will rename files
+		 * to ".nfs*" to emulate delete-on-last-close.
+		 *
+		 * In any case this should be fine as the reftable
+		 * library will never write files with leading dots
+		 * anyway.
+		 */
+		if (starts_with(d->d_name, "."))
 			continue;
 		len++;
 	}
--=20
2.44.0


--lVOhhQ7Lw62NesZC
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmX8VMcACgkQVbJhu7ck
PpRMFw/7B0htFVMcP+vZWDRgVtOoXcKAqy2Isx94z0l3SdMn6HfdrtgCaueivM2Z
pWNFxszPx8B22ogv2I+b6dIKHOslNMlyT0Zu3HHMmcoFwuOLlyg13eWvMSy65sGz
HJqqvYBqTnTK40HiK16rM2uxMvgR2LMb4JJKrIFzhwp1Eb1wfYmJYASvbxsuF3Kv
DvI56h+tv718WbIVrLGH8kILAOMohWSt7H2uHkXtpcHvqnDvRA6GZPmrPyWi+B2r
IrWEGG46GpPvn+lR33FHGNYGh16gNlNDACAINVXctSss3pf6IIcd2AfJqXj9jcaT
ZgmwrQx+1u0YNqM7+sf3X8du3fvVhQs+pxRuKzfPGBlfn2HfLryAC/dUOCC2sHPI
3+QDiA50pbl58iSNBkQduShucD1lqTj9Mj+YD4/YysJaf7RLiHV/uWiZ+erDMsUT
WQBcFPH5nF6Z06YTltfyj1subpjXJTQ1cydNcR0ikOCKS43WY9i66IbhgAXoRM4S
u9FrgYg1LQDgUnqBtglU5/v1ZlCWGH2Ys3LCmi/PN3PvMvUbMHnpExGxNx/2fVSN
mz/pRu5iE10l9TuM2jRSnjQdrCCvSCX3G7nAZeb/LbQxxiYT79k1ReT/CeoiXUZ6
4MCX44j5Tcdlhci5zJmH59dqDm/ERqwb3E0vV5yhtAUpATEmYcM=
=Wqvx
-----END PGP SIGNATURE-----

--lVOhhQ7Lw62NesZC--