[PATCH 0/7] Overlayfs exportfs tests

[PATCH 0/7] Overlayfs exportfs tests

[Amir Goldstein]

Eryu,

Following patches were used to test overlayfs NFS export.

Patches 1-5 improve generic/exportfs tests, by adding test coverage
of some directory file handle use cases.

Patches 6-7 add two overlay specific tests for NFS file handles
for overlay setup of same fs and non same fs.

The new overlay tests don't run on upstream kernel and pass with
my overlayfs NFS export V2 branch [1]. You may want to wait with
merging those tests until overlayfs NFS support is merged - up to you,
but a review would be most welcome.

Merging patches 1-5 seems like a good idea to me regardless of
overlayfs NFS export support.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/ovl-nfs-export-v2

Amir Goldstein (7):
  open_by_handle: store and load file handles from file
  open_by_handle: verify dir content only with -r flag
  generic/exportfs: golden output is not silent
  generic/exportfs: store and load file handles from file
  generic/exportfs: add a test case for renamed parent dir
  overlay: test encode/decode overlay file handles
  overlay: test encode/decode of non-samefs overlay file handles

 src/open_by_handle.c  | 112 +++++++++++++-----
 tests/generic/426     |   1 -
 tests/generic/426.out |   1 -
 tests/generic/467     |  37 ++++--
 tests/generic/467.out |  10 +-
 tests/overlay/050     | 291 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/050.out |  50 ++++++++
 tests/overlay/051     | 311 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/051.out |  50 ++++++++
 tests/overlay/group   |   2 +
 10 files changed, 822 insertions(+), 43 deletions(-)
 create mode 100755 tests/overlay/050
 create mode 100644 tests/overlay/050.out
 create mode 100755 tests/overlay/051
 create mode 100644 tests/overlay/051.out

-- 
2.7.4

[PATCH 1/7] open_by_handle: store and load file handles from file

[Amir Goldstein]

usage:
 open_by_handle -p -o <handles_file> <test_dir> [N]
 open_by_handle -p -i <handles_file> <test_dir> [N]

