[PATCH xfstests v1 1/2] statx: update headers to include newer statx fields

[PATCH xfstests v1 1/2] statx: update headers to include newer statx fields

[Aleksa Sarai]

These come from Linux v6.11-rc5.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 src/open_by_handle.c |  4 +++-
 src/statx.h          | 22 ++++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 0f74ed08b1f0..d9c802ca9bd1 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -82,12 +82,14 @@ Examples:
 #include <string.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <sys/stat.h>
 #include <sys/types.h>
 #include <errno.h>
 #include <linux/limits.h>
 #include <libgen.h>
 
+#include <sys/stat.h>
+#include "statx.h"
+
 #define MAXFILES 1024
 
 struct handle {
diff --git a/src/statx.h b/src/statx.h
index 3f239d791dfe..935cb2ed415e 100644
--- a/src/statx.h
+++ b/src/statx.h
@@ -102,7 +102,7 @@ struct statx {
 	__u64	stx_ino;	/* Inode number */
 	__u64	stx_size;	/* File size */
 	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
-	__u64	__spare1[1];
+	__u64	stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
 	/* 0x40 */
 	struct statx_timestamp	stx_atime;	/* Last access time */
 	struct statx_timestamp	stx_btime;	/* File creation time */
@@ -114,7 +114,18 @@ struct statx {
 	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
 	__u32	stx_dev_minor;
 	/* 0x90 */
-	__u64	__spare2[14];	/* Spare space for future expansion */
+	__u64	stx_mnt_id;
+	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
+	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
+	/* 0xa0 */
+	__u64	stx_subvol;	/* Subvolume identifier */
+	__u32	stx_atomic_write_unit_min;	/* Min atomic write unit in bytes */
+	__u32	stx_atomic_write_unit_max;	/* Max atomic write unit in bytes */
+	/* 0xb0 */
+	__u32   stx_atomic_write_segments_max;	/* Max atomic write segment count */
+	__u32   __spare1[1];
+	/* 0xb8 */
+	__u64	__spare3[9];	/* Spare space for future expansion */
 	/* 0x100 */
 };
 
@@ -139,6 +150,13 @@ struct statx {
 #define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
 #define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
 #define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
+#define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
+#define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
+#define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
+#define STATX_SUBVOL		0x00008000U	/* Want/got stx_subvol */
+#define STATX_WRITE_ATOMIC	0x00010000U	/* Want/got atomic_write_* fields */
+
+#define STATX__RESERVED		0x80000000U	/* Reserved for future struct statx expansion */
 #define STATX_ALL		0x00000fffU	/* All currently supported flags */
 
 /*
-- 
2.46.0

[PATCH xfstests v1 2/2] open_by_handle: add tests for u64 mount ID

[Aleksa Sarai]

Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
make sure they match properly as part of the regular open_by_handle
tests.

Link: https://lore.kernel.org/all/20240801-exportfs-u64-mount-id-v3-0-be5d6283144a@cyphar.com/
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 src/open_by_handle.c | 123 ++++++++++++++++++++++++++++++++-----------
 tests/generic/426    |   1 +
 2 files changed, 93 insertions(+), 31 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index d9c802ca9bd1..cbd68aeadac1 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -86,10 +86,15 @@ Examples:
 #include <errno.h>
 #include <linux/limits.h>
 #include <libgen.h>
+#include <stdint.h>
 
 #include <sys/stat.h>
 #include "statx.h"
 
+#ifndef AT_HANDLE_MNT_ID_UNIQUE
+#	define AT_HANDLE_MNT_ID_UNIQUE 0x001
+#endif
+
 #define MAXFILES 1024
 
 struct handle {
@@ -99,7 +104,7 @@ struct handle {
 
 void usage(void)
 {
-	fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
+	fprintf(stderr, "usage: open_by_handle [-cludMmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
 	fprintf(stderr, "\n");
 	fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
 	fprintf(stderr, "open_by_handle    <test_dir> [N] - get file handles of test files, drop caches and try to open by handle\n");
@@ -111,6 +116,7 @@ void usage(void)
 	fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
+	fprintf(stderr, "open_by_handle -M <test_dir> [N] - confirm that the mount id returned by name_to_handle_at matches the mount id in statx\n");
 	fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
 	fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
@@ -120,6 +126,81 @@ void usage(void)
 	exit(EXIT_FAILURE);
 }
 
+int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz,
+			 int checkmountid)
+{
+	int ret;
+	int mntid_short;
+
+	uint64_t mntid_unique;
+	uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
+	struct handle dummy_fh;
+
+	if (checkmountid) {
+		struct statx statxbuf;
+
+		/* Get both the short and unique mount id. */
+		if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
+			fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+		if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
+			fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
+			return EXIT_FAILURE;
+		}
+		statx_mntid_short = statxbuf.stx_mnt_id;
+
+		if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
+			fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+		if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)) {
+			fprintf(stderr, "%s: no STATX_MNT_ID_UNIQUE in stx_mask\n", fname);
+			return EXIT_FAILURE;
+		}
+		statx_mntid_unique = statxbuf.stx_mnt_id;
+	}
+
+	fh->handle_bytes = bufsz;
+	ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	if (bufsz < fh->handle_bytes) {
+		/* Query the filesystem required bufsz and the file handle */
+		if (ret != -1 || errno != EOVERFLOW) {
+			fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
+			return EXIT_FAILURE;
+		}
+		ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	}
+	if (ret < 0) {
+		fprintf(stderr, "%s: name_to_handle: %m\n", fname);
+		return EXIT_FAILURE;
+	}
+
+	if (checkmountid) {
+		if (mntid_short != (int) statx_mntid_short) {
+			fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
+			return EXIT_FAILURE;
+		}
+
+		/*
+		 * Get the unique mount ID. We don't need to get another copy of the
+		 * handle so store it in a dummy struct.
+		 */
+		dummy_fh.fh.handle_bytes = fh->handle_bytes;
+		if (name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE) < 0) {
+			fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+
+		if (mntid_unique != statx_mntid_unique) {
+			fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
+			return EXIT_FAILURE;
+		}
+	}
+
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	int	i, c;
@@ -132,19 +213,20 @@ int main(int argc, char **argv)
 	char	fname2[PATH_MAX];
 	char	*test_dir;
 	char	*mount_dir;
-	int	mount_fd, mount_id;
+	int	mount_fd;
 	char	*infile = NULL, *outfile = NULL;
 	int	in_fd = 0, out_fd = 0;
 	int	numfiles = 1;
 	int	create = 0, delete = 0, nlink = 1, move = 0;
 	int	rd = 0, wr = 0, wrafter = 0, parent = 0;
 	int	keepopen = 0, drop_caches = 1, sleep_loop = 0;
+	int	checkmountid = 0;
 	int	bufsz = MAX_HANDLE_SZ;
 
 	if (argc < 2)
 		usage();
 
