[PATCH 0/3] Yet another overlayfs exportfs test

[PATCH 0/3] Yet another overlayfs exportfs test

[Amir Goldstein]

Eryu,

This test should pass on upstream kernel.
It provides test coverage for a state of inode/dentry cache that
was not covered by previous tests.
I had to extend open_by_handle again to reproduce the uncovered
cache state.

Miklos,

I used tracing on index lookup with this test to verify correctness
of ("ovl: lookup in inode cache first when decoding lower file handle").
With your review fix, index lookup is called for non-dir after mount
cycle, but not called after opening by file handle again, when the
diconnected dentries are still in cache.

Thanks,
Amir.

Amir Goldstein (3):
  open_by_handle: add -n option to suppress drop caches
  open_by_handle: add -s option to sleep and keep files open by handle
  overlay: test decoding overlay file handles with warm/cold dentry
    cache

 src/open_by_handle.c  |  57 ++++++++++++++-------
 tests/overlay/058     | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/058.out |   9 ++++
 tests/overlay/group   |   1 +
 4 files changed, 186 insertions(+), 18 deletions(-)
 create mode 100755 tests/overlay/058
 create mode 100644 tests/overlay/058.out

-- 
2.7.4

[PATCH 1/3] open_by_handle: add -n option to suppress drop caches

[Amir Goldstein]

By default, open_by_handle drops inode and page caches before trying
to open by file handles. Add an option to suppress drop caches for
testing open by handle with warm caches.

Usage: open_by_handle -n <test_dir> [N]

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

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 78f7ef7..d3beb78 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -27,7 +27,7 @@
 
 /*
 
-usage: open_by_handle [-cludmrwapk] [<-i|-o> <handles_file>] <test_dir> [num_files]
+usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]
 
 Examples:
 
@@ -43,36 +43,39 @@ Examples:
 
    open_by_handle -p <test_dir> [N]
 
-3. Get file handles for existing test set and write them to a file.
-   Read file handles from file and open files by handle:
+3. Get file handles for existing test set and write them to a file:
 
    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,
+4. Read file handles from file and open files by handle without
+   dropping caches beforehand:
+
+   open_by_handle -np -i <handles_file> <test_dir> [N]
+
+5. 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]
 
-5. Get file handles for existing test set, unlink all test files,
+6. 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]
 
-6. Get file handles for existing test set, keep open file handles for all
+7. 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]
 
-7. Get file handles for existing test set, rename all test files,
+8. 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]
 
-8. Get file handles for existing test set, hardlink all test files,
+9. 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):
 
@@ -109,10 +112,11 @@ struct handle {
 
 void usage(void)
 {
-	fprintf(stderr, "usage: open_by_handle [-cludmrwapk] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
+	fprintf(stderr, "usage: open_by_handle [-cludmrwapknh] [<-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");
+	fprintf(stderr, "open_by_handle -n <test_dir> [N] - get file handles of test files and try to open by handle without drop caches\n");
 	fprintf(stderr, "open_by_handle -k <test_dir> [N] - get file handles of files that are kept open, drop caches and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -w <test_dir> [N] - write data to test files before open by handle\n");
 	fprintf(stderr, "open_by_handle -r <test_dir> [N] - read data from test files after open by handle and verify written data\n");
@@ -144,12 +148,12 @@ int main(int argc, char **argv)
 	int	numfiles = 1;
 	int	create = 0, delete = 0, nlink = 1, move = 0;
 	int	rd = 0, wr = 0, wrafter = 0, parent = 0;
-	int	keepopen = 0;
+	int	keepopen = 0, drop_caches = 1;
 
 	if (argc < 2)
 		usage();
 
-	while ((c = getopt(argc, argv, "cludmrwapki:o:")) != -1) {
+	while ((c = getopt(argc, argv, "cludmrwapknhi:o:")) != -1) {
 		switch (c) {
 		case 'c':
 			create = 1;
@@ -186,6 +190,9 @@ int main(int argc, char **argv)
 		case 'k':
 			keepopen = 1;
 			break;
+		case 'n':
+			drop_caches = 0;
+			break;
 		case 'i':
 			infile = optarg;
 			in_fd = open(infile, O_RDONLY);
@@ -438,10 +445,12 @@ int main(int argc, char **argv)
 	 * buftarg page cache is emptied so that the inode cluster has to be
 	 * fetched from disk again for the open_by_handle() call.
 	 */