This will be used to test decoding of file handles after various
file systems operations including mount cycle.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 src/open_by_handle.c | 110 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 85 insertions(+), 25 deletions(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index dbc5b0f..f9dfefc 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -43,30 +43,36 @@ Examples:
 
    open_by_handle -p <test_dir> [N]
 
-3. Get file handles for existing test set, write data to files,
+3. Get file handles for existing test set and write them to a file.
+   Read file handles from file and open files by handle:
+
+   open_by_handle -p -o <handles_file> <test_dir> [N]
+   open_by_handle -p -i <handles_file> <test_dir> [N]
+
+4. Get file handles for existing test set, write data to files,
    drop caches, open all files by handle, read and verify written
    data, write new data to file:
 
    open_by_handle -rwa <test_dir> [N]
 
-4. Get file handles for existing test set, unlink all test files,
+5. Get file handles for existing test set, unlink all test files,
    remove test_dir, drop caches, try to open all files by handle
    and expect ESTALE:
 
    open_by_handle -dp <test_dir> [N]
 
-5. Get file handles for existing test set, keep open file handles for all
+6. Get file handles for existing test set, keep open file handles for all
    test files, unlink all test files, drop caches and try to open all files
    by handle (should work):
 
    open_by_handle -dk <test_dir> [N]
 
-6. Get file handles for existing test set, rename all test files,
+7. Get file handles for existing test set, rename all test files,
    drop caches, try to open all files by handle (should work):
 
    open_by_handle -m <test_dir> [N]
 
-7. Get file handles for existing test set, hardlink all test files,
+8. Get file handles for existing test set, hardlink all test files,
    then unlink the original files, drop caches and try to open all
    files by handle (should work):
 
@@ -103,7 +109,7 @@ struct handle {
 
 void usage(void)
 {
-	fprintf(stderr, "usage: open_by_handle [-cludmrwapk] <test_dir> [num_files]\n");
+	fprintf(stderr, "usage: open_by_handle [-cludmrwapk] [<-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");
@@ -116,6 +122,8 @@ void usage(void)
 	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");
+	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");
 	exit(EXIT_FAILURE);
 }
 
@@ -131,15 +139,16 @@ int main(int argc, char **argv)
 	char	*test_dir;
 	char	*mount_dir;
 	int	mount_fd, mount_id;
+	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;
 
-	if (argc < 2 || argc > 4)
+	if (argc < 2)
 		usage();
 
-	while ((c = getopt(argc, argv, "cludmrwapk")) != -1) {
+	while ((c = getopt(argc, argv, "cludmrwapki:o:")) != -1) {
 		switch (c) {
 		case 'c':
 			create = 1;
@@ -176,13 +185,27 @@ int main(int argc, char **argv)
 		case 'k':
 			keepopen = 1;
 			break;
+		case 'i':
+			in_fd = open(optarg, O_RDONLY);
+			if (in_fd < 0) {
+				perror(optarg);
+				return EXIT_FAILURE;
+			}
+			break;
+		case 'o':
+			out_fd = creat(optarg, 0644);
+			if (out_fd < 0) {
+				perror(optarg);
+				return EXIT_FAILURE;
+			}
+			break;
 		default:
 			fprintf(stderr, "illegal option '%s'\n", argv[optind]);
 		case 'h':
 			usage();
 		}
 	}
-        if (optind == argc || optind > 2)
+        if (optind == argc)
             usage();
 	test_dir = argv[optind++];
 	if (optind < argc)
@@ -192,11 +215,14 @@ int main(int argc, char **argv)
 		usage();
 	}
 
-	if (parent) {
+	if (parent && !in_fd) {
 		strcpy(dname, test_dir);
 		/*
 		 * -p flag implies that test_dir is NOT a mount point,
 		 * so its parent can be used as mount_fd for open_by_handle_at.
+		 * -i flag implies that test_dir IS a mount point, because we
+		 *  are testing open by handle of dir, which may have been
+		 *  deleted or renamed.
 		 */
 		mount_dir = dirname(dname);
 		if (create)
@@ -241,15 +267,24 @@ int main(int argc, char **argv)
 	/* sync to get the new inodes to hit the disk */
 	sync();
 
-	/* create the handles */
+	/* create/read the handles */
 	for (i=0; i < numfiles; i++) {
 		sprintf(fname, "%s/file%06d", test_dir, i);
-		handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
-		ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
-		if (ret < 0) {
-			strcat(fname, ": name_to_handle");
-			perror(fname);
-			return EXIT_FAILURE;
+		if (in_fd) {
+			ret = read(in_fd, (char *)&handle[i], sizeof(*handle));
+			if (ret < sizeof(*handle)) {
+				strcat(fname, ": read handle");
+				perror(fname);
+				return EXIT_FAILURE;
+			}
+		} else {
+			handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
+			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
+			if (ret < 0) {
+				strcat(fname, ": name_to_handle");
+				perror(fname);
+				return EXIT_FAILURE;
+			}
 		}
 		if (keepopen) {
 			/* Open without close to keep unlinked files around */
@@ -260,15 +295,40 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 		}
+		if (out_fd) {
+			ret = write(out_fd, (char *)&handle[i], sizeof(*handle));
+			if (ret < sizeof(*handle)) {
+				strcat(fname, ": write handle");
+				perror(fname);
+				return EXIT_FAILURE;
+			}
+		}
 	}
 
 	if (parent) {
-		dir_handle.fh.handle_bytes = MAX_HANDLE_SZ;
-		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);
-			return EXIT_FAILURE;
+		if (in_fd) {
+			ret = read(in_fd, (char *)&dir_handle, sizeof(*handle));
+			if (ret < sizeof(*handle)) {
+				strcat(dname, ": read handle");
+				perror(dname);
+				return EXIT_FAILURE;
+			}
+		} else {
+			dir_handle.fh.handle_bytes = MAX_HANDLE_SZ;
+			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);
+				return EXIT_FAILURE;
+			}
+		}
+		if (out_fd) {
+			ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
+			if (ret < sizeof(*handle)) {
+				strcat(dname, ": write handle");
+				perror(dname);
+				return EXIT_FAILURE;
+			}
 		}
 	}
 
@@ -289,8 +349,8 @@ int main(int argc, char **argv)
 		close(fd);
 	}
 
-	/* after creating test set only check that fs supports exportfs */
-	if (create)
+	/* If creating test set or saving files handles, we are done */
+	if (create || out_fd)
 		return EXIT_SUCCESS;
 
 	/* hardlink the files */
-- 
2.7.4

[PATCH 2/7] open_by_handle: verify dir content only with -r flag

[Amir Goldstein]

Without -r flag file handles are opened, but file content is not read.
Treat dir file hanldes, similarly. without -r flag, open dir file
handle, but don't verify its content.

This is going to be used by tests for which dir content is changed
between encode and decode of dir file handle.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 src/open_by_handle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index f9dfefc..5b7a592 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -472,7 +472,7 @@ int main(int argc, char **argv)
 			if (!nlink) {
 				printf("open_by_handle(%s) opened an unlinked dir!\n", dname);
 				return EXIT_FAILURE;
-			} else {
+			} else if (rd) {
 				/*
 				 * Sanity check dir fd - expect to access orig file IFF
 				 * it was not unlinked nor renamed.
-- 
2.7.4

[PATCH 3/7] generic/exportfs: golden output is not silent

[Amir Goldstein]

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/generic/426     | 1 -
 tests/generic/426.out | 1 -
 tests/generic/467     | 1 -
 tests/generic/467.out | 1 -
 4 files changed, 4 deletions(-)

diff --git a/tests/generic/426 b/tests/generic/426
index 68ff169..17639bb 100755
--- a/tests/generic/426
+++ b/tests/generic/426
@@ -90,6 +90,5 @@ create_test_files $testdir
 test_file_handles $testdir -l
 test_file_handles $testdir -u
 
-echo "Silence is golden"
 status=0
 exit
diff --git a/tests/generic/426.out b/tests/generic/426.out
index 9a25123..b84e005 100644
--- a/tests/generic/426.out
+++ b/tests/generic/426.out
@@ -3,4 +3,3 @@ test_file_handles TEST_DIR/426-dir -d
 test_file_handles TEST_DIR/426-dir
 test_file_handles TEST_DIR/426-dir -l
 test_file_handles TEST_DIR/426-dir -u
-Silence is golden
diff --git a/tests/generic/467 b/tests/generic/467
index 6ff9270..56dfcbd 100755
--- a/tests/generic/467
+++ b/tests/generic/467
@@ -114,6 +114,5 @@ mkdir -p $testdir.new
 mv $testdir/* $testdir.new/
 test_file_handles $testdir.new -rp
 
-echo "Silence is golden"
 status=0
 exit
diff --git a/tests/generic/467.out b/tests/generic/467.out
index c649aa0..39d6a95 100644
--- a/tests/generic/467.out
+++ b/tests/generic/467.out
@@ -7,4 +7,3 @@ test_file_handles TEST_DIR/467-dir -ur
 test_file_handles TEST_DIR/467-dir -mr
 test_file_handles TEST_DIR/467-dir.renamed -rp
 test_file_handles TEST_DIR/467-dir.new -rp
-Silence is golden
-- 
2.7.4

[PATCH 4/7] generic/exportfs: store and load file handles from file

[Amir Goldstein]

For the test cases of renaming test directory and moving files
to new test directory, encoding the file handles is done after
files/dir have been renamed, so the test does not exercise decoding
file hanldes that were exported before the rename.

For those test cases, store the test set file handles to a temp
file, then move the files/dir, then try to open test set by stored
file handles.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/generic/467     | 29 ++++++++++++++++++-----------
 tests/generic/467.out |  6 ++++--
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/tests/generic/467 b/tests/generic/467
index 56dfcbd..592ffbe 100755
--- a/tests/generic/467
+++ b/tests/generic/467
@@ -74,10 +74,10 @@ create_test_files()
 test_file_handles()
 {
 	local dir=$1
-	local opt=$2
+	shift
 
-	echo test_file_handles $* | _filter_test_dir
-	$here/src/open_by_handle $opt $dir $NUMFILES
+	echo test_file_handles $dir $* | _filter_test_dir | sed -e "s,$tmp\.,,g"
+	$here/src/open_by_handle $* $dir $NUMFILES
 }
 
 # Check stale handles to deleted files/dir
@@ -101,18 +101,25 @@ test_file_handles $testdir -ur
 create_test_files $testdir
 test_file_handles $testdir -mr
 
-# Check non-stale file handles after rename of parent
-create_test_files $testdir
-rm -rf $testdir.renamed
-mv $testdir $testdir.renamed/
-test_file_handles $testdir.renamed -rp
-
-# Check non-stale file handles after move to new parent
+# Check non-stale file handles of moved files
 create_test_files $testdir
 rm -rf $testdir.new
 mkdir -p $testdir.new
+# Store file handles before move to new testdir
+test_file_handles $testdir -o $tmp.file_handles
 mv $testdir/* $testdir.new/
-test_file_handles $testdir.new -rp
+# Check open and read from stored file handles
+# (testdir argument is the mount point and NOT the old nor new testdir)
+test_file_handles $TEST_DIR -r -i $tmp.file_handles
+
+# Check non-stale file handles of renamed dirs
+create_test_files $testdir
+rm -rf $testdir.renamed
+# Store file handles before rename of testdir
+test_file_handles $testdir -p -o $tmp.file_handles
+mv $testdir $testdir.renamed/
+# Check open, read and readdir from stored file handles
+test_file_handles $TEST_DIR -rp -i $tmp.file_handles
 
 status=0
 exit
diff --git a/tests/generic/467.out b/tests/generic/467.out
index 39d6a95..c29289e 100644
--- a/tests/generic/467.out
+++ b/tests/generic/467.out
@@ -5,5 +5,7 @@ test_file_handles TEST_DIR/467-dir -dkr
 test_file_handles TEST_DIR/467-dir -lr
 test_file_handles TEST_DIR/467-dir -ur
 test_file_handles TEST_DIR/467-dir -mr
-test_file_handles TEST_DIR/467-dir.renamed -rp
-test_file_handles TEST_DIR/467-dir.new -rp
+test_file_handles TEST_DIR/467-dir -o file_handles
+test_file_handles TEST_DIR -r -i file_handles
+test_file_handles TEST_DIR/467-dir -p -o file_handles
+test_file_handles TEST_DIR -rp -i file_handles
-- 
2.7.4

[PATCH 5/7] generic/exportfs: add a test case for renamed parent dir

[Amir Goldstein]

Check that a dir file handle can be opened after its parent
has been renamed.

Retry decoding subdir file handle when parent is in dcache
and when parent is not in dcache.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/generic/467     | 7 +++++++
 tests/generic/467.out | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/tests/generic/467 b/tests/generic/467
index 592ffbe..555b44b 100755
--- a/tests/generic/467
+++ b/tests/generic/467
@@ -67,6 +67,7 @@ create_test_files()
 	local opt=$2
 
 	rm -rf $dir
+	mkdir -p $dir
 	$here/src/open_by_handle -cwp $dir $NUMFILES
 }
 
@@ -114,12 +115,18 @@ test_file_handles $TEST_DIR -r -i $tmp.file_handles
 
 # Check non-stale file handles of renamed dirs
 create_test_files $testdir
+create_test_files $testdir/subdir
 rm -rf $testdir.renamed
 # Store file handles before rename of testdir
 test_file_handles $testdir -p -o $tmp.file_handles
+test_file_handles $testdir/subdir -p -o $tmp.subdir_file_handles
 mv $testdir $testdir.renamed/
 # Check open, read and readdir from stored file handles
 test_file_handles $TEST_DIR -rp -i $tmp.file_handles
+test_file_handles $TEST_DIR -rp -i $tmp.subdir_file_handles
+# Retry decoding subdir file handle when testdir is in dcache
+# (providing renamed testdir argument pins testdir to dcache)
+test_file_handles $testdir.renamed -rp -i $tmp.subdir_file_handles
 
 status=0
 exit
diff --git a/tests/generic/467.out b/tests/generic/467.out
index c29289e..dfa3240 100644
--- a/tests/generic/467.out
+++ b/tests/generic/467.out
@@ -8,4 +8,7 @@ test_file_handles TEST_DIR/467-dir -mr
 test_file_handles TEST_DIR/467-dir -o file_handles
 test_file_handles TEST_DIR -r -i file_handles
 test_file_handles TEST_DIR/467-dir -p -o file_handles
+test_file_handles TEST_DIR/467-dir/subdir -p -o subdir_file_handles
 test_file_handles TEST_DIR -rp -i file_handles
+test_file_handles TEST_DIR -rp -i subdir_file_handles
+test_file_handles TEST_DIR/467-dir.renamed -rp -i subdir_file_handles
-- 
2.7.4

[PATCH 6/7] overlay: test encode/decode overlay file handles

[Amir Goldstein]

- Check encode/write/decode/read content of lower/upper file handles
- Check encode/decode/write/read content of lower/upper file handles
- Check decode/read of unlinked lower/upper files and directories
- Check decode/read of lower file handles after copy up, link and unlink
- Check decode/read of lower file handles after rename of parent and self

This test does not cover connectable file handles of non-directories,
because name_to_handle_at() syscall does not support requesting
connectable file handles.

This test covers only encode/decode of file handles for overlayfs
configuration of lower and upper on the same fs.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/050     | 291 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/050.out |  50 +++++++++
 tests/overlay/group   |   1 +
 3 files changed, 342 insertions(+)
 create mode 100755 tests/overlay/050
 create mode 100644 tests/overlay/050.out

diff --git a/tests/overlay/050 b/tests/overlay/050
new file mode 100755
index 0000000..4dcd66a
--- /dev/null
+++ b/tests/overlay/050
@@ -0,0 +1,291 @@
+#! /bin/bash
+# FS QA Test No. 050
+#
+# Test encode/decode overlay file handles
+#
+# - Check encode/write/decode/read content of lower/upper file handles
+# - Check encode/decode/write/read content of lower/upper file handles
+# - Check decode/read of unlinked lower/upper files and directories
+# - Check decode/read of lower file handles after copy up, link and unlink
+# - Check decode/read of lower file handles after rename of parent and self
+#
+# This test does not cover connectable file handles of non-directories,
+# because name_to_handle_at() syscall does not support requesting connectable
+# file handles.
+#
+#-----------------------------------------------------------------------
+# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
+# Author: Amir Goldstein <amir73il@gmail.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+# _require_exportfs already requires open_by_handle, but let's not count on it
+_require_test_program "open_by_handle"
+_require_exportfs
+
+lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER/lowertestdir
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER/uppertestdir
+lowertestdir=$SCRATCH_MNT/lowertestdir
+uppertestdir=$SCRATCH_MNT/uppertestdir
+NUMFILES=1
+
+# Create test dir and empty test files
+create_test_files()
+{
+	local dir=$1
+	local opt=$2
+
+	mkdir -p $dir
+	src/open_by_handle -cp $opt $dir $NUMFILES
+}
+
+# Test encode/decode file handles on overlay mount
+test_file_handles()
+{
+	local dir=$1
+	shift
+
+	echo test_file_handles $dir $* | _filter_scratch | _filter_ovl_dirs | \
+				sed -e "s,$tmp\.,,g"
+	$here/src/open_by_handle $* $dir $NUMFILES
+}
+
+# Re-create lower/upper/work dirs
+create_dirs()
+{
+	_scratch_mkfs
+}
+
+# Mount an overlay on $SCRATCH_MNT with all layers on scratch partition
+mount_dirs()
+{
+	_scratch_mount
+}
+
+# Unmount & check the overlay layers
+unmount_dirs()
+{
+	_scratch_unmount
+	_check_scratch_fs
+}
+
+# Check non-stale file handles of lower/upper files and verify
+# that handle decoded before copy up is encoded to upper after
+# copy up. Verify reading data from file open by file handle
+# and verify access_at() with dirfd open by file handle.
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check encode/decode of upper regular file handles
+test_file_handles $uppertestdir
+# Check encode/decode of upper dir file handle
+test_file_handles $uppertestdir -p
+# Check encode/write/decode/read/write of upper file handles
+test_file_handles $uppertestdir -wrap
+# Check encode/decode of lower regular file handles before copy up
+test_file_handles $lowertestdir
+# Check encode/decode of lower dir file handles before copy up
+test_file_handles $lowertestdir -p
+# Check encode/write/decode/read/write of lower file handles across copy up
+test_file_handles $lowertestdir -wrap
+unmount_dirs
+
+# Check copy up after encode/decode of lower/upper files
+# (copy up of disconnected dentry to index dir)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check encode/decode/write/read of upper regular file handles
+test_file_handles $uppertestdir -a
+test_file_handles $uppertestdir -r
+# Check encode/decode/write/read of lower regular file handles
+test_file_handles $lowertestdir -a
+test_file_handles $lowertestdir -r
+unmount_dirs
+
+# Check non-stale handles to unlinked but open lower/upper files
+create_dirs
+create_test_files $upperdir
+create_test_files $upperdir.rw
+create_test_files $lowerdir
+create_test_files $lowerdir.rw
+mount_dirs
+test_file_handles $uppertestdir -dk
+# Check encode/write/unlink/decode/read of upper regular file handles
+test_file_handles $uppertestdir.rw -rwdk
+test_file_handles $lowertestdir -dk
+# Check encode/write/unlink/decode/read of lower file handles across copy up
+test_file_handles $lowertestdir.rw -rwdk
+unmount_dirs
+
+# Check stale handles of unlinked lower/upper files (nlink = 1,0)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check decode of upper file handles after unlink/rmdir (nlink == 0)
+test_file_handles $uppertestdir -dp
+# Check decode of lower file handles after unlink/rmdir (nlink == 0)
+test_file_handles $lowertestdir -dp
+unmount_dirs
+
+# Check non-stale file handles of linked lower/upper files (nlink = 1,2,1)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check encode/decode of upper file handles (nlink == 1)
+test_file_handles $uppertestdir
+# Check decode/read of upper file handles after link (nlink == 2)
+test_file_handles $uppertestdir -wlr
+# Check decode/read of upper file handles after link + unlink (nlink == 1)
+test_file_handles $uppertestdir -ur
+# Check encode/decode of lower file handles before copy up (nlink == 1)
+test_file_handles $lowertestdir
+# Check decode/read of lower file handles after copy up + link (nlink == 2)
+test_file_handles $lowertestdir -wlr
+# Check decode/read of lower file handles after copy up + link + unlink (nlink == 1)
+test_file_handles $lowertestdir -ur
+unmount_dirs
+
+# Check non-stale file handles of linked lower/upper hardlinks (nlink = 2,1)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+# Create lower/upper hardlinks
+test_file_handles $lowerdir -l >/dev/null
+test_file_handles $upperdir -l >/dev/null
+mount_dirs
+# Check encode/decode of upper hardlink file handles (nlink == 2)
+test_file_handles $uppertestdir
+# Check decode/read of upper hardlink file handles after unlink (nlink == 1)
+test_file_handles $uppertestdir -wur
+# Check encode/decode of lower hardlink file handles before copy up (nlink == 2)
+test_file_handles $lowertestdir
+# Check decode/read of lower hardlink file handles after copy up + unlink (nlink == 1)
+test_file_handles $lowertestdir -wur
+unmount_dirs
+
+# Check stale file handles of unlinked lower/upper hardlinks (nlink = 2,0)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+# Create lower/upper hardlinks
+test_file_handles $lowerdir -l >/dev/null
+test_file_handles $upperdir -l >/dev/null
+mount_dirs
+# Check encode/decode of upper hardlink file handles (nlink == 2)
+test_file_handles $uppertestdir
+# Check decode of upper hardlink file handles after 2*unlink (nlink == 0)
+test_file_handles $uppertestdir -d
+# Check encode/decode of lower hardlink file handles before copy up (nlink == 2)
+test_file_handles $lowertestdir
+# Check decode of lower hardlink file handles after copy up + 2*unlink (nlink == 0)
+test_file_handles $lowertestdir -d
+unmount_dirs
+
+# Check non-stale file handles of lower/upper renamed files
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check decode/read of upper file handles after rename in same upper parent
+test_file_handles $uppertestdir -wmr
+# Check decode/read of lower file handles after copy up + rename in same merge parent
+test_file_handles $lowertestdir -wmr
+unmount_dirs
+
+# Check non-stale file handles of lower/upper moved files
+create_dirs
+create_test_files $upperdir -w
+create_test_files $lowerdir -w
+mkdir -p $lowerdir.lo $lowerdir.up $upperdir.lo $upperdir.up
+mount_dirs
+# Check encode/decode/read of lower/upper file handles after move to new upper testdir
+test_file_handles $uppertestdir -o $tmp.upper_file_handles
+test_file_handles $lowertestdir -o $tmp.lower_file_handles
+mv $uppertestdir/* $uppertestdir.up/
+mv $lowertestdir/* $uppertestdir.lo/
+# Check open and read from stored file handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.lower_file_handles
+# Check encode/decode/read of lower/upper file handles after move to new merge testdir
+test_file_handles $uppertestdir.up -o $tmp.upper_file_handles
+test_file_handles $uppertestdir.lo -o $tmp.lower_file_handles
+mv $uppertestdir.up/* $lowertestdir.up/
+mv $uppertestdir.lo/* $lowertestdir.lo/
+# Check open and read from stored file handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.lower_file_handles
+unmount_dirs
+
+# Check non-stale file handles of lower/upper renamed dirs
+create_dirs
+create_test_files $upperdir -w
+create_test_files $lowerdir -w
+create_test_files $upperdir/subdir -w
+create_test_files $lowerdir/subdir -w
+mount_dirs
+# Check encode/decode/read of lower/upper file handles after rename of testdir
+test_file_handles $uppertestdir -p -o $tmp.upper_file_handles
+test_file_handles $lowertestdir -p -o $tmp.lower_file_handles
+# Check encode/decode/read of lower/upper file handles after rename of testdir's parent
+test_file_handles $uppertestdir/subdir -p -o $tmp.upper_subdir_file_handles
+test_file_handles $lowertestdir/subdir -p -o $tmp.lower_subdir_file_handles
+# Rename pure upper dir
+mv $uppertestdir $uppertestdir.new/
+# Copy up lower dir, index and rename
+mv $lowertestdir $lowertestdir.new/
+# Check open, read and readdir from stored file handles
+# (testdir argument is the mount point and NOT the dir
+#  we are trying to open by stored file handle)
+test_file_handles $SCRATCH_MNT -rp -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -rp -i $tmp.lower_file_handles
+test_file_handles $SCRATCH_MNT -rp -i $tmp.upper_subdir_file_handles
+test_file_handles $SCRATCH_MNT -rp -i $tmp.lower_subdir_file_handles
+# Retry decoding lower subdir file handle when indexed testdir is in dcache
+# (providing renamed testdir argument pins the indexed testdir to dcache)
+test_file_handles $lowertestdir.new -rp -i $tmp.lower_subdir_file_handles
+unmount_dirs
+
+status=0
+exit
diff --git a/tests/overlay/050.out b/tests/overlay/050.out
new file mode 100644
index 0000000..d8bac04
--- /dev/null
+++ b/tests/overlay/050.out
@@ -0,0 +1,50 @@
+QA output created by 050
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -p
+test_file_handles SCRATCH_MNT/uppertestdir -wrap
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -p
+test_file_handles SCRATCH_MNT/lowertestdir -wrap
+test_file_handles SCRATCH_MNT/uppertestdir -a
+test_file_handles SCRATCH_MNT/uppertestdir -r
+test_file_handles SCRATCH_MNT/lowertestdir -a
+test_file_handles SCRATCH_MNT/lowertestdir -r
+test_file_handles SCRATCH_MNT/uppertestdir -dk
+test_file_handles SCRATCH_MNT/uppertestdir.rw -rwdk
+test_file_handles SCRATCH_MNT/lowertestdir -dk
+test_file_handles SCRATCH_MNT/lowertestdir.rw -rwdk
+test_file_handles SCRATCH_MNT/uppertestdir -dp
+test_file_handles SCRATCH_MNT/lowertestdir -dp
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -wlr
+test_file_handles SCRATCH_MNT/uppertestdir -ur
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -wlr
+test_file_handles SCRATCH_MNT/lowertestdir -ur
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -wur
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -wur
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -d
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -d
+test_file_handles SCRATCH_MNT/uppertestdir -wmr
+test_file_handles SCRATCH_MNT/lowertestdir -wmr
+test_file_handles SCRATCH_MNT/uppertestdir -o upper_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir -o lower_file_handles
+test_file_handles SCRATCH_MNT -r -i upper_file_handles
+test_file_handles SCRATCH_MNT -r -i lower_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir.up -o upper_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir.lo -o lower_file_handles
+test_file_handles SCRATCH_MNT -r -i upper_file_handles
+test_file_handles SCRATCH_MNT -r -i lower_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir -p -o upper_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir -p -o lower_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir/subdir -p -o upper_subdir_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
+test_file_handles SCRATCH_MNT -rp -i upper_file_handles
+test_file_handles SCRATCH_MNT -rp -i lower_file_handles
+test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
+test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir.new -rp -i lower_subdir_file_handles
diff --git a/tests/overlay/group b/tests/overlay/group
index 7e541e4..6cb3763 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -49,3 +49,4 @@
 044 auto quick copyup hardlink nonsamefs
 047 auto quick copyup hardlink
 048 auto quick copyup hardlink
+050 auto quick copyup hardlink exportfs
-- 
2.7.4

[PATCH 7/7] overlay: test encode/decode of non-samefs overlay file handles

[Amir Goldstein]

This is a variant of overlay file handles test for an overlayfs that
is composed of multiple lower layers not on the same underlying fs.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/051     | 311 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/051.out |  50 ++++++++
 tests/overlay/group   |   1 +
 3 files changed, 362 insertions(+)
 create mode 100755 tests/overlay/051
 create mode 100644 tests/overlay/051.out

diff --git a/tests/overlay/051 b/tests/overlay/051
new file mode 100755
index 0000000..6f11e9e
--- /dev/null
+++ b/tests/overlay/051
@@ -0,0 +1,311 @@
+#! /bin/bash
+# FS QA Test No. 051
+#
+# Test encode/decode overlay file handles
+#
+# This is a variant of overlay file handles test for an overlayfs that is
+# composed of multiple lower layers not on the same underlying fs.
+#
+# - Check encode/write/decode/read content of lower/upper file handles
+# - Check encode/decode/write/read content of lower/upper file handles
+# - Check decode/read of unlinked lower/upper files and directories
+# - Check decode/read of lower file handles after copy up, link and unlink
+# - Check decode/read of lower file handles after rename of parent and self
+#
+# This test does not cover connectable file handles of non-directories,
+# because name_to_handle_at() syscall does not support requesting connectable
+# file handles.
+#
+#-----------------------------------------------------------------------
+# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
+# Author: Amir Goldstein <amir73il@gmail.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	# Unmount the non-samefs overlay mount
+	$UMOUNT_PROG $SCRATCH_MNT 2>/dev/null
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+
+_supported_fs overlay
+_supported_os Linux
+_require_test
+_require_scratch
+# _require_exportfs already requires open_by_handle, but let's not count on it
+_require_test_program "open_by_handle"
+_require_exportfs
+
+# Lower is on test partition
+lower=$OVL_BASE_TEST_DIR/$OVL_LOWER-$seq
+# Upper/work are on scratch partition
+middle=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
+upper=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
+work=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
+
+lowerdir=$lower/lowertestdir
+upperdir=$upper/uppertestdir
+lowertestdir=$SCRATCH_MNT/lowertestdir
+uppertestdir=$SCRATCH_MNT/uppertestdir
+NUMFILES=1
+
+# Create test dir and empty test files
+create_test_files()
+{
+	local dir=$1
+	local opt=$2
+
+	mkdir -p $dir
+	src/open_by_handle -cp $opt $dir $NUMFILES
+}
+
+# Test encode/decode file handles on overlay mount
+test_file_handles()
+{
+	local dir=$1
+	shift
+
+	echo test_file_handles $dir $* | _filter_scratch | _filter_ovl_dirs | \
+				sed -e "s,$tmp\.,,g"
+	$here/src/open_by_handle $* $dir $NUMFILES
+}
+
+# Re-create lower/upper/work dirs
+create_dirs()
+{
+	# Create lower dir on test partition
+	rm -rf $lower
+	mkdir $lower
+
+	# Create upper/work dirs on scratch partition
+	_scratch_mkfs
+}
+
+# Mount an overlay on $SCRATCH_MNT with lower layer on test partition
+# and middle and upper layer on scratch partition
+mount_dirs()
+{
+	_overlay_mount_dirs $middle:$lower $upper $work \
+		$OVERLAY_MOUNT_OPTIONS overlay $SCRATCH_MNT
+}
+
+# Unmount & check the non samefs overlay
+unmount_dirs()
+{
+	$UMOUNT_PROG $SCRATCH_MNT
+	_overlay_check_dirs $lower $upper $work
+}
+
+# Check non-stale file handles of lower/upper files and verify
+# that handle decoded before copy up is encoded to upper after
+# copy up. Verify reading data from file open by file handle
+# and verify access_at() with dirfd open by file handle.
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check encode/decode of upper regular file handles
+test_file_handles $uppertestdir
+# Check encode/decode of upper dir file handle
+test_file_handles $uppertestdir -p
+# Check encode/write/decode/read/write of upper file handles
+test_file_handles $uppertestdir -wrap
+# Check encode/decode of lower regular file handles before copy up
+test_file_handles $lowertestdir
+# Check encode/decode of lower dir file handles before copy up
+test_file_handles $lowertestdir -p
+# Check encode/write/decode/read/write of lower file handles across copy up
+test_file_handles $lowertestdir -wrap
+unmount_dirs
+
+# Check copy up after encode/decode of lower/upper files
+# (copy up of disconnected dentry to index dir)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check encode/decode/write/read of upper regular file handles
+test_file_handles $uppertestdir -a
+test_file_handles $uppertestdir -r
+# Check encode/decode/write/read of lower regular file handles
+test_file_handles $lowertestdir -a
+test_file_handles $lowertestdir -r
+unmount_dirs
+
+# Check non-stale handles to unlinked but open lower/upper files
+create_dirs
+create_test_files $upperdir
+create_test_files $upperdir.rw
+create_test_files $lowerdir
+create_test_files $lowerdir.rw
+mount_dirs
+test_file_handles $uppertestdir -dk
+# Check encode/write/unlink/decode/read of upper regular file handles
+test_file_handles $uppertestdir.rw -rwdk
+test_file_handles $lowertestdir -dk
+# Check encode/write/unlink/decode/read of lower file handles across copy up
+test_file_handles $lowertestdir.rw -rwdk
+unmount_dirs
+
+# Check stale handles of unlinked lower/upper files (nlink = 1,0)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check decode of upper file handles after unlink/rmdir (nlink == 0)
+test_file_handles $uppertestdir -dp
+# Check decode of lower file handles after unlink/rmdir (nlink == 0)
+test_file_handles $lowertestdir -dp
+unmount_dirs
+
+# Check non-stale file handles of linked lower/upper files (nlink = 1,2,1)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check encode/decode of upper file handles (nlink == 1)
+test_file_handles $uppertestdir
+# Check decode/read of upper file handles after link (nlink == 2)
+test_file_handles $uppertestdir -wlr
+# Check decode/read of upper file handles after link + unlink (nlink == 1)
+test_file_handles $uppertestdir -ur
+# Check encode/decode of lower file handles before copy up (nlink == 1)
+test_file_handles $lowertestdir
+# Check decode/read of lower file handles after copy up + link (nlink == 2)
+test_file_handles $lowertestdir -wlr
+# Check decode/read of lower file handles after copy up + link + unlink (nlink == 1)
+test_file_handles $lowertestdir -ur
+unmount_dirs
+
+# Check non-stale file handles of linked lower/upper hardlinks (nlink = 2,1)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+# Create lower/upper hardlinks
+test_file_handles $lowerdir -l >/dev/null
+test_file_handles $upperdir -l >/dev/null
+mount_dirs
+# Check encode/decode of upper hardlink file handles (nlink == 2)
+test_file_handles $uppertestdir
+# Check decode/read of upper hardlink file handles after unlink (nlink == 1)
+test_file_handles $uppertestdir -wur
+# Check encode/decode of lower hardlink file handles before copy up (nlink == 2)
+test_file_handles $lowertestdir
+# Check decode/read of lower hardlink file handles after copy up + unlink (nlink == 1)
+test_file_handles $lowertestdir -wur
+unmount_dirs
+
+# Check stale file handles of unlinked lower/upper hardlinks (nlink = 2,0)
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+# Create lower/upper hardlinks
+test_file_handles $lowerdir -l >/dev/null
+test_file_handles $upperdir -l >/dev/null
+mount_dirs
+# Check encode/decode of upper hardlink file handles (nlink == 2)
+test_file_handles $uppertestdir
+# Check decode of upper hardlink file handles after 2*unlink (nlink == 0)
+test_file_handles $uppertestdir -d
+# Check encode/decode of lower hardlink file handles before copy up (nlink == 2)
+test_file_handles $lowertestdir
+# Check decode of lower hardlink file handles after copy up + 2*unlink (nlink == 0)
+test_file_handles $lowertestdir -d
+unmount_dirs
+
+# Check non-stale file handles of lower/upper renamed files
+create_dirs
+create_test_files $upperdir
+create_test_files $lowerdir
+mount_dirs
+# Check decode/read of upper file handles after rename in same upper parent
+test_file_handles $uppertestdir -wmr
+# Check decode/read of lower file handles after copy up + rename in same merge parent
+test_file_handles $lowertestdir -wmr
+unmount_dirs
+
+# Check non-stale file handles of lower/upper moved files
+create_dirs
+create_test_files $upperdir -w
+create_test_files $lowerdir -w
+mkdir -p $lowerdir.lo $lowerdir.up $upperdir.lo $upperdir.up
+mount_dirs
+# Check encode/decode/read of lower/upper file handles after move to new upper testdir
+test_file_handles $uppertestdir -o $tmp.upper_file_handles
+test_file_handles $lowertestdir -o $tmp.lower_file_handles
+mv $uppertestdir/* $uppertestdir.up/
+mv $lowertestdir/* $uppertestdir.lo/
+# Check open and read from stored file handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.lower_file_handles
+# Check encode/decode/read of lower/upper file handles after move to new merge testdir
+test_file_handles $uppertestdir.up -o $tmp.upper_file_handles
+test_file_handles $uppertestdir.lo -o $tmp.lower_file_handles
+mv $uppertestdir.up/* $lowertestdir.up/
+mv $uppertestdir.lo/* $lowertestdir.lo/
+# Check open and read from stored file handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -r -i $tmp.lower_file_handles
+unmount_dirs
+
+# Check non-stale file handles of lower/upper renamed dirs
+create_dirs
+create_test_files $upperdir -w
+create_test_files $lowerdir -w
+create_test_files $upperdir/subdir -w
+create_test_files $lowerdir/subdir -w
+mount_dirs
+# Check encode/decode/read of lower/upper file handles after rename of testdir
+test_file_handles $uppertestdir -p -o $tmp.upper_file_handles
+test_file_handles $lowertestdir -p -o $tmp.lower_file_handles
+# Check encode/decode/read of lower/upper file handles after rename of testdir's parent
+test_file_handles $uppertestdir/subdir -p -o $tmp.upper_subdir_file_handles
+test_file_handles $lowertestdir/subdir -p -o $tmp.lower_subdir_file_handles
+# Rename pure upper dir
+mv $uppertestdir $uppertestdir.new/
+# Copy up lower dir, index and rename
+mv $lowertestdir $lowertestdir.new/
+# Check open, read and readdir from stored file handles
+# (testdir argument is the mount point and NOT the dir
+#  we are trying to open by stored file handle)
+test_file_handles $SCRATCH_MNT -rp -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -rp -i $tmp.lower_file_handles
+test_file_handles $SCRATCH_MNT -rp -i $tmp.upper_subdir_file_handles
+test_file_handles $SCRATCH_MNT -rp -i $tmp.lower_subdir_file_handles
+# Retry decoding lower subdir file handle when indexed testdir is in dcache
+# (providing renamed testdir argument pins the indexed testdir to dcache)
+test_file_handles $lowertestdir.new -rp -i $tmp.lower_subdir_file_handles
+unmount_dirs
+
+status=0
+exit
diff --git a/tests/overlay/051.out b/tests/overlay/051.out
new file mode 100644
index 0000000..505e37e
--- /dev/null
+++ b/tests/overlay/051.out
@@ -0,0 +1,50 @@
+QA output created by 051
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -p
+test_file_handles SCRATCH_MNT/uppertestdir -wrap
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -p
+test_file_handles SCRATCH_MNT/lowertestdir -wrap
+test_file_handles SCRATCH_MNT/uppertestdir -a
+test_file_handles SCRATCH_MNT/uppertestdir -r
+test_file_handles SCRATCH_MNT/lowertestdir -a
+test_file_handles SCRATCH_MNT/lowertestdir -r
+test_file_handles SCRATCH_MNT/uppertestdir -dk
+test_file_handles SCRATCH_MNT/uppertestdir.rw -rwdk
+test_file_handles SCRATCH_MNT/lowertestdir -dk
+test_file_handles SCRATCH_MNT/lowertestdir.rw -rwdk
+test_file_handles SCRATCH_MNT/uppertestdir -dp
+test_file_handles SCRATCH_MNT/lowertestdir -dp
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -wlr
+test_file_handles SCRATCH_MNT/uppertestdir -ur
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -wlr
+test_file_handles SCRATCH_MNT/lowertestdir -ur
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -wur
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -wur
+test_file_handles SCRATCH_MNT/uppertestdir
+test_file_handles SCRATCH_MNT/uppertestdir -d
+test_file_handles SCRATCH_MNT/lowertestdir
+test_file_handles SCRATCH_MNT/lowertestdir -d
+test_file_handles SCRATCH_MNT/uppertestdir -wmr
+test_file_handles SCRATCH_MNT/lowertestdir -wmr
+test_file_handles SCRATCH_MNT/uppertestdir -o upper_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir -o lower_file_handles
+test_file_handles SCRATCH_MNT -r -i upper_file_handles
+test_file_handles SCRATCH_MNT -r -i lower_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir.up -o upper_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir.lo -o lower_file_handles
+test_file_handles SCRATCH_MNT -r -i upper_file_handles
+test_file_handles SCRATCH_MNT -r -i lower_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir -p -o upper_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir -p -o lower_file_handles
+test_file_handles SCRATCH_MNT/uppertestdir/subdir -p -o upper_subdir_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
+test_file_handles SCRATCH_MNT -rp -i upper_file_handles
+test_file_handles SCRATCH_MNT -rp -i lower_file_handles
+test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
+test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
+test_file_handles SCRATCH_MNT/lowertestdir.new -rp -i lower_subdir_file_handles
diff --git a/tests/overlay/group b/tests/overlay/group
index 6cb3763..380a382 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -50,3 +50,4 @@
 047 auto quick copyup hardlink
 048 auto quick copyup hardlink
 050 auto quick copyup hardlink exportfs
+051 auto quick copyup hardlink exportfs nonsamefs
-- 
2.7.4

Re: [PATCH 0/7] Overlayfs exportfs tests

[Eryu Guan]

On Sun, Jan 07, 2018 at 08:07:18PM +0200, Amir Goldstein wrote:
> Eryu,
> 
> Following patches were used to test overlayfs NFS export.
> 
> Patches 1-5 improve generic/exportfs tests, by adding test coverage
> of some directory file handle use cases.

open_by_handle test is covering more and more cases, thanks! OTOH, it's
getting more and more complicated :) Everytime I start looking at
open_by_handle tests/patches I need some time to get familiar with it
again, so the review process might be slow..

But from a quick scan, I think it's time to stop adding new sub-tests to
existing tests (maybe we should have done this since last time we added
new sub-tests to generic/467), because generic/467 is getting more
complex too and makes review harder. Perhaps we can factor out some
helper functions from generic/467 to common helpers and use them in new
tests.

> 
> Patches 6-7 add two overlay specific tests for NFS file handles
> for overlay setup of same fs and non same fs.
> 
> The new overlay tests don't run on upstream kernel and pass with
> my overlayfs NFS export V2 branch [1]. You may want to wait with
> merging those tests until overlayfs NFS support is merged - up to you,
> but a review would be most welcome.

OK, I'll see after reviewing all the tests.

> 
> Merging patches 1-5 seems like a good idea to me regardless of
> overlayfs NFS export support.

Agreed.

Thanks,
Eryu

> 
> Thanks,
> Amir.
> 
> [1] https://github.com/amir73il/linux/commits/ovl-nfs-export-v2
> 
> Amir Goldstein (7):
>   open_by_handle: store and load file handles from file
>   open_by_handle: verify dir content only with -r flag
>   generic/exportfs: golden output is not silent
>   generic/exportfs: store and load file handles from file
>   generic/exportfs: add a test case for renamed parent dir
>   overlay: test encode/decode overlay file handles
>   overlay: test encode/decode of non-samefs overlay file handles
> 
>  src/open_by_handle.c  | 112 +++++++++++++-----
>  tests/generic/426     |   1 -
>  tests/generic/426.out |   1 -
>  tests/generic/467     |  37 ++++--
>  tests/generic/467.out |  10 +-
>  tests/overlay/050     | 291 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/050.out |  50 ++++++++
>  tests/overlay/051     | 311 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/051.out |  50 ++++++++
>  tests/overlay/group   |   2 +
>  10 files changed, 822 insertions(+), 43 deletions(-)
>  create mode 100755 tests/overlay/050
>  create mode 100644 tests/overlay/050.out
>  create mode 100755 tests/overlay/051
>  create mode 100644 tests/overlay/051.out
> 
> -- 
> 2.7.4
> 

Re: [PATCH 0/7] Overlayfs exportfs tests

[Amir Goldstein]

On Thu, Jan 11, 2018 at 1:43 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Sun, Jan 07, 2018 at 08:07:18PM +0200, Amir Goldstein wrote:
>> Eryu,
>>
>> Following patches were used to test overlayfs NFS export.
>>
>> Patches 1-5 improve generic/exportfs tests, by adding test coverage
>> of some directory file handle use cases.
>
> open_by_handle test is covering more and more cases, thanks! OTOH, it's
> getting more and more complicated :)

Yeh, sorry about that ;-)

>
> But from a quick scan, I think it's time to stop adding new sub-tests to
> existing tests (maybe we should have done this since last time we added
> new sub-tests to generic/467), because generic/467 is getting more
> complex too and makes review harder.

Probably a good idea, but please note that patch 4/7 is fixing a test that was
not implemented correctly and test 5/7 adds a very minor variant.
An option is to split the directory related tests (those that are fixed by 4/7)
out of generic/467 and then fix them and add the new variant.

> Perhaps we can factor out some
> helper functions from generic/467 to common helpers and use them in new
> tests.
>

I don't think there is much to factor out, but we can add new tests
and clone the helpers. Let me know if you think differently after review.

Thanks,
Amir.

Re: [PATCH 1/7] open_by_handle: store and load file handles from file

[Eryu Guan]

On Sun, Jan 07, 2018 at 08:07:19PM +0200, Amir Goldstein wrote:
> usage:
>  open_by_handle -p -o <handles_file> <test_dir> [N]
>  open_by_handle -p -i <handles_file> <test_dir> [N]
> 
> This will be used to test decoding of file handles after various
> file systems operations including mount cycle.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

I found that it might be easier to review if the new test that takes use
of this new function is included in the same patch, so reviewer could
know how are these new functions being used without switching between
different patches.

> ---
>  src/open_by_handle.c | 110 +++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 85 insertions(+), 25 deletions(-)
> 
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index dbc5b0f..f9dfefc 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -43,30 +43,36 @@ Examples:

The usage info above the "Examples:" can be updated too.

"usage: open_by_handle [-cludmrwapk] <test_dir> [num_files]"

>  
>     open_by_handle -p <test_dir> [N]
>  
> -3. Get file handles for existing test set, write data to files,
> +3. Get file handles for existing test set and write them to a file.
> +   Read file handles from file and open files by handle:
> +
> +   open_by_handle -p -o <handles_file> <test_dir> [N]
> +   open_by_handle -p -i <handles_file> <test_dir> [N]
> +
> +4. Get file handles for existing test set, write data to files,
>     drop caches, open all files by handle, read and verify written
>     data, write new data to file:
>  
>     open_by_handle -rwa <test_dir> [N]
>  
> -4. Get file handles for existing test set, unlink all test files,
> +5. Get file handles for existing test set, unlink all test files,
>     remove test_dir, drop caches, try to open all files by handle
>     and expect ESTALE:
>  
>     open_by_handle -dp <test_dir> [N]
>  
> -5. Get file handles for existing test set, keep open file handles for all
> +6. Get file handles for existing test set, keep open file handles for all
>     test files, unlink all test files, drop caches and try to open all files
>     by handle (should work):
>  
>     open_by_handle -dk <test_dir> [N]
>  
> -6. Get file handles for existing test set, rename all test files,
> +7. Get file handles for existing test set, rename all test files,
>     drop caches, try to open all files by handle (should work):
>  
>     open_by_handle -m <test_dir> [N]
>  
> -7. Get file handles for existing test set, hardlink all test files,
> +8. Get file handles for existing test set, hardlink all test files,
>     then unlink the original files, drop caches and try to open all
>     files by handle (should work):
>  
> @@ -103,7 +109,7 @@ struct handle {
>  
>  void usage(void)
>  {
> -	fprintf(stderr, "usage: open_by_handle [-cludmrwapk] <test_dir> [num_files]\n");
> +	fprintf(stderr, "usage: open_by_handle [-cludmrwapk] [<-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");
> @@ -116,6 +122,8 @@ void usage(void)
>  	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");
> +	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");
>  	exit(EXIT_FAILURE);
>  }
>  
> @@ -131,15 +139,16 @@ int main(int argc, char **argv)
>  	char	*test_dir;
>  	char	*mount_dir;
>  	int	mount_fd, mount_id;
> +	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;
>  
> -	if (argc < 2 || argc > 4)
> +	if (argc < 2)
>  		usage();
>  
> -	while ((c = getopt(argc, argv, "cludmrwapk")) != -1) {
> +	while ((c = getopt(argc, argv, "cludmrwapki:o:")) != -1) {
>  		switch (c) {
>  		case 'c':
>  			create = 1;
> @@ -176,13 +185,27 @@ int main(int argc, char **argv)
>  		case 'k':
>  			keepopen = 1;
>  			break;
> +		case 'i':
> +			in_fd = open(optarg, O_RDONLY);
> +			if (in_fd < 0) {
> +				perror(optarg);
> +				return EXIT_FAILURE;
> +			}
> +			break;
> +		case 'o':
> +			out_fd = creat(optarg, 0644);
> +			if (out_fd < 0) {
> +				perror(optarg);
> +				return EXIT_FAILURE;
> +			}
> +			break;
>  		default:
>  			fprintf(stderr, "illegal option '%s'\n", argv[optind]);
>  		case 'h':
>  			usage();
>  		}
>  	}
> -        if (optind == argc || optind > 2)
> +        if (optind == argc)
>              usage();
>  	test_dir = argv[optind++];
>  	if (optind < argc)
> @@ -192,11 +215,14 @@ int main(int argc, char **argv)
>  		usage();
>  	}
>  
> -	if (parent) {
> +	if (parent && !in_fd) {
>  		strcpy(dname, test_dir);
>  		/*
>  		 * -p flag implies that test_dir is NOT a mount point,
>  		 * so its parent can be used as mount_fd for open_by_handle_at.
> +		 * -i flag implies that test_dir IS a mount point, because we
> +		 *  are testing open by handle of dir, which may have been
> +		 *  deleted or renamed.

I'm a bit confused by this comment, is test_dir a mount point if I
specify both -i and -p? Does that mean -i would override -p regarding to
test_dir being mount point or not?

>  		 */
>  		mount_dir = dirname(dname);
>  		if (create)
> @@ -241,15 +267,24 @@ int main(int argc, char **argv)
>  	/* sync to get the new inodes to hit the disk */
>  	sync();
>  
> -	/* create the handles */
> +	/* create/read the handles */

This loop also write handles out, update the comment accordingly?

>  	for (i=0; i < numfiles; i++) {
>  		sprintf(fname, "%s/file%06d", test_dir, i);
> -		handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
> -		ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> -		if (ret < 0) {
> -			strcat(fname, ": name_to_handle");
> -			perror(fname);
> -			return EXIT_FAILURE;
> +		if (in_fd) {
> +			ret = read(in_fd, (char *)&handle[i], sizeof(*handle));
> +			if (ret < sizeof(*handle)) {
> +				strcat(fname, ": read handle");
> +				perror(fname);
> +				return EXIT_FAILURE;
> +			}
> +		} else {
> +			handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
> +			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> +			if (ret < 0) {
> +				strcat(fname, ": name_to_handle");
> +				perror(fname);
> +				return EXIT_FAILURE;
> +			}
>  		}
>  		if (keepopen) {
>  			/* Open without close to keep unlinked files around */
> @@ -260,15 +295,40 @@ int main(int argc, char **argv)
>  				return EXIT_FAILURE;
>  			}
>  		}
> +		if (out_fd) {
> +			ret = write(out_fd, (char *)&handle[i], sizeof(*handle));
> +			if (ret < sizeof(*handle)) {
> +				strcat(fname, ": write handle");
> +				perror(fname);

A short write is not an error, so errno is 0 and error message can be
confusing: "$file: write handle: Success"

> +				return EXIT_FAILURE;
> +			}
> +		}
>  	}
>  
>  	if (parent) {
> -		dir_handle.fh.handle_bytes = MAX_HANDLE_SZ;
> -		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);
> -			return EXIT_FAILURE;
> +		if (in_fd) {
> +			ret = read(in_fd, (char *)&dir_handle, sizeof(*handle));
> +			if (ret < sizeof(*handle)) {

Same here, perror doesn't work with a short read.

> +				strcat(dname, ": read handle");

With both -i and -p specified, dname is not initialized, then error
message is getting into:

: read handle: Success

> +				perror(dname);
> +				return EXIT_FAILURE;
> +			}
> +		} else {
> +			dir_handle.fh.handle_bytes = MAX_HANDLE_SZ;
> +			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);
> +				return EXIT_FAILURE;
> +			}
> +		}
> +		if (out_fd) {
> +			ret = write(out_fd, (char *)&dir_handle, sizeof(*handle));
> +			if (ret < sizeof(*handle)) {
> +				strcat(dname, ": write handle");
> +				perror(dname);

Short write issue again.

Thanks,
Eryu

> +				return EXIT_FAILURE;
> +			}
>  		}
>  	}
>  
> @@ -289,8 +349,8 @@ int main(int argc, char **argv)
>  		close(fd);
>  	}
>  
> -	/* after creating test set only check that fs supports exportfs */
> -	if (create)
> +	/* If creating test set or saving files handles, we are done */
> +	if (create || out_fd)
>  		return EXIT_SUCCESS;
>  
>  	/* hardlink the files */
> -- 
> 2.7.4
> 