-	while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
+	while ((c = getopt(argc, argv, "cludMmrwapknhi:o:sz")) != -1) {
 		switch (c) {
 		case 'c':
 			create = 1;
@@ -172,6 +254,9 @@ int main(int argc, char **argv)
 			delete = 1;
 			nlink = 0;
 			break;
+		case 'M':
+			checkmountid = 1;
+			break;
 		case 'm':
 			move = 1;
 			break;
@@ -307,21 +392,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			handle[i].fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			if (bufsz < handle[i].fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(fname, ": name_to_handle");
-				perror(fname);
+			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz, checkmountid);
+			if (ret < 0)
 				return EXIT_FAILURE;
-			}
 		}
 		if (keepopen) {
 			/* Open without close to keep unlinked files around */
@@ -349,21 +422,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			dir_handle.fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			if (bufsz < dir_handle.fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(dname, ": name_to_handle");
-				perror(dname);
+			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz, checkmountid);
+			if (ret < 0)
 				return EXIT_FAILURE;
-			}
 		}
 		if (out_fd) {
 			ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
diff --git a/tests/generic/426 b/tests/generic/426
index 25909f220e1e..df481c58562c 100755
--- a/tests/generic/426
+++ b/tests/generic/426
@@ -51,6 +51,7 @@ test_file_handles $testdir -d
 # Check non-stale handles to linked files
 create_test_files $testdir
 test_file_handles $testdir
+test_file_handles $testdir -M
 
 # Check non-stale handles to files that were hardlinked and original deleted
 create_test_files $testdir
-- 
2.46.0

Re: [PATCH xfstests v1 2/2] open_by_handle: add tests for u64 mount ID

[Amir Goldstein]

On Wed, Aug 28, 2024 at 12:37 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> make sure they match properly as part of the regular open_by_handle
> tests.
>
> Link: https://lore.kernel.org/all/20240801-exportfs-u64-mount-id-v3-0-be5d6283144a@cyphar.com/
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
>  src/open_by_handle.c | 123 ++++++++++++++++++++++++++++++++-----------
>  tests/generic/426    |   1 +
>  2 files changed, 93 insertions(+), 31 deletions(-)
>
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index d9c802ca9bd1..cbd68aeadac1 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -86,10 +86,15 @@ Examples:
>  #include <errno.h>
>  #include <linux/limits.h>
>  #include <libgen.h>
> +#include <stdint.h>
>
>  #include <sys/stat.h>
>  #include "statx.h"
>
> +#ifndef AT_HANDLE_MNT_ID_UNIQUE
> +#      define AT_HANDLE_MNT_ID_UNIQUE 0x001
> +#endif
> +
>  #define MAXFILES 1024
>
>  struct handle {
> @@ -99,7 +104,7 @@ struct handle {
>
>  void usage(void)
>  {
> -       fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> +       fprintf(stderr, "usage: open_by_handle [-cludMmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
>         fprintf(stderr, "\n");
>         fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
>         fprintf(stderr, "open_by_handle    <test_dir> [N] - get file handles of test files, drop caches and try to open by handle\n");
> @@ -111,6 +116,7 @@ void usage(void)
>         fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
> +       fprintf(stderr, "open_by_handle -M <test_dir> [N] - confirm that the mount id returned by name_to_handle_at matches the mount id in statx\n");
>         fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
>         fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
> @@ -120,6 +126,81 @@ void usage(void)
>         exit(EXIT_FAILURE);
>  }
>
> +int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz,
> +                        int checkmountid)
> +{
> +       int ret;
> +       int mntid_short;
> +
> +       uint64_t mntid_unique;
> +       uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
> +       struct handle dummy_fh;
> +
> +       if (checkmountid) {
> +               struct statx statxbuf;
> +
> +               /* Get both the short and unique mount id. */
> +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
> +                       fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +               if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> +                       fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +               statx_mntid_short = statxbuf.stx_mnt_id;
> +
> +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
> +                       fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
> +                       return EXIT_FAILURE;

This failure will break the test on LTS kernels  - we don't want that.
Instead I think you should:
- drop the -M option
- get statx_mntid_unique here IF kernel supports STATX_MNT_ID_UNIQUE
and then...

> +               }
> +               if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)) {
> +                       fprintf(stderr, "%s: no STATX_MNT_ID_UNIQUE in stx_mask\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +               statx_mntid_unique = statxbuf.stx_mnt_id;
> +       }
> +
> +       fh->handle_bytes = bufsz;
> +       ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> +       if (bufsz < fh->handle_bytes) {
> +               /* Query the filesystem required bufsz and the file handle */
> +               if (ret != -1 || errno != EOVERFLOW) {
> +                       fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
> +                       return EXIT_FAILURE;
> +               }
> +               ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> +       }
> +       if (ret < 0) {
> +               fprintf(stderr, "%s: name_to_handle: %m\n", fname);
> +               return EXIT_FAILURE;
> +       }
> +
> +       if (checkmountid) {
> +               if (mntid_short != (int) statx_mntid_short) {
> +                       fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
> +                       return EXIT_FAILURE;
> +               }
> +
> +               /*
> +                * Get the unique mount ID. We don't need to get another copy of the
> +                * handle so store it in a dummy struct.
> +                */
> +               dummy_fh.fh.handle_bytes = fh->handle_bytes;
> +               if (name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE) < 0) {
> +                       fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +
> +               if (mntid_unique != statx_mntid_unique) {
> +                       fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
> +                       return EXIT_FAILURE;
> +               }

- check statx_mntid_unique here IFF statx_mntid_unique is set
- always check statx_mntid_short (what could be a reason to not check it?)

> +       }
> +
> +       return 0;
> +}
> +
>  int main(int argc, char **argv)
>  {
>         int     i, c;
> @@ -132,19 +213,20 @@ int main(int argc, char **argv)
>         char    fname2[PATH_MAX];
>         char    *test_dir;
>         char    *mount_dir;
> -       int     mount_fd, mount_id;
> +       int     mount_fd;
>         char    *infile = NULL, *outfile = NULL;
>         int     in_fd = 0, out_fd = 0;
>         int     numfiles = 1;
>         int     create = 0, delete = 0, nlink = 1, move = 0;
>         int     rd = 0, wr = 0, wrafter = 0, parent = 0;
>         int     keepopen = 0, drop_caches = 1, sleep_loop = 0;
> +       int     checkmountid = 0;
>         int     bufsz = MAX_HANDLE_SZ;
>
>         if (argc < 2)
>                 usage();
>
> -       while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
> +       while ((c = getopt(argc, argv, "cludMmrwapknhi:o:sz")) != -1) {
>                 switch (c) {
>                 case 'c':
>                         create = 1;
> @@ -172,6 +254,9 @@ int main(int argc, char **argv)
>                         delete = 1;
>                         nlink = 0;
>                         break;
> +               case 'M':
> +                       checkmountid = 1;
> +                       break;
>                 case 'm':
>                         move = 1;
>                         break;
> @@ -307,21 +392,9 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       handle[i].fh.handle_bytes = bufsz;
> -                       ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> -                       if (bufsz < handle[i].fh.handle_bytes) {
> -                               /* Query the filesystem required bufsz and the file handle */
> -                               if (ret != -1 || errno != EOVERFLOW) {
> -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
> -                                       return EXIT_FAILURE;
> -                               }
> -                               ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> -                       }
> -                       if (ret < 0) {
> -                               strcat(fname, ": name_to_handle");
> -                               perror(fname);
> +                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz, checkmountid);
> +                       if (ret < 0)
>                                 return EXIT_FAILURE;
> -                       }
>                 }
>                 if (keepopen) {
>                         /* Open without close to keep unlinked files around */
> @@ -349,21 +422,9 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       dir_handle.fh.handle_bytes = bufsz;
> -                       ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> -                       if (bufsz < dir_handle.fh.handle_bytes) {
> -                               /* Query the filesystem required bufsz and the file handle */
> -                               if (ret != -1 || errno != EOVERFLOW) {
> -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
> -                                       return EXIT_FAILURE;
> -                               }
> -                               ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> -                       }
> -                       if (ret < 0) {
> -                               strcat(dname, ": name_to_handle");
> -                               perror(dname);
> +                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz, checkmountid);
> +                       if (ret < 0)
>                                 return EXIT_FAILURE;
> -                       }
>                 }
>                 if (out_fd) {
>                         ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
> diff --git a/tests/generic/426 b/tests/generic/426
> index 25909f220e1e..df481c58562c 100755
> --- a/tests/generic/426
> +++ b/tests/generic/426
> @@ -51,6 +51,7 @@ test_file_handles $testdir -d
>  # Check non-stale handles to linked files
>  create_test_files $testdir
>  test_file_handles $testdir
> +test_file_handles $testdir -M

I see no reason to add option -M and add a second invocation.

Something I am missing?

Thanks,
Amir.

Re: [PATCH xfstests v1 2/2] open_by_handle: add tests for u64 mount ID

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 12532 bytes --]

On 2024-08-30, Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Aug 28, 2024 at 12:37 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> > make sure they match properly as part of the regular open_by_handle
> > tests.
> >
> > Link: https://lore.kernel.org/all/20240801-exportfs-u64-mount-id-v3-0-be5d6283144a@cyphar.com/
> > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > ---
> >  src/open_by_handle.c | 123 ++++++++++++++++++++++++++++++++-----------
> >  tests/generic/426    |   1 +
> >  2 files changed, 93 insertions(+), 31 deletions(-)
> >
> > diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> > index d9c802ca9bd1..cbd68aeadac1 100644
> > --- a/src/open_by_handle.c
> > +++ b/src/open_by_handle.c
> > @@ -86,10 +86,15 @@ Examples:
> >  #include <errno.h>
> >  #include <linux/limits.h>
> >  #include <libgen.h>
> > +#include <stdint.h>
> >
> >  #include <sys/stat.h>
> >  #include "statx.h"
> >
> > +#ifndef AT_HANDLE_MNT_ID_UNIQUE
> > +#      define AT_HANDLE_MNT_ID_UNIQUE 0x001
> > +#endif
> > +
> >  #define MAXFILES 1024
> >
> >  struct handle {
> > @@ -99,7 +104,7 @@ struct handle {
> >
> >  void usage(void)
> >  {
> > -       fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> > +       fprintf(stderr, "usage: open_by_handle [-cludMmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> >         fprintf(stderr, "\n");
> >         fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
> >         fprintf(stderr, "open_by_handle    <test_dir> [N] - get file handles of test files, drop caches and try to open by handle\n");
> > @@ -111,6 +116,7 @@ void usage(void)
> >         fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
> >         fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
> >         fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
> > +       fprintf(stderr, "open_by_handle -M <test_dir> [N] - confirm that the mount id returned by name_to_handle_at matches the mount id in statx\n");
> >         fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
> >         fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
> >         fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
> > @@ -120,6 +126,81 @@ void usage(void)
> >         exit(EXIT_FAILURE);
> >  }
> >
> > +int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz,
> > +                        int checkmountid)
> > +{
> > +       int ret;
> > +       int mntid_short;
> > +
> > +       uint64_t mntid_unique;
> > +       uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
> > +       struct handle dummy_fh;
> > +
> > +       if (checkmountid) {
> > +               struct statx statxbuf;
> > +
> > +               /* Get both the short and unique mount id. */
> > +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
> > +                       fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> > +                       return EXIT_FAILURE;
> > +               }
> > +               if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> > +                       fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
> > +                       return EXIT_FAILURE;
> > +               }
> > +               statx_mntid_short = statxbuf.stx_mnt_id;
> > +
> > +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
> > +                       fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
> > +                       return EXIT_FAILURE;
> 
> This failure will break the test on LTS kernels  - we don't want that.
> Instead I think you should:
> - drop the -M option
> - get statx_mntid_unique here IF kernel supports STATX_MNT_ID_UNIQUE
> and then...

Ah okay, I wasn't sure if the xfstests policy was like selftests where
only the latest kernel matters. I'll send a v2 with the suggestions you
mentioned.

However, presumably this means we would also not do the
STATX_MNT_ID_UNIQUE check if name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE)
returns -EINVAL, right?

> > +               }
> > +               if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)) {
> > +                       fprintf(stderr, "%s: no STATX_MNT_ID_UNIQUE in stx_mask\n", fname);
> > +                       return EXIT_FAILURE;
> > +               }
> > +               statx_mntid_unique = statxbuf.stx_mnt_id;
> > +       }
> > +
> > +       fh->handle_bytes = bufsz;
> > +       ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> > +       if (bufsz < fh->handle_bytes) {
> > +               /* Query the filesystem required bufsz and the file handle */
> > +               if (ret != -1 || errno != EOVERFLOW) {
> > +                       fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
> > +                       return EXIT_FAILURE;
> > +               }
> > +               ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> > +       }
> > +       if (ret < 0) {
> > +               fprintf(stderr, "%s: name_to_handle: %m\n", fname);
> > +               return EXIT_FAILURE;
> > +       }
> > +
> > +       if (checkmountid) {
> > +               if (mntid_short != (int) statx_mntid_short) {
> > +                       fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
> > +                       return EXIT_FAILURE;
> > +               }
> > +
> > +               /*
> > +                * Get the unique mount ID. We don't need to get another copy of the
> > +                * handle so store it in a dummy struct.
> > +                */
> > +               dummy_fh.fh.handle_bytes = fh->handle_bytes;
> > +               if (name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE) < 0) {
> > +                       fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
> > +                       return EXIT_FAILURE;
> > +               }
> > +
> > +               if (mntid_unique != statx_mntid_unique) {
> > +                       fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
> > +                       return EXIT_FAILURE;
> > +               }
> 
> - check statx_mntid_unique here IFF statx_mntid_unique is set
> - always check statx_mntid_short (what could be a reason to not check it?)
> 
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >         int     i, c;
> > @@ -132,19 +213,20 @@ int main(int argc, char **argv)
> >         char    fname2[PATH_MAX];
> >         char    *test_dir;
> >         char    *mount_dir;
> > -       int     mount_fd, mount_id;
> > +       int     mount_fd;
> >         char    *infile = NULL, *outfile = NULL;
> >         int     in_fd = 0, out_fd = 0;
> >         int     numfiles = 1;
> >         int     create = 0, delete = 0, nlink = 1, move = 0;
> >         int     rd = 0, wr = 0, wrafter = 0, parent = 0;
> >         int     keepopen = 0, drop_caches = 1, sleep_loop = 0;
> > +       int     checkmountid = 0;
> >         int     bufsz = MAX_HANDLE_SZ;
> >
> >         if (argc < 2)
> >                 usage();
> >
> > -       while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
> > +       while ((c = getopt(argc, argv, "cludMmrwapknhi:o:sz")) != -1) {
> >                 switch (c) {
> >                 case 'c':
> >                         create = 1;
> > @@ -172,6 +254,9 @@ int main(int argc, char **argv)
> >                         delete = 1;
> >                         nlink = 0;
> >                         break;
> > +               case 'M':
> > +                       checkmountid = 1;
> > +                       break;
> >                 case 'm':
> >                         move = 1;
> >                         break;
> > @@ -307,21 +392,9 @@ int main(int argc, char **argv)
> >                                 return EXIT_FAILURE;
> >                         }
> >                 } else {
> > -                       handle[i].fh.handle_bytes = bufsz;
> > -                       ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> > -                       if (bufsz < handle[i].fh.handle_bytes) {
> > -                               /* Query the filesystem required bufsz and the file handle */
> > -                               if (ret != -1 || errno != EOVERFLOW) {
> > -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
> > -                                       return EXIT_FAILURE;
> > -                               }
> > -                               ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> > -                       }
> > -                       if (ret < 0) {
> > -                               strcat(fname, ": name_to_handle");
> > -                               perror(fname);
> > +                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz, checkmountid);
> > +                       if (ret < 0)
> >                                 return EXIT_FAILURE;
> > -                       }
> >                 }
> >                 if (keepopen) {
> >                         /* Open without close to keep unlinked files around */
> > @@ -349,21 +422,9 @@ int main(int argc, char **argv)
> >                                 return EXIT_FAILURE;
> >                         }
> >                 } else {
> > -                       dir_handle.fh.handle_bytes = bufsz;
> > -                       ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> > -                       if (bufsz < dir_handle.fh.handle_bytes) {
> > -                               /* Query the filesystem required bufsz and the file handle */
> > -                               if (ret != -1 || errno != EOVERFLOW) {
> > -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
> > -                                       return EXIT_FAILURE;
> > -                               }
> > -                               ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> > -                       }
> > -                       if (ret < 0) {
> > -                               strcat(dname, ": name_to_handle");
> > -                               perror(dname);
> > +                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz, checkmountid);
> > +                       if (ret < 0)
> >                                 return EXIT_FAILURE;
> > -                       }
> >                 }
> >                 if (out_fd) {
> >                         ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
> > diff --git a/tests/generic/426 b/tests/generic/426
> > index 25909f220e1e..df481c58562c 100755
> > --- a/tests/generic/426
> > +++ b/tests/generic/426
> > @@ -51,6 +51,7 @@ test_file_handles $testdir -d
> >  # Check non-stale handles to linked files
> >  create_test_files $testdir
> >  test_file_handles $testdir
> > +test_file_handles $testdir -M
> 
> I see no reason to add option -M and add a second invocation.
> 
> Something I am missing?

Given how many other custom modes there were, I assumed that providing
it as an additional flag would've been preferred. I'll just make it
automatic.

Thanks!

> 
> Thanks,
> Amir.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH xfstests v2 1/2] statx: update headers to include newer statx fields

[Aleksa Sarai]

These come from Linux v6.11-rc5.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 src/open_by_handle.c |  4 +++-
 src/statx.h          | 22 ++++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 0f74ed08b1f0..d9c802ca9bd1 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -82,12 +82,14 @@ Examples:
 #include <string.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <sys/stat.h>
 #include <sys/types.h>
 #include <errno.h>
 #include <linux/limits.h>
 #include <libgen.h>
 
+#include <sys/stat.h>
+#include "statx.h"
+
 #define MAXFILES 1024
 
 struct handle {
diff --git a/src/statx.h b/src/statx.h
index 3f239d791dfe..935cb2ed415e 100644
--- a/src/statx.h
+++ b/src/statx.h
@@ -102,7 +102,7 @@ struct statx {
 	__u64	stx_ino;	/* Inode number */
 	__u64	stx_size;	/* File size */
 	__u64	stx_blocks;	/* Number of 512-byte blocks allocated */
-	__u64	__spare1[1];
+	__u64	stx_attributes_mask; /* Mask to show what's supported in stx_attributes */
 	/* 0x40 */
 	struct statx_timestamp	stx_atime;	/* Last access time */
 	struct statx_timestamp	stx_btime;	/* File creation time */
@@ -114,7 +114,18 @@ struct statx {
 	__u32	stx_dev_major;	/* ID of device containing file [uncond] */
 	__u32	stx_dev_minor;
 	/* 0x90 */
-	__u64	__spare2[14];	/* Spare space for future expansion */
+	__u64	stx_mnt_id;
+	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
+	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
+	/* 0xa0 */
+	__u64	stx_subvol;	/* Subvolume identifier */
+	__u32	stx_atomic_write_unit_min;	/* Min atomic write unit in bytes */
+	__u32	stx_atomic_write_unit_max;	/* Max atomic write unit in bytes */
+	/* 0xb0 */
+	__u32   stx_atomic_write_segments_max;	/* Max atomic write segment count */
+	__u32   __spare1[1];
+	/* 0xb8 */
+	__u64	__spare3[9];	/* Spare space for future expansion */
 	/* 0x100 */
 };
 
@@ -139,6 +150,13 @@ struct statx {
 #define STATX_BLOCKS		0x00000400U	/* Want/got stx_blocks */
 #define STATX_BASIC_STATS	0x000007ffU	/* The stuff in the normal stat struct */
 #define STATX_BTIME		0x00000800U	/* Want/got stx_btime */
+#define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
+#define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
+#define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
+#define STATX_SUBVOL		0x00008000U	/* Want/got stx_subvol */
+#define STATX_WRITE_ATOMIC	0x00010000U	/* Want/got atomic_write_* fields */
+
+#define STATX__RESERVED		0x80000000U	/* Reserved for future struct statx expansion */
 #define STATX_ALL		0x00000fffU	/* All currently supported flags */
 
 /*
-- 
2.46.0

[PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Aleksa Sarai]

Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
make sure they match properly as part of the regular open_by_handle
tests.

Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
v2:
- Remove -M argument and always do the mount ID tests. [Amir Goldstein]
- Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
  or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
- v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>

 src/open_by_handle.c | 128 +++++++++++++++++++++++++++++++++----------
 1 file changed, 99 insertions(+), 29 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index d9c802ca9bd1..0ad591da632e 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -86,10 +86,16 @@ Examples:
 #include <errno.h>
 #include <linux/limits.h>
 #include <libgen.h>
+#include <stdint.h>
+#include <stdbool.h>
 
 #include <sys/stat.h>
 #include "statx.h"
 
+#ifndef AT_HANDLE_MNT_ID_UNIQUE
+#	define AT_HANDLE_MNT_ID_UNIQUE 0x001
+#endif
+
 #define MAXFILES 1024
 
 struct handle {
@@ -120,6 +126,94 @@ void usage(void)
 	exit(EXIT_FAILURE);
 }
 
+int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz)
+{
+	int ret;
+	int mntid_short;
+
+	static bool skip_mntid_unique;
+
+	uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
+	struct statx statxbuf;
+
+	/* Get both the short and unique mount id. */
+	if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
+		fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
+		return EXIT_FAILURE;
+	}
+	if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
+		fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
+		return EXIT_FAILURE;
+	}
+	statx_mntid_short = statxbuf.stx_mnt_id;
+
+	if (!skip_mntid_unique) {
+		if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
+			fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+		/*
+		 * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
+		 * kernel doesn't give us a unique mount ID just skip it.
+		 */
+		if ((skip_mntid_unique |= !(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)))
+			printf("statx(STATX_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");
+		else
+			statx_mntid_unique = statxbuf.stx_mnt_id;
+	}
+
+	fh->handle_bytes = bufsz;
+	ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	if (bufsz < fh->handle_bytes) {
+		/* Query the filesystem required bufsz and the file handle */
+		if (ret != -1 || errno != EOVERFLOW) {
+			fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
+			return EXIT_FAILURE;
+		}
+		ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	}
+	if (ret < 0) {
+		fprintf(stderr, "%s: name_to_handle: %m\n", fname);
+		return EXIT_FAILURE;
+	}
+
+	if (mntid_short != (int) statx_mntid_short) {
+		fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
+		return EXIT_FAILURE;
+	}
+
+	if (!skip_mntid_unique && statx_mntid_unique != 0) {
+		struct handle dummy_fh;
+		uint64_t mntid_unique = 0;
+
+		/*
+		 * Get the unique mount ID. We don't need to get another copy of the
+		 * handle so store it in a dummy struct.
+		 */
+		dummy_fh.fh.handle_bytes = fh->handle_bytes;
+		ret = name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE);
+		if (ret < 0) {
+			if (errno != EINVAL) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
+				return EXIT_FAILURE;
+			}
+			/*
+			 * EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported, so skip
+			 * the check in that case.
+			 */
+			printf("name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");
+			skip_mntid_unique = true;
+		} else {
+			if (mntid_unique != statx_mntid_unique) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
+				return EXIT_FAILURE;
+			}
+		}
+	}
+
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	int	i, c;
@@ -132,7 +226,7 @@ int main(int argc, char **argv)
 	char	fname2[PATH_MAX];
 	char	*test_dir;
 	char	*mount_dir;
