public inbox for fstests@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] fstests: generic test for NFS handles
@ 2017-04-19 16:29 Amir Goldstein
  2017-04-19 16:29 ` [PATCH v2 1/4] fstests: remove IRIX test program open_unlink Amir Goldstein
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Amir Goldstein @ 2017-04-19 16:29 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

Hi Eryu,

Changes since v1:
- Build time checks for open_by_handle_at()
- Update docs
- Improve usage and commit messages
- Remove unused program open_unlink
- Squash _require_exportfs into patch 3
- Change test to run on $TEST_DIR/$seq-dir

Amir Goldstein (4):
  fstests: remove IRIX test program open_unlink
  src/open_by_handle: program to exercise open_by_handle_at() syscall
  src/open_by_handle: flexible usage options
  fstests: add generic test for file handles

 .gitignore                   |   1 +
 aclocal.m4                   |  15 +++
 common/rc                    |  10 ++
 configure.ac                 |   1 +
 doc/auxiliary-programs.txt   |  16 ++-
 doc/requirement-checking.txt |   7 ++
 include/builddefs.in         |   1 +
 src/Makefile                 |   7 +-
 src/open_by_handle.c         | 261 +++++++++++++++++++++++++++++++++++++++++++
 src/open_unlink.c            | 147 ------------------------
 tests/generic/426            |  74 ++++++++++++
 tests/generic/426.out        |   2 +
 tests/generic/group          |   1 +
 13 files changed, 390 insertions(+), 153 deletions(-)
 create mode 100644 src/open_by_handle.c
 delete mode 100644 src/open_unlink.c
 create mode 100755 tests/generic/426
 create mode 100644 tests/generic/426.out

-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] fstests: remove IRIX test program open_unlink
  2017-04-19 16:29 [PATCH v2 0/4] fstests: generic test for NFS handles Amir Goldstein
@ 2017-04-19 16:29 ` Amir Goldstein
  2017-04-19 16:29 ` [PATCH v2 2/4] src/open_by_handle: program to exercise open_by_handle_at() syscall Amir Goldstein
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Amir Goldstein @ 2017-04-19 16:29 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

It does not seem to be used by any test.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 src/Makefile      |   3 --
 src/open_unlink.c | 147 ------------------------------------------------------
 2 files changed, 150 deletions(-)
 delete mode 100644 src/open_unlink.c

diff --git a/src/Makefile b/src/Makefile
index e62d7a9..247383c 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -40,14 +40,11 @@ ifeq ($(HAVE_FALLOCATE), true)
 LCFLAGS += -DHAVE_FALLOCATE
 endif
 
-IRIX_TARGETS = open_unlink
-
 ifeq ($(PKG_PLATFORM),linux)
 TARGETS += $(LINUX_TARGETS)
 endif
 
 ifeq ($(PKG_PLATFORM),irix)
-TARGETS += $(IRIX_TARGETS)
 LLDLIBS += -lgen
 endif
 
diff --git a/src/open_unlink.c b/src/open_unlink.c
deleted file mode 100644
index 9f83929..0000000
--- a/src/open_unlink.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/*
- * Copyright (c) 2005 Silicon Graphics, Inc.
- * All Rights Reserved.
- *
- * 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
- */
-#include <stdlib.h>
-#include <stdio.h>
-#include <unistd.h>
-#include <string.h>
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <sys/attributes.h>
-#include <sys/fs/xfs_itable.h>
-
-#ifndef ATTR_PARENT
-#define ATTR_PARENT 0x0040
-#endif
-
-#define EA_LISTBUF_SZ 16384
-
-int
-main(int argc, char *argv[])
-{
-	char *path;
-	int fd;
-	char *prog = argv[0];
-	void *handle;
-	size_t hlen;
-	attrlist_cursor_t cursor;
-	attrlist_t *ea_listbuf = (attrlist_t *)malloc(EA_LISTBUF_SZ);
-	uint64_t parent_ino;
-	uint64_t link_cnt;
-	int sts;
-	int nameix;
-	attrlist_ent_t *entp;
-
-	if (argc < 2) {
-		fprintf(stderr, "%s: missing pathname argument\n", prog);
-		return 1;
-	}
-	path = argv[1];
-
-	if (argc > 2) {
-		fprintf(stderr, "%s: too many arguments\n", prog);
-		return 1;
-	}
-
-	/* if file already exists then error out */
-	if (access(path, F_OK) == 0) {
-		fprintf(stderr, "%s: file \"%s\" already exists\n", prog, path);
-		return 1;
-	}
-
-	fd = open(path, O_RDWR|O_CREAT|O_EXCL);
-	if (fd == -1) {
-		fprintf(stderr, "%s: failed to create \"%s\": %s\n", prog, path, strerror(errno));
-		return 1;
-	}
-
-
-	/* for linux libhandle version - to set libhandle fsfd cache */
-	{
-		void *fshandle;
-		size_t fshlen;
-
-		if (path_to_fshandle(path, &fshandle, &fshlen) != 0) {
-			fprintf(stderr, "%s: failed path_to_fshandle \"%s\": %s\n",
-				prog, path, strerror(errno));
-			return 1;
-		}
-	}
-
-
-	/* 
-	 * look at parentptr EAs and see if the path exists now that
-	 * it has been unlinked.
-	 */ 
-	if (fd_to_handle(fd, &handle, &hlen) != 0) {
-		fprintf(stderr, "%s: failed to fd_to_handle \"%s\": %s\n",
-			prog, path, strerror(errno));
-		return 1;
-	}
-
-	if (unlink(path) == -1) {
-		fprintf(stderr, "%s: failed to unlink \"%s\": %s\n", prog, path, strerror(errno));
-		return 1;
-	}
-
-	memset(&cursor, 0, sizeof(cursor));
-
-	/* just do one call - don't bother with continue logic */
-	sts = attr_list_by_handle(handle,
-			hlen,
-			(char*)ea_listbuf,
-			EA_LISTBUF_SZ,
-			ATTR_PARENT,
-			&cursor);
-	if (sts != 0) {
-		fprintf(stderr, "%s: failed to list attr for \"%s\": %s\n", prog, path, strerror(errno));
-		return 1;
-	}
-
-	printf("ea count = %d\n", ea_listbuf->al_count);
-
-	/*
-	 * process EA list
-	 */
-	for (nameix = 0; nameix < ea_listbuf->al_count; nameix++) {
-		entp = ATTR_ENTRY(ea_listbuf, nameix);
-
-		sts = sscanf(entp->a_name, "%llx %llx", &parent_ino, &link_cnt);
-		if (sts != 2) {
-			fprintf(stderr,
-				"inode-path for \"%s\" is corrupted\n",
-				path);
-
-			/* go onto next EA name */
-			continue;
-		}
-		/* just print the info out */
-		printf("[%d] parent_ino = %llu, link_cnt = %llu\n", nameix, parent_ino, link_cnt);
-	}
-
-
-	free_handle(handle, hlen);
-
-	if (close(fd) == -1) {
-		fprintf(stderr, "%s: failed to close \"%s\": %s\n", prog, path, strerror(errno));
-		return 1;
-	}
-
-	return 0;
-}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] src/open_by_handle: program to exercise open_by_handle_at() syscall
  2017-04-19 16:29 [PATCH v2 0/4] fstests: generic test for NFS handles Amir Goldstein
  2017-04-19 16:29 ` [PATCH v2 1/4] fstests: remove IRIX test program open_unlink Amir Goldstein