Re: [PATCH 1/7] open_by_handle: store and load file handles from file

[Amir Goldstein]

On Thu, Jan 11, 2018 at 1:59 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Sun, Jan 07, 2018 at 08:07:19PM +0200, Amir Goldstein wrote:
>> usage:
>>  open_by_handle -p -o <handles_file> <test_dir> [N]
>>  open_by_handle -p -i <handles_file> <test_dir> [N]
>>
>> This will be used to test decoding of file handles after various
>> file systems operations including mount cycle.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> I found that it might be easier to review if the new test that takes use
> of this new function is included in the same patch, so reviewer could
> know how are these new functions being used without switching between
> different patches.
>

Eryu,

Thanks for your comments. I will fix them later, but let me first address
this meta comment, so maybe it will help you with review of the tests.

I find that the comment about posting the new functionality along with the
test would have been a good thing to do for most of the options I added
so far. It could be a good option for this test as well.
However, the -i -o options are quite generic in nature, so it should
be fine to explain them without a test, only I probably did not explain well.

The basic flow of open_by_handle program is:
- store file handle of parent and test files in memory array
- perform operations on files
- drop caches
- open by file handles from array
- perform tests on open files

-i -o options allow to perform name_to_handle and open_by_handle with
2 different program invocations, so you don't need to rely on the program
performing drop caches and you can test advances flows like:
- encode files
- cycle mount
- decode files