-	int	mount_fd, mount_id;
+	int	mount_fd;
 	char	*infile = NULL, *outfile = NULL;
 	int	in_fd = 0, out_fd = 0;
 	int	numfiles = 1;
@@ -307,21 +401,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			handle[i].fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			if (bufsz < handle[i].fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(fname, ": name_to_handle");
-				perror(fname);
+			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
+			if (ret < 0)
 				return EXIT_FAILURE;
-			}
 		}
 		if (keepopen) {
 			/* Open without close to keep unlinked files around */
@@ -349,21 +431,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			dir_handle.fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			if (bufsz < dir_handle.fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(dname, ": name_to_handle");
-				perror(dname);
+			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
+			if (ret < 0)
 				return EXIT_FAILURE;
-			}
 		}
 		if (out_fd) {
 			ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
-- 
2.46.0

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Amir Goldstein]

On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> make sure they match properly as part of the regular open_by_handle
> tests.
>
> Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> v2:
> - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
>   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>

Looks good.

You may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

It'd be nice to get a verification that this is indeed tested on the latest
upstream and does not regress the tests that run the open_by_handle program.

Thanks,
Amir.

>
>  src/open_by_handle.c | 128 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 99 insertions(+), 29 deletions(-)
>
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index d9c802ca9bd1..0ad591da632e 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -86,10 +86,16 @@ Examples:
>  #include <errno.h>
>  #include <linux/limits.h>
>  #include <libgen.h>
> +#include <stdint.h>
> +#include <stdbool.h>
>
>  #include <sys/stat.h>
>  #include "statx.h"
>
> +#ifndef AT_HANDLE_MNT_ID_UNIQUE
> +#      define AT_HANDLE_MNT_ID_UNIQUE 0x001
> +#endif
> +
>  #define MAXFILES 1024
>
>  struct handle {
> @@ -120,6 +126,94 @@ void usage(void)
>         exit(EXIT_FAILURE);
>  }
>
> +int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz)
> +{
> +       int ret;
> +       int mntid_short;
> +
> +       static bool skip_mntid_unique;
> +
> +       uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
> +       struct statx statxbuf;
> +
> +       /* Get both the short and unique mount id. */
> +       if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
> +               fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> +               return EXIT_FAILURE;
> +       }
> +       if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> +               fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
> +               return EXIT_FAILURE;
> +       }
> +       statx_mntid_short = statxbuf.stx_mnt_id;
> +
> +       if (!skip_mntid_unique) {
> +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
> +                       fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +               /*
> +                * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
> +                * kernel doesn't give us a unique mount ID just skip it.
> +                */
> +               if ((skip_mntid_unique |= !(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)))
> +                       printf("statx(STATX_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");
> +               else
> +                       statx_mntid_unique = statxbuf.stx_mnt_id;
> +       }
> +
> +       fh->handle_bytes = bufsz;
> +       ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> +       if (bufsz < fh->handle_bytes) {
> +               /* Query the filesystem required bufsz and the file handle */
> +               if (ret != -1 || errno != EOVERFLOW) {
> +                       fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
> +                       return EXIT_FAILURE;
> +               }
> +               ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> +       }
> +       if (ret < 0) {
> +               fprintf(stderr, "%s: name_to_handle: %m\n", fname);
> +               return EXIT_FAILURE;
> +       }
> +
> +       if (mntid_short != (int) statx_mntid_short) {
> +               fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
> +               return EXIT_FAILURE;
> +       }
> +
> +       if (!skip_mntid_unique && statx_mntid_unique != 0) {
> +               struct handle dummy_fh;
> +               uint64_t mntid_unique = 0;
> +
> +               /*
> +                * Get the unique mount ID. We don't need to get another copy of the
> +                * handle so store it in a dummy struct.
> +                */
> +               dummy_fh.fh.handle_bytes = fh->handle_bytes;
> +               ret = name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE);
> +               if (ret < 0) {
> +                       if (errno != EINVAL) {
> +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
> +                               return EXIT_FAILURE;
> +                       }
> +                       /*
> +                        * EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported, so skip
> +                        * the check in that case.
> +                        */
> +                       printf("name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");
> +                       skip_mntid_unique = true;
> +               } else {
> +                       if (mntid_unique != statx_mntid_unique) {
> +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
> +                               return EXIT_FAILURE;
> +                       }
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  int main(int argc, char **argv)
>  {
>         int     i, c;
> @@ -132,7 +226,7 @@ int main(int argc, char **argv)
>         char    fname2[PATH_MAX];
>         char    *test_dir;
>         char    *mount_dir;
> -       int     mount_fd, mount_id;
> +       int     mount_fd;
>         char    *infile = NULL, *outfile = NULL;
>         int     in_fd = 0, out_fd = 0;
>         int     numfiles = 1;
> @@ -307,21 +401,9 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       handle[i].fh.handle_bytes = bufsz;
> -                       ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> -                       if (bufsz < handle[i].fh.handle_bytes) {
> -                               /* Query the filesystem required bufsz and the file handle */
> -                               if (ret != -1 || errno != EOVERFLOW) {
> -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
> -                                       return EXIT_FAILURE;
> -                               }
> -                               ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> -                       }
> -                       if (ret < 0) {
> -                               strcat(fname, ": name_to_handle");
> -                               perror(fname);
> +                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
> +                       if (ret < 0)
>                                 return EXIT_FAILURE;
> -                       }
>                 }
>                 if (keepopen) {
>                         /* Open without close to keep unlinked files around */
> @@ -349,21 +431,9 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       dir_handle.fh.handle_bytes = bufsz;
> -                       ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> -                       if (bufsz < dir_handle.fh.handle_bytes) {
> -                               /* Query the filesystem required bufsz and the file handle */
> -                               if (ret != -1 || errno != EOVERFLOW) {
> -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
> -                                       return EXIT_FAILURE;
> -                               }
> -                               ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> -                       }
> -                       if (ret < 0) {
> -                               strcat(dname, ": name_to_handle");
> -                               perror(dname);
> +                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
> +                       if (ret < 0)
>                                 return EXIT_FAILURE;
> -                       }
>                 }
>                 if (out_fd) {
>                         ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
> --
> 2.46.0
>

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 10128 bytes --]

On 2024-09-02, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> > make sure they match properly as part of the regular open_by_handle
> > tests.
> >
> > Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > ---
> > v2:
> > - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> > - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
> >   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> > - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
> 
> Looks good.
> 
> You may add:
> 
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> It'd be nice to get a verification that this is indeed tested on the latest
> upstream and does not regress the tests that run the open_by_handle program.

I've tested that the fallback works on mainline and correctly does the
test on patched kernels (by running open_by_handle directly) but I
haven't run the suite yet (still getting my mkosi testing setup working
to run fstests...).

> Thanks,
> Amir.
> 
> >
> >  src/open_by_handle.c | 128 +++++++++++++++++++++++++++++++++----------
> >  1 file changed, 99 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> > index d9c802ca9bd1..0ad591da632e 100644
> > --- a/src/open_by_handle.c
> > +++ b/src/open_by_handle.c
> > @@ -86,10 +86,16 @@ Examples:
> >  #include <errno.h>
> >  #include <linux/limits.h>
> >  #include <libgen.h>
> > +#include <stdint.h>
> > +#include <stdbool.h>
> >
> >  #include <sys/stat.h>
> >  #include "statx.h"
> >
> > +#ifndef AT_HANDLE_MNT_ID_UNIQUE
> > +#      define AT_HANDLE_MNT_ID_UNIQUE 0x001
> > +#endif
> > +
> >  #define MAXFILES 1024
> >
> >  struct handle {
> > @@ -120,6 +126,94 @@ void usage(void)
> >         exit(EXIT_FAILURE);
> >  }
> >
> > +int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz)
> > +{
> > +       int ret;
> > +       int mntid_short;
> > +
> > +       static bool skip_mntid_unique;
> > +
> > +       uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
> > +       struct statx statxbuf;
> > +
> > +       /* Get both the short and unique mount id. */
> > +       if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
> > +               fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> > +               return EXIT_FAILURE;
> > +       }
> > +       if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> > +               fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
> > +               return EXIT_FAILURE;
> > +       }
> > +       statx_mntid_short = statxbuf.stx_mnt_id;
> > +
> > +       if (!skip_mntid_unique) {
> > +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
> > +                       fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
> > +                       return EXIT_FAILURE;
> > +               }
> > +               /*
> > +                * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
> > +                * kernel doesn't give us a unique mount ID just skip it.
> > +                */
> > +               if ((skip_mntid_unique |= !(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)))
> > +                       printf("statx(STATX_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");
> > +               else
> > +                       statx_mntid_unique = statxbuf.stx_mnt_id;
> > +       }
> > +
> > +       fh->handle_bytes = bufsz;
> > +       ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> > +       if (bufsz < fh->handle_bytes) {
> > +               /* Query the filesystem required bufsz and the file handle */
> > +               if (ret != -1 || errno != EOVERFLOW) {
> > +                       fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
> > +                       return EXIT_FAILURE;
> > +               }
> > +               ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> > +       }
> > +       if (ret < 0) {
> > +               fprintf(stderr, "%s: name_to_handle: %m\n", fname);
> > +               return EXIT_FAILURE;
> > +       }
> > +
> > +       if (mntid_short != (int) statx_mntid_short) {
> > +               fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
> > +               return EXIT_FAILURE;
> > +       }
> > +
> > +       if (!skip_mntid_unique && statx_mntid_unique != 0) {
> > +               struct handle dummy_fh;
> > +               uint64_t mntid_unique = 0;
> > +
> > +               /*
> > +                * Get the unique mount ID. We don't need to get another copy of the
> > +                * handle so store it in a dummy struct.
> > +                */
> > +               dummy_fh.fh.handle_bytes = fh->handle_bytes;
> > +               ret = name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE);
> > +               if (ret < 0) {
> > +                       if (errno != EINVAL) {
> > +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
> > +                               return EXIT_FAILURE;
> > +                       }
> > +                       /*
> > +                        * EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported, so skip
> > +                        * the check in that case.
> > +                        */
> > +                       printf("name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");
> > +                       skip_mntid_unique = true;
> > +               } else {
> > +                       if (mntid_unique != statx_mntid_unique) {
> > +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
> > +                               return EXIT_FAILURE;
> > +                       }
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >         int     i, c;
> > @@ -132,7 +226,7 @@ int main(int argc, char **argv)
> >         char    fname2[PATH_MAX];
> >         char    *test_dir;
> >         char    *mount_dir;
> > -       int     mount_fd, mount_id;
> > +       int     mount_fd;
> >         char    *infile = NULL, *outfile = NULL;
> >         int     in_fd = 0, out_fd = 0;
> >         int     numfiles = 1;
> > @@ -307,21 +401,9 @@ int main(int argc, char **argv)
> >                                 return EXIT_FAILURE;
> >                         }
> >                 } else {
> > -                       handle[i].fh.handle_bytes = bufsz;
> > -                       ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> > -                       if (bufsz < handle[i].fh.handle_bytes) {
> > -                               /* Query the filesystem required bufsz and the file handle */
> > -                               if (ret != -1 || errno != EOVERFLOW) {
> > -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
> > -                                       return EXIT_FAILURE;
> > -                               }
> > -                               ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> > -                       }
> > -                       if (ret < 0) {
> > -                               strcat(fname, ": name_to_handle");
> > -                               perror(fname);
> > +                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
> > +                       if (ret < 0)
> >                                 return EXIT_FAILURE;
> > -                       }
> >                 }
> >                 if (keepopen) {
> >                         /* Open without close to keep unlinked files around */
> > @@ -349,21 +431,9 @@ int main(int argc, char **argv)
> >                                 return EXIT_FAILURE;
> >                         }
> >                 } else {
> > -                       dir_handle.fh.handle_bytes = bufsz;
> > -                       ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> > -                       if (bufsz < dir_handle.fh.handle_bytes) {
> > -                               /* Query the filesystem required bufsz and the file handle */
> > -                               if (ret != -1 || errno != EOVERFLOW) {
> > -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
> > -                                       return EXIT_FAILURE;
> > -                               }
> > -                               ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> > -                       }
> > -                       if (ret < 0) {
> > -                               strcat(dname, ": name_to_handle");
> > -                               perror(dname);
> > +                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
> > +                       if (ret < 0)
> >                                 return EXIT_FAILURE;
> > -                       }
> >                 }
> >                 if (out_fd) {
> >                         ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
> > --
> > 2.46.0
> >

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Amir Goldstein]

On Tue, Sep 3, 2024 at 8:41 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> On 2024-09-02, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> > >
> > > Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> > > make sure they match properly as part of the regular open_by_handle
> > > tests.
> > >
> > > Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> > > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > > ---
> > > v2:
> > > - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> > > - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
> > >   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> > > - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
> >
> > Looks good.
> >
> > You may add:
> >
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> >
> > It'd be nice to get a verification that this is indeed tested on the latest
> > upstream and does not regress the tests that run the open_by_handle program.
>
> I've tested that the fallback works on mainline and correctly does the
> test on patched kernels (by running open_by_handle directly) but I
> haven't run the suite yet (still getting my mkosi testing setup working
> to run fstests...).

I am afraid this has to be tested.
I started testing myself and found that it breaks existing tests.
Even if you make the test completely opt-in as in v1 it need to be
tested and _notrun on old kernels.

If you have a new version, I can test it until you get your fstests setup
ready, because anyway I would want to check that your test also
works with overlayfs which has some specialized exportfs tests.
Test by running ./check -overlay -g exportfs, but I can also do that for you.

Thanks,
Amir.

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]

On 2024-09-03, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Sep 3, 2024 at 8:41 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > On 2024-09-02, Amir Goldstein <amir73il@gmail.com> wrote:
> > > On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> > > >
> > > > Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> > > > make sure they match properly as part of the regular open_by_handle
> > > > tests.
> > > >
> > > > Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> > > > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > > > ---
> > > > v2:
> > > > - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> > > > - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
> > > >   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> > > > - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
> > >
> > > Looks good.
> > >
> > > You may add:
> > >
> > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > >
> > > It'd be nice to get a verification that this is indeed tested on the latest
> > > upstream and does not regress the tests that run the open_by_handle program.
> >
> > I've tested that the fallback works on mainline and correctly does the
> > test on patched kernels (by running open_by_handle directly) but I
> > haven't run the suite yet (still getting my mkosi testing setup working
> > to run fstests...).
> 
> I am afraid this has to be tested.
> I started testing myself and found that it breaks existing tests.
> Even if you make the test completely opt-in as in v1 it need to be
> tested and _notrun on old kernels.
> 
> If you have a new version, I can test it until you get your fstests setup
> ready, because anyway I would want to check that your test also
> works with overlayfs which has some specialized exportfs tests.
> Test by running ./check -overlay -g exportfs, but I can also do that for you.

I managed to get fstests running, sorry about that...

For the v3 I have ready (which includes a new test using -M), the
following runs work in my VM:

 - ./check -g exportfs
 - ./check -overlay -g exportfs

Should I check anything else before sending it?

Also, when running the tests I think I may have found a bug? Using
overlayfs+xfs leads to the following error when doing ./check -overlay
if the scratch device is XFS:

  ./common/rc: line 299: _xfs_has_feature: command not found
    not run: upper fs needs to support d_type

The fix I applied was simply:

diff --git a/common/rc b/common/rc
index 0beaf2ff1126..e6af1b16918f 100644
--- a/common/rc
+++ b/common/rc
@@ -296,6 +296,7 @@ _supports_filetype()
 	local fstyp=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $2}'`
 	case "$fstyp" in
 	xfs)
+		. common/xfs
 		_xfs_has_feature $dir ftype
 		;;
 	ext2|ext3|ext4)

Should I include this patch as well, or did I make a mistake somewhere?
(I could add the import to the top instead if you'd prefer that.)

Thanks.

> 
> Thanks,
> Amir.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Amir Goldstein]

On Wed, Sep 4, 2024 at 6:31 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> On 2024-09-03, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Tue, Sep 3, 2024 at 8:41 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
> > >
> > > On 2024-09-02, Amir Goldstein <amir73il@gmail.com> wrote:
> > > > On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> > > > >
> > > > > Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> > > > > make sure they match properly as part of the regular open_by_handle
> > > > > tests.
> > > > >
> > > > > Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> > > > > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > > > > ---
> > > > > v2:
> > > > > - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> > > > > - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
> > > > >   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> > > > > - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
> > > >
> > > > Looks good.
> > > >
> > > > You may add:
> > > >
> > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > >
> > > > It'd be nice to get a verification that this is indeed tested on the latest
> > > > upstream and does not regress the tests that run the open_by_handle program.
> > >
> > > I've tested that the fallback works on mainline and correctly does the
> > > test on patched kernels (by running open_by_handle directly) but I
> > > haven't run the suite yet (still getting my mkosi testing setup working
> > > to run fstests...).
> >
> > I am afraid this has to be tested.
> > I started testing myself and found that it breaks existing tests.
> > Even if you make the test completely opt-in as in v1 it need to be
> > tested and _notrun on old kernels.
> >
> > If you have a new version, I can test it until you get your fstests setup
> > ready, because anyway I would want to check that your test also
> > works with overlayfs which has some specialized exportfs tests.
> > Test by running ./check -overlay -g exportfs, but I can also do that for you.
>
> I managed to get fstests running, sorry about that...
>
> For the v3 I have ready (which includes a new test using -M), the
> following runs work in my VM:
>
>  - ./check -g exportfs
>  - ./check -overlay -g exportfs
>
> Should I check anything else before sending it?
>

That should be enough.
So you have one new test that does not run on upstream kernel
and runs and passes on patched kernel?

> Also, when running the tests I think I may have found a bug? Using
> overlayfs+xfs leads to the following error when doing ./check -overlay
> if the scratch device is XFS:
>
>   ./common/rc: line 299: _xfs_has_feature: command not found
>     not run: upper fs needs to support d_type
>
> The fix I applied was simply:
>
> diff --git a/common/rc b/common/rc
> index 0beaf2ff1126..e6af1b16918f 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -296,6 +296,7 @@ _supports_filetype()
>         local fstyp=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $2}'`
>         case "$fstyp" in
>         xfs)
> +               . common/xfs
>                 _xfs_has_feature $dir ftype
>                 ;;
>         ext2|ext3|ext4)
>
> Should I include this patch as well, or did I make a mistake somewhere?
> (I could add the import to the top instead if you'd prefer that.)

This should already be handled by
if [ -n "$OVL_BASE_FSTYP" ];then
        _source_specific_fs $OVL_BASE_FSTYP
fi

in common/overlay

I think what you are missing is to
export FSTYP=xfs
as README.overlay suggests.

It's true that ./check does not *require* defining FSTYP
and can auto detect the test filesystem, but for running -overlay
is it a requirement to define the base FSTYP.

Thanks,
Amir.

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 4677 bytes --]

On 2024-09-04, Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Sep 4, 2024 at 6:31 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > On 2024-09-03, Amir Goldstein <amir73il@gmail.com> wrote:
> > > On Tue, Sep 3, 2024 at 8:41 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
> > > >
> > > > On 2024-09-02, Amir Goldstein <amir73il@gmail.com> wrote:
> > > > > On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> > > > > >
> > > > > > Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> > > > > > make sure they match properly as part of the regular open_by_handle
> > > > > > tests.
> > > > > >
> > > > > > Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> > > > > > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > > > > > ---
> > > > > > v2:
> > > > > > - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> > > > > > - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
> > > > > >   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> > > > > > - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
> > > > >
> > > > > Looks good.
> > > > >
> > > > > You may add:
> > > > >
> > > > > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > > > >
> > > > > It'd be nice to get a verification that this is indeed tested on the latest
> > > > > upstream and does not regress the tests that run the open_by_handle program.
> > > >
> > > > I've tested that the fallback works on mainline and correctly does the
> > > > test on patched kernels (by running open_by_handle directly) but I
> > > > haven't run the suite yet (still getting my mkosi testing setup working
> > > > to run fstests...).
> > >
> > > I am afraid this has to be tested.
> > > I started testing myself and found that it breaks existing tests.
> > > Even if you make the test completely opt-in as in v1 it need to be
> > > tested and _notrun on old kernels.
> > >
> > > If you have a new version, I can test it until you get your fstests setup
> > > ready, because anyway I would want to check that your test also
> > > works with overlayfs which has some specialized exportfs tests.
> > > Test by running ./check -overlay -g exportfs, but I can also do that for you.
> >
> > I managed to get fstests running, sorry about that...
> >
> > For the v3 I have ready (which includes a new test using -M), the
> > following runs work in my VM:
> >
> >  - ./check -g exportfs
> >  - ./check -overlay -g exportfs
> >
> > Should I check anything else before sending it?
> >
> 
> That should be enough.
> So you have one new test that does not run on upstream kernel
> and runs and passes on patched kernel?

Yes. Both of those suite runs succeed without issues on v6.6.49,
v6.11-rc6 and with the AT_HANDLE_MNT_ID_UNIQUE patches.

I also added skipping logic such that it _should_ work on pre-5.8
kernels (pre-STATX_MNT_ID) as well, but I can't test that at the moment
(mkosi-kernel fails to boot kernels that old it seems...).

I'll send it now.

> > Also, when running the tests I think I may have found a bug? Using
> > overlayfs+xfs leads to the following error when doing ./check -overlay
> > if the scratch device is XFS:
> >
> >   ./common/rc: line 299: _xfs_has_feature: command not found
> >     not run: upper fs needs to support d_type
> >
> > The fix I applied was simply:
> >
> > diff --git a/common/rc b/common/rc
> > index 0beaf2ff1126..e6af1b16918f 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -296,6 +296,7 @@ _supports_filetype()
> >         local fstyp=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $2}'`
> >         case "$fstyp" in
> >         xfs)
> > +               . common/xfs
> >                 _xfs_has_feature $dir ftype
> >                 ;;
> >         ext2|ext3|ext4)
> >
> > Should I include this patch as well, or did I make a mistake somewhere?
> > (I could add the import to the top instead if you'd prefer that.)
> 
> This should already be handled by
> if [ -n "$OVL_BASE_FSTYP" ];then
>         _source_specific_fs $OVL_BASE_FSTYP
> fi
> 
> in common/overlay
> 
> I think what you are missing is to
> export FSTYP=xfs
> as README.overlay suggests.
> 
> It's true that ./check does not *require* defining FSTYP
> and can auto detect the test filesystem, but for running -overlay
> is it a requirement to define the base FSTYP.

Ah okay, I missed that. That fixed the issue, thanks!

> Thanks,
> Amir.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Amir Goldstein]

On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> make sure they match properly as part of the regular open_by_handle
> tests.
>
> Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> v2:
> - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
>   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
>
>  src/open_by_handle.c | 128 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 99 insertions(+), 29 deletions(-)
>
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index d9c802ca9bd1..0ad591da632e 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -86,10 +86,16 @@ Examples:
>  #include <errno.h>
>  #include <linux/limits.h>
>  #include <libgen.h>
> +#include <stdint.h>
> +#include <stdbool.h>
>
>  #include <sys/stat.h>
>  #include "statx.h"
>
> +#ifndef AT_HANDLE_MNT_ID_UNIQUE
> +#      define AT_HANDLE_MNT_ID_UNIQUE 0x001
> +#endif
> +
>  #define MAXFILES 1024
>
>  struct handle {
> @@ -120,6 +126,94 @@ void usage(void)
>         exit(EXIT_FAILURE);
>  }
>
> +int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz)
> +{
> +       int ret;
> +       int mntid_short;
> +
> +       static bool skip_mntid_unique;
> +
> +       uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
> +       struct statx statxbuf;
> +
> +       /* Get both the short and unique mount id. */
> +       if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {

This fails build on top of latest for-next branch with commit
873e36c9 - statx.h: update to latest kernel UAPI

It can be fixed by changing to use the private xfstests_statx()
implementation, same as in stat_test.c.

I am not sure how elegant this is, but that's the easy fix.

> +               fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> +               return EXIT_FAILURE;
> +       }
> +       if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> +               fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
> +               return EXIT_FAILURE;
> +       }
> +       statx_mntid_short = statxbuf.stx_mnt_id;
> +
> +       if (!skip_mntid_unique) {
> +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
> +                       fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +               /*
> +                * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
> +                * kernel doesn't give us a unique mount ID just skip it.
> +                */
> +               if ((skip_mntid_unique |= !(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)))
> +                       printf("statx(STATX_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");

This verbose print breaks all existing "exportfs" tests which do not
expect it in the golden output.

I understand that silently ignoring the failure is not good, but I also
would like to add this test coverage to all the existing tests.

One solution is to resurrect the command line option -M from v1,
but instead of meaning "test unique mount id" let it mean
"do not allow to skip unique mount id" test.

Then you can add a new test that runs open_by_handle -M, but also
implement a helper _require_unique_mntid similar to _require_btime
which is needed for the new test to run only on new kernels.

I'm sorry for this complication, but fstest is a testsuite that runs on
disto and stable kernels as well and we need to allow test coverage
of new features along with stability of the test env.

Thanks,
Amir.

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 4615 bytes --]

On 2024-09-03, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> > make sure they match properly as part of the regular open_by_handle
> > tests.
> >
> > Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> > ---
> > v2:
> > - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> > - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
> >   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> > - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
> >
> >  src/open_by_handle.c | 128 +++++++++++++++++++++++++++++++++----------
> >  1 file changed, 99 insertions(+), 29 deletions(-)
> >
> > diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> > index d9c802ca9bd1..0ad591da632e 100644
> > --- a/src/open_by_handle.c
> > +++ b/src/open_by_handle.c
> > @@ -86,10 +86,16 @@ Examples:
> >  #include <errno.h>
> >  #include <linux/limits.h>
> >  #include <libgen.h>
> > +#include <stdint.h>
> > +#include <stdbool.h>
> >
> >  #include <sys/stat.h>
> >  #include "statx.h"
> >
> > +#ifndef AT_HANDLE_MNT_ID_UNIQUE
> > +#      define AT_HANDLE_MNT_ID_UNIQUE 0x001
> > +#endif
> > +
> >  #define MAXFILES 1024
> >
> >  struct handle {
> > @@ -120,6 +126,94 @@ void usage(void)
> >         exit(EXIT_FAILURE);
> >  }
> >
> > +int do_name_to_handle_at(const char *fname, struct file_handle *fh, int bufsz)
> > +{
> > +       int ret;
> > +       int mntid_short;
> > +
> > +       static bool skip_mntid_unique;
> > +
> > +       uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
> > +       struct statx statxbuf;
> > +
> > +       /* Get both the short and unique mount id. */
> > +       if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
> 
> This fails build on top of latest for-next branch with commit
> 873e36c9 - statx.h: update to latest kernel UAPI
> 
> It can be fixed by changing to use the private xfstests_statx()
> implementation, same as in stat_test.c.
> 
> I am not sure how elegant this is, but that's the easy fix.

Ah, I was using master as the base. Sorry about that...

> > +               fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> > +               return EXIT_FAILURE;
> > +       }
> > +       if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> > +               fprintf(stderr, "%s: no STATX_MNT_ID in stx_mask\n", fname);
> > +               return EXIT_FAILURE;
> > +       }
> > +       statx_mntid_short = statxbuf.stx_mnt_id;
> > +
> > +       if (!skip_mntid_unique) {
> > +               if (statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
> > +                       fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
> > +                       return EXIT_FAILURE;
> > +               }
> > +               /*
> > +                * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
> > +                * kernel doesn't give us a unique mount ID just skip it.
> > +                */
> > +               if ((skip_mntid_unique |= !(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)))
> > +                       printf("statx(STATX_MNT_ID_UNIQUE) not supported by running kernel -- skipping unique mount ID test\n");
> 
> This verbose print breaks all existing "exportfs" tests which do not
> expect it in the golden output.
> 
> I understand that silently ignoring the failure is not good, but I also
> would like to add this test coverage to all the existing tests.
> 
> One solution is to resurrect the command line option -M from v1,
> but instead of meaning "test unique mount id" let it mean
> "do not allow to skip unique mount id" test.
> 
> Then you can add a new test that runs open_by_handle -M, but also
> implement a helper _require_unique_mntid similar to _require_btime
> which is needed for the new test to run only on new kernels.
> 
> I'm sorry for this complication, but fstest is a testsuite that runs on
> disto and stable kernels as well and we need to allow test coverage
> of new features along with stability of the test env.

No worries, I'll write it up. I'm not familiar with the exact
requirements of xfstests, sorry for the noise! (^_^")

> 
> Thanks,
> Amir.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH xfstests v2 1/2] statx: update headers to include newer statx fields

[Amir Goldstein]

On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> These come from Linux v6.11-rc5.
>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
>  src/open_by_handle.c |  4 +++-
>  src/statx.h          | 22 ++++++++++++++++++++--
>  2 files changed, 23 insertions(+), 3 deletions(-)
>

This patch conflicts with commit
873e36c9 - statx.h: update to latest kernel UAPI
already in for-next branch (this is the branch to base patches on)

> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index 0f74ed08b1f0..d9c802ca9bd1 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -82,12 +82,14 @@ Examples:
>  #include <string.h>
>  #include <fcntl.h>
>  #include <unistd.h>
> -#include <sys/stat.h>
>  #include <sys/types.h>
>  #include <errno.h>
>  #include <linux/limits.h>
>  #include <libgen.h>
>
> +#include <sys/stat.h>
> +#include "statx.h"
> +

So probably best to squash this one liner into the 2nd patch.
I guess Zorro can do it on commit if needed.

Thanks,
Amir.

[PATCH xfstests v3 1/2] open_by_handle: verify u32 and u64 mount IDs

[Aleksa Sarai]

Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
make sure they match properly as part of the regular open_by_handle
tests. Also, add automatic tests for the old u32 mount IDs as well.

By default, we do mount ID checks but silently skip the tests if the
syscalls are not supported by the running kernel (to ensure the tests
continue to work for old kernels). We will add some tests explicitly
checking the new features (with no silent skipping) in a future patch.

The u32 mount ID tests require STATX_MNT_ID (Linux 5.8), while the u64
mount ID tests require STATX_MNT_ID_UNIQUE (Linux 6.9) and
AT_HANDLE_MNT_ID_UNIQUE (linux-next).

Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Changed in v3:
- Make skipping completely silent in regular open_by_handle mode. [Amir Goldstein]
- Re-add -M to turn skipping into errors and add a new test that uses
  -M, but is skipped on older kernels. [Amir Goldstein]
- v2: <https://lore.kernel.org/all/20240902164554.928371-1-cyphar@cyphar.com/>
Changed in v2:
- Remove -M argument and always do the mount ID tests. [Amir Goldstein]
- Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
  or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
- v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
---
 src/open_by_handle.c | 132 +++++++++++++++++++++++++++++++++----------
 1 file changed, 103 insertions(+), 29 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 0f74ed08b1f0..920ec7d9170b 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -87,6 +87,15 @@ Examples:
 #include <errno.h>
 #include <linux/limits.h>
 #include <libgen.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#include <sys/stat.h>
+#include "statx.h"
+
+#ifndef AT_HANDLE_MNT_ID_UNIQUE
+#	define AT_HANDLE_MNT_ID_UNIQUE 0x001
+#endif
 
 #define MAXFILES 1024
 
@@ -108,6 +117,7 @@ void usage(void)
 	fprintf(stderr, "open_by_handle -a <test_dir> [N] - write data to test files after open by handle\n");
 	fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
+	fprintf(stderr, "open_by_handle -U <test_dir> [N] - verify the mount ID returned with AT_HANDLE_MNT_ID_UNIQUE is correct\n");
 	fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
@@ -118,6 +128,94 @@ void usage(void)
 	exit(EXIT_FAILURE);
 }
 