-	ret = system("echo 3 > /proc/sys/vm/drop_caches");
-	if (ret < 0) {
-		perror("drop_caches");
-		return EXIT_FAILURE;
+	if (drop_caches) {
+		ret = system("echo 3 > /proc/sys/vm/drop_caches");
+		if (ret < 0) {
+			perror("drop_caches");
+			return EXIT_FAILURE;
+		}
 	}
 
 	/*
-- 
2.7.4

[PATCH 2/3] open_by_handle: add -s option to sleep and keep files open by handle

[Amir Goldstein]

This feature is needed for tests that need to open file by handle and
then perform operations while that file is open. This will be used by
an overlay test to keep disconnected dentries in dcache.

Usage: open_by_handle -s <test_dir>

On success, the program will run until it gets a terminating signal.

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

diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index d3beb78..e4bd29b 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -27,7 +27,7 @@
 
 /*
 
-usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]
+usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]
 
 Examples:
 
@@ -48,9 +48,9 @@ Examples:
    open_by_handle -p -o <handles_file> <test_dir> [N]
 
 4. Read file handles from file and open files by handle without
-   dropping caches beforehand:
+   dropping caches beforehand. Sleep afterhand to keep files open:
 
-   open_by_handle -np -i <handles_file> <test_dir> [N]
+   open_by_handle -nps -i <handles_file> <test_dir> [N]
 
 5. Get file handles for existing test set, write data to files,
    drop caches, open all files by handle, read and verify written
@@ -112,7 +112,7 @@ struct handle {
 
 void usage(void)
 {
-	fprintf(stderr, "usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
+	fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-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");
@@ -128,6 +128,7 @@ void usage(void)
 	fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
 	fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
 	fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
+	fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
 	exit(EXIT_FAILURE);
 }
 
@@ -148,12 +149,12 @@ int main(int argc, char **argv)
 	int	numfiles = 1;
 	int	create = 0, delete = 0, nlink = 1, move = 0;
 	int	rd = 0, wr = 0, wrafter = 0, parent = 0;
-	int	keepopen = 0, drop_caches = 1;
+	int	keepopen = 0, drop_caches = 1, sleep_loop = 0;
 
 	if (argc < 2)
 		usage();
 
-	while ((c = getopt(argc, argv, "cludmrwapknhi:o:")) != -1) {
+	while ((c = getopt(argc, argv, "cludmrwapknhi:o:s")) != -1) {
 		switch (c) {
 		case 'c':
 			create = 1;
@@ -209,6 +210,9 @@ int main(int argc, char **argv)
 				return EXIT_FAILURE;
 			}
 			break;
+		case 's':
+			sleep_loop = 1;
+			break;
 		default:
 			fprintf(stderr, "illegal option '%s'\n", argv[optind]);
 		case 'h':
@@ -308,6 +312,7 @@ int main(int argc, char **argv)
 				fprintf(stderr, "failed reading file handle #%d from '%s'\n", i, infile);
 				return EXIT_FAILURE;
 			}
+			continue;
 		} else {
 			handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
 			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
@@ -478,7 +483,8 @@ int main(int argc, char **argv)
 				perror(fname);
 				return EXIT_FAILURE;
 			}
-			close(fd);
+			if (!sleep_loop)
+				close(fd);
 			continue;
 		} else if (!nlink && !keepopen && fd < 0 && (errno == ENOENT || errno == ESTALE)) {
 			continue;
@@ -529,7 +535,8 @@ int main(int argc, char **argv)
 					return EXIT_FAILURE;
 				}
 			}
-			close(fd);
+			if (!sleep_loop)
+				close(fd);
 		} else if (nlink || !(errno == ENOENT || errno == ESTALE)) {
 			printf("open_by_handle(%s) returned %d incorrectly on %s dir!\n",
 					dname, errno,
@@ -540,5 +547,10 @@ int main(int argc, char **argv)
 
 	if (failed)
 		return EXIT_FAILURE;
+
+	/* Sleep keeping files open by handle */
+	while (sleep_loop)
+		sleep(1);
+
 	return EXIT_SUCCESS;
 }
