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

Aleksa Sarai posted 2 patches 1 year, 3 months ago
[PATCH xfstests v3 1/2] open_by_handle: verify u32 and u64 mount IDs
Posted by Aleksa Sarai 1 year, 3 months ago
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
Re: [PATCH xfstests v3 1/2] open_by_handle: verify u32 and u64 mount IDs
Posted by Amir Goldstein 1 year, 3 months ago
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 v3 2/2] generic/756: test name_to_handle_at(AT_HANDLE_MNT_ID_UNIQUE) explicitly
Posted by Aleksa Sarai 1 year, 3 months ago
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
Posted by Amir Goldstein 1 year, 3 months ago
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
Posted by Aleksa Sarai 1 year, 3 months ago
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/>