+static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
+				int bufsz)
+{
+	int ret;
+	int mntid_short;
+
+	static bool skip_mntid, skip_mntid_unique;
+
+	uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
+	struct statx statxbuf;
+
+	/* Get both the short and unique mount id. */
+	if (!skip_mntid) {
+		if (xfstests_statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
+			fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+		if (!(statxbuf.stx_mask & STATX_MNT_ID))
+			skip_mntid = true;
+		else
+			statx_mntid_short = statxbuf.stx_mnt_id;
+	}
+
+	if (!skip_mntid_unique) {
+		if (xfstests_statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
+			fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+		/*
+		 * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
+		 * kernel doesn't give us a unique mount ID just skip it.
+		 */
+		if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE))
+			skip_mntid_unique = true;
+		else
+			statx_mntid_unique = statxbuf.stx_mnt_id;
+	}
+
+	fh->handle_bytes = bufsz;
+	ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	if (bufsz < fh->handle_bytes) {
+		/* Query the filesystem required bufsz and the file handle */
+		if (ret != -1 || errno != EOVERFLOW) {
+			fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
+			return EXIT_FAILURE;
+		}
+		ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	}
+	if (ret < 0) {
+		fprintf(stderr, "%s: name_to_handle: %m\n", fname);
+		return EXIT_FAILURE;
+	}
+
+	if (!skip_mntid) {
+		if (mntid_short != (int) statx_mntid_short) {
+			fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
+			return EXIT_FAILURE;
+		}
+	}
+
+	if (!skip_mntid_unique) {
+		struct handle dummy_fh;
+		uint64_t mntid_unique = 0;
+
+		/*
+		 * Get the unique mount ID. We don't need to get another copy of the
+		 * handle so store it in a dummy struct.
+		 */
+		dummy_fh.fh.handle_bytes = fh->handle_bytes;
+		ret = name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE);
+		if (ret < 0) {
+			if (errno != EINVAL) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
+				return EXIT_FAILURE;
+			}
+			/* EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported */
+			skip_mntid_unique = true;
+		} else {
+			if (mntid_unique != statx_mntid_unique) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
+				return EXIT_FAILURE;
+			}
+		}
+	}
+
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	int	i, c;
@@ -130,7 +228,7 @@ int main(int argc, char **argv)
 	char	fname2[PATH_MAX];
 	char	*test_dir;
 	char	*mount_dir;
-	int	mount_fd, mount_id;
+	int	mount_fd;
 	char	*infile = NULL, *outfile = NULL;
 	int	in_fd = 0, out_fd = 0;
 	int	numfiles = 1;
@@ -305,21 +403,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			handle[i].fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			if (bufsz < handle[i].fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(fname, ": name_to_handle");
-				perror(fname);
+			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
+			if (ret)
 				return EXIT_FAILURE;
-			}
 		}
 		if (keepopen) {
 			/* Open without close to keep unlinked files around */
@@ -347,21 +433,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			dir_handle.fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			if (bufsz < dir_handle.fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(dname, ": name_to_handle");
-				perror(dname);
+			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
+			if (ret)
 				return EXIT_FAILURE;
-			}
 		}
 		if (out_fd) {
 			ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
-- 
2.46.0

[PATCH xfstests v3 2/2] generic/756: test name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) explicitly

[Aleksa Sarai]

In order to make sure we are actually testing AT_HANDLE_MNT_ID_UNIQUE,
add a test (based on generic/426) which runs the open_by_handle in a
mode where it will error out if there is a problem with getting mount
IDs. The test is skipped if the kernel doesn't support the necessary
features.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 common/rc             | 24 ++++++++++++++++
 src/open_by_handle.c  | 63 ++++++++++++++++++++++++++++++++++-------
 tests/generic/756     | 65 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/756.out |  5 ++++
 4 files changed, 147 insertions(+), 10 deletions(-)
 create mode 100755 tests/generic/756
 create mode 100644 tests/generic/756.out

diff --git a/common/rc b/common/rc
index 9da9fe188297..0beaf2ff1126 100644
--- a/common/rc
+++ b/common/rc
@@ -5178,6 +5178,30 @@ _require_fibmap()
 	rm -f $file
 }
 
+_require_statx_unique_mountid()
+{
+	# statx(STATX_MNT_ID=0x1000) was added in Linux 5.8.
+	# statx(STATX_MNT_ID_UNIQUE=0x4000) was added in Linux 6.9.
+	# We only need to check the latter.
+
+	export STATX_MNT_ID_UNIQUE=0x4000
+	local statx_mask=$(
+		${XFS_IO_PROG} -c "statx -m $STATX_MNT_ID_UNIQUE -r" "$TEST_DIR" |
+		sed -En 's/stat\.mask = (0x[0-9a-f]+)/\1/p'
+	)
+
+	[[ $(( statx_mask & STATX_MNT_ID_UNIQUE )) == $((STATX_MNT_ID_UNIQUE)) ]] ||
+		_notrun "statx does not support STATX_MNT_ID_UNIQUE on this kernel"
+}
+
+_require_open_by_handle_unique_mountid()
+{
+	_require_test_program "open_by_handle"
+
+	$here/src/open_by_handle -C AT_HANDLE_MNT_ID_UNIQUE 2>&1 \
+		|| _notrun "name_to_handle_at does not support AT_HANDLE_MNT_ID_UNIQUE"
+}
+
 _try_wipe_scratch_devs()
 {
 	test -x "$WIPEFS_PROG" || return 0
diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 920ec7d9170b..b5c1a30abbbc 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -106,9 +106,11 @@ struct handle {
 
 void usage(void)
 {
-	fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
+	fprintf(stderr, "usage: open_by_handle [-cludmMrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
+	fprintf(stderr, "       open_by_handle -C <feature>\n");
 	fprintf(stderr, "\n");
 	fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
+	fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
 	fprintf(stderr, "open_by_handle    <test_dir> [N] - get file handles of test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -n <test_dir> [N] - get file handles of test files and try to open by handle without drop caches\n");
 	fprintf(stderr, "open_by_handle -k <test_dir> [N] - get file handles of files that are kept open, drop caches and try to open by handle\n");
@@ -117,19 +119,23 @@ void usage(void)
 	fprintf(stderr, "open_by_handle -a <test_dir> [N] - write data to test files after open by handle\n");
 	fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
-	fprintf(stderr, "open_by_handle -U <test_dir> [N] - verify the mount ID returned with AT_HANDLE_MNT_ID_UNIQUE is correct\n");
 	fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
+	fprintf(stderr, "open_by_handle -M <test_dir> [N] - do not silently skip the mount ID verifications\n");
 	fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
 	fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
 	fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
 	fprintf(stderr, "open_by_handle -z <test_dir> [N] - query filesystem required buffer size\n");
+	fprintf(stderr, "\n");
+	fprintf(stderr, "open_by_handle -C <feature>      - check if <feature> is supported by the kernel.\n");
+	fprintf(stderr, "  <feature> can be any of the following values:\n");
+	fprintf(stderr, "  - AT_HANDLE_MNT_ID_UNIQUE\n");
 	exit(EXIT_FAILURE);
 }
 
 static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
-				int bufsz)
+				int bufsz, bool force_check_mountid)
 {
 	int ret;
 	int mntid_short;
@@ -145,10 +151,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 			fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
 			return EXIT_FAILURE;
 		}
-		if (!(statxbuf.stx_mask & STATX_MNT_ID))
+		if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
+			if (force_check_mountid) {
+				fprintf(stderr, "%s: statx(STATX_MNT_ID) not supported by running kernel\n", fname);
+				return EXIT_FAILURE;
+			}
 			skip_mntid = true;
-		else
+		} else {
 			statx_mntid_short = statxbuf.stx_mnt_id;
+		}
 	}
 
 	if (!skip_mntid_unique) {
@@ -160,10 +171,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 		 * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
 		 * kernel doesn't give us a unique mount ID just skip it.
 		 */
-		if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE))
+		if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)) {
+			if (force_check_mountid) {
+				fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
+				return EXIT_FAILURE;
+			}
 			skip_mntid_unique = true;
-		else
+		} else {
 			statx_mntid_unique = statxbuf.stx_mnt_id;
+		}
 	}
 
 	fh->handle_bytes = bufsz;
@@ -204,6 +220,10 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 				return EXIT_FAILURE;
 			}
 			/* EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported */
+			if (force_check_mountid) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
+				return EXIT_FAILURE;
+			}
 			skip_mntid_unique = true;
 		} else {
 			if (mntid_unique != statx_mntid_unique) {
@@ -216,6 +236,22 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 	return 0;
 }
 
+static int check_feature(const char *feature)
+{
+	if (!strcmp(feature, "AT_HANDLE_MNT_ID_UNIQUE")) {
+		int ret = name_to_handle_at(AT_FDCWD, ".", NULL, NULL, AT_HANDLE_MNT_ID_UNIQUE);
+		/* If AT_HANDLE_MNT_ID_UNIQUE is supported, we get EFAULT. */
+		if (ret < 0 && errno == EINVAL) {
+			fprintf(stderr, "name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n");
+			return EXIT_FAILURE;
+		}
+		return 0;
+	}
+
+	fprintf(stderr, "unknown feature name '%s'\n", feature);
+	return EXIT_FAILURE;
+}
+
 int main(int argc, char **argv)
 {
 	int	i, c;
@@ -235,16 +271,20 @@ int main(int argc, char **argv)
 	int	create = 0, delete = 0, nlink = 1, move = 0;
 	int	rd = 0, wr = 0, wrafter = 0, parent = 0;
 	int	keepopen = 0, drop_caches = 1, sleep_loop = 0;
+	int force_check_mountid = 0;
 	int	bufsz = MAX_HANDLE_SZ;
 
 	if (argc < 2)
 		usage();
 
-	while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
+	while ((c = getopt(argc, argv, "cC:ludmMrwapknhi:o:sz")) != -1) {
 		switch (c) {
 		case 'c':
 			create = 1;
 			break;
+		case 'C':
+			/* Check kernel feature support. */
+			return check_feature(optarg);
 		case 'w':
 			/* Write data before open_by_handle_at() */
 			wr = 1;
@@ -271,6 +311,9 @@ int main(int argc, char **argv)
 		case 'm':
 			move = 1;
 			break;
+		case 'M':
+			force_check_mountid = 1;
+			break;
 		case 'p':
 			parent = 1;
 			break;
@@ -403,7 +446,7 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
+			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz, force_check_mountid);
 			if (ret)
 				return EXIT_FAILURE;
 		}
@@ -433,7 +476,7 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
+			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz, force_check_mountid);
 			if (ret)
 				return EXIT_FAILURE;
 		}