-- 
2.7.4

[PATCH 3/3] overlay: test decoding overlay file handles with warm/cold dentry cache

[Amir Goldstein]

When opening a non-dir by file handle and the decoded inode/dentry
are not in cache, the resulting dentry is "disconnected" (i.e. unknown
path). This is a common case that is already covered by previous tests.
This test covers the case of decoding an overlay file handle, while a
disconnected dentry is still in cache.

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

diff --git a/tests/overlay/058 b/tests/overlay/058
new file mode 100755
index 0000000..4c4ca45
--- /dev/null
+++ b/tests/overlay/058
@@ -0,0 +1,137 @@
+#! /bin/bash
+# FS QA Test No. 058
+#
+# Test decoding overlay file handles with warm/cold dentry cache
+#
+# When opening a non-dir by file handle and the decoded inode/dentry
+# are not in cache, the resulting dentry is "disconnected" (i.e. unknown
+# path). This is a common case that is already covered by previous tests.
+# This test covers the case of decoding an overlay file handle, while a
+# disconnected dentry is still in cache.
+#
+# This test requires and enables overlayfs NFS export support.
+# NFS export support depends on and requires overlayfs index feature.
+#
+#-----------------------------------------------------------------------
+# Copyright (C) 2018 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()
+{
+	$KILLALL_PROG -9 open_by_handle >/dev/null 2>&1
+	wait
+	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_test_program "open_by_handle"
+# We need to require all features together, because nfs_export cannot
+# be enabled when index is disabled
+_require_scratch_overlay_features index nfs_export
+_require_command "$KILLALL_PROG" killall
+
+# All overlay dirs are on scratch partition
+lower=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
+upper=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
+work=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
+
+# Keep this number > 1, because open_by_handle -rp does faccessat()
+# on file file000000, which makes that dentry connected
+NUMFILES=10
+
+# Create test dir and empty test files
+create_test_files()
+{
+	local dir=$1
+	local opt=$2
+
+	mkdir -p $dir
+	$here/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 | \
+				sed -e "s,$tmp\.,,g"
+	$here/src/open_by_handle $* $dir $NUMFILES
+}
+
+# Create lower/upper dir and files
+_scratch_mkfs
+create_test_files $upper/uppertestdir -w
+create_test_files $lower/lowertestdir -w
+_scratch_mount -o "index=on,nfs_export=on"
+# Encode upper file handles
+test_file_handles $SCRATCH_MNT/uppertestdir -p -o $tmp.upper_file_handles
+# Encode lower file handles
+test_file_handles $SCRATCH_MNT/lowertestdir -p -o $tmp.lower_file_handles
+
+# Check decode and read from stored file handles with warm caches -
+# At this time, all non-dir dentries are connected, because the entries
+# were created on lookup, before encoding the file handles.
+test_file_handles $SCRATCH_MNT -rnp -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -rnp -i $tmp.lower_file_handles
+
+# Check decode and read/readdir from stored file handles with cold caches -
+# -s sleeps in the background to keep files open and keep disconnected
+# overlay dentries in cache
+_scratch_cycle_mount "index=on,nfs_export=on"
+test_file_handles $SCRATCH_MNT -rnps -i $tmp.upper_file_handles &
+# Give the above 1 second to get to sleep loop
+sleep 1
+test_file_handles $SCRATCH_MNT -rnps -i $tmp.lower_file_handles &
+# Give the above 1 second to get to sleep loop
+sleep 1
+
+# Check decode and read/readdir from stored file handles with warm caches -
+# At this time, all non-dir dentries are disconnected, because there was
+# no lookup to the files since drop caches. The expection to this rule
+# is lower and upper file000000. open_by_handle -rp above did faccessat()
+# on file file000000, which created a connected dentry alias in addition
+# to the disconnected dentry alias.
+test_file_handles $SCRATCH_MNT -rnp -i $tmp.upper_file_handles
+test_file_handles $SCRATCH_MNT -rnp -i $tmp.lower_file_handles
+
+# SIGPIPE avoids Terminated/Killed message from bash
+$KILLALL_PROG -q -13 open_by_handle
+wait
+
+status=0
+exit
diff --git a/tests/overlay/058.out b/tests/overlay/058.out
new file mode 100644
index 0000000..6ff9811
--- /dev/null
+++ b/tests/overlay/058.out
@@ -0,0 +1,9 @@
+QA output created by 058
+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 -rnp -i upper_file_handles
+test_file_handles SCRATCH_MNT -rnp -i lower_file_handles
+test_file_handles SCRATCH_MNT -rnps -i upper_file_handles
+test_file_handles SCRATCH_MNT -rnps -i lower_file_handles
+test_file_handles SCRATCH_MNT -rnp -i upper_file_handles
+test_file_handles SCRATCH_MNT -rnp -i lower_file_handles
diff --git a/tests/overlay/group b/tests/overlay/group
index c49ae3a..359ed42 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -60,3 +60,4 @@
 055 auto quick copyup redirect exportfs nonsamefs
 056 auto quick fsck
 057 auto quick redirect
+058 auto quick exportfs
-- 
2.7.4

Re: [PATCH 1/3] open_by_handle: add -n option to suppress drop caches

[Eryu Guan]

On Sun, Mar 25, 2018 at 05:53:09PM +0300, Amir Goldstein wrote:
> By default, open_by_handle drops inode and page caches before trying
> to open by file handles. Add an option to suppress drop caches for
> testing open by handle with warm caches.
> 
> Usage: open_by_handle -n <test_dir> [N]
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  src/open_by_handle.c | 41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index 78f7ef7..d3beb78 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -27,7 +27,7 @@
>  
>  /*
>  
> -usage: open_by_handle [-cludmrwapk] [<-i|-o> <handles_file>] <test_dir> [num_files]
> +usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]

Better to mention that it fixes the 'h' option too in the commit log, or
maybe just send a separate patch?

Otherwise looks fine to me.

Thanks,
Eryu

Re: [PATCH 2/3] open_by_handle: add -s option to sleep and keep files open by handle

[Eryu Guan]

On Sun, Mar 25, 2018 at 05:53:10PM +0300, Amir Goldstein wrote:
> This feature is needed for tests that need to open file by handle and
> then perform operations while that file is open. This will be used by
> an overlay test to keep disconnected dentries in dcache.
> 
> Usage: open_by_handle -s <test_dir>
> 
> On success, the program will run until it gets a terminating signal.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  src/open_by_handle.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
> index d3beb78..e4bd29b 100644
> --- a/src/open_by_handle.c
> +++ b/src/open_by_handle.c
> @@ -27,7 +27,7 @@
>  
>  /*
>  
> -usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]
> +usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]
>  
>  Examples:
>  
> @@ -48,9 +48,9 @@ Examples:
>     open_by_handle -p -o <handles_file> <test_dir> [N]
>  
>  4. Read file handles from file and open files by handle without
> -   dropping caches beforehand:
> +   dropping caches beforehand. Sleep afterhand to keep files open:
>  
> -   open_by_handle -np -i <handles_file> <test_dir> [N]
> +   open_by_handle -nps -i <handles_file> <test_dir> [N]
>  
>  5. Get file handles for existing test set, write data to files,
>     drop caches, open all files by handle, read and verify written
> @@ -112,7 +112,7 @@ struct handle {
>  
>  void usage(void)
>  {
> -	fprintf(stderr, "usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
> +	fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-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");
> @@ -128,6 +128,7 @@ void usage(void)
>  	fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
>  	fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
>  	fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
> +	fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
>  	exit(EXIT_FAILURE);
>  }
>  
> @@ -148,12 +149,12 @@ int main(int argc, char **argv)
>  	int	numfiles = 1;
>  	int	create = 0, delete = 0, nlink = 1, move = 0;
>  	int	rd = 0, wr = 0, wrafter = 0, parent = 0;
> -	int	keepopen = 0, drop_caches = 1;
> +	int	keepopen = 0, drop_caches = 1, sleep_loop = 0;
>  
>  	if (argc < 2)
>  		usage();
>  
> -	while ((c = getopt(argc, argv, "cludmrwapknhi:o:")) != -1) {
> +	while ((c = getopt(argc, argv, "cludmrwapknhi:o:s")) != -1) {
>  		switch (c) {
>  		case 'c':
>  			create = 1;
> @@ -209,6 +210,9 @@ int main(int argc, char **argv)
>  				return EXIT_FAILURE;
>  			}
>  			break;
> +		case 's':
> +			sleep_loop = 1;
> +			break;
>  		default:
>  			fprintf(stderr, "illegal option '%s'\n", argv[optind]);
>  		case 'h':
> @@ -308,6 +312,7 @@ int main(int argc, char **argv)
>  				fprintf(stderr, "failed reading file handle #%d from '%s'\n", i, infile);
>  				return EXIT_FAILURE;
>  			}
> +			continue;

With this 'continue', it seems we can't keep file open when reading file
handles from a file?

>  		} else {
>  			handle[i].fh.handle_bytes = MAX_HANDLE_SZ;
>  			ret = name_to_handle_at(AT_FDCWD, fname, &handle[i].fh, &mount_id, 0);
> @@ -478,7 +483,8 @@ int main(int argc, char **argv)
>  				perror(fname);
>  				return EXIT_FAILURE;
>  			}
> -			close(fd);
> +			if (!sleep_loop)
> +				close(fd);
>  			continue;
>  		} else if (!nlink && !keepopen && fd < 0 && (errno == ENOENT || errno == ESTALE)) {
>  			continue;
> @@ -529,7 +535,8 @@ int main(int argc, char **argv)
>  					return EXIT_FAILURE;
>  				}
>  			}
> -			close(fd);
> +			if (!sleep_loop)
> +				close(fd);
>  		} else if (nlink || !(errno == ENOENT || errno == ESTALE)) {
>  			printf("open_by_handle(%s) returned %d incorrectly on %s dir!\n",
>  					dname, errno,
> @@ -540,5 +547,10 @@ int main(int argc, char **argv)
>  
>  	if (failed)
>  		return EXIT_FAILURE;
> +
> +	/* Sleep keeping files open by handle */
> +	while (sleep_loop)
> +		sleep(1);
> +

Better to mention that the program needs to be killed explicitly in
comment.

Otherwise looks fine to me.

Thanks,
Eryu

>  	return EXIT_SUCCESS;
>  }
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] overlay: test decoding overlay file handles with warm/cold dentry cache

[Eryu Guan]

On Sun, Mar 25, 2018 at 05:53:11PM +0300, Amir Goldstein wrote:
> When opening a non-dir by file handle and the decoded inode/dentry
> are not in cache, the resulting dentry is "disconnected" (i.e. unknown
> path). This is a common case that is already covered by previous tests.
> This test covers the case of decoding an overlay file handle, while a
> disconnected dentry is still in cache.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  tests/overlay/058     | 137 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/058.out |   9 ++++
>  tests/overlay/group   |   1 +
>  3 files changed, 147 insertions(+)
>  create mode 100755 tests/overlay/058
>  create mode 100644 tests/overlay/058.out
> 
> diff --git a/tests/overlay/058 b/tests/overlay/058
> new file mode 100755
> index 0000000..4c4ca45
> --- /dev/null
> +++ b/tests/overlay/058
> @@ -0,0 +1,137 @@
> +#! /bin/bash
> +# FS QA Test No. 058
> +#
> +# Test decoding overlay file handles with warm/cold dentry cache
> +#
> +# When opening a non-dir by file handle and the decoded inode/dentry
> +# are not in cache, the resulting dentry is "disconnected" (i.e. unknown
> +# path). This is a common case that is already covered by previous tests.
> +# This test covers the case of decoding an overlay file handle, while a
> +# disconnected dentry is still in cache.
> +#
> +# This test requires and enables overlayfs NFS export support.
> +# NFS export support depends on and requires overlayfs index feature.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (C) 2018 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()
> +{
> +	$KILLALL_PROG -9 open_by_handle >/dev/null 2>&1
> +	wait
> +	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_test_program "open_by_handle"
> +# We need to require all features together, because nfs_export cannot
> +# be enabled when index is disabled
> +_require_scratch_overlay_features index nfs_export
> +_require_command "$KILLALL_PROG" killall
> +
> +# All overlay dirs are on scratch partition
> +lower=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
> +upper=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> +work=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
> +
> +# Keep this number > 1, because open_by_handle -rp does faccessat()
> +# on file file000000, which makes that dentry connected
> +NUMFILES=10
> +
> +# Create test dir and empty test files
> +create_test_files()
> +{
> +	local dir=$1
> +	local opt=$2
> +
> +	mkdir -p $dir
> +	$here/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 | \
> +				sed -e "s,$tmp\.,,g"
> +	$here/src/open_by_handle $* $dir $NUMFILES
> +}
> +
> +# Create lower/upper dir and files
> +_scratch_mkfs
> +create_test_files $upper/uppertestdir -w
> +create_test_files $lower/lowertestdir -w
> +_scratch_mount -o "index=on,nfs_export=on"
> +# Encode upper file handles
> +test_file_handles $SCRATCH_MNT/uppertestdir -p -o $tmp.upper_file_handles
> +# Encode lower file handles
> +test_file_handles $SCRATCH_MNT/lowertestdir -p -o $tmp.lower_file_handles
> +
> +# Check decode and read from stored file handles with warm caches -
> +# At this time, all non-dir dentries are connected, because the entries
> +# were created on lookup, before encoding the file handles.
> +test_file_handles $SCRATCH_MNT -rnp -i $tmp.upper_file_handles
> +test_file_handles $SCRATCH_MNT -rnp -i $tmp.lower_file_handles
> +
> +# Check decode and read/readdir from stored file handles with cold caches -
> +# -s sleeps in the background to keep files open and keep disconnected
> +# overlay dentries in cache
> +_scratch_cycle_mount "index=on,nfs_export=on"
> +test_file_handles $SCRATCH_MNT -rnps -i $tmp.upper_file_handles &
> +# Give the above 1 second to get to sleep loop
> +sleep 1
> +test_file_handles $SCRATCH_MNT -rnps -i $tmp.lower_file_handles &
> +# Give the above 1 second to get to sleep loop
> +sleep 1
> +
> +# Check decode and read/readdir from stored file handles with warm caches -
> +# At this time, all non-dir dentries are disconnected, because there was
> +# no lookup to the files since drop caches. The expection to this rule
> +# is lower and upper file000000. open_by_handle -rp above did faccessat()
> +# on file file000000, which created a connected dentry alias in addition
> +# to the disconnected dentry alias.
> +test_file_handles $SCRATCH_MNT -rnp -i $tmp.upper_file_handles
> +test_file_handles $SCRATCH_MNT -rnp -i $tmp.lower_file_handles
> +
> +# SIGPIPE avoids Terminated/Killed message from bash
> +$KILLALL_PROG -q -13 open_by_handle

I think SIGINT is more proper?

Thanks,
Eryu

> +wait
> +
> +status=0
> +exit
> diff --git a/tests/overlay/058.out b/tests/overlay/058.out
> new file mode 100644
> index 0000000..6ff9811
> --- /dev/null
> +++ b/tests/overlay/058.out
> @@ -0,0 +1,9 @@
> +QA output created by 058
> +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 -rnp -i upper_file_handles
> +test_file_handles SCRATCH_MNT -rnp -i lower_file_handles
> +test_file_handles SCRATCH_MNT -rnps -i upper_file_handles
> +test_file_handles SCRATCH_MNT -rnps -i lower_file_handles
> +test_file_handles SCRATCH_MNT -rnp -i upper_file_handles
> +test_file_handles SCRATCH_MNT -rnp -i lower_file_handles
> diff --git a/tests/overlay/group b/tests/overlay/group
> index c49ae3a..359ed42 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -60,3 +60,4 @@
>  055 auto quick copyup redirect exportfs nonsamefs
>  056 auto quick fsck
>  057 auto quick redirect
> +058 auto quick exportfs
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] open_by_handle: add -s option to sleep and keep files open by handle

[Amir Goldstein]

On Thu, Mar 29, 2018 at 6:30 AM, Eryu Guan <guaneryu@gmail.com> wrote:
> On Sun, Mar 25, 2018 at 05:53:10PM +0300, Amir Goldstein wrote:
>> This feature is needed for tests that need to open file by handle and
>> then perform operations while that file is open. This will be used by
>> an overlay test to keep disconnected dentries in dcache.
>>
>> Usage: open_by_handle -s <test_dir>
>>
>> On success, the program will run until it gets a terminating signal.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  src/open_by_handle.c | 28 ++++++++++++++++++++--------
>>  1 file changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/open_by_handle.c b/src/open_by_handle.c
>> index d3beb78..e4bd29b 100644
>> --- a/src/open_by_handle.c
>> +++ b/src/open_by_handle.c
>> @@ -27,7 +27,7 @@
>>
>>  /*
>>
>> -usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]
>> +usage: open_by_handle [-cludmrwapknhs] [<-i|-o> <handles_file>] <test_dir> [num_files]
>>
>>  Examples:
>>
>> @@ -48,9 +48,9 @@ Examples:
>>     open_by_handle -p -o <handles_file> <test_dir> [N]
>>
>>  4. Read file handles from file and open files by handle without
>> -   dropping caches beforehand:
>> +   dropping caches beforehand. Sleep afterhand to keep files open:
>>
>> -   open_by_handle -np -i <handles_file> <test_dir> [N]
>> +   open_by_handle -nps -i <handles_file> <test_dir> [N]
>>
>>  5. Get file handles for existing test set, write data to files,
>>     drop caches, open all files by handle, read and verify written
>> @@ -112,7 +112,7 @@ struct handle {
>>
>>  void usage(void)
>>  {
>> -     fprintf(stderr, "usage: open_by_handle [-cludmrwapknh] [<-i|-o> <handles_file>] <test_dir> [num_files]\n");
>> +     fprintf(stderr, "usage: open_by_handle [-cludmrwapknhs] [<-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");
>> @@ -128,6 +128,7 @@ void usage(void)
>>       fprintf(stderr, "open_by_handle -p <test_dir>     - create/delete and try to open by handle also test_dir itself\n");
>>       fprintf(stderr, "open_by_handle -i <handles_file> <test_dir> [N] - read test files handles from file and try to open by handle\n");
>>       fprintf(stderr, "open_by_handle -o <handles_file> <test_dir> [N] - get file handles of test files and write handles to file\n");
>> +     fprintf(stderr, "open_by_handle -s <test_dir> [N] - wait in sleep loop after opening files by handle to keep them open\n");
>>       exit(EXIT_FAILURE);
>>  }
>>
>> @@ -148,12 +149,12 @@ int main(int argc, char **argv)
>>       int     numfiles = 1;
>>       int     create = 0, delete = 0, nlink = 1, move = 0;
>>       int     rd = 0, wr = 0, wrafter = 0, parent = 0;
>> -     int     keepopen = 0, drop_caches = 1;
>> +     int     keepopen = 0, drop_caches = 1, sleep_loop = 0;
>>
>>       if (argc < 2)
>>               usage();
>>
>> -     while ((c = getopt(argc, argv, "cludmrwapknhi:o:")) != -1) {
>> +     while ((c = getopt(argc, argv, "cludmrwapknhi:o:s")) != -1) {
>>               switch (c) {
>>               case 'c':
>>                       create = 1;
>> @@ -209,6 +210,9 @@ int main(int argc, char **argv)
>>                               return EXIT_FAILURE;
>>                       }
>>                       break;
>> +             case 's':
>> +                     sleep_loop = 1;
>> +                     break;
>>               default:
>>                       fprintf(stderr, "illegal option '%s'\n", argv[optind]);
>>               case 'h':
>> @@ -308,6 +312,7 @@ int main(int argc, char **argv)
>>                               fprintf(stderr, "failed reading file handle #%d from '%s'\n", i, infile);
>>                               return EXIT_FAILURE;
>>                       }
>> +                     continue;
>
> With this 'continue', it seems we can't keep file open when reading file
> handles from a file?

It's a leftover - I was thinking to reuse the meaning of -k -i for
this functionality
but decided to go with a new flag.

With fix in v2.

Thanks,
Amir.