In my use case, the new tests in generic/467 are:
- ecnode parent and files
- rename parent
- decode parent and files

- encode files
- move files to new parent
- decode files

- ecnode parent and files
- rename grandparent
- decode parent and files

Cheers,
Amir.

Re: [PATCH 0/7] Overlayfs exportfs tests

[Eryu Guan]

On Thu, Jan 11, 2018 at 01:52:02PM +0200, Amir Goldstein wrote:
> On Thu, Jan 11, 2018 at 1:43 PM, Eryu Guan <eguan@redhat.com> wrote:
> > On Sun, Jan 07, 2018 at 08:07:18PM +0200, Amir Goldstein wrote:
> >> Eryu,
> >>
> >> Following patches were used to test overlayfs NFS export.
> >>
> >> Patches 1-5 improve generic/exportfs tests, by adding test coverage
> >> of some directory file handle use cases.
> >
> > open_by_handle test is covering more and more cases, thanks! OTOH, it's
> > getting more and more complicated :)
> 
> Yeh, sorry about that ;-)
> 
> >
> > But from a quick scan, I think it's time to stop adding new sub-tests to
> > existing tests (maybe we should have done this since last time we added
> > new sub-tests to generic/467), because generic/467 is getting more
> > complex too and makes review harder.
> 
> Probably a good idea, but please note that patch 4/7 is fixing a test that was
> not implemented correctly and test 5/7 adds a very minor variant.