@ 2017-04-19 16:29 ` Amir Goldstein
  2017-04-22  7:55   ` Rock Lee
  2017-04-19 16:29 ` [PATCH v2 3/4] src/open_by_handle: flexible usage options Amir Goldstein
  2017-04-19 16:29 ` [PATCH v2 4/4] fstests: add generic test for file handles Amir Goldstein
  3 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2017-04-19 16:29 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

This is a clone of src/stale_handle.c test that uses generic
open_by_handle_at() syscall instead of the xfs specific ioctl.

No test is using this program yet.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .gitignore           |   1 +
 aclocal.m4           |  15 ++++++
 configure.ac         |   1 +
 include/builddefs.in |   1 +
 src/Makefile         |   4 ++
 src/open_by_handle.c | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 165 insertions(+)
 create mode 100644 src/open_by_handle.c

diff --git a/.gitignore b/.gitignore
index 0336555..ded4a61 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@
 /src/multi_open_unlink
 /src/nametest
 /src/nsexec
+/src/open_by_handle
 /src/permname
 /src/preallo_rw_pattern_reader
 /src/preallo_rw_pattern_writer
diff --git a/aclocal.m4 b/aclocal.m4
index f3412e1..829fa10 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -37,6 +37,21 @@ AC_DEFUN([AC_PACKAGE_WANT_FALLOCATE],
       [ have_fallocate=false; AC_MSG_RESULT(no) ])
     AC_SUBST(have_fallocate)
   ])
+
+AC_DEFUN([AC_PACKAGE_WANT_OPEN_BY_HANDLE_AT],
+  [ AC_MSG_CHECKING([for open_by_handle_at])
+    AC_TRY_LINK([
+#define _GNU_SOURCE
+#include <fcntl.h>
+      ],
+      [
+          struct file_handle fh;
+          open_by_handle_at(0, &fh, 0);
+      ],
+      [ have_open_by_handle_at=true; AC_MSG_RESULT(yes) ],
+      [ have_open_by_handle_at=false; AC_MSG_RESULT(no) ])
+    AC_SUBST(have_open_by_handle_at)
+  ])
 m4_include([m4/multilib.m4])
 m4_include([m4/package_acldev.m4])
 m4_include([m4/package_aiodev.m4])
diff --git a/configure.ac b/configure.ac
index 246f92e..1285bf4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,7 @@ in
 		AC_PACKAGE_WANT_DMAPI
 		AC_PACKAGE_WANT_LINUX_FIEMAP_H
 		AC_PACKAGE_WANT_FALLOCATE
+		AC_PACKAGE_WANT_OPEN_BY_HANDLE_AT
 		AC_PACKAGE_WANT_LINUX_PRCTL_H
 		AC_PACKAGE_WANT_LINUX_FS_H
 		AC_PACKAGE_WANT_SSL
diff --git a/include/builddefs.in b/include/builddefs.in
index 24f838f..2725037 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -61,6 +61,7 @@ ENABLE_SHARED = @enable_shared@
 HAVE_DB = @have_db@
 HAVE_AIO = @have_aio@
 HAVE_FALLOCATE = @have_fallocate@
+HAVE_OPEN_BY_HANDLE_AT = @have_open_by_handle_at@
 HAVE_SSL = @have_ssl@
 HAVE_DMAPI = @have_dmapi@
 HAVE_ATTR_LIST = @have_attr_list@
diff --git a/src/Makefile b/src/Makefile
index 247383c..abfd873 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -36,6 +36,10 @@ ifeq ($(HAVE_FIEMAP), true)
 LINUX_TARGETS += fiemap-tester
 endif
 
+ifeq ($(HAVE_OPEN_BY_HANDLE_AT), true)
+LINUX_TARGETS += open_by_handle
+endif
+
 ifeq ($(HAVE_FALLOCATE), true)
 LCFLAGS += -DHAVE_FALLOCATE
 endif