diff --git a/tests/generic/756 b/tests/generic/756
new file mode 100755
index 000000000000..c7a82cfd25f4
--- /dev/null
+++ b/tests/generic/756
@@ -0,0 +1,65 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
+# Copyright (C) 2024 Aleksa Sarai <cyphar@cyphar.com>
+#
+# FS QA Test No. 756
+#
+# Check stale handles pointing to unlinked files and non-stale handles pointing
+# to linked files while verifying that u64 mount IDs are correctly returned.
+#
+. ./common/preamble
+_begin_fstest auto quick exportfs
+
+# Import common functions.
+. ./common/filter
+
+
+# Modify as appropriate.
+_require_test
+# _require_exportfs and  already requires open_by_handle, but let's not count on it
+_require_test_program "open_by_handle"
+_require_exportfs
+# We need both STATX_MNT_ID_UNIQUE and AT_HANDLE_MNT_ID_UNIQUE.
+_require_statx_unique_mountid
+_require_open_by_handle_unique_mountid
+
+NUMFILES=1024
+testdir=$TEST_DIR/$seq-dir
+mkdir -p $testdir
+
+# Create empty test files in test dir
+create_test_files()
+{
+	local dir=$1
+
+	mkdir -p $dir
+	rm -f $dir/*
+	$here/src/open_by_handle -c $dir $NUMFILES
+}
+
+# Test encode/decode file handles
+test_file_handles()
+{
+	local dir=$1
+	local opt=$2
+
+	echo test_file_handles $* | _filter_test_dir
+	$here/src/open_by_handle $opt $dir $NUMFILES
+}
+
+# Check stale handles to deleted files
+create_test_files $testdir
+test_file_handles $testdir -Md
+
+# Check non-stale handles to linked files
+create_test_files $testdir
+test_file_handles $testdir -M
+
+# Check non-stale handles to files that were hardlinked and original deleted
+create_test_files $testdir
+test_file_handles $testdir -Ml
+test_file_handles $testdir -Mu
+
+status=0
+exit
diff --git a/tests/generic/756.out b/tests/generic/756.out
new file mode 100644
index 000000000000..48aed88d87b9
--- /dev/null
+++ b/tests/generic/756.out
@@ -0,0 +1,5 @@
+QA output created by 756
+test_file_handles TEST_DIR/756-dir -Md
+test_file_handles TEST_DIR/756-dir -M
+test_file_handles TEST_DIR/756-dir -Ml
+test_file_handles TEST_DIR/756-dir -Mu
-- 
2.46.0

Re: [PATCH xfstests v3 2/2] generic/756: test name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) explicitly

[Amir Goldstein]

On Wed, Sep 4, 2024 at 7:57 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> In order to make sure we are actually testing AT_HANDLE_MNT_ID_UNIQUE,
> add a test (based on generic/426) which runs the open_by_handle in a
> mode where it will error out if there is a problem with getting mount
> IDs. The test is skipped if the kernel doesn't support the necessary
> features.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Apart from one minor nits below, you may add:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  common/rc             | 24 ++++++++++++++++
>  src/open_by_handle.c  | 63 ++++++++++++++++++++++++++++++++++-------
>  tests/generic/756     | 65 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/756.out |  5 ++++
>  4 files changed, 147 insertions(+), 10 deletions(-)
>  create mode 100755 tests/generic/756
>  create mode 100644 tests/generic/756.out
>
> diff --git a/common/rc b/common/rc
> index 9da9fe188297..0beaf2ff1126 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5178,6 +5178,30 @@ _require_fibmap()
>         rm -f $file
>  }
>
> +_require_statx_unique_mountid()
> +{
> +       # statx(STATX_MNT_ID=0x1000) was added in Linux 5.8.
> +       # statx(STATX_MNT_ID_UNIQUE=0x4000) was added in Linux 6.9.
> +       # We only need to check the latter.
> +
> +       export STATX_MNT_ID_UNIQUE=0x4000
> +       local statx_mask=$(
> +               ${XFS_IO_PROG} -c "statx -m $STATX_MNT_ID_UNIQUE -r" "$TEST_DIR" |
> +               sed -En 's/stat\.mask = (0x[0-9a-f]+)/\1/p'
> +       )
> +
> +       [[ $(( statx_mask & STATX_MNT_ID_UNIQUE )) == $((STATX_MNT_ID_UNIQUE)) ]] ||
> +               _notrun "statx does not support STATX_MNT_ID_UNIQUE on this kernel"
> +}
> +
> +_require_open_by_handle_unique_mountid()
> +{
> +       _require_test_program "open_by_handle"
> +
> +       $here/src/open_by_handle -C AT_HANDLE_MNT_ID_UNIQUE 2>&1 \
> +               || _notrun "name_to_handle_at does not support AT_HANDLE_MNT_ID_UNIQUE"
> +}
> +
>  _try_wipe_scratch_devs()
>  {
>         test -x "$WIPEFS_PROG" || return 0
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index 920ec7d9170b..b5c1a30abbbc 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -106,9 +106,11 @@ struct handle {
>
>  void usage(void)
>  {
> -       fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> +       fprintf(stderr, "usage: open_by_handle [-cludmMrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> +       fprintf(stderr, "       open_by_handle -C <feature>\n");
>         fprintf(stderr, "\n");
>         fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
> +       fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
>         fprintf(stderr, "open_by_handle    <test_dir> [N] - get file handles of test files, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -n <test_dir> [N] - get file handles of test files and try to open by handle without drop caches\n");
>         fprintf(stderr, "open_by_handle -k <test_dir> [N] - get file handles of files that are kept open, drop caches and try to open by handle\n");
> @@ -117,19 +119,23 @@ void usage(void)
>         fprintf(stderr, "open_by_handle -a <test_dir> [N] - write data to test files after open by handle\n");
>         fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
> -       fprintf(stderr, "open_by_handle -U <test_dir> [N] - verify the mount ID returned with AT_HANDLE_MNT_ID_UNIQUE is correct\n");

I guess this was not supposed to be in the first patch

>         fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
> +       fprintf(stderr, "open_by_handle -M <test_dir> [N] - do not silently skip the mount ID verifications\n");
>         fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
>         fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
>         fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
>         fprintf(stderr, "open_by_handle -z <test_dir> [N] - query filesystem required buffer size\n");
> +       fprintf(stderr, "\n");
> +       fprintf(stderr, "open_by_handle -C <feature>      - check if <feature> is supported by the kernel.\n");
> +       fprintf(stderr, "  <feature> can be any of the following values:\n");
> +       fprintf(stderr, "  - AT_HANDLE_MNT_ID_UNIQUE\n");
>         exit(EXIT_FAILURE);
>  }
>
>  static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
> -                               int bufsz)
> +                               int bufsz, bool force_check_mountid)
>  {
>         int ret;
>         int mntid_short;
> @@ -145,10 +151,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
>                         fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
>                         return EXIT_FAILURE;
>                 }
> -               if (!(statxbuf.stx_mask & STATX_MNT_ID))
> +               if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> +                       if (force_check_mountid) {
> +                               fprintf(stderr, "%s: statx(STATX_MNT_ID) not supported by running kernel\n", fname);
> +                               return EXIT_FAILURE;
> +                       }
>                         skip_mntid = true;
> -               else
> +               } else {
>                         statx_mntid_short = statxbuf.stx_mnt_id;
> +               }
>         }
>
>         if (!skip_mntid_unique) {
> @@ -160,10 +171,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
>                  * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
>                  * kernel doesn't give us a unique mount ID just skip it.
>                  */
> -               if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE))
> +               if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)) {
> +                       if (force_check_mountid) {
> +                               fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
> +                               return EXIT_FAILURE;
> +                       }
>                         skip_mntid_unique = true;
> -               else
> +               } else {
>                         statx_mntid_unique = statxbuf.stx_mnt_id;
> +               }
>         }
>
>         fh->handle_bytes = bufsz;
> @@ -204,6 +220,10 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
>                                 return EXIT_FAILURE;
>                         }
>                         /* EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported */
> +                       if (force_check_mountid) {
> +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
> +                               return EXIT_FAILURE;
> +                       }
>                         skip_mntid_unique = true;
>                 } else {
>                         if (mntid_unique != statx_mntid_unique) {
> @@ -216,6 +236,22 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
>         return 0;
>  }
>
> +static int check_feature(const char *feature)
> +{
> +       if (!strcmp(feature, "AT_HANDLE_MNT_ID_UNIQUE")) {
> +               int ret = name_to_handle_at(AT_FDCWD, ".", NULL, NULL, AT_HANDLE_MNT_ID_UNIQUE);
> +               /* If AT_HANDLE_MNT_ID_UNIQUE is supported, we get EFAULT. */
> +               if (ret < 0 && errno == EINVAL) {
> +                       fprintf(stderr, "name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n");
> +                       return EXIT_FAILURE;
> +               }
> +               return 0;
> +       }
> +
> +       fprintf(stderr, "unknown feature name '%s'\n", feature);
> +       return EXIT_FAILURE;
> +}
> +
>  int main(int argc, char **argv)
>  {
>         int     i, c;
> @@ -235,16 +271,20 @@ int main(int argc, char **argv)
>         int     create = 0, delete = 0, nlink = 1, move = 0;
>         int     rd = 0, wr = 0, wrafter = 0, parent = 0;
>         int     keepopen = 0, drop_caches = 1, sleep_loop = 0;
> +       int force_check_mountid = 0;
>         int     bufsz = MAX_HANDLE_SZ;
>
>         if (argc < 2)
>                 usage();
>
> -       while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
> +       while ((c = getopt(argc, argv, "cC:ludmMrwapknhi:o:sz")) != -1) {
>                 switch (c) {
>                 case 'c':
>                         create = 1;
>                         break;
> +               case 'C':
> +                       /* Check kernel feature support. */
> +                       return check_feature(optarg);
>                 case 'w':
>                         /* Write data before open_by_handle_at() */
>                         wr = 1;
> @@ -271,6 +311,9 @@ int main(int argc, char **argv)
>                 case 'm':
>                         move = 1;
>                         break;
> +               case 'M':
> +                       force_check_mountid = 1;
> +                       break;
>                 case 'p':
>                         parent = 1;
>                         break;
> @@ -403,7 +446,7 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
> +                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz, force_check_mountid);
>                         if (ret)
>                                 return EXIT_FAILURE;
>                 }
> @@ -433,7 +476,7 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
> +                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz, force_check_mountid);
>                         if (ret)
>                                 return EXIT_FAILURE;
>                 }
> diff --git a/tests/generic/756 b/tests/generic/756
> new file mode 100755
> index 000000000000..c7a82cfd25f4
> --- /dev/null
> +++ b/tests/generic/756
> @@ -0,0 +1,65 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
> +# Copyright (C) 2024 Aleksa Sarai <cyphar@cyphar.com>
> +#
> +# FS QA Test No. 756
> +#
> +# Check stale handles pointing to unlinked files and non-stale handles pointing
> +# to linked files while verifying that u64 mount IDs are correctly returned.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick exportfs
> +
> +# Import common functions.
> +. ./common/filter
> +
> +
> +# Modify as appropriate.
> +_require_test
> +# _require_exportfs and  already requires open_by_handle, but let's not count on it
> +_require_test_program "open_by_handle"
> +_require_exportfs
> +# We need both STATX_MNT_ID_UNIQUE and AT_HANDLE_MNT_ID_UNIQUE.
> +_require_statx_unique_mountid
> +_require_open_by_handle_unique_mountid
> +
> +NUMFILES=1024
> +testdir=$TEST_DIR/$seq-dir
> +mkdir -p $testdir
> +
> +# Create empty test files in test dir
> +create_test_files()
> +{
> +       local dir=$1
> +
> +       mkdir -p $dir
> +       rm -f $dir/*
> +       $here/src/open_by_handle -c $dir $NUMFILES
> +}
> +
> +# Test encode/decode file handles
> +test_file_handles()
> +{
> +       local dir=$1
> +       local opt=$2
> +
> +       echo test_file_handles $* | _filter_test_dir
> +       $here/src/open_by_handle $opt $dir $NUMFILES
> +}
> +
> +# Check stale handles to deleted files
> +create_test_files $testdir
> +test_file_handles $testdir -Md
> +
> +# Check non-stale handles to linked files
> +create_test_files $testdir
> +test_file_handles $testdir -M
> +
> +# Check non-stale handles to files that were hardlinked and original deleted
> +create_test_files $testdir
> +test_file_handles $testdir -Ml
> +test_file_handles $testdir -Mu
> +
> +status=0
> +exit
> diff --git a/tests/generic/756.out b/tests/generic/756.out
> new file mode 100644
> index 000000000000..48aed88d87b9
> --- /dev/null
> +++ b/tests/generic/756.out
> @@ -0,0 +1,5 @@
> +QA output created by 756
> +test_file_handles TEST_DIR/756-dir -Md
> +test_file_handles TEST_DIR/756-dir -M
> +test_file_handles TEST_DIR/756-dir -Ml
> +test_file_handles TEST_DIR/756-dir -Mu
> --
> 2.46.0
>

Re: [PATCH xfstests v3 2/2] generic/756: test name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) explicitly

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 14683 bytes --]

On 2024-09-04, Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Sep 4, 2024 at 7:57 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > In order to make sure we are actually testing AT_HANDLE_MNT_ID_UNIQUE,
> > add a test (based on generic/426) which runs the open_by_handle in a
> > mode where it will error out if there is a problem with getting mount
> > IDs. The test is skipped if the kernel doesn't support the necessary
> > features.
> >
> > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> 
> Apart from one minor nits below, you may add:
> 
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> > ---
> >  common/rc             | 24 ++++++++++++++++
> >  src/open_by_handle.c  | 63 ++++++++++++++++++++++++++++++++++-------
> >  tests/generic/756     | 65 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/756.out |  5 ++++
> >  4 files changed, 147 insertions(+), 10 deletions(-)
> >  create mode 100755 tests/generic/756
> >  create mode 100644 tests/generic/756.out
> >
> > diff --git a/common/rc b/common/rc
> > index 9da9fe188297..0beaf2ff1126 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -5178,6 +5178,30 @@ _require_fibmap()
> >         rm -f $file
> >  }
> >
> > +_require_statx_unique_mountid()
> > +{
> > +       # statx(STATX_MNT_ID=0x1000) was added in Linux 5.8.
> > +       # statx(STATX_MNT_ID_UNIQUE=0x4000) was added in Linux 6.9.
> > +       # We only need to check the latter.
> > +
> > +       export STATX_MNT_ID_UNIQUE=0x4000
> > +       local statx_mask=$(
> > +               ${XFS_IO_PROG} -c "statx -m $STATX_MNT_ID_UNIQUE -r" "$TEST_DIR" |
> > +               sed -En 's/stat\.mask = (0x[0-9a-f]+)/\1/p'
> > +       )
> > +
> > +       [[ $(( statx_mask & STATX_MNT_ID_UNIQUE )) == $((STATX_MNT_ID_UNIQUE)) ]] ||
> > +               _notrun "statx does not support STATX_MNT_ID_UNIQUE on this kernel"
> > +}
> > +
> > +_require_open_by_handle_unique_mountid()
> > +{
> > +       _require_test_program "open_by_handle"
> > +
> > +       $here/src/open_by_handle -C AT_HANDLE_MNT_ID_UNIQUE 2>&1 \
> > +               || _notrun "name_to_handle_at does not support AT_HANDLE_MNT_ID_UNIQUE"
> > +}
> > +
> >  _try_wipe_scratch_devs()
> >  {
> >         test -x "$WIPEFS_PROG" || return 0
> > diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> > index 920ec7d9170b..b5c1a30abbbc 100644
> > --- a/src/open_by_handle.c
> > +++ b/src/open_by_handle.c
> > @@ -106,9 +106,11 @@ struct handle {
> >
> >  void usage(void)
> >  {
> > -       fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> > +       fprintf(stderr, "usage: open_by_handle [-cludmMrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> > +       fprintf(stderr, "       open_by_handle -C <feature>\n");
> >         fprintf(stderr, "\n");
> >         fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
> > +       fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
> >         fprintf(stderr, "open_by_handle    <test_dir> [N] - get file handles of test files, drop caches and try to open by handle\n");
> >         fprintf(stderr, "open_by_handle -n <test_dir> [N] - get file handles of test files and try to open by handle without drop caches\n");
> >         fprintf(stderr, "open_by_handle -k <test_dir> [N] - get file handles of files that are kept open, drop caches and try to open by handle\n");
> > @@ -117,19 +119,23 @@ void usage(void)
> >         fprintf(stderr, "open_by_handle -a <test_dir> [N] - write data to test files after open by handle\n");
> >         fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
> >         fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
> > -       fprintf(stderr, "open_by_handle -U <test_dir> [N] - verify the mount ID returned with AT_HANDLE_MNT_ID_UNIQUE is correct\n");
> 
> I guess this was not supposed to be in the first patch

Yeah, it got mixed up when splitting the patch. I'll fix it up.

> >         fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
> >         fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
> > +       fprintf(stderr, "open_by_handle -M <test_dir> [N] - do not silently skip the mount ID verifications\n");
> >         fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
> >         fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
> >         fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
> >         fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
> >         fprintf(stderr, "open_by_handle -z <test_dir> [N] - query filesystem required buffer size\n");
> > +       fprintf(stderr, "\n");
> > +       fprintf(stderr, "open_by_handle -C <feature>      - check if <feature> is supported by the kernel.\n");
> > +       fprintf(stderr, "  <feature> can be any of the following values:\n");
> > +       fprintf(stderr, "  - AT_HANDLE_MNT_ID_UNIQUE\n");
> >         exit(EXIT_FAILURE);
> >  }
> >
> >  static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
> > -                               int bufsz)
> > +                               int bufsz, bool force_check_mountid)
> >  {
> >         int ret;
> >         int mntid_short;
> > @@ -145,10 +151,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
> >                         fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> >                         return EXIT_FAILURE;
> >                 }
> > -               if (!(statxbuf.stx_mask & STATX_MNT_ID))
> > +               if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
> > +                       if (force_check_mountid) {
> > +                               fprintf(stderr, "%s: statx(STATX_MNT_ID) not supported by running kernel\n", fname);
> > +                               return EXIT_FAILURE;
> > +                       }
> >                         skip_mntid = true;
> > -               else
> > +               } else {
> >                         statx_mntid_short = statxbuf.stx_mnt_id;
> > +               }
> >         }
> >
> >         if (!skip_mntid_unique) {
> > @@ -160,10 +171,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
> >                  * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
> >                  * kernel doesn't give us a unique mount ID just skip it.
> >                  */
> > -               if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE))
> > +               if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)) {
> > +                       if (force_check_mountid) {
> > +                               fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
> > +                               return EXIT_FAILURE;
> > +                       }
> >                         skip_mntid_unique = true;
> > -               else
> > +               } else {
> >                         statx_mntid_unique = statxbuf.stx_mnt_id;
> > +               }
> >         }
> >
> >         fh->handle_bytes = bufsz;
> > @@ -204,6 +220,10 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
> >                                 return EXIT_FAILURE;
> >                         }
> >                         /* EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported */
> > +                       if (force_check_mountid) {
> > +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
> > +                               return EXIT_FAILURE;
> > +                       }
> >                         skip_mntid_unique = true;
> >                 } else {
> >                         if (mntid_unique != statx_mntid_unique) {
> > @@ -216,6 +236,22 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
> >         return 0;
> >  }
> >
> > +static int check_feature(const char *feature)
> > +{
> > +       if (!strcmp(feature, "AT_HANDLE_MNT_ID_UNIQUE")) {
> > +               int ret = name_to_handle_at(AT_FDCWD, ".", NULL, NULL, AT_HANDLE_MNT_ID_UNIQUE);
> > +               /* If AT_HANDLE_MNT_ID_UNIQUE is supported, we get EFAULT. */
> > +               if (ret < 0 && errno == EINVAL) {
> > +                       fprintf(stderr, "name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n");
> > +                       return EXIT_FAILURE;
> > +               }
> > +               return 0;
> > +       }
> > +
> > +       fprintf(stderr, "unknown feature name '%s'\n", feature);
> > +       return EXIT_FAILURE;
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >         int     i, c;
> > @@ -235,16 +271,20 @@ int main(int argc, char **argv)
> >         int     create = 0, delete = 0, nlink = 1, move = 0;
> >         int     rd = 0, wr = 0, wrafter = 0, parent = 0;
> >         int     keepopen = 0, drop_caches = 1, sleep_loop = 0;
> > +       int force_check_mountid = 0;
> >         int     bufsz = MAX_HANDLE_SZ;
> >
> >         if (argc < 2)
> >                 usage();
> >
> > -       while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
> > +       while ((c = getopt(argc, argv, "cC:ludmMrwapknhi:o:sz")) != -1) {
> >                 switch (c) {
> >                 case 'c':
> >                         create = 1;
> >                         break;
> > +               case 'C':
> > +                       /* Check kernel feature support. */
> > +                       return check_feature(optarg);
> >                 case 'w':
> >                         /* Write data before open_by_handle_at() */
> >                         wr = 1;
> > @@ -271,6 +311,9 @@ int main(int argc, char **argv)
> >                 case 'm':
> >                         move = 1;
> >                         break;
> > +               case 'M':
> > +                       force_check_mountid = 1;
> > +                       break;
> >                 case 'p':
> >                         parent = 1;
> >                         break;
> > @@ -403,7 +446,7 @@ int main(int argc, char **argv)
> >                                 return EXIT_FAILURE;
> >                         }
> >                 } else {
> > -                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
> > +                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz, force_check_mountid);
> >                         if (ret)
> >                                 return EXIT_FAILURE;
> >                 }
> > @@ -433,7 +476,7 @@ int main(int argc, char **argv)
> >                                 return EXIT_FAILURE;
> >                         }
> >                 } else {
> > -                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
> > +                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz, force_check_mountid);
> >                         if (ret)
> >                                 return EXIT_FAILURE;
> >                 }
> > diff --git a/tests/generic/756 b/tests/generic/756
> > new file mode 100755
> > index 000000000000..c7a82cfd25f4
> > --- /dev/null
> > +++ b/tests/generic/756
> > @@ -0,0 +1,65 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
> > +# Copyright (C) 2024 Aleksa Sarai <cyphar@cyphar.com>
> > +#
> > +# FS QA Test No. 756
> > +#
> > +# Check stale handles pointing to unlinked files and non-stale handles pointing
> > +# to linked files while verifying that u64 mount IDs are correctly returned.
> > +#
> > +. ./common/preamble
> > +_begin_fstest auto quick exportfs
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +
> > +# Modify as appropriate.
> > +_require_test
> > +# _require_exportfs and  already requires open_by_handle, but let's not count on it
> > +_require_test_program "open_by_handle"
> > +_require_exportfs
> > +# We need both STATX_MNT_ID_UNIQUE and AT_HANDLE_MNT_ID_UNIQUE.
> > +_require_statx_unique_mountid
> > +_require_open_by_handle_unique_mountid
> > +
> > +NUMFILES=1024
> > +testdir=$TEST_DIR/$seq-dir
> > +mkdir -p $testdir
> > +
> > +# Create empty test files in test dir
> > +create_test_files()
> > +{
> > +       local dir=$1
> > +
> > +       mkdir -p $dir
> > +       rm -f $dir/*
> > +       $here/src/open_by_handle -c $dir $NUMFILES
> > +}
> > +
> > +# Test encode/decode file handles
> > +test_file_handles()
> > +{
> > +       local dir=$1
> > +       local opt=$2
> > +
> > +       echo test_file_handles $* | _filter_test_dir
> > +       $here/src/open_by_handle $opt $dir $NUMFILES
> > +}
> > +
> > +# Check stale handles to deleted files
> > +create_test_files $testdir
> > +test_file_handles $testdir -Md
> > +
> > +# Check non-stale handles to linked files
> > +create_test_files $testdir
> > +test_file_handles $testdir -M
> > +
> > +# Check non-stale handles to files that were hardlinked and original deleted
> > +create_test_files $testdir
> > +test_file_handles $testdir -Ml
> > +test_file_handles $testdir -Mu
> > +
> > +status=0
> > +exit
> > diff --git a/tests/generic/756.out b/tests/generic/756.out
> > new file mode 100644
> > index 000000000000..48aed88d87b9
> > --- /dev/null
> > +++ b/tests/generic/756.out
> > @@ -0,0 +1,5 @@
> > +QA output created by 756
> > +test_file_handles TEST_DIR/756-dir -Md
> > +test_file_handles TEST_DIR/756-dir -M
> > +test_file_handles TEST_DIR/756-dir -Ml
> > +test_file_handles TEST_DIR/756-dir -Mu
> > --
> > 2.46.0
> >

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH xfstests v3 1/2] open_by_handle: verify u32 and u64 mount IDs

[Amir Goldstein]

On Wed, Sep 4, 2024 at 7:57 PM Aleksa Sarai <cyphar@cyphar.com> wrote:
>
> Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
> make sure they match properly as part of the regular open_by_handle
> tests. Also, add automatic tests for the old u32 mount IDs as well.
>
> By default, we do mount ID checks but silently skip the tests if the
> syscalls are not supported by the running kernel (to ensure the tests
> continue to work for old kernels). We will add some tests explicitly
> checking the new features (with no silent skipping) in a future patch.
>
> The u32 mount ID tests require STATX_MNT_ID (Linux 5.8), while the u64
> mount ID tests require STATX_MNT_ID_UNIQUE (Linux 6.9) and
> AT_HANDLE_MNT_ID_UNIQUE (linux-next).
>
> Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
> Changed in v3:
> - Make skipping completely silent in regular open_by_handle mode. [Amir Goldstein]
> - Re-add -M to turn skipping into errors and add a new test that uses
>   -M, but is skipped on older kernels. [Amir Goldstein]
> - v2: <https://lore.kernel.org/all/20240902164554.928371-1-cyphar@cyphar.com/>
> Changed in v2:
> - Remove -M argument and always do the mount ID tests. [Amir Goldstein]
> - Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
>   or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
> - v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
> ---
>  src/open_by_handle.c | 132 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 103 insertions(+), 29 deletions(-)
>
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index 0f74ed08b1f0..920ec7d9170b 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -87,6 +87,15 @@ Examples:
>  #include <errno.h>
>  #include <linux/limits.h>
>  #include <libgen.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +#include <sys/stat.h>
> +#include "statx.h"
> +
> +#ifndef AT_HANDLE_MNT_ID_UNIQUE
> +#      define AT_HANDLE_MNT_ID_UNIQUE 0x001
> +#endif
>
>  #define MAXFILES 1024
>
> @@ -108,6 +117,7 @@ void usage(void)
>         fprintf(stderr, "open_by_handle -a <test_dir> [N] - write data to test files after open by handle\n");
>         fprintf(stderr, "open_by_handle -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
> +       fprintf(stderr, "open_by_handle -U <test_dir> [N] - verify the mount ID returned with AT_HANDLE_MNT_ID_UNIQUE is correct\n");
>         fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
>         fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
> @@ -118,6 +128,94 @@ void usage(void)
>         exit(EXIT_FAILURE);
>  }
>
> +static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
> +                               int bufsz)
> +{
> +       int ret;
> +       int mntid_short;
> +
> +       static bool skip_mntid, skip_mntid_unique;
> +
> +       uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
> +       struct statx statxbuf;
> +
> +       /* Get both the short and unique mount id. */
> +       if (!skip_mntid) {
> +               if (xfstests_statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
> +                       fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +               if (!(statxbuf.stx_mask & STATX_MNT_ID))
> +                       skip_mntid = true;
> +               else
> +                       statx_mntid_short = statxbuf.stx_mnt_id;
> +       }
> +
> +       if (!skip_mntid_unique) {
> +               if (xfstests_statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
> +                       fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
> +                       return EXIT_FAILURE;
> +               }
> +               /*
> +                * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
> +                * kernel doesn't give us a unique mount ID just skip it.
> +                */
> +               if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE))
> +                       skip_mntid_unique = true;
> +               else
> +                       statx_mntid_unique = statxbuf.stx_mnt_id;
> +       }
> +
> +       fh->handle_bytes = bufsz;
> +       ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> +       if (bufsz < fh->handle_bytes) {
> +               /* Query the filesystem required bufsz and the file handle */
> +               if (ret != -1 || errno != EOVERFLOW) {
> +                       fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
> +                       return EXIT_FAILURE;
> +               }
> +               ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
> +       }
> +       if (ret < 0) {
> +               fprintf(stderr, "%s: name_to_handle: %m\n", fname);
> +               return EXIT_FAILURE;
> +       }
> +
> +       if (!skip_mntid) {
> +               if (mntid_short != (int) statx_mntid_short) {
> +                       fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
> +                       return EXIT_FAILURE;
> +               }
> +       }
> +
> +       if (!skip_mntid_unique) {
> +               struct handle dummy_fh;
> +               uint64_t mntid_unique = 0;
> +
> +               /*
> +                * Get the unique mount ID. We don't need to get another copy of the
> +                * handle so store it in a dummy struct.
> +                */
> +               dummy_fh.fh.handle_bytes = fh->handle_bytes;
> +               ret = name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE);
> +               if (ret < 0) {
> +                       if (errno != EINVAL) {
> +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
> +                               return EXIT_FAILURE;
> +                       }
> +                       /* EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported */
> +                       skip_mntid_unique = true;
> +               } else {
> +                       if (mntid_unique != statx_mntid_unique) {
> +                               fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
> +                               return EXIT_FAILURE;
> +                       }
> +               }
> +       }
> +
> +       return 0;
> +}
> +
>  int main(int argc, char **argv)
>  {
>         int     i, c;
> @@ -130,7 +228,7 @@ int main(int argc, char **argv)
>         char    fname2[PATH_MAX];
>         char    *test_dir;
>         char    *mount_dir;
> -       int     mount_fd, mount_id;
> +       int     mount_fd;
>         char    *infile = NULL, *outfile = NULL;
>         int     in_fd = 0, out_fd = 0;
>         int     numfiles = 1;
> @@ -305,21 +403,9 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       handle[i].fh.handle_bytes = bufsz;
> -                       ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> -                       if (bufsz < handle[i].fh.handle_bytes) {
> -                               /* Query the filesystem required bufsz and the file handle */
> -                               if (ret != -1 || errno != EOVERFLOW) {
> -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
> -                                       return EXIT_FAILURE;
> -                               }
> -                               ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> -                       }
> -                       if (ret < 0) {
> -                               strcat(fname, ": name_to_handle");
> -                               perror(fname);
> +                       ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
> +                       if (ret)
>                                 return EXIT_FAILURE;
> -                       }
>                 }
>                 if (keepopen) {
>                         /* Open without close to keep unlinked files around */
> @@ -347,21 +433,9 @@ int main(int argc, char **argv)
>                                 return EXIT_FAILURE;
>                         }
>                 } else {
> -                       dir_handle.fh.handle_bytes = bufsz;
> -                       ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> -                       if (bufsz < dir_handle.fh.handle_bytes) {
> -                               /* Query the filesystem required bufsz and the file handle */
> -                               if (ret != -1 || errno != EOVERFLOW) {
> -                                       fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
> -                                       return EXIT_FAILURE;
> -                               }
> -                               ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
> -                       }
> -                       if (ret < 0) {
> -                               strcat(dname, ": name_to_handle");
> -                               perror(dname);
> +                       ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
> +                       if (ret)
>                                 return EXIT_FAILURE;
> -                       }
>                 }
>                 if (out_fd) {
>                         ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
> --
> 2.46.0
>

[PATCH xfstests v4 1/2] open_by_handle: verify u32 and u64 mount IDs

[Aleksa Sarai]

Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
make sure they match properly as part of the regular open_by_handle
tests. Also, add automatic tests for the old u32 mount IDs as well.

By default, we do mount ID checks but silently skip the tests if the
syscalls are not supported by the running kernel (to ensure the tests
continue to work for old kernels). We will add some tests explicitly
checking the new features (with no silent skipping) in a future patch.

The u32 mount ID tests require STATX_MNT_ID (Linux 5.8), while the u64
mount ID tests require STATX_MNT_ID_UNIQUE (Linux 6.9) and
AT_HANDLE_MNT_ID_UNIQUE (linux-next).

Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
Changed in v4:
- Fix minor flub in patch split. [Amir Goldstein]
- v3: <https://lore.kernel.org/all/20240904175639.2269694-1-cyphar@cyphar.com/>
Changed in v3:
- Make skipping completely silent in regular open_by_handle mode. [Amir Goldstein]
- Re-add -M to turn skipping into errors and add a new test that uses
  -M, but is skipped on older kernels. [Amir Goldstein]
- v2: <https://lore.kernel.org/all/20240902164554.928371-1-cyphar@cyphar.com/>
Changed in v2:
- Remove -M argument and always do the mount ID tests. [Amir Goldstein]
- Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
  or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
- v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/>
---
 src/open_by_handle.c | 131 +++++++++++++++++++++++++++++++++----------
 1 file changed, 102 insertions(+), 29 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 0f74ed08b1f0..dcbcd35561fb 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -87,6 +87,15 @@ Examples:
 #include <errno.h>
 #include <linux/limits.h>
 #include <libgen.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#include <sys/stat.h>
+#include "statx.h"
+
+#ifndef AT_HANDLE_MNT_ID_UNIQUE
+#	define AT_HANDLE_MNT_ID_UNIQUE 0x001
+#endif
 
 #define MAXFILES 1024
 
@@ -118,6 +127,94 @@ void usage(void)
 	exit(EXIT_FAILURE);
 }
 
+static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
+				int bufsz)
+{
+	int ret;
+	int mntid_short;
+
+	static bool skip_mntid, skip_mntid_unique;
+
+	uint64_t statx_mntid_short = 0, statx_mntid_unique = 0;
+	struct statx statxbuf;
+
+	/* Get both the short and unique mount id. */
+	if (!skip_mntid) {
+		if (xfstests_statx(AT_FDCWD, fname, 0, STATX_MNT_ID, &statxbuf) < 0) {
+			fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+		if (!(statxbuf.stx_mask & STATX_MNT_ID))
+			skip_mntid = true;
+		else
+			statx_mntid_short = statxbuf.stx_mnt_id;
+	}
+
+	if (!skip_mntid_unique) {
+		if (xfstests_statx(AT_FDCWD, fname, 0, STATX_MNT_ID_UNIQUE, &statxbuf) < 0) {
+			fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE): %m\n", fname);
+			return EXIT_FAILURE;
+		}
+		/*
+		 * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
+		 * kernel doesn't give us a unique mount ID just skip it.
+		 */
+		if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE))
+			skip_mntid_unique = true;
+		else
+			statx_mntid_unique = statxbuf.stx_mnt_id;
+	}
+
+	fh->handle_bytes = bufsz;
+	ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	if (bufsz < fh->handle_bytes) {
+		/* Query the filesystem required bufsz and the file handle */
+		if (ret != -1 || errno != EOVERFLOW) {
+			fprintf(stderr, "%s: unexpected result from name_to_handle_at: %d (%m)\n", fname, ret);
+			return EXIT_FAILURE;
+		}
+		ret = name_to_handle_at(AT_FDCWD, fname, fh, &mntid_short, 0);
+	}
+	if (ret < 0) {
+		fprintf(stderr, "%s: name_to_handle: %m\n", fname);
+		return EXIT_FAILURE;
+	}
+
+	if (!skip_mntid) {
+		if (mntid_short != (int) statx_mntid_short) {
+			fprintf(stderr, "%s: name_to_handle_at returned a different mount ID to STATX_MNT_ID: %u != %lu\n", fname, mntid_short, statx_mntid_short);
+			return EXIT_FAILURE;
+		}
+	}
+
+	if (!skip_mntid_unique) {
+		struct handle dummy_fh;
+		uint64_t mntid_unique = 0;
+
+		/*
+		 * Get the unique mount ID. We don't need to get another copy of the
+		 * handle so store it in a dummy struct.
+		 */
+		dummy_fh.fh.handle_bytes = fh->handle_bytes;
+		ret = name_to_handle_at(AT_FDCWD, fname, &dummy_fh.fh, (int *) &mntid_unique, AT_HANDLE_MNT_ID_UNIQUE);
+		if (ret < 0) {
+			if (errno != EINVAL) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE): %m\n", fname);
+				return EXIT_FAILURE;
+			}
+			/* EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported */
+			skip_mntid_unique = true;
+		} else {
+			if (mntid_unique != statx_mntid_unique) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) returned a different mount ID to STATX_MNT_ID_UNIQUE: %lu != %lu\n", fname, mntid_unique, statx_mntid_unique);
+				return EXIT_FAILURE;
+			}
+		}
+	}
+
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	int	i, c;
@@ -130,7 +227,7 @@ int main(int argc, char **argv)
 	char	fname2[PATH_MAX];
 	char	*test_dir;
 	char	*mount_dir;
-	int	mount_fd, mount_id;
+	int	mount_fd;
 	char	*infile = NULL, *outfile = NULL;
 	int	in_fd = 0, out_fd = 0;
 	int	numfiles = 1;
@@ -305,21 +402,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			handle[i].fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			if (bufsz < handle[i].fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", fname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(fname, ": name_to_handle");
-				perror(fname);
+			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
+			if (ret)
 				return EXIT_FAILURE;
-			}
 		}
 		if (keepopen) {
 			/* Open without close to keep unlinked files around */
@@ -347,21 +432,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			dir_handle.fh.handle_bytes = bufsz;
-			ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			if (bufsz < dir_handle.fh.handle_bytes) {
-				/* Query the filesystem required bufsz and the file handle */
-				if (ret != -1 || errno != EOVERFLOW) {
-					fprintf(stderr, "Unexpected result from name_to_handle_at(%s)\n", dname);
-					return EXIT_FAILURE;
-				}
-				ret = name_to_handle_at(AT_FDCWD, test_dir, &dir_handle.fh, &mount_id, 0);
-			}
-			if (ret < 0) {
-				strcat(dname, ": name_to_handle");
-				perror(dname);
+			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
+			if (ret)
 				return EXIT_FAILURE;
-			}
 		}
 		if (out_fd) {
 			ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
-- 
2.46.0

[PATCH xfstests v4 2/2] generic/756: test name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) explicitly

[Aleksa Sarai]

In order to make sure we are actually testing AT_HANDLE_MNT_ID_UNIQUE,
add a test (based on generic/426) which runs the open_by_handle in a
mode where it will error out if there is a problem with getting mount
IDs. The test is skipped if the kernel doesn't support the necessary
features.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 common/rc             | 24 ++++++++++++++++
 src/open_by_handle.c  | 61 ++++++++++++++++++++++++++++++++++------
 tests/generic/756     | 65 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/756.out |  5 ++++
 4 files changed, 146 insertions(+), 9 deletions(-)
 create mode 100755 tests/generic/756
 create mode 100644 tests/generic/756.out

diff --git a/common/rc b/common/rc
index 9da9fe188297..0beaf2ff1126 100644
--- a/common/rc
+++ b/common/rc
@@ -5178,6 +5178,30 @@ _require_fibmap()
 	rm -f $file
 }
 
+_require_statx_unique_mountid()
+{
+	# statx(STATX_MNT_ID=0x1000) was added in Linux 5.8.
+	# statx(STATX_MNT_ID_UNIQUE=0x4000) was added in Linux 6.9.
+	# We only need to check the latter.
+
+	export STATX_MNT_ID_UNIQUE=0x4000
+	local statx_mask=$(
+		${XFS_IO_PROG} -c "statx -m $STATX_MNT_ID_UNIQUE -r" "$TEST_DIR" |
+		sed -En 's/stat\.mask = (0x[0-9a-f]+)/\1/p'
+	)
+
+	[[ $(( statx_mask & STATX_MNT_ID_UNIQUE )) == $((STATX_MNT_ID_UNIQUE)) ]] ||
+		_notrun "statx does not support STATX_MNT_ID_UNIQUE on this kernel"
+}
+
+_require_open_by_handle_unique_mountid()
+{
+	_require_test_program "open_by_handle"
+
+	$here/src/open_by_handle -C AT_HANDLE_MNT_ID_UNIQUE 2>&1 \
+		|| _notrun "name_to_handle_at does not support AT_HANDLE_MNT_ID_UNIQUE"
+}
+
 _try_wipe_scratch_devs()
 {
 	test -x "$WIPEFS_PROG" || return 0
diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index dcbcd35561fb..a99cce4b3558 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -106,7 +106,8 @@ struct handle {
 
 void usage(void)
 {
-	fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
+	fprintf(stderr, "usage: open_by_handle [-cludmMrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
+	fprintf(stderr, "       open_by_handle -C <feature>\n");
 	fprintf(stderr, "\n");
 	fprintf(stderr, "open_by_handle -c <test_dir> [N] - create N test files under test_dir, try to get file handles and exit\n");
 	fprintf(stderr, "open_by_handle    <test_dir> [N] - get file handles of test files, drop caches and try to open by handle\n");
@@ -119,16 +120,21 @@ void usage(void)
 	fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -m <test_dir> [N] - rename test files, drop caches and try to open by handle\n");
+	fprintf(stderr, "open_by_handle -M <test_dir> [N] - do not silently skip the mount ID verifications\n");
 	fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
 	fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
 	fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
 	fprintf(stderr, "open_by_handle -z <test_dir> [N] - query filesystem required buffer size\n");
+	fprintf(stderr, "\n");
+	fprintf(stderr, "open_by_handle -C <feature>      - check if <feature> is supported by the kernel.\n");
+	fprintf(stderr, "  <feature> can be any of the following values:\n");
+	fprintf(stderr, "  - AT_HANDLE_MNT_ID_UNIQUE\n");
 	exit(EXIT_FAILURE);
 }
 
 static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
-				int bufsz)
+				int bufsz, bool force_check_mountid)
 {
 	int ret;
 	int mntid_short;
@@ -144,10 +150,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 			fprintf(stderr, "%s: statx(STATX_MNT_ID): %m\n", fname);
 			return EXIT_FAILURE;
 		}
-		if (!(statxbuf.stx_mask & STATX_MNT_ID))
+		if (!(statxbuf.stx_mask & STATX_MNT_ID)) {
+			if (force_check_mountid) {
+				fprintf(stderr, "%s: statx(STATX_MNT_ID) not supported by running kernel\n", fname);
+				return EXIT_FAILURE;
+			}
 			skip_mntid = true;
-		else
+		} else {
 			statx_mntid_short = statxbuf.stx_mnt_id;
+		}
 	}
 
 	if (!skip_mntid_unique) {
@@ -159,10 +170,15 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 		 * STATX_MNT_ID_UNIQUE was added fairly recently in Linux 6.8, so if the
 		 * kernel doesn't give us a unique mount ID just skip it.
 		 */
-		if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE))
+		if (!(statxbuf.stx_mask & STATX_MNT_ID_UNIQUE)) {
+			if (force_check_mountid) {
+				fprintf(stderr, "%s: statx(STATX_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
+				return EXIT_FAILURE;
+			}
 			skip_mntid_unique = true;
-		else
+		} else {
 			statx_mntid_unique = statxbuf.stx_mnt_id;
+		}
 	}
 
 	fh->handle_bytes = bufsz;
@@ -203,6 +219,10 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 				return EXIT_FAILURE;
 			}
 			/* EINVAL means AT_HANDLE_MNT_ID_UNIQUE is not supported */
+			if (force_check_mountid) {
+				fprintf(stderr, "%s: name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n", fname);
+				return EXIT_FAILURE;
+			}
 			skip_mntid_unique = true;
 		} else {
 			if (mntid_unique != statx_mntid_unique) {
@@ -215,6 +235,22 @@ static int do_name_to_handle_at(const char *fname, struct file_handle *fh,
 	return 0;
 }
 
+static int check_feature(const char *feature)
+{
+	if (!strcmp(feature, "AT_HANDLE_MNT_ID_UNIQUE")) {
+		int ret = name_to_handle_at(AT_FDCWD, ".", NULL, NULL, AT_HANDLE_MNT_ID_UNIQUE);
+		/* If AT_HANDLE_MNT_ID_UNIQUE is supported, we get EFAULT. */
+		if (ret < 0 && errno == EINVAL) {
+			fprintf(stderr, "name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) not supported by running kernel\n");
+			return EXIT_FAILURE;
+		}
+		return 0;
+	}
+
+	fprintf(stderr, "unknown feature name '%s'\n", feature);
+	return EXIT_FAILURE;
+}
+
 int main(int argc, char **argv)
 {
 	int	i, c;
@@ -234,16 +270,20 @@ int main(int argc, char **argv)
 	int	create = 0, delete = 0, nlink = 1, move = 0;
 	int	rd = 0, wr = 0, wrafter = 0, parent = 0;
 	int	keepopen = 0, drop_caches = 1, sleep_loop = 0;
+	int	force_check_mountid = 0;
 	int	bufsz = MAX_HANDLE_SZ;
 
 	if (argc < 2)
 		usage();
 
-	while ((c = getopt(argc, argv, "cludmrwapknhi:o:sz")) != -1) {
+	while ((c = getopt(argc, argv, "cC:ludmMrwapknhi:o:sz")) != -1) {
 		switch (c) {
 		case 'c':
 			create = 1;
 			break;
+		case 'C':
+			/* Check kernel feature support. */
+			return check_feature(optarg);
 		case 'w':
 			/* Write data before open_by_handle_at() */
 			wr = 1;
@@ -270,6 +310,9 @@ int main(int argc, char **argv)
 		case 'm':
 			move = 1;
 			break;
+		case 'M':
+			force_check_mountid = 1;
+			break;
 		case 'p':
 			parent = 1;
 			break;
@@ -402,7 +445,7 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz);
+			ret = do_name_to_handle_at(fname, &handle[i].fh, bufsz, force_check_mountid);
 			if (ret)
 				return EXIT_FAILURE;
 		}
@@ -432,7 +475,7 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		} else {
-			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz);
+			ret = do_name_to_handle_at(test_dir, &dir_handle.fh, bufsz, force_check_mountid);
 			if (ret)
 				return EXIT_FAILURE;
 		}
diff --git a/tests/generic/756 b/tests/generic/756
new file mode 100755
index 000000000000..c7a82cfd25f4
--- /dev/null
+++ b/tests/generic/756
@@ -0,0 +1,65 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
+# Copyright (C) 2024 Aleksa Sarai <cyphar@cyphar.com>
+#
+# FS QA Test No. 756
+#
+# Check stale handles pointing to unlinked files and non-stale handles pointing
+# to linked files while verifying that u64 mount IDs are correctly returned.
+#
+. ./common/preamble
+_begin_fstest auto quick exportfs
+
+# Import common functions.
+. ./common/filter
+
+
+# Modify as appropriate.
+_require_test
+# _require_exportfs and  already requires open_by_handle, but let's not count on it
+_require_test_program "open_by_handle"
+_require_exportfs
+# We need both STATX_MNT_ID_UNIQUE and AT_HANDLE_MNT_ID_UNIQUE.
+_require_statx_unique_mountid
+_require_open_by_handle_unique_mountid
+
+NUMFILES=1024
+testdir=$TEST_DIR/$seq-dir
+mkdir -p $testdir
+
+# Create empty test files in test dir
+create_test_files()
+{
+	local dir=$1
+
+	mkdir -p $dir
+	rm -f $dir/*
+	$here/src/open_by_handle -c $dir $NUMFILES
+}
+
+# Test encode/decode file handles
+test_file_handles()
+{
+	local dir=$1
+	local opt=$2
+
+	echo test_file_handles $* | _filter_test_dir
+	$here/src/open_by_handle $opt $dir $NUMFILES
+}
+
+# Check stale handles to deleted files
+create_test_files $testdir
+test_file_handles $testdir -Md
+
+# Check non-stale handles to linked files
+create_test_files $testdir
+test_file_handles $testdir -M
+
+# Check non-stale handles to files that were hardlinked and original deleted
+create_test_files $testdir
+test_file_handles $testdir -Ml
+test_file_handles $testdir -Mu
+
+status=0
+exit
diff --git a/tests/generic/756.out b/tests/generic/756.out
new file mode 100644
index 000000000000..48aed88d87b9
--- /dev/null
+++ b/tests/generic/756.out
@@ -0,0 +1,5 @@
+QA output created by 756
+test_file_handles TEST_DIR/756-dir -Md
+test_file_handles TEST_DIR/756-dir -M
+test_file_handles TEST_DIR/756-dir -Ml
+test_file_handles TEST_DIR/756-dir -Mu
-- 
2.46.0