I've gone through the first 5 patches now. All look fine except the
comments to patch 1/7.

I think we can add a new test that contains the last part of patch 4/7
("Check non-stale file handles of renamed dirs") and patch 5/7. And
leave the "fix test" part in patch 4/7.

> An option is to split the directory related tests (those that are fixed by 4/7)
> out of generic/467 and then fix them and add the new variant.

This should be fine too, but I slightly tend to not move existing tests,
just add new cases to new tests.

BTW, thanks for the explaining in another reply, that did help.

Thanks,
Eryu

> 
> > Perhaps we can factor out some
> > helper functions from generic/467 to common helpers and use them in new
> > tests.
> >
> 
> I don't think there is much to factor out, but we can add new tests
> and clone the helpers. Let me know if you think differently after review.
> 
> Thanks,
> Amir.

Re: [PATCH 0/7] Overlayfs exportfs tests

[Amir Goldstein]

On Fri, Jan 12, 2018 at 1:52 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Thu, Jan 11, 2018 at 01:52:02PM +0200, Amir Goldstein wrote:
>> On Thu, Jan 11, 2018 at 1:43 PM, Eryu Guan <eguan@redhat.com> wrote:
>> > On Sun, Jan 07, 2018 at 08:07:18PM +0200, Amir Goldstein wrote:
>> >> Eryu,
>> >>
>> >> Following patches were used to test overlayfs NFS export.
>> >>
>> >> Patches 1-5 improve generic/exportfs tests, by adding test coverage
>> >> of some directory file handle use cases.
>> >
>> > open_by_handle test is covering more and more cases, thanks! OTOH, it's
>> > getting more and more complicated :)
>>
>> Yeh, sorry about that ;-)
>>
>> >
>> > But from a quick scan, I think it's time to stop adding new sub-tests to
>> > existing tests (maybe we should have done this since last time we added
>> > new sub-tests to generic/467), because generic/467 is getting more
>> > complex too and makes review harder.
>>
>> Probably a good idea, but please note that patch 4/7 is fixing a test that was
>> not implemented correctly and test 5/7 adds a very minor variant.
>
> I've gone through the first 5 patches now. All look fine except the
> comments to patch 1/7.
>
> I think we can add a new test that contains the last part of patch 4/7
> ("Check non-stale file handles of renamed dirs") and patch 5/7. And
> leave the "fix test" part in patch 4/7.
>
>> An option is to split the directory related tests (those that are fixed by 4/7)
>> out of generic/467 and then fix them and add the new variant.
>
> This should be fine too, but I slightly tend to not move existing tests,
> just add new cases to new tests.
>

So what I think I will do is not change generic/467, but add a variant of
generic/467 that tests open_by_handle after mount cycle that will use -i -o.
I will try re-work patch 1/7 and maybe add more changes so open_by_handle
becomes more of a tool to write file handle tests instead of a file handle
test program that does all tests and just invoked from a tests script.
If that doesn't work out, I'll just add more commentary to the patch ;-)

Thanks,
Amir.

Re: [PATCH 6/7] overlay: test encode/decode overlay file handles

[Eryu Guan]

On Sun, Jan 07, 2018 at 08:07:24PM +0200, Amir Goldstein wrote:
> - Check encode/write/decode/read content of lower/upper file handles
> - Check encode/decode/write/read content of lower/upper file handles
> - Check decode/read of unlinked lower/upper files and directories
> - Check decode/read of lower file handles after copy up, link and unlink
> - Check decode/read of lower file handles after rename of parent and self

I'm wondering that if this should be split into multiple tests somehow,
e.g. tests on regular files, tests on dirs and tests on hardlinks? It
might be eaiser to review and debug when there're test failures. But I
have no strong preference on this.

> 
> This test does not cover connectable file handles of non-directories,
> because name_to_handle_at() syscall does not support requesting
> connectable file handles.
> 
> This test covers only encode/decode of file handles for overlayfs
> configuration of lower and upper on the same fs.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  tests/overlay/050     | 291 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/050.out |  50 +++++++++
>  tests/overlay/group   |   1 +
>  3 files changed, 342 insertions(+)
>  create mode 100755 tests/overlay/050
>  create mode 100644 tests/overlay/050.out

I ran the test on your ovl-nfs-export-v2 branch and saw failures like:

--- tests/overlay/050.out       2018-01-16 14:51:11.350000000 +0800
+++ /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad      2018-01-16 15:08:43.487000000 +0800
@@ -45,6 +45,9 @@
 test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
 test_file_handles SCRATCH_MNT -rp -i upper_file_handles
 test_file_handles SCRATCH_MNT -rp -i lower_file_handles
+open_by_handle() returned 116 incorrectly on a linked dir!
 test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
 test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
+open_by_handle() returned 116 incorrectly on a linked dir!
 test_file_handles SCRATCH_MNT/lowertestdir.new -rp -i lower_subdir_file_handles
+open_by_handle() returned 116 incorrectly on a linked dir!

Are these failures expected?

> 
> diff --git a/tests/overlay/050 b/tests/overlay/050
> new file mode 100755
> index 0000000..4dcd66a
> --- /dev/null
> +++ b/tests/overlay/050
> @@ -0,0 +1,291 @@
> +#! /bin/bash
> +# FS QA Test No. 050
> +#
> +# Test encode/decode overlay file handles
> +#
> +# - Check encode/write/decode/read content of lower/upper file handles
> +# - Check encode/decode/write/read content of lower/upper file handles
> +# - Check decode/read of unlinked lower/upper files and directories
> +# - Check decode/read of lower file handles after copy up, link and unlink
> +# - Check decode/read of lower file handles after rename of parent and self
> +#
> +# This test does not cover connectable file handles of non-directories,
> +# because name_to_handle_at() syscall does not support requesting connectable
> +# file handles.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
                   ^^^^ 2018? :)

> +# Author: Amir Goldstein <amir73il@gmail.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +
> +_supported_fs overlay
> +_supported_os Linux
> +_require_scratch
> +# _require_exportfs already requires open_by_handle, but let's not count on it
> +_require_test_program "open_by_handle"
> +_require_exportfs

>From the commits in your development branch, I know that overlay
exportfs support depends on "verify" feature, I'm wondering if we should
check "verify" feature explicitly? So we know we need to enable "verify"
feature, otherwise we just got "[not run] overlay does not support NFS
export", which seems not so informative.

Perhaps we should mention it in commit log too.

> +
> +lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER/lowertestdir
> +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER/uppertestdir
> +lowertestdir=$SCRATCH_MNT/lowertestdir
> +uppertestdir=$SCRATCH_MNT/uppertestdir

Hmm, the "lowerdir"/"upperdir" always make me think them as the
lowerdir/upperdir used in overlay mount options.., and they actually
should be lowertestdir/uppertestdir, i.e.

losertestdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER/lowertestdir
uppertestdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER/uppertestdir

and rename the previous "$lowertestdir"/"$uppertestdir" to other names,
maybe

ovl_lowertestdir=$SCRATCH_MNT/lowertestdir
ovl_uppertestdir=$SCRATCH_MNT/uppertestdir

I'm not good at naming variables..

> +NUMFILES=1
> +
> +# Create test dir and empty test files
> +create_test_files()
> +{
> +	local dir=$1
> +	local opt=$2
> +
> +	mkdir -p $dir
> +	src/open_by_handle -cp $opt $dir $NUMFILES

$here/src/open_by_handle? Using "$here" for consistency.

> +}
> +
> +# Test encode/decode file handles on overlay mount
> +test_file_handles()
> +{
> +	local dir=$1
> +	shift
> +
> +	echo test_file_handles $dir $* | _filter_scratch | _filter_ovl_dirs | \
> +				sed -e "s,$tmp\.,,g"
> +	$here/src/open_by_handle $* $dir $NUMFILES
> +}
> +
> +# Re-create lower/upper/work dirs
> +create_dirs()
> +{
> +	_scratch_mkfs
> +}
> +
> +# Mount an overlay on $SCRATCH_MNT with all layers on scratch partition
> +mount_dirs()
> +{
> +	_scratch_mount
> +}
> +
> +# Unmount & check the overlay layers
> +unmount_dirs()
> +{
> +	_scratch_unmount
> +	_check_scratch_fs
> +}
> +
> +# Check non-stale file handles of lower/upper files and verify
> +# that handle decoded before copy up is encoded to upper after
                 ^^^^^^^ encoded?          ^^^^^^^ decoded?
> +# copy up. Verify reading data from file open by file handle
> +# and verify access_at() with dirfd open by file handle.
> +create_dirs
> +create_test_files $upperdir
> +create_test_files $lowerdir
> +mount_dirs
> +# Check encode/decode of upper regular file handles
> +test_file_handles $uppertestdir
> +# Check encode/decode of upper dir file handle
> +test_file_handles $uppertestdir -p
> +# Check encode/write/decode/read/write of upper file handles
> +test_file_handles $uppertestdir -wrap
> +# Check encode/decode of lower regular file handles before copy up
> +test_file_handles $lowertestdir
> +# Check encode/decode of lower dir file handles before copy up
> +test_file_handles $lowertestdir -p
> +# Check encode/write/decode/read/write of lower file handles across copy up
> +test_file_handles $lowertestdir -wrap
> +unmount_dirs
> +
> +# Check copy up after encode/decode of lower/upper files
> +# (copy up of disconnected dentry to index dir)
> +create_dirs
> +create_test_files $upperdir
> +create_test_files $lowerdir
> +mount_dirs
> +# Check encode/decode/write/read of upper regular file handles
> +test_file_handles $uppertestdir -a
> +test_file_handles $uppertestdir -r
> +# Check encode/decode/write/read of lower regular file handles
> +test_file_handles $lowertestdir -a
> +test_file_handles $lowertestdir -r
> +unmount_dirs
> +
> +# Check non-stale handles to unlinked but open lower/upper files
> +create_dirs
> +create_test_files $upperdir
> +create_test_files $upperdir.rw
> +create_test_files $lowerdir
> +create_test_files $lowerdir.rw
> +mount_dirs
> +test_file_handles $uppertestdir -dk
> +# Check encode/write/unlink/decode/read of upper regular file handles
> +test_file_handles $uppertestdir.rw -rwdk
> +test_file_handles $lowertestdir -dk
> +# Check encode/write/unlink/decode/read of lower file handles across copy up
> +test_file_handles $lowertestdir.rw -rwdk
> +unmount_dirs
> +
> +# Check stale handles of unlinked lower/upper files (nlink = 1,0)

Seems there's no nlink=1 case in this subtest.

> +create_dirs
> +create_test_files $upperdir
> +create_test_files $lowerdir
> +mount_dirs
> +# Check decode of upper file handles after unlink/rmdir (nlink == 0)
> +test_file_handles $uppertestdir -dp
> +# Check decode of lower file handles after unlink/rmdir (nlink == 0)
> +test_file_handles $lowertestdir -dp
> +unmount_dirs
> +
> +# Check non-stale file handles of linked lower/upper files (nlink = 1,2,1)
> +create_dirs
> +create_test_files $upperdir
> +create_test_files $lowerdir
> +mount_dirs
> +# Check encode/decode of upper file handles (nlink == 1)
> +test_file_handles $uppertestdir

This nlink=1 case has been tested in the first subtest.

> +# Check decode/read of upper file handles after link (nlink == 2)
> +test_file_handles $uppertestdir -wlr
> +# Check decode/read of upper file handles after link + unlink (nlink == 1)
> +test_file_handles $uppertestdir -ur
> +# Check encode/decode of lower file handles before copy up (nlink == 1)
> +test_file_handles $lowertestdir

Same here.

> +# Check decode/read of lower file handles after copy up + link (nlink == 2)
> +test_file_handles $lowertestdir -wlr
> +# Check decode/read of lower file handles after copy up + link + unlink (nlink == 1)
> +test_file_handles $lowertestdir -ur
> +unmount_dirs
> +
> +# Check non-stale file handles of linked lower/upper hardlinks (nlink = 2,1)
> +create_dirs
> +create_test_files $upperdir
> +create_test_files $lowerdir
> +# Create lower/upper hardlinks
> +test_file_handles $lowerdir -l >/dev/null
> +test_file_handles $upperdir -l >/dev/null

Add comments on why discard the stdout above?

> +mount_dirs
> +# Check encode/decode of upper hardlink file handles (nlink == 2)
> +test_file_handles $uppertestdir
> +# Check decode/read of upper hardlink file handles after unlink (nlink == 1)
> +test_file_handles $uppertestdir -wur
> +# Check encode/decode of lower hardlink file handles before copy up (nlink == 2)
> +test_file_handles $lowertestdir
> +# Check decode/read of lower hardlink file handles after copy up + unlink (nlink == 1)
> +test_file_handles $lowertestdir -wur