diff --git a/src/open_by_handle.c b/src/open_by_handle.c
new file mode 100644
index 0000000..76510ff
--- /dev/null
+++ b/src/open_by_handle.c
@@ -0,0 +1,143 @@
+/*
+ * open_by_handle.c - attempt to create a file handle and open it
+ *                    with open_by_handle_at() syscall
+ *
+ * Copyright (C) 2017 CTERA Networks. All Rights Reserved.
+ * Author: Amir Goldstein <amir73il@gmail.com>
+ *
+ * from:
+ *  stale_handle.c
+ *
+ *  Copyright (C) 2010 Red Hat, Inc. All Rights reserved.
+ *
+ *  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; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will 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 to the Free Software
+ *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <errno.h>
+#include <linux/limits.h>
+
+#define NUMFILES 1024
+
+struct handle {
+	struct file_handle fh;
+	unsigned char fid[MAX_HANDLE_SZ];
+} handle[NUMFILES];
+
+int main(int argc, char **argv)
+{
+	int	i;
+	int	fd;
+	int	ret;
+	int	failed = 0;
+	char	fname[PATH_MAX];
+	char	*test_dir;
+	int	mount_fd, mount_id;
+
+	if (argc != 2) {
+		fprintf(stderr, "usage: open_by_handle <test_dir>\n");
+		return EXIT_FAILURE;
+	}
+
+	test_dir = argv[1];
+	mount_fd = open(test_dir, O_RDONLY|O_DIRECTORY);
+	if (mount_fd < 0) {
+		perror("open test_dir");
+		return EXIT_FAILURE;
+	}
+
+	/*
+	 * create a large number of files to force allocation of new inode
+	 * chunks on disk.
+	 */
+	for (i=0; i < NUMFILES; i++) {
+		sprintf(fname, "%s/file%06d", test_dir, i);
+		fd = open(fname, O_RDWR | O_CREAT | O_TRUNC, 0644);
+		if (fd < 0) {
+			printf("Warning (%s,%d), open(%s) failed.\n", __FILE__, __LINE__, fname);
+			perror(fname);
+			return EXIT_FAILURE;
+		}
+		close(fd);
+	}
+
+	/* sync to get the new inodes to hit the disk */
+	sync();
+
+	/* create 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) {
+			perror("name_to_handle");
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* unlink the files */
+	for (i=0; i < NUMFILES; i++) {
+		sprintf(fname, "%s/file%06d", test_dir, i);
+		ret = unlink(fname);
+		if (ret < 0) {
+			perror("unlink");
+			return EXIT_FAILURE;
+		}
+	}
+
+	/* sync to get log forced for unlink transactions to hit the disk */
+	sync();
+
+	/* sync once more FTW */
+	sync();
+
+	/*
+	 * now drop the caches so that unlinked inodes are reclaimed and
+	 * 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;
+	}
+
+	/*
+	 * now try to open the files by the stored handles. Expecting ENOENT
+	 * for all of them.
+	 */
+	for (i=0; i < NUMFILES; i++) {
+		errno = 0;
+		fd = open_by_handle_at(mount_fd, &handle[i].fh, O_RDWR);
+		if (fd < 0 && (errno == ENOENT || errno == ESTALE)) {
+			continue;
+		}
+		if (fd >= 0) {
+			printf("open_by_handle(%d) opened an unlinked file!\n", i);
+			close(fd);
+		} else
+			printf("open_by_handle(%d) returned %d incorrectly on an unlinked file!\n", i, errno);
+		failed++;
+	}
+	if (failed)
+		return EXIT_FAILURE;
+	return EXIT_SUCCESS;
+}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] src/open_by_handle: flexible usage options
  2017-04-19 16:29 [PATCH v2 0/4] fstests: generic test for NFS handles Amir Goldstein
  2017-04-19 16:29 ` [PATCH v2 1/4] fstests: remove IRIX test program open_unlink Amir Goldstein
  2017-04-19 16:29 ` [PATCH v2 2/4] src/open_by_handle: program to exercise open_by_handle_at() syscall Amir Goldstein
@ 2017-04-19 16:29 ` Amir Goldstein
  2017-04-19 16:29 ` [PATCH v2 4/4] fstests: add generic test for file handles Amir Goldstein
  3 siblings, 0 replies; 11+ messages in thread
From: Amir Goldstein @ 2017-04-19 16:29 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

More usage options for testing open_by_handle, which are needed
for testing stable handles across copy up in overlayfs.

usage: open_by_handle [-c|-l|-u|-d] <test_dir> [num_files]

Examples:

1. Create test set of N files and try to get their NFS handles:

   open_by_handle -c <test_dir> [N]

   This is used by new helper _require_exportfs() to check
   if filesystem supports exportfs

2. Get file handles for existing test set, drop caches and try to
   open all files by handle:

   open_by_handle <test_dir> [N]

3. Get file handles for existing test set, unlink all test files,
   drop caches, try to open all files by handle and expect ESTALE:

   open_by_handle -d <test_dir> [N]

4. 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):

   open_by_handle -l <test_dir> [N]
   open_by_handle -u <test_dir> [N]

   This test is done with 2 invocations of the program, first to
   hardlink (-l) and then to unlink the originals (-u), because
   we would like to be able to perform the hardlinks on overlay
   lower layer and unlink on upper layer.

   NOTE that open_by_handle -u doesn't check if the files are
   hardlinked, it just assumes that they are.  If they are not
   then the test will fail, because file handles would be stale.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/rc                    |  10 +++
 doc/auxiliary-programs.txt   |  16 ++++-
 doc/requirement-checking.txt |   7 ++
 src/open_by_handle.c         | 156 +++++++++++++++++++++++++++++++++++++------
 4 files changed, 167 insertions(+), 22 deletions(-)

diff --git a/common/rc b/common/rc
index 685b859..a706920 100644
--- a/common/rc
+++ b/common/rc
@@ -2857,6 +2857,16 @@ _require_freeze()
 	[ $result -eq 0 ] || _notrun "$FSTYP does not support freezing"
 }
 