At first I suspected that this would fail if index feature was disabled,
then I found that verify feature depends on index feature. I think this
should be mentioned in commit log and in test as comment too.

Thanks,
Eryu

> +unmount_dirs
> +
> +# Check stale file handles of unlinked lower/upper hardlinks (nlink = 2,0)
> +create_dirs
> +create_test_files $upperdir
> +create_test_files $lowerdir
> +# Create lower/upper hardlinks
> +test_file_handles $lowerdir -l >/dev/null
> +test_file_handles $upperdir -l >/dev/null
> +mount_dirs
> +# Check encode/decode of upper hardlink file handles (nlink == 2)
> +test_file_handles $uppertestdir
> +# Check decode of upper hardlink file handles after 2*unlink (nlink == 0)
> +test_file_handles $uppertestdir -d
> +# Check encode/decode of lower hardlink file handles before copy up (nlink == 2)
> +test_file_handles $lowertestdir
> +# Check decode of lower hardlink file handles after copy up + 2*unlink (nlink == 0)
> +test_file_handles $lowertestdir -d
> +unmount_dirs
> +
> +# Check non-stale file handles of lower/upper renamed files
> +create_dirs
> +create_test_files $upperdir
> +create_test_files $lowerdir
> +mount_dirs
> +# Check decode/read of upper file handles after rename in same upper parent
> +test_file_handles $uppertestdir -wmr
> +# Check decode/read of lower file handles after copy up + rename in same merge parent
> +test_file_handles $lowertestdir -wmr
> +unmount_dirs
> +
> +# Check non-stale file handles of lower/upper moved files
> +create_dirs
> +create_test_files $upperdir -w
> +create_test_files $lowerdir -w
> +mkdir -p $lowerdir.lo $lowerdir.up $upperdir.lo $upperdir.up
> +mount_dirs
> +# Check encode/decode/read of lower/upper file handles after move to new upper testdir
> +test_file_handles $uppertestdir -o $tmp.upper_file_handles
> +test_file_handles $lowertestdir -o $tmp.lower_file_handles
> +mv $uppertestdir/* $uppertestdir.up/
> +mv $lowertestdir/* $uppertestdir.lo/
> +# Check open and read from stored file handles
> +test_file_handles $SCRATCH_MNT -r -i $tmp.upper_file_handles
> +test_file_handles $SCRATCH_MNT -r -i $tmp.lower_file_handles
> +# Check encode/decode/read of lower/upper file handles after move to new merge testdir
> +test_file_handles $uppertestdir.up -o $tmp.upper_file_handles
> +test_file_handles $uppertestdir.lo -o $tmp.lower_file_handles
> +mv $uppertestdir.up/* $lowertestdir.up/
> +mv $uppertestdir.lo/* $lowertestdir.lo/
> +# Check open and read from stored file handles
> +test_file_handles $SCRATCH_MNT -r -i $tmp.upper_file_handles
> +test_file_handles $SCRATCH_MNT -r -i $tmp.lower_file_handles
> +unmount_dirs
> +
> +# Check non-stale file handles of lower/upper renamed dirs
> +create_dirs
> +create_test_files $upperdir -w
> +create_test_files $lowerdir -w
> +create_test_files $upperdir/subdir -w
> +create_test_files $lowerdir/subdir -w
> +mount_dirs
> +# Check encode/decode/read of lower/upper file handles after rename of testdir
> +test_file_handles $uppertestdir -p -o $tmp.upper_file_handles
> +test_file_handles $lowertestdir -p -o $tmp.lower_file_handles
> +# Check encode/decode/read of lower/upper file handles after rename of testdir's parent
> +test_file_handles $uppertestdir/subdir -p -o $tmp.upper_subdir_file_handles
> +test_file_handles $lowertestdir/subdir -p -o $tmp.lower_subdir_file_handles
> +# Rename pure upper dir
> +mv $uppertestdir $uppertestdir.new/
> +# Copy up lower dir, index and rename
> +mv $lowertestdir $lowertestdir.new/
> +# Check open, read and readdir from stored file handles
> +# (testdir argument is the mount point and NOT the dir
> +#  we are trying to open by stored file handle)
> +test_file_handles $SCRATCH_MNT -rp -i $tmp.upper_file_handles
> +test_file_handles $SCRATCH_MNT -rp -i $tmp.lower_file_handles
> +test_file_handles $SCRATCH_MNT -rp -i $tmp.upper_subdir_file_handles
> +test_file_handles $SCRATCH_MNT -rp -i $tmp.lower_subdir_file_handles
> +# Retry decoding lower subdir file handle when indexed testdir is in dcache
> +# (providing renamed testdir argument pins the indexed testdir to dcache)
> +test_file_handles $lowertestdir.new -rp -i $tmp.lower_subdir_file_handles
> +unmount_dirs
> +
> +status=0
> +exit
> diff --git a/tests/overlay/050.out b/tests/overlay/050.out
> new file mode 100644
> index 0000000..d8bac04
> --- /dev/null
> +++ b/tests/overlay/050.out
> @@ -0,0 +1,50 @@
> +QA output created by 050
> +test_file_handles SCRATCH_MNT/uppertestdir
> +test_file_handles SCRATCH_MNT/uppertestdir -p
> +test_file_handles SCRATCH_MNT/uppertestdir -wrap
> +test_file_handles SCRATCH_MNT/lowertestdir
> +test_file_handles SCRATCH_MNT/lowertestdir -p
> +test_file_handles SCRATCH_MNT/lowertestdir -wrap
> +test_file_handles SCRATCH_MNT/uppertestdir -a
> +test_file_handles SCRATCH_MNT/uppertestdir -r
> +test_file_handles SCRATCH_MNT/lowertestdir -a
> +test_file_handles SCRATCH_MNT/lowertestdir -r
> +test_file_handles SCRATCH_MNT/uppertestdir -dk
> +test_file_handles SCRATCH_MNT/uppertestdir.rw -rwdk
> +test_file_handles SCRATCH_MNT/lowertestdir -dk
> +test_file_handles SCRATCH_MNT/lowertestdir.rw -rwdk
> +test_file_handles SCRATCH_MNT/uppertestdir -dp
> +test_file_handles SCRATCH_MNT/lowertestdir -dp
> +test_file_handles SCRATCH_MNT/uppertestdir
> +test_file_handles SCRATCH_MNT/uppertestdir -wlr
> +test_file_handles SCRATCH_MNT/uppertestdir -ur
> +test_file_handles SCRATCH_MNT/lowertestdir
> +test_file_handles SCRATCH_MNT/lowertestdir -wlr
> +test_file_handles SCRATCH_MNT/lowertestdir -ur
> +test_file_handles SCRATCH_MNT/uppertestdir
> +test_file_handles SCRATCH_MNT/uppertestdir -wur
> +test_file_handles SCRATCH_MNT/lowertestdir
> +test_file_handles SCRATCH_MNT/lowertestdir -wur
> +test_file_handles SCRATCH_MNT/uppertestdir
> +test_file_handles SCRATCH_MNT/uppertestdir -d
> +test_file_handles SCRATCH_MNT/lowertestdir
> +test_file_handles SCRATCH_MNT/lowertestdir -d
> +test_file_handles SCRATCH_MNT/uppertestdir -wmr
> +test_file_handles SCRATCH_MNT/lowertestdir -wmr
> +test_file_handles SCRATCH_MNT/uppertestdir -o upper_file_handles
> +test_file_handles SCRATCH_MNT/lowertestdir -o lower_file_handles
> +test_file_handles SCRATCH_MNT -r -i upper_file_handles
> +test_file_handles SCRATCH_MNT -r -i lower_file_handles
> +test_file_handles SCRATCH_MNT/uppertestdir.up -o upper_file_handles
> +test_file_handles SCRATCH_MNT/uppertestdir.lo -o lower_file_handles
> +test_file_handles SCRATCH_MNT -r -i upper_file_handles
> +test_file_handles SCRATCH_MNT -r -i lower_file_handles
> +test_file_handles SCRATCH_MNT/uppertestdir -p -o upper_file_handles
> +test_file_handles SCRATCH_MNT/lowertestdir -p -o lower_file_handles
> +test_file_handles SCRATCH_MNT/uppertestdir/subdir -p -o upper_subdir_file_handles
> +test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
> +test_file_handles SCRATCH_MNT -rp -i upper_file_handles
> +test_file_handles SCRATCH_MNT -rp -i lower_file_handles
> +test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
> +test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
> +test_file_handles SCRATCH_MNT/lowertestdir.new -rp -i lower_subdir_file_handles
> diff --git a/tests/overlay/group b/tests/overlay/group
> index 7e541e4..6cb3763 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -49,3 +49,4 @@
>  044 auto quick copyup hardlink nonsamefs
>  047 auto quick copyup hardlink
>  048 auto quick copyup hardlink
> +050 auto quick copyup hardlink exportfs
> -- 
> 2.7.4
> 

Re: [PATCH 7/7] overlay: test encode/decode of non-samefs overlay file handles

[Eryu Guan]

On Sun, Jan 07, 2018 at 08:07:25PM +0200, Amir Goldstein wrote:
> This is a variant of overlay file handles test for an overlayfs that
> is composed of multiple lower layers not on the same underlying fs.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

This test follows the same pattern of overlay/050, I just noticed one
other issue. We don't have _overlay_check_dirs yet, so test failed
complaining (apart from the "+open_by_handle() returned 116 incorrectly
on a linked dir!" issue):

+./tests/overlay/051: line 126: _overlay_check_dirs: command not found

Thanks,
Eryu

Re: [PATCH 7/7] overlay: test encode/decode of non-samefs overlay file handles

[Amir Goldstein]

On Tue, Jan 16, 2018 at 9:42 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Sun, Jan 07, 2018 at 08:07:25PM +0200, Amir Goldstein wrote:
>> This is a variant of overlay file handles test for an overlayfs that
>> is composed of multiple lower layers not on the same underlying fs.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> This test follows the same pattern of overlay/050, I just noticed one
> other issue. We don't have _overlay_check_dirs yet, so test failed
> complaining (apart from the "+open_by_handle() returned 116 incorrectly
> on a linked dir!" issue):
>
> +./tests/overlay/051: line 126: _overlay_check_dirs: command not found
>

My bad, I re-ordered patches and forgot to remove these.

Thanks,
Amir.

Re: [PATCH 6/7] overlay: test encode/decode overlay file handles

[Amir Goldstein]

On Tue, Jan 16, 2018 at 9:38 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Sun, Jan 07, 2018 at 08:07:24PM +0200, Amir Goldstein wrote:
>> - Check encode/write/decode/read content of lower/upper file handles
>> - Check encode/decode/write/read content of lower/upper file handles
>> - Check decode/read of unlinked lower/upper files and directories
>> - Check decode/read of lower file handles after copy up, link and unlink
>> - Check decode/read of lower file handles after rename of parent and self
>
> I'm wondering that if this should be split into multiple tests somehow,
> e.g. tests on regular files, tests on dirs and tests on hardlinks? It
> might be eaiser to review and debug when there're test failures. But I
> have no strong preference on this.
>

I prefer not splitting the test, this is a classic test with sub-test cases.
I may end up splitting the dir rename tests (open_by_handle -i/-o)
to conform with a similar split that you requested in the generic test.

>>
>> This test does not cover connectable file handles of non-directories,
>> because name_to_handle_at() syscall does not support requesting
>> connectable file handles.
>>
>> This test covers only encode/decode of file handles for overlayfs
>> configuration of lower and upper on the same fs.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  tests/overlay/050     | 291 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/overlay/050.out |  50 +++++++++
>>  tests/overlay/group   |   1 +
>>  3 files changed, 342 insertions(+)
>>  create mode 100755 tests/overlay/050
>>  create mode 100644 tests/overlay/050.out
>
> I ran the test on your ovl-nfs-export-v2 branch and saw failures like:
>
> --- tests/overlay/050.out       2018-01-16 14:51:11.350000000 +0800
> +++ /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad      2018-01-16 15:08:43.487000000 +0800
> @@ -45,6 +45,9 @@
>  test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
>  test_file_handles SCRATCH_MNT -rp -i upper_file_handles
>  test_file_handles SCRATCH_MNT -rp -i lower_file_handles
> +open_by_handle() returned 116 incorrectly on a linked dir!
>  test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
>  test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
> +open_by_handle() returned 116 incorrectly on a linked dir!
>  test_file_handles SCRATCH_MNT/lowertestdir.new -rp -i lower_subdir_file_handles
> +open_by_handle() returned 116 incorrectly on a linked dir!
>
> Are these failures expected?
>

No. not expected. I wonder which base fs did you test with?
Did you have OVERLAY_FS_VERIFY=y in config or verify=on in MOUNT_OPTIONS?
(Not that I know any of the above should matter)

Do you see any overlayfs warnings in dmesg?

>>
>> diff --git a/tests/overlay/050 b/tests/overlay/050
>> new file mode 100755
>> index 0000000..4dcd66a
>> --- /dev/null
>> +++ b/tests/overlay/050
>> @@ -0,0 +1,291 @@
>> +#! /bin/bash
>> +# FS QA Test No. 050
>> +#
>> +# Test encode/decode overlay file handles
>> +#
>> +# - Check encode/write/decode/read content of lower/upper file handles
>> +# - Check encode/decode/write/read content of lower/upper file handles
>> +# - Check decode/read of unlinked lower/upper files and directories
>> +# - Check decode/read of lower file handles after copy up, link and unlink
>> +# - Check decode/read of lower file handles after rename of parent and self
>> +#
>> +# This test does not cover connectable file handles of non-directories,
>> +# because name_to_handle_at() syscall does not support requesting connectable
>> +# file handles.
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (C) 2017 CTERA Networks. All Rights Reserved.
>                    ^^^^ 2018? :)

Yeh, just added that.
That's now officially a multi year development effort ;-)

>
>> +# Author: Amir Goldstein <amir73il@gmail.com>
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write the Free Software Foundation,
>> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1     # failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +     cd /
>> +     rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +
>> +_supported_fs overlay
>> +_supported_os Linux
>> +_require_scratch
>> +# _require_exportfs already requires open_by_handle, but let's not count on it
>> +_require_test_program "open_by_handle"
>> +_require_exportfs
>
> From the commits in your development branch, I know that overlay
> exportfs support depends on "verify" feature, I'm wondering if we should
> check "verify" feature explicitly? So we know we need to enable "verify"
> feature, otherwise we just got "[not run] overlay does not support NFS
> export", which seems not so informative.
>
> Perhaps we should mention it in commit log too.

For V3 the opt-in feature name is "nfs_export", so I changed the
test to:

-_require_exportfs
+_require_scratch_feature nfs_export

 mount_dirs()
 {
-       _scratch_mount
+       _scratch_mount -o nfs_export=on
 }

Now I think that is pretty much self explanatory,
but will add a note in commit message.

However, please note that the generic/exportfs tests check for
_require_exportfs (on test partition), so those tests will still
"[not run] overlay does not support NFS export" unless user
sets CONFIG_OVERLAY_FS_NFS_EXPORT=y or
MOUNT_OPTIONS="-o nfs_export=on"

I think that is fine and should stay like this.
If user sees that overlayfs does not support NFS  export, she can go
to overlayfs documentation and see how to enable NFS export support.

>
>> +
>> +lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER/lowertestdir
>> +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER/uppertestdir
>> +lowertestdir=$SCRATCH_MNT/lowertestdir
>> +uppertestdir=$SCRATCH_MNT/uppertestdir
>
> Hmm, the "lowerdir"/"upperdir" always make me think them as the
> lowerdir/upperdir used in overlay mount options.., and they actually
> should be lowertestdir/uppertestdir, i.e.
>
> losertestdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER/lowertestdir
> uppertestdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER/uppertestdir
>
> and rename the previous "$lowertestdir"/"$uppertestdir" to other names,
> maybe
>
> ovl_lowertestdir=$SCRATCH_MNT/lowertestdir
> ovl_uppertestdir=$SCRATCH_MNT/uppertestdir
>
> I'm not good at naming variables..
>

Totally agree with you.
Instead of using goofy long variable names, I will use more explicit
expressions, e.g.:

create_test_files $upper/uppertestdir
create_test_files $lower/lowertestdir
test_file_handles $SCRATCH_MNT/uppertestdir
test_file_handles $SCRATCH_MNT/lowertestdir

Which is more similar to how golden output looks.

>> +NUMFILES=1
>> +
>> +# Create test dir and empty test files
>> +create_test_files()
>> +{
>> +     local dir=$1
>> +     local opt=$2
>> +
>> +     mkdir -p $dir
>> +     src/open_by_handle -cp $opt $dir $NUMFILES
>
> $here/src/open_by_handle? Using "$here" for consistency.

OK.

>
>> +}
>> +
>> +# Test encode/decode file handles on overlay mount
>> +test_file_handles()
>> +{
>> +     local dir=$1
>> +     shift
>> +
>> +     echo test_file_handles $dir $* | _filter_scratch | _filter_ovl_dirs | \
>> +                             sed -e "s,$tmp\.,,g"
>> +     $here/src/open_by_handle $* $dir $NUMFILES
>> +}
>> +
>> +# Re-create lower/upper/work dirs
>> +create_dirs()
>> +{
>> +     _scratch_mkfs
>> +}
>> +
>> +# Mount an overlay on $SCRATCH_MNT with all layers on scratch partition
>> +mount_dirs()
>> +{
>> +     _scratch_mount
>> +}
>> +
>> +# Unmount & check the overlay layers
>> +unmount_dirs()
>> +{
>> +     _scratch_unmount
>> +     _check_scratch_fs
>> +}
>> +
>> +# Check non-stale file handles of lower/upper files and verify
>> +# that handle decoded before copy up is encoded to upper after
>                  ^^^^^^^ encoded?          ^^^^^^^ decoded?

Oops. Thanks.

>> +# copy up. Verify reading data from file open by file handle
>> +# and verify access_at() with dirfd open by file handle.
>> +create_dirs
>> +create_test_files $upperdir
>> +create_test_files $lowerdir
>> +mount_dirs
>> +# Check encode/decode of upper regular file handles
>> +test_file_handles $uppertestdir
>> +# Check encode/decode of upper dir file handle
>> +test_file_handles $uppertestdir -p
>> +# Check encode/write/decode/read/write of upper file handles
>> +test_file_handles $uppertestdir -wrap
>> +# Check encode/decode of lower regular file handles before copy up
>> +test_file_handles $lowertestdir
>> +# Check encode/decode of lower dir file handles before copy up
>> +test_file_handles $lowertestdir -p
>> +# Check encode/write/decode/read/write of lower file handles across copy up
>> +test_file_handles $lowertestdir -wrap
>> +unmount_dirs
>> +
>> +# Check copy up after encode/decode of lower/upper files
>> +# (copy up of disconnected dentry to index dir)
>> +create_dirs
>> +create_test_files $upperdir
>> +create_test_files $lowerdir
>> +mount_dirs
>> +# Check encode/decode/write/read of upper regular file handles
>> +test_file_handles $uppertestdir -a
>> +test_file_handles $uppertestdir -r
>> +# Check encode/decode/write/read of lower regular file handles
>> +test_file_handles $lowertestdir -a
>> +test_file_handles $lowertestdir -r
>> +unmount_dirs
>> +
>> +# Check non-stale handles to unlinked but open lower/upper files
>> +create_dirs
>> +create_test_files $upperdir
>> +create_test_files $upperdir.rw
>> +create_test_files $lowerdir
>> +create_test_files $lowerdir.rw
>> +mount_dirs
>> +test_file_handles $uppertestdir -dk
>> +# Check encode/write/unlink/decode/read of upper regular file handles
>> +test_file_handles $uppertestdir.rw -rwdk
>> +test_file_handles $lowertestdir -dk
>> +# Check encode/write/unlink/decode/read of lower file handles across copy up
>> +test_file_handles $lowertestdir.rw -rwdk
>> +unmount_dirs
>> +
>> +# Check stale handles of unlinked lower/upper files (nlink = 1,0)
>
> Seems there's no nlink=1 case in this subtest.

nlink=1 on encode and nlink=0 on decode.
it's just not adding anything to test encode/decode with nlink=1
because that has already been tests by first subtest, as you corretly
noted in the cases below.

>
>> +create_dirs
>> +create_test_files $upperdir
>> +create_test_files $lowerdir
>> +mount_dirs
>> +# Check decode of upper file handles after unlink/rmdir (nlink == 0)
>> +test_file_handles $uppertestdir -dp
>> +# Check decode of lower file handles after unlink/rmdir (nlink == 0)
>> +test_file_handles $lowertestdir -dp
>> +unmount_dirs
>> +
>> +# Check non-stale file handles of linked lower/upper files (nlink = 1,2,1)
>> +create_dirs
>> +create_test_files $upperdir
>> +create_test_files $lowerdir
>> +mount_dirs
>> +# Check encode/decode of upper file handles (nlink == 1)
>> +test_file_handles $uppertestdir
>
> This nlink=1 case has been tested in the first subtest.

Correct. Removed.

>
>> +# Check decode/read of upper file handles after link (nlink == 2)
>> +test_file_handles $uppertestdir -wlr
>> +# Check decode/read of upper file handles after link + unlink (nlink == 1)
>> +test_file_handles $uppertestdir -ur
>> +# Check encode/decode of lower file handles before copy up (nlink == 1)
>> +test_file_handles $lowertestdir
>
> Same here.

Removed.

>
>> +# Check decode/read of lower file handles after copy up + link (nlink == 2)
>> +test_file_handles $lowertestdir -wlr
>> +# Check decode/read of lower file handles after copy up + link + unlink (nlink == 1)
>> +test_file_handles $lowertestdir -ur
>> +unmount_dirs
>> +
>> +# Check non-stale file handles of linked lower/upper hardlinks (nlink = 2,1)
>> +create_dirs
>> +create_test_files $upperdir
>> +create_test_files $lowerdir
>> +# Create lower/upper hardlinks
>> +test_file_handles $lowerdir -l >/dev/null
>> +test_file_handles $upperdir -l >/dev/null
>
> Add comments on why discard the stdout above?

I created a link_test_files helper instead of hacking around test_file_handles
output

>
>> +mount_dirs
>> +# Check encode/decode of upper hardlink file handles (nlink == 2)
>> +test_file_handles $uppertestdir
>> +# Check decode/read of upper hardlink file handles after unlink (nlink == 1)
>> +test_file_handles $uppertestdir -wur
>> +# Check encode/decode of lower hardlink file handles before copy up (nlink == 2)
>> +test_file_handles $lowertestdir
>> +# Check decode/read of lower hardlink file handles after copy up + unlink (nlink == 1)
>> +test_file_handles $lowertestdir -wur
>
> At first I suspected that this would fail if index feature was disabled,
> then I found that verify feature depends on index feature. I think this
> should be mentioned in commit log and in test as comment too.
>

OK. Will add that too.

Thanks a lot for review!

Amir.

Re: [PATCH 6/7] overlay: test encode/decode overlay file handles

[Eryu Guan]

On Tue, Jan 16, 2018 at 12:53:38PM +0200, Amir Goldstein wrote:
> On Tue, Jan 16, 2018 at 9:38 AM, Eryu Guan <eguan@redhat.com> wrote:
> > On Sun, Jan 07, 2018 at 08:07:24PM +0200, Amir Goldstein wrote:
> >> - Check encode/write/decode/read content of lower/upper file handles
> >> - Check encode/decode/write/read content of lower/upper file handles
> >> - Check decode/read of unlinked lower/upper files and directories
> >> - Check decode/read of lower file handles after copy up, link and unlink
> >> - Check decode/read of lower file handles after rename of parent and self
> >
> > I'm wondering that if this should be split into multiple tests somehow,
> > e.g. tests on regular files, tests on dirs and tests on hardlinks? It
> > might be eaiser to review and debug when there're test failures. But I
> > have no strong preference on this.
> >
> 
> I prefer not splitting the test, this is a classic test with sub-test cases.
> I may end up splitting the dir rename tests (open_by_handle -i/-o)
> to conform with a similar split that you requested in the generic test.
> 
> >>
> >> This test does not cover connectable file handles of non-directories,
> >> because name_to_handle_at() syscall does not support requesting
> >> connectable file handles.
> >>
> >> This test covers only encode/decode of file handles for overlayfs
> >> configuration of lower and upper on the same fs.
> >>
> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >> ---
> >>  tests/overlay/050     | 291 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/overlay/050.out |  50 +++++++++
> >>  tests/overlay/group   |   1 +
> >>  3 files changed, 342 insertions(+)
> >>  create mode 100755 tests/overlay/050
> >>  create mode 100644 tests/overlay/050.out
> >
> > I ran the test on your ovl-nfs-export-v2 branch and saw failures like:
> >
> > --- tests/overlay/050.out       2018-01-16 14:51:11.350000000 +0800
> > +++ /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad      2018-01-16 15:08:43.487000000 +0800
> > @@ -45,6 +45,9 @@
> >  test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
> >  test_file_handles SCRATCH_MNT -rp -i upper_file_handles
> >  test_file_handles SCRATCH_MNT -rp -i lower_file_handles
> > +open_by_handle() returned 116 incorrectly on a linked dir!
> >  test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
> >  test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
> > +open_by_handle() returned 116 incorrectly on a linked dir!
> >  test_file_handles SCRATCH_MNT/lowertestdir.new -rp -i lower_subdir_file_handles
> > +open_by_handle() returned 116 incorrectly on a linked dir!
> >
> > Are these failures expected?
> >
> 
> No. not expected. I wonder which base fs did you test with?
> Did you have OVERLAY_FS_VERIFY=y in config or verify=on in MOUNT_OPTIONS?
> (Not that I know any of the above should matter)

I didn't have OVERLAY_FS_VERIFY set in .config, but I did mount with "-o
verify=on", and underlying fs is xfs. Here is the screenshot:

[root@bootp-73-5-205 xfstests]# OVERLAY_MOUNT_OPTIONS="-o verify=on" ./check -s xfs_4k_crc -overlay overlay/050
SECTION       -- xfs_4k_crc
RECREATING    -- overlay on /mnt/testarea/test
FSTYP         -- overlay
PLATFORM      -- Linux/x86_64 bootp-73-5-205 4.15.0-rc2.ovl+
MKFS_OPTIONS  -- -f -b size=4k -m crc=1 /mnt/testarea/scratch
MOUNT_OPTIONS -- -o verify=on /mnt/testarea/scratch /mnt/testarea/scratch/ovl-mnt

overlay/050      - output mismatch (see /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad)
    --- tests/overlay/050.out   2018-01-16 14:51:11.350000000 +0800
    +++ /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad  2018-01-16 19:01:54.984000000 +0800
    @@ -45,6 +45,9 @@
     test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
     test_file_handles SCRATCH_MNT -rp -i upper_file_handles
     test_file_handles SCRATCH_MNT -rp -i lower_file_handles
    +open_by_handle() returned 116 incorrectly on a linked dir!
     test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
     test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
    +open_by_handle() returned 116 incorrectly on a linked dir!
    ...
    (Run 'diff -u tests/overlay/050.out /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad'  to see the entire diff)
Ran: overlay/050
Failures: overlay/050
Failed 1 of 1 tests

And I just tried with ext4 as underlying fs and got the same result.

> 
> Do you see any overlayfs warnings in dmesg?

No, there's no warnings nor other useful information in dmesg, just
mount/umount xfs and drop caches messages.

Thanks,
Eryu

Re: [PATCH 6/7] overlay: test encode/decode overlay file handles

[Amir Goldstein]

On Tue, Jan 16, 2018 at 1:06 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Tue, Jan 16, 2018 at 12:53:38PM +0200, Amir Goldstein wrote:
>> On Tue, Jan 16, 2018 at 9:38 AM, Eryu Guan <eguan@redhat.com> wrote:
>> > On Sun, Jan 07, 2018 at 08:07:24PM +0200, Amir Goldstein wrote:
>> >> - Check encode/write/decode/read content of lower/upper file handles
>> >> - Check encode/decode/write/read content of lower/upper file handles
>> >> - Check decode/read of unlinked lower/upper files and directories
>> >> - Check decode/read of lower file handles after copy up, link and unlink
>> >> - Check decode/read of lower file handles after rename of parent and self
>> >
>> > I'm wondering that if this should be split into multiple tests somehow,
>> > e.g. tests on regular files, tests on dirs and tests on hardlinks? It
>> > might be eaiser to review and debug when there're test failures. But I
>> > have no strong preference on this.
>> >
>>
>> I prefer not splitting the test, this is a classic test with sub-test cases.
>> I may end up splitting the dir rename tests (open_by_handle -i/-o)
>> to conform with a similar split that you requested in the generic test.
>>
>> >>
>> >> This test does not cover connectable file handles of non-directories,
>> >> because name_to_handle_at() syscall does not support requesting
>> >> connectable file handles.
>> >>
>> >> This test covers only encode/decode of file handles for overlayfs
>> >> configuration of lower and upper on the same fs.
>> >>
>> >> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> >> ---
>> >>  tests/overlay/050     | 291 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  tests/overlay/050.out |  50 +++++++++
>> >>  tests/overlay/group   |   1 +
>> >>  3 files changed, 342 insertions(+)
>> >>  create mode 100755 tests/overlay/050
>> >>  create mode 100644 tests/overlay/050.out
>> >
>> > I ran the test on your ovl-nfs-export-v2 branch and saw failures like:
>> >
>> > --- tests/overlay/050.out       2018-01-16 14:51:11.350000000 +0800
>> > +++ /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad      2018-01-16 15:08:43.487000000 +0800
>> > @@ -45,6 +45,9 @@
>> >  test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
>> >  test_file_handles SCRATCH_MNT -rp -i upper_file_handles
>> >  test_file_handles SCRATCH_MNT -rp -i lower_file_handles
>> > +open_by_handle() returned 116 incorrectly on a linked dir!
>> >  test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
>> >  test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
>> > +open_by_handle() returned 116 incorrectly on a linked dir!
>> >  test_file_handles SCRATCH_MNT/lowertestdir.new -rp -i lower_subdir_file_handles
>> > +open_by_handle() returned 116 incorrectly on a linked dir!
>> >
>> > Are these failures expected?
>> >
>>
>> No. not expected. I wonder which base fs did you test with?
>> Did you have OVERLAY_FS_VERIFY=y in config or verify=on in MOUNT_OPTIONS?
>> (Not that I know any of the above should matter)
>
> I didn't have OVERLAY_FS_VERIFY set in .config, but I did mount with "-o
> verify=on", and underlying fs is xfs. Here is the screenshot:
>
> [root@bootp-73-5-205 xfstests]# OVERLAY_MOUNT_OPTIONS="-o verify=on" ./check -s xfs_4k_crc -overlay overlay/050
> SECTION       -- xfs_4k_crc
> RECREATING    -- overlay on /mnt/testarea/test
> FSTYP         -- overlay
> PLATFORM      -- Linux/x86_64 bootp-73-5-205 4.15.0-rc2.ovl+
> MKFS_OPTIONS  -- -f -b size=4k -m crc=1 /mnt/testarea/scratch
> MOUNT_OPTIONS -- -o verify=on /mnt/testarea/scratch /mnt/testarea/scratch/ovl-mnt
>
> overlay/050      - output mismatch (see /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad)
>     --- tests/overlay/050.out   2018-01-16 14:51:11.350000000 +0800
>     +++ /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad  2018-01-16 19:01:54.984000000 +0800
>     @@ -45,6 +45,9 @@
>      test_file_handles SCRATCH_MNT/lowertestdir/subdir -p -o lower_subdir_file_handles
>      test_file_handles SCRATCH_MNT -rp -i upper_file_handles
>      test_file_handles SCRATCH_MNT -rp -i lower_file_handles
>     +open_by_handle() returned 116 incorrectly on a linked dir!
>      test_file_handles SCRATCH_MNT -rp -i upper_subdir_file_handles
>      test_file_handles SCRATCH_MNT -rp -i lower_subdir_file_handles
>     +open_by_handle() returned 116 incorrectly on a linked dir!
>     ...
>     (Run 'diff -u tests/overlay/050.out /root/xfstests/results//xfs_4k_crc/overlay/050.out.bad'  to see the entire diff)
> Ran: overlay/050
> Failures: overlay/050
> Failed 1 of 1 tests
>
> And I just tried with ext4 as underlying fs and got the same result.
>
>>
>> Do you see any overlayfs warnings in dmesg?
>
> No, there's no warnings nor other useful information in dmesg, just
> mount/umount xfs and drop caches messages.
>

What happened here is quite nice.
You do not have redirect_dir enabled by default and the test didn't
enable it as well.

Then this line in the test:
# Copy up lower dir, index and rename
mv $SCRATCH_MNT/lowertestdir $SCRATCH_MNT/lowertestdir.new/

...doesn't fail, because mv gets -EXDEV and falls back to "recursive move":
- Create new dir
- Move files from old dir to new dir
- Remove old file

When test later tries to decode the file handle encoded from old dir,
old dir has been removed, so the 116 (ESTALE) error is correct for
what happened here.

This error was detected thanks to the patch using the -i/-o options
from "open_by_handle: store and load file handles from file"
The earlier version of this directory rename test (like upstream generic/467)
would not have detected this error because dir file handle was encoded
after the mv command.

The take aways are:
1. Split the dir rename test cases to a separate test
2. Require and enable redirect_dir feature in the dir rename test

Thanks,
Amir.

Re: [PATCH 1/7] open_by_handle: store and load file handles from file

[Amir Goldstein]

On Thu, Jan 11, 2018 at 1:59 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Sun, Jan 07, 2018 at 08:07:19PM +0200, Amir Goldstein wrote:
>> usage:
>>  open_by_handle -p -o <handles_file> <test_dir> [N]
>>  open_by_handle -p -i <handles_file> <test_dir> [N]
>>
>> This will be used to test decoding of file handles after various
>> file systems operations including mount cycle.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> I found that it might be easier to review if the new test that takes use
> of this new function is included in the same patch, so reviewer could
> know how are these new functions being used without switching between
> different patches.
>

OK. I will give it a shot to see how that works out.

>> ---
>>  src/open_by_handle.c | 110 +++++++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 85 insertions(+), 25 deletions(-)
>>
>> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
>> index dbc5b0f..f9dfefc 100644
>> --- a/src/open_by_handle.c
>> +++ b/src/open_by_handle.c
>> @@ -43,30 +43,36 @@ Examples:
>
> The usage info above the "Examples:" can be updated too.
>
> "usage: open_by_handle [-cludmrwapk] <test_dir> [num_files]"
>

OK.

>>
>>     open_by_handle -p <test_dir> [N]
>>
>> -3. Get file handles for existing test set, write data to files,
>> +3. Get file handles for existing test set and write them to a file.
>> +   Read file handles from file and open files by handle:
>> +
>> +   open_by_handle -p -o <handles_file> <test_dir> [N]
>> +   open_by_handle -p -i <handles_file> <test_dir> [N]
>> +
>> +4. Get file handles for existing test set, write data to files,
>>     drop caches, open all files by handle, read and verify written
>>     data, write new data to file:
>>
>>     open_by_handle -rwa <test_dir> [N]
>>
>> -4. Get file handles for existing test set, unlink all test files,
>> +5. Get file handles for existing test set, unlink all test files,
>>     remove test_dir, drop caches, try to open all files by handle
>>     and expect ESTALE:
>>
>>     open_by_handle -dp <test_dir> [N]
>>
>> -5. Get file handles for existing test set, keep open file handles for all
>> +6. Get file handles for existing test set, keep open file handles for all
>>     test files, unlink all test files, drop caches and try to open all files
>>     by handle (should work):
>>
>>     open_by_handle -dk <test_dir> [N]
>>
>> -6. Get file handles for existing test set, rename all test files,
>> +7. Get file handles for existing test set, rename all test files,
>>     drop caches, try to open all files by handle (should work):
>>
>>     open_by_handle -m <test_dir> [N]
>>
>> -7. Get file handles for existing test set, hardlink all test files,
>> +8. Get file handles for existing test set, hardlink all test files,
>>     then unlink the original files, drop caches and try to open all
>>     files by handle (should work):
>>
>> @@ -103,7 +109,7 @@ struct handle {
>>
>>  void usage(void)
>>  {
>> -     fprintf(stderr, "usage: open_by_handle [-cludmrwapk] <test_dir> [num_files]\n");
>> +     fprintf(stderr, "usage: open_by_handle [-cludmrwapk] [<-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");
>> @@ -116,6 +122,8 @@ void usage(void)
>>       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");
>> +     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");
>>       exit(EXIT_FAILURE);
>>  }
>>
>> @@ -131,15 +139,16 @@ int main(int argc, char **argv)
>>       char    *test_dir;
>>       char    *mount_dir;
>>       int     mount_fd, mount_id;
>> +     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;
>>
>> -     if (argc < 2 || argc > 4)
>> +     if (argc < 2)
>>               usage();
>>
>> -     while ((c = getopt(argc, argv, "cludmrwapk")) != -1) {
>> +     while ((c = getopt(argc, argv, "cludmrwapki:o:")) != -1) {
>>               switch (c) {
>>               case 'c':
>>                       create = 1;
>> @@ -176,13 +185,27 @@ int main(int argc, char **argv)
>>               case 'k':
>>                       keepopen = 1;
>>                       break;
>> +             case 'i':
>> +                     in_fd = open(optarg, O_RDONLY);
>> +                     if (in_fd < 0) {
>> +                             perror(optarg);
>> +                             return EXIT_FAILURE;
>> +                     }
>> +                     break;
>> +             case 'o':
>> +                     out_fd = creat(optarg, 0644);
>> +                     if (out_fd < 0) {
>> +                             perror(optarg);
>> +                             return EXIT_FAILURE;
>> +                     }
>> +                     break;
>>               default:
>>                       fprintf(stderr, "illegal option '%s'\n", argv[optind]);
>>               case 'h':
>>                       usage();
>>               }
>>       }
>> -        if (optind == argc || optind > 2)
>> +        if (optind == argc)
>>              usage();
>>       test_dir = argv[optind++];
>>       if (optind < argc)
>> @@ -192,11 +215,14 @@ int main(int argc, char **argv)
>>               usage();
>>       }
>>
>> -     if (parent) {
>> +     if (parent && !in_fd) {
>>               strcpy(dname, test_dir);
>>               /*
>>                * -p flag implies that test_dir is NOT a mount point,
>>                * so its parent can be used as mount_fd for open_by_handle_at.
>> +              * -i flag implies that test_dir IS a mount point, because we
>> +              *  are testing open by handle of dir, which may have been
>> +              *  deleted or renamed.
>
> I'm a bit confused by this comment, is test_dir a mount point if I
> specify both -i and -p? Does that mean -i would override -p regarding to
> test_dir being mount point or not?
>

Yes, I elaborated the documentation.
For a generic utility, the mount point argument should have been split
to a different optional option and default to same value as test_dir, but
I rather keep the test arguments simple, so here goes:

        /*
         * The way we determine the mount_dir to be used for mount_fd argument
         * for open_by_handle_at() depends on other command line arguments:
         *
         * -p flag usually (see -i below) implies that test_dir is NOT a mount
         *    point, but a directory inside a mount point that we will create
         *    and/or encode/decode during the test, so we use test_dir's parent
         *    for mount_fd. Even when not creatig test_dir, if we would use
         *    test_dir as mount_fd, then drop_caches will not drop the test_dir
         *    dcache entry.
         *
         * If -p is not specified, we don't have a hint whether test_dir is a
         *    mount point or not, so we assume the worst case, that it is a
         *    mount point and therefore, we cannnot use parent as mount_fd,
         *    because parent may be on a differnt file system.
         *
         * -i flag, even with -p flag, implies that test_dir IS a mount point,
         *    because we are testing open by handle of dir, which may have been
         *    deleted or renamed and we are not creating nor encoding the
         *    directory file handle. -i flag is meant to be used for tests
         *    after encoding file handles and mount cycle the file system. If
         *    we would require the test to pass in with -ip the test_dir we
         *    want to decode and not the mount point, that would have populated
         *    the dentry cache and the use of -ip flag combination would not
         *    allow testing decode of dir file handle in cold dcache scenario.
         */


>>                */
>>               mount_dir = dirname(dname);
>>               if (create)
>> @@ -241,15 +267,24 @@ int main(int argc, char **argv)
>>       /* sync to get the new inodes to hit the disk */
>>       sync();
>>
>> -     /* create the handles */
>> +     /* create/read the handles */
>
> This loop also write handles out, update the comment accordingly?
>

OK.

>>       for (i=0; i < numfiles; i++) {
>>               sprintf(fname, "%s/file%06d", test_dir, i);
>> -             handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
>> -             ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
>> -             if (ret < 0) {
>> -                     strcat(fname, ": name_to_handle");
>> -                     perror(fname);
>> -                     return EXIT_FAILURE;
>> +             if (in_fd) {
>> +                     ret = read(in_fd, (char *)&handle[i], sizeof(*handle));
>> +                     if (ret < sizeof(*handle)) {
>> +                             strcat(fname, ": read handle");
>> +                             perror(fname);
>> +                             return EXIT_FAILURE;
>> +                     }
>> +             } else {
>> +                     handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
>> +                     ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
>> +                     if (ret < 0) {
>> +                             strcat(fname, ": name_to_handle");
>> +                             perror(fname);
>> +                             return EXIT_FAILURE;
>> +                     }
>>               }
>>               if (keepopen) {
>>                       /* Open without close to keep unlinked files around */
>> @@ -260,15 +295,40 @@ int main(int argc, char **argv)
>>                               return EXIT_FAILURE;
>>                       }
>>               }
>> +             if (out_fd) {
>> +                     ret = write(out_fd, (char *)&handle[i], sizeof(*handle));
>> +                     if (ret < sizeof(*handle)) {
>> +                             strcat(fname, ": write handle");
>> +                             perror(fname);
>
> A short write is not an error, so errno is 0 and error message can be
> confusing: "$file: write handle: Success"

OK. fixed all of those.

Thanks,
Amir.