+# Does NFS export work on this fs?
+_require_exportfs()
+{
+	_require_test_program "open_by_handle"
+	mkdir -p "$TEST_DIR"/exportfs_test
+	$here/src/open_by_handle -c "$TEST_DIR"/exportfs_test 2>&1 \
+		|| _notrun "$FSTYP does not support NFS export"
+}
+
+
 # Does shutdown work on this fs?
 _require_scratch_shutdown()
 {
diff --git a/doc/auxiliary-programs.txt b/doc/auxiliary-programs.txt
index 17797b0..2e2060a 100644
--- a/doc/auxiliary-programs.txt
+++ b/doc/auxiliary-programs.txt
@@ -15,9 +15,10 @@ note the dependency with:
 
 Contents:
 
- - af_unix	-- Create an AF_UNIX socket
- - stat_test	-- statx syscall exercise
- - xfs_io	-- General I/O operation exercise
+ - af_unix		-- Create an AF_UNIX socket
+ - open_by_handle	-- open_by_handle_at syscall exercise
+ - stat_test		-- statx syscall exercise
+ - xfs_io		-- General I/O operation exercise
 
 
 ==================
@@ -28,6 +29,15 @@ af_unix
 
 	The af_unix program creates an AF_UNIX socket at the given location.
 
+open_by_handle
+
+	The open_by_handle program exercises the open_by_handle_at() system
+	call.  It can check if file handles are valid or stale after certain
+	filesystem operations.
+
+	See also:
+		_require_exportfs
+
 stat_test
 
 	The stat_test program is primarily designed to exercise the statx()
diff --git a/doc/requirement-checking.txt b/doc/requirement-checking.txt
index 730c7ac..95d10e6 100644
--- a/doc/requirement-checking.txt
+++ b/doc/requirement-checking.txt
@@ -15,6 +15,7 @@ they have.  This is done with _require_<xxx> macros, which may take parameters.
  (2) Filesystem capability requirements.
 
 	_require_chattr <letters>
+	_require_exportfs
 
  (3) System call requirements.
 
@@ -86,6 +87,12 @@ _require_chattr <letters>
      tests to see if setting the append-only and immutable attributes on a file
      (chattr +a +i) is supported.
 
+_require_exportfs
+
+     The test requires that the $TEST_DEV filesystem supports NFS export.
+     The test also requires the use of the open_by_handle_at() system call and
+     will be skipped if it isn't available in the kernel.
+
 
 ========================
 SYSTEM CALL REQUIREMENTS
diff --git a/src/open_by_handle.c b/src/open_by_handle.c
index 76510ff..63ebdac 100644
--- a/src/open_by_handle.c
+++ b/src/open_by_handle.c
@@ -25,6 +25,46 @@
  *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
  */
 
+/*
+
+usage: open_by_handle [-c|-l|-u|-d] <test_dir> [num_files]
+
+Examples:
+
+1. Create test set of N files and try to get their NFS handles:
+
+   open_by_handle -c <test_dir> [N]
+
+   This is used by new helper _require_exportfs() to check
+   if filesystem supports exportfs
+
+2. Get file handles for existing test set, drop caches and try to
+   open all files by handle:
+
+   open_by_handle <test_dir> [N]
+
+3. Get file handles for existing test set, unlink all test files,
+   drop caches, try to open all files by handle and expect ESTALE:
+
+   open_by_handle -d <test_dir> [N]
+
+4. 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):
+
+   open_by_handle -l <test_dir> [N]
+   open_by_handle -u <test_dir> [N]
+
+   This test is done with 2 invocations of the program, first to
+   hardlink (-l) and then to unlink the originals (-u), because
+   we would like to be able to perform the hardlinks on overlay
+   lower layer and unlink on upper layer.
+
+   NOTE that open_by_handle -u doesn't check if the files are
+   hardlinked, it just assumes that they are.  If they are not
+   then the test will fail, because file handles would be stale.
+*/
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -35,41 +75,85 @@
 #include <errno.h>
 #include <linux/limits.h>
 
-#define NUMFILES 1024
+#define MAXFILES 1024
 
 struct handle {
 	struct file_handle fh;
 	unsigned char fid[MAX_HANDLE_SZ];
-} handle[NUMFILES];
+} handle[MAXFILES];
+
+void usage(void)
+{
+	fprintf(stderr, "usage: open_by_handle [-c|-l|-u|-d] <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 -l <test_dir> [N] - create hardlinks to test files, drop caches and try to open by handle\n");
+	fprintf(stderr, "open_by_handle -u <test_dir> [N] - unlink (hardlinked) test files, drop caches and try to open by handle\n");
+	fprintf(stderr, "open_by_handle -d <test_dir> [N] - unlink test files and hardlinks, drop caches and try to open by handle\n");
+	exit(EXIT_FAILURE);
+}
 
 int main(int argc, char **argv)
 {
-	int	i;
+	int	i, c;
 	int	fd;
 	int	ret;
 	int	failed = 0;
 	char	fname[PATH_MAX];
+	char	fname2[PATH_MAX];
 	char	*test_dir;
 	int	mount_fd, mount_id;
+	int	numfiles = 1;
+	int	create = 0, delete = 0, nlink = 1;
 
-	if (argc != 2) {
-		fprintf(stderr, "usage: open_by_handle <test_dir>\n");
-		return EXIT_FAILURE;
+	if (argc < 2 || argc > 4)
+		usage();
+
+	while ((c = getopt(argc, argv, "clud")) != -1) {
+		switch (c) {
+		case 'c':
+			create = 1;
+			break;
+		case 'l':
+			nlink = 2;
+			break;
+		case 'u':
+			delete = 1;
+			nlink = 1;
+			break;
+		case 'd':
+			delete = 1;
+			nlink = 0;
+			break;
+		default:
+			fprintf(stderr, "illegal option '%s'\n", argv[optind]);
+		case 'h':
+			usage();
+		}
+	}
+        if (optind == argc || optind > 2)
+            usage();
+	test_dir = argv[optind++];
+	if (optind < argc)
+		numfiles = atoi(argv[optind]);
+	if (!numfiles || numfiles > MAXFILES) {
+		fprintf(stderr, "illegal value '%s' for num_files\n", argv[optind]);
+		usage();
 	}
 
-	test_dir = argv[1];
 	mount_fd = open(test_dir, O_RDONLY|O_DIRECTORY);
 	if (mount_fd < 0) {
-		perror("open test_dir");
+		perror(test_dir);
 		return EXIT_FAILURE;
 	}
 
 	/*
-	 * create a large number of files to force allocation of new inode
-	 * chunks on disk.
+	 * Create the test files and remove leftover hardlinks from previous run
 	 */
-	for (i=0; i < NUMFILES; i++) {
+	for (i=0; create && i < numfiles; i++) {
 		sprintf(fname, "%s/file%06d", test_dir, i);
+		sprintf(fname2, "%s/link%06d", test_dir, i);
 		fd = open(fname, O_RDWR | O_CREAT | O_TRUNC, 0644);
 		if (fd < 0) {
 			printf("Warning (%s,%d), open(%s) failed.\n", __FILE__, __LINE__, fname);
@@ -77,13 +161,19 @@ int main(int argc, char **argv)
 			return EXIT_FAILURE;
 		}
 		close(fd);
+		/* blow up leftovers hardlinks if they exist */
+		ret = unlink(fname2);
+		if (ret < 0 && errno != ENOENT) {
+			perror("unlink");
+			return EXIT_FAILURE;
+		}
 	}
 
 	/* sync to get the new inodes to hit the disk */
 	sync();
 
 	/* create the handles */
-	for (i=0; i < NUMFILES; i++) {
+	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);
@@ -93,14 +183,37 @@ int main(int argc, char **argv)
 		}
 	}
 
+	/* after creating test set only check that fs supports exportfs */
+	if (create)
+		return EXIT_SUCCESS;
+
+	/* hardlink the files */
+	for (i=0; nlink > 1 && i < numfiles; i++) {
+		sprintf(fname, "%s/file%06d", test_dir, i);
+		sprintf(fname2, "%s/link%06d", test_dir, i);
+		ret = link(fname, fname2);
+		if (ret < 0) {
+			perror("link");
+			return EXIT_FAILURE;
+		}
+	}
+
 	/* unlink the files */
-	for (i=0; i < NUMFILES; i++) {
+	for (i=0; delete && i < numfiles; i++) {
 		sprintf(fname, "%s/file%06d", test_dir, i);
+		sprintf(fname2, "%s/link%06d", test_dir, i);
 		ret = unlink(fname);
 		if (ret < 0) {
 			perror("unlink");
 			return EXIT_FAILURE;
 		}
+		/* with -d flag, delete the hardlink if it exists */
+		if (!nlink)
+			ret = unlink(fname2);
+		if (ret < 0 && errno != ENOENT) {
+			perror("unlink");
+			return EXIT_FAILURE;
+		}
 	}
 
 	/* sync to get log forced for unlink transactions to hit the disk */
@@ -121,20 +234,25 @@ int main(int argc, char **argv)
 	}
 
 	/*
-	 * now try to open the files by the stored handles. Expecting ENOENT
-	 * for all of them.
+	 * now try to open the files by the stored handles. Expecting ESTALE
+	 * if all files and their hardlinks have been unlinked.
 	 */
-	for (i=0; i < NUMFILES; i++) {
+	for (i=0; i < numfiles; i++) {
 		errno = 0;
 		fd = open_by_handle_at(mount_fd, &handle[i].fh, O_RDWR);
-		if (fd < 0 && (errno == ENOENT || errno == ESTALE)) {
+		if (nlink && fd >= 0) {
+			close(fd);
+			continue;
+		} else if (!nlink && fd < 0 && (errno == ENOENT || errno == ESTALE)) {
 			continue;
 		}
 		if (fd >= 0) {
 			printf("open_by_handle(%d) opened an unlinked file!\n", i);
 			close(fd);
-		} else
-			printf("open_by_handle(%d) returned %d incorrectly on an unlinked file!\n", i, errno);
+		} else {
+			printf("open_by_handle(%d) returned %d incorrectly on %s file!\n", i, errno,
+					nlink ? "a linked" : "an unlinked");
+		}
 		failed++;
 	}
 	if (failed)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] fstests: add generic test for file handles
  2017-04-19 16:29 [PATCH v2 0/4] fstests: generic test for NFS handles Amir Goldstein
                   ` (2 preceding siblings ...)
  2017-04-19 16:29 ` [PATCH v2 3/4] src/open_by_handle: flexible usage options Amir Goldstein
@ 2017-04-19 16:29 ` Amir Goldstein
  2017-04-20  6:00   ` Eryu Guan
  2017-04-20  8:09   ` [PATCH v3 " Amir Goldstein
  3 siblings, 2 replies; 11+ messages in thread
From: Amir Goldstein @ 2017-04-19 16:29 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

Cloned from xfs specific test xfs/238, which checks
stale file handles of deleted files.

This test uses the generic open_by_handle_at() syscall
and also tests for non-stale file handles of linked files.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/generic/426     | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/426.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 77 insertions(+)
 create mode 100755 tests/generic/426
 create mode 100644 tests/generic/426.out

diff --git a/tests/generic/426 b/tests/generic/426
new file mode 100755
index 0000000..c39c165
--- /dev/null
+++ b/tests/generic/426
@@ -0,0 +1,74 @@
+#! /bin/bash
+# FS QA Test No. 426
+#
+# Check stale handles pointing to unlinked files
+# and non-stale handles pointing to linked files
+#
+#-----------------------------------------------------------------------
+# 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
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+# _require_exportfs already requires open_by_handle, but let's not count on it
+_require_test_program "open_by_handle"
+_require_exportfs
+
+numfiles=1024
+testdir=$TEST_DIR/$seq-dir
+mkdir -p $testdir
+
+# Check stale handles to deleted files
+rm -f $testdir/*
+src/open_by_handle -c $testdir $numfiles
+src/open_by_handle -d $testdir $numfiles
+
+# Check non-stale handles to linked files
+rm -f $testdir/*
+src/open_by_handle -c $testdir $numfiles
+src/open_by_handle    $testdir $numfiles
+
+# Check non-stale handles to files that were hardlinked and original deleted
+src/open_by_handle -l $testdir $numfiles
+src/open_by_handle -u $testdir $numfiles
+
+echo "Silence is golden"
+status=$?
+exit
diff --git a/tests/generic/426.out b/tests/generic/426.out
new file mode 100644
index 0000000..777cbcd
--- /dev/null
+++ b/tests/generic/426.out
@@ -0,0 +1,2 @@
+QA output created by 426
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 6d6e4f6..f29009c 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -428,3 +428,4 @@
 423 auto quick
 424 auto quick
 425 auto quick attr
+426 auto quick exportfs
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] fstests: add generic test for file handles
  2017-04-19 16:29 ` [PATCH v2 4/4] fstests: add generic test for file handles Amir Goldstein
@ 2017-04-20  6:00   ` Eryu Guan
  2017-04-20  6:34     ` Amir Goldstein
  2017-04-20  8:09   ` [PATCH v3 " Amir Goldstein
  1 sibling, 1 reply; 11+ messages in thread
From: Eryu Guan @ 2017-04-20  6:00 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

On Wed, Apr 19, 2017 at 07:29:19PM +0300, Amir Goldstein wrote:
> Cloned from xfs specific test xfs/238, which checks
> stale file handles of deleted files.
> 
> This test uses the generic open_by_handle_at() syscall
> and also tests for non-stale file handles of linked files.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  tests/generic/426     | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/426.out |  2 ++
>  tests/generic/group   |  1 +
>  3 files changed, 77 insertions(+)
>  create mode 100755 tests/generic/426
>  create mode 100644 tests/generic/426.out
> 
> diff --git a/tests/generic/426 b/tests/generic/426
> new file mode 100755
> index 0000000..c39c165
> --- /dev/null
> +++ b/tests/generic/426
> @@ -0,0 +1,74 @@
> +#! /bin/bash
> +# FS QA Test No. 426
> +#
> +# Check stale handles pointing to unlinked files
> +# and non-stale handles pointing to linked files
> +#
> +#-----------------------------------------------------------------------
> +# 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
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +# _require_exportfs already requires open_by_handle, but let's not count on it
> +_require_test_program "open_by_handle"
> +_require_exportfs
> +
> +numfiles=1024
> +testdir=$TEST_DIR/$seq-dir
> +mkdir -p $testdir
> +
> +# Check stale handles to deleted files
> +rm -f $testdir/*
> +src/open_by_handle -c $testdir $numfiles
> +src/open_by_handle -d $testdir $numfiles
> +
> +# Check non-stale handles to linked files
> +rm -f $testdir/*
> +src/open_by_handle -c $testdir $numfiles
> +src/open_by_handle    $testdir $numfiles
> +
> +# Check non-stale handles to files that were hardlinked and original deleted
> +src/open_by_handle -l $testdir $numfiles
> +src/open_by_handle -u $testdir $numfiles

This last test still depends on test files created in the second test
implicitly. Forgot to update this part?

# Check non-stale handles to files that were hardlinked and original deleted
rm -f $testdir/*
src/open_by_handle -c $testdir $numfiles
src/open_by_handle -l $testdir $numfiles
src/open_by_handle -u $testdir $numfiles

> +
> +echo "Silence is golden"
> +status=$?

$? is always 0 after echo. And any test failure could break golden
image, I think status can be set to 0 unconditionally.

Thanks,
Eryu

> +exit
> diff --git a/tests/generic/426.out b/tests/generic/426.out
> new file mode 100644
> index 0000000..777cbcd
> --- /dev/null
> +++ b/tests/generic/426.out
> @@ -0,0 +1,2 @@
> +QA output created by 426
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index 6d6e4f6..f29009c 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -428,3 +428,4 @@
>  423 auto quick
>  424 auto quick
>  425 auto quick attr
> +426 auto quick exportfs
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] fstests: add generic test for file handles
  2017-04-20  6:00   ` Eryu Guan
@ 2017-04-20  6:34     ` Amir Goldstein
  2017-04-20  7:21       ` Eryu Guan
  0 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2017-04-20  6:34 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

On Thu, Apr 20, 2017 at 9:00 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Wed, Apr 19, 2017 at 07:29:19PM +0300, Amir Goldstein wrote:
>> Cloned from xfs specific test xfs/238, which checks
>> stale file handles of deleted files.
>>
>> This test uses the generic open_by_handle_at() syscall
>> and also tests for non-stale file handles of linked files.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  tests/generic/426     | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/426.out |  2 ++
>>  tests/generic/group   |  1 +
>>  3 files changed, 77 insertions(+)
>>  create mode 100755 tests/generic/426
>>  create mode 100644 tests/generic/426.out
>>
>> diff --git a/tests/generic/426 b/tests/generic/426
>> new file mode 100755
>> index 0000000..c39c165
>> --- /dev/null
>> +++ b/tests/generic/426
>> @@ -0,0 +1,74 @@
>> +#! /bin/bash
>> +# FS QA Test No. 426
>> +#
>> +# Check stale handles pointing to unlinked files
>> +# and non-stale handles pointing to linked files
>> +#
>> +#-----------------------------------------------------------------------
>> +# 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
>> +
>> +# Modify as appropriate.
>> +_supported_fs generic
>> +_supported_os Linux
>> +# _require_exportfs already requires open_by_handle, but let's not count on it
>> +_require_test_program "open_by_handle"
>> +_require_exportfs
>> +
>> +numfiles=1024
>> +testdir=$TEST_DIR/$seq-dir
>> +mkdir -p $testdir
>> +
>> +# Check stale handles to deleted files
>> +rm -f $testdir/*
>> +src/open_by_handle -c $testdir $numfiles
>> +src/open_by_handle -d $testdir $numfiles
>> +
>> +# Check non-stale handles to linked files
>> +rm -f $testdir/*
>> +src/open_by_handle -c $testdir $numfiles
>> +src/open_by_handle    $testdir $numfiles
>> +
>> +# Check non-stale handles to files that were hardlinked and original deleted
>> +src/open_by_handle -l $testdir $numfiles
>> +src/open_by_handle -u $testdir $numfiles
>
> This last test still depends on test files created in the second test
> implicitly. Forgot to update this part?

Intentional.
It's the intended usage to create a test set with -c
(imagine this is going to done on lower layer)
then link the test set with -l (imagine this on either lower or overlay)
then unlink the test set with -u (imagine this on overlay).

So as a rule of thumb, rm -f is only added as extra safety
before a src/open_by_handle -c line

>
> # Check non-stale handles to files that were hardlinked and original deleted
> rm -f $testdir/*
> src/open_by_handle -c $testdir $numfiles
> src/open_by_handle -l $testdir $numfiles
> src/open_by_handle -u $testdir $numfiles
>
>> +
>> +echo "Silence is golden"
>> +status=$?
>
> $? is always 0 after echo. And any test failure could break golden
> image, I think status can be set to 0 unconditionally.
>

Indeed. a combination of copy&paste from xfs/238 and me moving
Silence is golden to end. Let me know if you want me to re-post for this

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] fstests: add generic test for file handles
  2017-04-20  6:34     ` Amir Goldstein
@ 2017-04-20  7:21       ` Eryu Guan
  2017-04-20  8:03         ` Amir Goldstein
  0 siblings, 1 reply; 11+ messages in thread
From: Eryu Guan @ 2017-04-20  7:21 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

On Thu, Apr 20, 2017 at 09:34:48AM +0300, Amir Goldstein wrote:
..
> >> +# Check non-stale handles to linked files
> >> +rm -f $testdir/*
> >> +src/open_by_handle -c $testdir $numfiles
> >> +src/open_by_handle    $testdir $numfiles
> >> +
> >> +# Check non-stale handles to files that were hardlinked and original deleted
> >> +src/open_by_handle -l $testdir $numfiles
> >> +src/open_by_handle -u $testdir $numfiles
> >
> > This last test still depends on test files created in the second test
> > implicitly. Forgot to update this part?
> 
> Intentional.
> It's the intended usage to create a test set with -c
> (imagine this is going to done on lower layer)
> then link the test set with -l (imagine this on either lower or overlay)
> then unlink the test set with -u (imagine this on overlay).

Yeah, that makes more sense in an overlay-specific test & operating on
multiple layers. But I don't see the point in this generic test by doing
so, as all these operations are clearly done on the same layer. So I
think adding "rm -f" and "-c" run in this test won't change test
behavior, but could avoid the implicit dependency.

Anyway, at least this is worth some comments on the implicit dependency.

> 
> So as a rule of thumb, rm -f is only added as extra safety
> before a src/open_by_handle -c line
> 
> >
> > # Check non-stale handles to files that were hardlinked and original deleted
> > rm -f $testdir/*
> > src/open_by_handle -c $testdir $numfiles
> > src/open_by_handle -l $testdir $numfiles
> > src/open_by_handle -u $testdir $numfiles
> >
> >> +
> >> +echo "Silence is golden"
> >> +status=$?
> >
> > $? is always 0 after echo. And any test failure could break golden
> > image, I think status can be set to 0 unconditionally.
> >
> 
> Indeed. a combination of copy&paste from xfs/238 and me moving
> Silence is golden to end. Let me know if you want me to re-post for this

I can fix this status issue, but seems you need to re-post anyway to add
more comments or empty $testdir before each test :)

Thanks,
Eryu

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] fstests: add generic test for file handles
  2017-04-20  7:21       ` Eryu Guan
@ 2017-04-20  8:03         ` Amir Goldstein
  0 siblings, 0 replies; 11+ messages in thread
From: Amir Goldstein @ 2017-04-20  8:03 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

On Thu, Apr 20, 2017 at 10:21 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Thu, Apr 20, 2017 at 09:34:48AM +0300, Amir Goldstein wrote:
> ..
>> >> +# Check non-stale handles to linked files
>> >> +rm -f $testdir/*
>> >> +src/open_by_handle -c $testdir $numfiles
>> >> +src/open_by_handle    $testdir $numfiles
>> >> +
>> >> +# Check non-stale handles to files that were hardlinked and original deleted
>> >> +src/open_by_handle -l $testdir $numfiles
>> >> +src/open_by_handle -u $testdir $numfiles
>> >
>> > This last test still depends on test files created in the second test
>> > implicitly. Forgot to update this part?
>>
>> Intentional.
>> It's the intended usage to create a test set with -c
>> (imagine this is going to done on lower layer)
>> then link the test set with -l (imagine this on either lower or overlay)
>> then unlink the test set with -u (imagine this on overlay).
>
> Yeah, that makes more sense in an overlay-specific test & operating on
> multiple layers. But I don't see the point in this generic test by doing
> so, as all these operations are clearly done on the same layer. So I
> think adding "rm -f" and "-c" run in this test won't change test
> behavior, but could avoid the implicit dependency.
>

You are right. It wont change the test.

> Anyway, at least this is worth some comments on the implicit dependency.
>

I'll take the easier choice ;-)

>>
>> So as a rule of thumb, rm -f is only added as extra safety
>> before a src/open_by_handle -c line
>>
>> >
>> > # Check non-stale handles to files that were hardlinked and original deleted
>> > rm -f $testdir/*
>> > src/open_by_handle -c $testdir $numfiles
>> > src/open_by_handle -l $testdir $numfiles
>> > src/open_by_handle -u $testdir $numfiles
>> >
>> >> +
>> >> +echo "Silence is golden"
>> >> +status=$?
>> >
>> > $? is always 0 after echo. And any test failure could break golden
>> > image, I think status can be set to 0 unconditionally.
>> >
>>
>> Indeed. a combination of copy&paste from xfs/238 and me moving
>> Silence is golden to end. Let me know if you want me to re-post for this
>
> I can fix this status issue, but seems you need to re-post anyway to add
> more comments or empty $testdir before each test :)
>
> Thanks,
> Eryu

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 4/4] fstests: add generic test for file handles
  2017-04-19 16:29 ` [PATCH v2 4/4] fstests: add generic test for file handles Amir Goldstein
  2017-04-20  6:00   ` Eryu Guan
@ 2017-04-20  8:09   ` Amir Goldstein
  1 sibling, 0 replies; 11+ messages in thread
From: Amir Goldstein @ 2017-04-20  8:09 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Trond Myklebust, Jeff Layton, J . Bruce Fields, David Howells,
	Miklos Szeredi, fstests, linux-unionfs

Cloned from xfs specific test xfs/238, which checks
stale file handles of deleted files.

This test uses the generic open_by_handle_at() syscall
and also tests for non-stale file handles of linked files.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/generic/426     | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/426.out |  2 ++
 tests/generic/group   |  1 +
 3 files changed, 79 insertions(+)
 create mode 100755 tests/generic/426
 create mode 100644 tests/generic/426.out

Eryu,

Avoiding unneeded spam to the list,
here is a re-send of the last patch with your requested fixes.

Thanks,
Amir.

diff --git a/tests/generic/426 b/tests/generic/426
new file mode 100755
index 0000000..d2e555f
--- /dev/null
+++ b/tests/generic/426
@@ -0,0 +1,76 @@
+#! /bin/bash
+# FS QA Test No. 426
+#
+# Check stale handles pointing to unlinked files
+# and non-stale handles pointing to linked files
+#
+#-----------------------------------------------------------------------
+# 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
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+# _require_exportfs already requires open_by_handle, but let's not count on it
+_require_test_program "open_by_handle"
+_require_exportfs
+
+numfiles=1024
+testdir=$TEST_DIR/$seq-dir
+mkdir -p $testdir
+
+# Check stale handles to deleted files
+rm -f $testdir/*
+src/open_by_handle -c $testdir $numfiles
+src/open_by_handle -d $testdir $numfiles
+
+# Check non-stale handles to linked files
+rm -f $testdir/*
+src/open_by_handle -c $testdir $numfiles
+src/open_by_handle    $testdir $numfiles
+
+# Check non-stale handles to files that were hardlinked and original deleted
+rm -f $testdir/*
+src/open_by_handle -c $testdir $numfiles
+src/open_by_handle -l $testdir $numfiles
+src/open_by_handle -u $testdir $numfiles
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/426.out b/tests/generic/426.out
new file mode 100644
index 0000000..777cbcd
--- /dev/null
+++ b/tests/generic/426.out
@@ -0,0 +1,2 @@
+QA output created by 426
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index 6d6e4f6..f29009c 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -428,3 +428,4 @@
 423 auto quick
 424 auto quick
 425 auto quick attr
+426 auto quick exportfs
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] src/open_by_handle: program to exercise open_by_handle_at() syscall
  2017-04-19 16:29 ` [PATCH v2 2/4] src/open_by_handle: program to exercise open_by_handle_at() syscall Amir Goldstein
@ 2017-04-22  7:55   ` Rock Lee
  0 siblings, 0 replies; 11+ messages in thread
From: Rock Lee @ 2017-04-22  7:55 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Eryu Guan, Trond Myklebust, Jeff Layton, J . Bruce Fields,
	David Howells, Miklos Szeredi, fstests, linux-unionfs

Hi, Amir:

> +       /*
> +        * now drop the caches so that unlinked inodes are reclaimed and
> +        * 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;
> +       }
> +

The original value of  /proc/sys/vm/drop_caches is 0 in my rpi3. After
this test, it turns to 3. Is it necessary to restore it ?

-- 
Cheers,
Rock

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-04-22  7:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-19 16:29 [PATCH v2 0/4] fstests: generic test for NFS handles Amir Goldstein
2017-04-19 16:29 ` [PATCH v2 1/4] fstests: remove IRIX test program open_unlink Amir Goldstein
2017-04-19 16:29 ` [PATCH v2 2/4] src/open_by_handle: program to exercise open_by_handle_at() syscall Amir Goldstein
2017-04-22  7:55   ` Rock Lee
2017-04-19 16:29 ` [PATCH v2 3/4] src/open_by_handle: flexible usage options Amir Goldstein
2017-04-19 16:29 ` [PATCH v2 4/4] fstests: add generic test for file handles Amir Goldstein
2017-04-20  6:00   ` Eryu Guan
2017-04-20  6:34     ` Amir Goldstein
2017-04-20  7:21       ` Eryu Guan
2017-04-20  8:03         ` Amir Goldstein
2017-04-20  8:09   ` [PATCH v3 " Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox