fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] fstests: various fixes
@ 2019-05-20 22:30 Darrick J. Wong
  2019-05-20 22:30 ` [PATCH 1/3] generic/530: revert commit f8f57747222 Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Darrick J. Wong @ 2019-05-20 22:30 UTC (permalink / raw)
  To: guaneryu, darrick.wong; +Cc: linux-xfs, jefflexu, amir73il, fstests

Hi all,

The first two patches fix t_open_tmpfiles to shut down the scratch
filesystem properly by reverting a broken fix and teaching xfstests to
pass the relevant handles around.  The final patch cleans up some open
coded src/godown calls against the scratch fs to use the wrapper.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes

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

* [PATCH 1/3] generic/530: revert commit f8f57747222
  2019-05-20 22:30 [PATCH 0/3] fstests: various fixes Darrick J. Wong
@ 2019-05-20 22:30 ` Darrick J. Wong
  2019-05-21  5:01   ` Amir Goldstein
  2019-05-20 22:31 ` [PATCH 2/3] generic/530, xfs/501: pass fs shutdown handle to t_open_tmpfiles Darrick J. Wong
  2019-05-20 22:31 ` [PATCH 3/3] generic, xfs: use _scratch_shutdown instead of calling src/godown Darrick J. Wong
  2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2019-05-20 22:30 UTC (permalink / raw)
  To: guaneryu, darrick.wong; +Cc: linux-xfs, jefflexu, amir73il, fstests

From: Darrick J. Wong <darrick.wong@oracle.com>

Commit f8f57747222 ("generic/530: fix shutdown failure of generic/530 in
overlay") improperly clears an overlayfs test failure by shutting down
the filesystem after all the tempfiles are closed, which totally defeats
the purpose of both generic/530 and xfs/501.  Revert this commit so we
can fix it properly.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 src/t_open_tmpfiles.c |   19 +++++++++++++++++++
 tests/generic/530     |    4 +---
 tests/xfs/501         |    4 +---
 3 files changed, 21 insertions(+), 6 deletions(-)


diff --git a/src/t_open_tmpfiles.c b/src/t_open_tmpfiles.c
index 0393c6bd..da9390fd 100644
--- a/src/t_open_tmpfiles.c
+++ b/src/t_open_tmpfiles.c
@@ -24,6 +24,7 @@ static int min_fd = -1;
 static int max_fd = -1;
 static unsigned int nr_opened = 0;
 static float start_time;
+static int shutdown_fs = 0;
 
 void clock_time(float *time)
 {
@@ -68,6 +69,22 @@ void die(void)
 				end_time - start_time);
 		fflush(stdout);
 
+		if (shutdown_fs) {
+			/*
+			 * Flush the log so that we have to process the
+			 * unlinked inodes the next time we mount.
+			 */
+			int flag = XFS_FSOP_GOING_FLAGS_LOGFLUSH;
+			int ret;
+
+			ret = ioctl(min_fd, XFS_IOC_GOINGDOWN, &flag);
+			if (ret) {
+				perror("shutdown");
+				exit(2);
+			}
+			exit(0);
+		}
+
 		clock_time(&start_time);
 		for (fd = min_fd; fd <= max_fd; fd++)
 			close(fd);
@@ -143,6 +160,8 @@ int main(int argc, char *argv[])
 		if (ret)
 			perror(argv[1]);
 	}
+	if (argc > 2 && !strcmp(argv[2], "shutdown"))
+		shutdown_fs = 1;
 
 	clock_time(&start_time);
 	while (1)
diff --git a/tests/generic/530 b/tests/generic/530
index 56c6d32a..b0d188b1 100755
--- a/tests/generic/530
+++ b/tests/generic/530
@@ -49,9 +49,7 @@ ulimit -n $max_files
 
 # Open a lot of unlinked files
 echo create >> $seqres.full
-$here/src/t_open_tmpfiles $SCRATCH_MNT >> $seqres.full
-_scratch_shutdown -f
-
+$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
 
 # Unmount to prove that we can clean it all
 echo umount >> $seqres.full
diff --git a/tests/xfs/501 b/tests/xfs/501
index 4be9997c..974f3414 100755
--- a/tests/xfs/501
+++ b/tests/xfs/501
@@ -54,9 +54,7 @@ ulimit -n $max_files
 
 # Open a lot of unlinked files
 echo create >> $seqres.full
-$here/src/t_open_tmpfiles $SCRATCH_MNT >> $seqres.full
-_scratch_shutdown -f
-
+$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
 
 # Unmount to prove that we can clean it all
 echo umount >> $seqres.full

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

* [PATCH 2/3] generic/530, xfs/501: pass fs shutdown handle to t_open_tmpfiles
  2019-05-20 22:30 [PATCH 0/3] fstests: various fixes Darrick J. Wong
  2019-05-20 22:30 ` [PATCH 1/3] generic/530: revert commit f8f57747222 Darrick J. Wong
@ 2019-05-20 22:31 ` Darrick J. Wong
  2019-05-21  5:12   ` Amir Goldstein
  2019-05-20 22:31 ` [PATCH 3/3] generic, xfs: use _scratch_shutdown instead of calling src/godown Darrick J. Wong
  2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2019-05-20 22:31 UTC (permalink / raw)
  To: guaneryu, darrick.wong; +Cc: linux-xfs, jefflexu, amir73il, fstests

From: Darrick J. Wong <darrick.wong@oracle.com>

So it turns out that overlayfs can't pass FS_IOC_SHUTDOWN to the lower
filesystems and so xfstests works around this by creating shutdown
helpers for the scratch fs to direct the shutdown ioctl to wherever it
needs to go to shut down the filesystem -- SCRATCH_MNT on normal
filesystems and OVL_BASE_SCRATCH_MNT when -overlay is enabled.  This
means that t_open_tmpfiles cannot simply use one of the open tempfiles
to shut down the filesystem.

Commit f8f57747222 tried to "fix" this by ripping the shutdown code out,
but this made the tests useless.  Fix this instead by creating a
xfstests helper to return a path that can be used to shut down the
filesystem and then pass that path to t_open_tmpfiles so that we can
shut down the filesystem when overlayfs is enabled.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 common/rc             |   11 +++++++++++
 src/t_open_tmpfiles.c |   20 +++++++++++++-------
 tests/generic/530     |    2 +-
 tests/xfs/501         |    2 +-
 4 files changed, 26 insertions(+), 9 deletions(-)


diff --git a/common/rc b/common/rc
index 27c8bb7a..f577e5e3 100644
--- a/common/rc
+++ b/common/rc
@@ -393,6 +393,17 @@ _scratch_shutdown()
 	fi
 }
 
+# Return a file path that can be used to shut down the scratch filesystem.
+# Caller should _require_scratch_shutdown before using this.
+_scratch_shutdown_handle()
+{
+	if [ $FSTYP = "overlay" ]; then
+		echo $OVL_BASE_SCRATCH_MNT
+	else
+		echo $SCRATCH_MNT
+	fi
+}
+
 _test_mount()
 {
     if [ "$FSTYP" == "overlay" ]; then
diff --git a/src/t_open_tmpfiles.c b/src/t_open_tmpfiles.c
index da9390fd..258b0c95 100644
--- a/src/t_open_tmpfiles.c
+++ b/src/t_open_tmpfiles.c
@@ -24,7 +24,7 @@ static int min_fd = -1;
 static int max_fd = -1;
 static unsigned int nr_opened = 0;
 static float start_time;
-static int shutdown_fs = 0;
+static int shutdown_fd = -1;
 
 void clock_time(float *time)
 {
@@ -69,7 +69,7 @@ void die(void)
 				end_time - start_time);
 		fflush(stdout);
 
-		if (shutdown_fs) {
+		if (shutdown_fd >= 0) {
 			/*
 			 * Flush the log so that we have to process the
 			 * unlinked inodes the next time we mount.
@@ -77,7 +77,7 @@ void die(void)
 			int flag = XFS_FSOP_GOING_FLAGS_LOGFLUSH;
 			int ret;
 
-			ret = ioctl(min_fd, XFS_IOC_GOINGDOWN, &flag);
+			ret = ioctl(shutdown_fd, XFS_IOC_GOINGDOWN, &flag);
 			if (ret) {
 				perror("shutdown");
 				exit(2);
@@ -148,8 +148,9 @@ void leak_tmpfile(void)
 
 /*
  * Try to put as many files on the unlinked list and then kill them.
- * The first argument is a directory to chdir into; passing any second arg
- * will shut down the fs instead of closing files.
+ * The first argument is a directory to chdir into; the second argumennt (if
+ * provided) is a file path that will be opened and then used to shut down the
+ * fs before the program exits.
  */
 int main(int argc, char *argv[])
 {
@@ -160,8 +161,13 @@ int main(int argc, char *argv[])
 		if (ret)
 			perror(argv[1]);
 	}
-	if (argc > 2 && !strcmp(argv[2], "shutdown"))
-		shutdown_fs = 1;
+	if (argc > 2) {
+		shutdown_fd = open(argv[2], O_RDONLY);
+		if (shutdown_fd < 0) {
+			perror(argv[2]);
+			return 1;
+		}
+	}
 
 	clock_time(&start_time);
 	while (1)
diff --git a/tests/generic/530 b/tests/generic/530
index b0d188b1..cb874ace 100755
--- a/tests/generic/530
+++ b/tests/generic/530
@@ -49,7 +49,7 @@ ulimit -n $max_files
 
 # Open a lot of unlinked files
 echo create >> $seqres.full
-$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
+$here/src/t_open_tmpfiles $SCRATCH_MNT $(_scratch_shutdown_handle) >> $seqres.full
 
 # Unmount to prove that we can clean it all
 echo umount >> $seqres.full
diff --git a/tests/xfs/501 b/tests/xfs/501
index 974f3414..4be03b31 100755
--- a/tests/xfs/501
+++ b/tests/xfs/501
@@ -54,7 +54,7 @@ ulimit -n $max_files
 
 # Open a lot of unlinked files
 echo create >> $seqres.full
-$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
+$here/src/t_open_tmpfiles $SCRATCH_MNT $(_scratch_shutdown_handle) >> $seqres.full
 
 # Unmount to prove that we can clean it all
 echo umount >> $seqres.full

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

* [PATCH 3/3] generic, xfs: use _scratch_shutdown instead of calling src/godown
  2019-05-20 22:30 [PATCH 0/3] fstests: various fixes Darrick J. Wong
  2019-05-20 22:30 ` [PATCH 1/3] generic/530: revert commit f8f57747222 Darrick J. Wong
  2019-05-20 22:31 ` [PATCH 2/3] generic/530, xfs/501: pass fs shutdown handle to t_open_tmpfiles Darrick J. Wong
@ 2019-05-20 22:31 ` Darrick J. Wong
  2019-05-21  5:04   ` Amir Goldstein
  2 siblings, 1 reply; 7+ messages in thread
From: Darrick J. Wong @ 2019-05-20 22:31 UTC (permalink / raw)
  To: guaneryu, darrick.wong; +Cc: linux-xfs, jefflexu, amir73il, fstests

From: Darrick J. Wong <darrick.wong@oracle.com>

Overlayfs introduces some complexity with regards to what path we have
to use to shut down the scratch filesystem: it's SCRATCH_MNT for regular
filesystems, but it's OVL_BASE_SCRATCH_MNT (i.e. the lower mount of the
overlay) if overlayfs is enabled.  The helper works through all that, so
we might as well use it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tests/generic/050 |    2 +-
 tests/xfs/051     |    2 +-
 tests/xfs/079     |    2 +-
 tests/xfs/121     |    4 ++--
 tests/xfs/181     |    4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)


diff --git a/tests/generic/050 b/tests/generic/050
index 9a327165..91632d2d 100755
--- a/tests/generic/050
+++ b/tests/generic/050
@@ -92,7 +92,7 @@ echo "touch files"
 touch $SCRATCH_MNT/{0,1,2,3,4,5,6,7,8,9}{0,1,2,3,4,5,6,7,8,9}
 
 echo "going down:"
-src/godown -f $SCRATCH_MNT
+_scratch_shutdown -f
 
 echo "unmounting shutdown filesystem:"
 _scratch_unmount 2>&1 | _filter_scratch
diff --git a/tests/xfs/051 b/tests/xfs/051
index bcc824f8..105fa9ff 100755
--- a/tests/xfs/051
+++ b/tests/xfs/051
@@ -47,7 +47,7 @@ _scratch_mount
 # recovery.
 $FSSTRESS_PROG -n 9999 -p 2 -w -d $SCRATCH_MNT > /dev/null 2>&1 &
 sleep 5
-src/godown -f $SCRATCH_MNT
+_scratch_shutdown -f
 $KILLALL_PROG -q $FSSTRESS_PROG
 wait
 _scratch_unmount
diff --git a/tests/xfs/079 b/tests/xfs/079
index bf965a7f..67250495 100755
--- a/tests/xfs/079
+++ b/tests/xfs/079
@@ -56,7 +56,7 @@ _scratch_mount "-o logbsize=32k"
 # Run a workload to dirty the log, wait a bit and shutdown the fs.
 $FSSTRESS_PROG -d $SCRATCH_MNT -p 4 -n 99999999 >> $seqres.full 2>&1 &
 sleep 10
-./src/godown -f $SCRATCH_MNT
+_scratch_shutdown -f
 wait
 
 # Remount with a different log buffer size. Going from 32k to 64k increases the
diff --git a/tests/xfs/121 b/tests/xfs/121
index d82a367f..2e3914b7 100755
--- a/tests/xfs/121
+++ b/tests/xfs/121
@@ -52,7 +52,7 @@ src/multi_open_unlink -f $SCRATCH_MNT/test_file -n $num_files -s $delay &
 sleep 3
 
 echo "godown"
-src/godown -v -f $SCRATCH_MNT >> $seqres.full
+_scratch_shutdown -v -f >> $seqres.full
 
 # time for multi_open_unlink to exit out after its delay
 # so we have no references and can unmount
@@ -69,7 +69,7 @@ _try_scratch_mount $mnt >>$seqres.full 2>&1 \
     || _fail "mount failed: $mnt $MOUNT_OPTIONS"
 
 echo "godown"
-src/godown -v -f $SCRATCH_MNT >> $seqres.full
+_scratch_shutdown -v -f >> $seqres.full
 
 echo "unmount"
 _scratch_unmount
diff --git a/tests/xfs/181 b/tests/xfs/181
index 882a974b..dba69a70 100755
--- a/tests/xfs/181
+++ b/tests/xfs/181
@@ -65,7 +65,7 @@ pid=$!
 sleep 10
 
 echo "godown"
-src/godown -v -f $SCRATCH_MNT >> $seqres.full
+_scratch_shutdown -v -f >> $seqres.full
 
 # kill the multi_open_unlink
 kill $pid 2>/dev/null
@@ -83,7 +83,7 @@ _scratch_mount $mnt >>$seqres.full 2>&1 \
     || _fail "mount failed: $mnt $MOUNT_OPTIONS"
 
 echo "godown"
-src/godown -v -f $SCRATCH_MNT >> $seqres.full
+_scratch_shutdown -v -f >> $seqres.full
 
 echo "unmount"
 _scratch_unmount

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

* Re: [PATCH 1/3] generic/530: revert commit f8f57747222
  2019-05-20 22:30 ` [PATCH 1/3] generic/530: revert commit f8f57747222 Darrick J. Wong
@ 2019-05-21  5:01   ` Amir Goldstein
  0 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2019-05-21  5:01 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Eryu Guan, linux-xfs, Jeffle Xu, fstests

On Tue, May 21, 2019 at 1:31 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Commit f8f57747222 ("generic/530: fix shutdown failure of generic/530 in
> overlay") improperly clears an overlayfs test failure by shutting down
> the filesystem after all the tempfiles are closed, which totally defeats
> the purpose of both generic/530 and xfs/501.  Revert this commit so we
> can fix it properly.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  src/t_open_tmpfiles.c |   19 +++++++++++++++++++
>  tests/generic/530     |    4 +---
>  tests/xfs/501         |    4 +---
>  3 files changed, 21 insertions(+), 6 deletions(-)
>
>
> diff --git a/src/t_open_tmpfiles.c b/src/t_open_tmpfiles.c
> index 0393c6bd..da9390fd 100644
> --- a/src/t_open_tmpfiles.c
> +++ b/src/t_open_tmpfiles.c
> @@ -24,6 +24,7 @@ static int min_fd = -1;
>  static int max_fd = -1;
>  static unsigned int nr_opened = 0;
>  static float start_time;
> +static int shutdown_fs = 0;
>
>  void clock_time(float *time)
>  {
> @@ -68,6 +69,22 @@ void die(void)
>                                 end_time - start_time);
>                 fflush(stdout);
>
> +               if (shutdown_fs) {
> +                       /*
> +                        * Flush the log so that we have to process the
> +                        * unlinked inodes the next time we mount.
> +                        */
> +                       int flag = XFS_FSOP_GOING_FLAGS_LOGFLUSH;
> +                       int ret;
> +
> +                       ret = ioctl(min_fd, XFS_IOC_GOINGDOWN, &flag);
> +                       if (ret) {
> +                               perror("shutdown");
> +                               exit(2);
> +                       }
> +                       exit(0);
> +               }
> +
>                 clock_time(&start_time);
>                 for (fd = min_fd; fd <= max_fd; fd++)
>                         close(fd);
> @@ -143,6 +160,8 @@ int main(int argc, char *argv[])
>                 if (ret)
>                         perror(argv[1]);
>         }
> +       if (argc > 2 && !strcmp(argv[2], "shutdown"))
> +               shutdown_fs = 1;
>
>         clock_time(&start_time);
>         while (1)
> diff --git a/tests/generic/530 b/tests/generic/530
> index 56c6d32a..b0d188b1 100755
> --- a/tests/generic/530
> +++ b/tests/generic/530
> @@ -49,9 +49,7 @@ ulimit -n $max_files
>
>  # Open a lot of unlinked files
>  echo create >> $seqres.full
> -$here/src/t_open_tmpfiles $SCRATCH_MNT >> $seqres.full
> -_scratch_shutdown -f
> -
> +$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
>
>  # Unmount to prove that we can clean it all
>  echo umount >> $seqres.full
> diff --git a/tests/xfs/501 b/tests/xfs/501
> index 4be9997c..974f3414 100755
> --- a/tests/xfs/501
> +++ b/tests/xfs/501
> @@ -54,9 +54,7 @@ ulimit -n $max_files
>
>  # Open a lot of unlinked files
>  echo create >> $seqres.full
> -$here/src/t_open_tmpfiles $SCRATCH_MNT >> $seqres.full
> -_scratch_shutdown -f
> -
> +$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
>
>  # Unmount to prove that we can clean it all
>  echo umount >> $seqres.full
>

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

* Re: [PATCH 3/3] generic, xfs: use _scratch_shutdown instead of calling src/godown
  2019-05-20 22:31 ` [PATCH 3/3] generic, xfs: use _scratch_shutdown instead of calling src/godown Darrick J. Wong
@ 2019-05-21  5:04   ` Amir Goldstein
  0 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2019-05-21  5:04 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Eryu Guan, linux-xfs, Jeffle Xu, fstests

On Tue, May 21, 2019 at 1:31 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Overlayfs introduces some complexity with regards to what path we have
> to use to shut down the scratch filesystem: it's SCRATCH_MNT for regular
> filesystems, but it's OVL_BASE_SCRATCH_MNT (i.e. the lower mount of the
> overlay) if overlayfs is enabled.  The helper works through all that, so
> we might as well use it.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks for cleaning that up

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

> ---
>  tests/generic/050 |    2 +-
>  tests/xfs/051     |    2 +-
>  tests/xfs/079     |    2 +-
>  tests/xfs/121     |    4 ++--
>  tests/xfs/181     |    4 ++--
>  5 files changed, 7 insertions(+), 7 deletions(-)
>
>
> diff --git a/tests/generic/050 b/tests/generic/050
> index 9a327165..91632d2d 100755
> --- a/tests/generic/050
> +++ b/tests/generic/050
> @@ -92,7 +92,7 @@ echo "touch files"
>  touch $SCRATCH_MNT/{0,1,2,3,4,5,6,7,8,9}{0,1,2,3,4,5,6,7,8,9}
>
>  echo "going down:"
> -src/godown -f $SCRATCH_MNT
> +_scratch_shutdown -f
>
>  echo "unmounting shutdown filesystem:"
>  _scratch_unmount 2>&1 | _filter_scratch
> diff --git a/tests/xfs/051 b/tests/xfs/051
> index bcc824f8..105fa9ff 100755
> --- a/tests/xfs/051
> +++ b/tests/xfs/051
> @@ -47,7 +47,7 @@ _scratch_mount
>  # recovery.
>  $FSSTRESS_PROG -n 9999 -p 2 -w -d $SCRATCH_MNT > /dev/null 2>&1 &
>  sleep 5
> -src/godown -f $SCRATCH_MNT
> +_scratch_shutdown -f
>  $KILLALL_PROG -q $FSSTRESS_PROG
>  wait
>  _scratch_unmount
> diff --git a/tests/xfs/079 b/tests/xfs/079
> index bf965a7f..67250495 100755
> --- a/tests/xfs/079
> +++ b/tests/xfs/079
> @@ -56,7 +56,7 @@ _scratch_mount "-o logbsize=32k"
>  # Run a workload to dirty the log, wait a bit and shutdown the fs.
>  $FSSTRESS_PROG -d $SCRATCH_MNT -p 4 -n 99999999 >> $seqres.full 2>&1 &
>  sleep 10
> -./src/godown -f $SCRATCH_MNT
> +_scratch_shutdown -f
>  wait
>
>  # Remount with a different log buffer size. Going from 32k to 64k increases the
> diff --git a/tests/xfs/121 b/tests/xfs/121
> index d82a367f..2e3914b7 100755
> --- a/tests/xfs/121
> +++ b/tests/xfs/121
> @@ -52,7 +52,7 @@ src/multi_open_unlink -f $SCRATCH_MNT/test_file -n $num_files -s $delay &
>  sleep 3
>
>  echo "godown"
> -src/godown -v -f $SCRATCH_MNT >> $seqres.full
> +_scratch_shutdown -v -f >> $seqres.full
>
>  # time for multi_open_unlink to exit out after its delay
>  # so we have no references and can unmount
> @@ -69,7 +69,7 @@ _try_scratch_mount $mnt >>$seqres.full 2>&1 \
>      || _fail "mount failed: $mnt $MOUNT_OPTIONS"
>
>  echo "godown"
> -src/godown -v -f $SCRATCH_MNT >> $seqres.full
> +_scratch_shutdown -v -f >> $seqres.full
>
>  echo "unmount"
>  _scratch_unmount
> diff --git a/tests/xfs/181 b/tests/xfs/181
> index 882a974b..dba69a70 100755
> --- a/tests/xfs/181
> +++ b/tests/xfs/181
> @@ -65,7 +65,7 @@ pid=$!
>  sleep 10
>
>  echo "godown"
> -src/godown -v -f $SCRATCH_MNT >> $seqres.full
> +_scratch_shutdown -v -f >> $seqres.full
>
>  # kill the multi_open_unlink
>  kill $pid 2>/dev/null
> @@ -83,7 +83,7 @@ _scratch_mount $mnt >>$seqres.full 2>&1 \
>      || _fail "mount failed: $mnt $MOUNT_OPTIONS"
>
>  echo "godown"
> -src/godown -v -f $SCRATCH_MNT >> $seqres.full
> +_scratch_shutdown -v -f >> $seqres.full
>
>  echo "unmount"
>  _scratch_unmount
>

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

* Re: [PATCH 2/3] generic/530, xfs/501: pass fs shutdown handle to t_open_tmpfiles
  2019-05-20 22:31 ` [PATCH 2/3] generic/530, xfs/501: pass fs shutdown handle to t_open_tmpfiles Darrick J. Wong
@ 2019-05-21  5:12   ` Amir Goldstein
  0 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2019-05-21  5:12 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Eryu Guan, linux-xfs, Jeffle Xu, fstests

On Tue, May 21, 2019 at 1:33 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> So it turns out that overlayfs can't pass FS_IOC_SHUTDOWN to the lower
> filesystems and so xfstests works around this by creating shutdown
> helpers for the scratch fs to direct the shutdown ioctl to wherever it
> needs to go to shut down the filesystem -- SCRATCH_MNT on normal
> filesystems and OVL_BASE_SCRATCH_MNT when -overlay is enabled.  This
> means that t_open_tmpfiles cannot simply use one of the open tempfiles
> to shut down the filesystem.
>
> Commit f8f57747222 tried to "fix" this by ripping the shutdown code out,
> but this made the tests useless.  Fix this instead by creating a
> xfstests helper to return a path that can be used to shut down the
> filesystem and then pass that path to t_open_tmpfiles so that we can
> shut down the filesystem when overlayfs is enabled.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Thanks for sorting that out.

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

> ---
>  common/rc             |   11 +++++++++++
>  src/t_open_tmpfiles.c |   20 +++++++++++++-------
>  tests/generic/530     |    2 +-
>  tests/xfs/501         |    2 +-
>  4 files changed, 26 insertions(+), 9 deletions(-)
>
>
> diff --git a/common/rc b/common/rc
> index 27c8bb7a..f577e5e3 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -393,6 +393,17 @@ _scratch_shutdown()
>         fi
>  }
>
> +# Return a file path that can be used to shut down the scratch filesystem.
> +# Caller should _require_scratch_shutdown before using this.
> +_scratch_shutdown_handle()
> +{
> +       if [ $FSTYP = "overlay" ]; then
> +               echo $OVL_BASE_SCRATCH_MNT
> +       else
> +               echo $SCRATCH_MNT
> +       fi
> +}
> +
>  _test_mount()
>  {
>      if [ "$FSTYP" == "overlay" ]; then
> diff --git a/src/t_open_tmpfiles.c b/src/t_open_tmpfiles.c
> index da9390fd..258b0c95 100644
> --- a/src/t_open_tmpfiles.c
> +++ b/src/t_open_tmpfiles.c
> @@ -24,7 +24,7 @@ static int min_fd = -1;
>  static int max_fd = -1;
>  static unsigned int nr_opened = 0;
>  static float start_time;
> -static int shutdown_fs = 0;
> +static int shutdown_fd = -1;
>
>  void clock_time(float *time)
>  {
> @@ -69,7 +69,7 @@ void die(void)
>                                 end_time - start_time);
>                 fflush(stdout);
>
> -               if (shutdown_fs) {
> +               if (shutdown_fd >= 0) {
>                         /*
>                          * Flush the log so that we have to process the
>                          * unlinked inodes the next time we mount.
> @@ -77,7 +77,7 @@ void die(void)
>                         int flag = XFS_FSOP_GOING_FLAGS_LOGFLUSH;
>                         int ret;
>
> -                       ret = ioctl(min_fd, XFS_IOC_GOINGDOWN, &flag);
> +                       ret = ioctl(shutdown_fd, XFS_IOC_GOINGDOWN, &flag);
>                         if (ret) {
>                                 perror("shutdown");
>                                 exit(2);
> @@ -148,8 +148,9 @@ void leak_tmpfile(void)
>
>  /*
>   * Try to put as many files on the unlinked list and then kill them.
> - * The first argument is a directory to chdir into; passing any second arg
> - * will shut down the fs instead of closing files.
> + * The first argument is a directory to chdir into; the second argumennt (if
> + * provided) is a file path that will be opened and then used to shut down the
> + * fs before the program exits.
>   */
>  int main(int argc, char *argv[])
>  {
> @@ -160,8 +161,13 @@ int main(int argc, char *argv[])
>                 if (ret)
>                         perror(argv[1]);
>         }
> -       if (argc > 2 && !strcmp(argv[2], "shutdown"))
> -               shutdown_fs = 1;
> +       if (argc > 2) {
> +               shutdown_fd = open(argv[2], O_RDONLY);
> +               if (shutdown_fd < 0) {
> +                       perror(argv[2]);
> +                       return 1;
> +               }
> +       }
>
>         clock_time(&start_time);
>         while (1)
> diff --git a/tests/generic/530 b/tests/generic/530
> index b0d188b1..cb874ace 100755
> --- a/tests/generic/530
> +++ b/tests/generic/530
> @@ -49,7 +49,7 @@ ulimit -n $max_files
>
>  # Open a lot of unlinked files
>  echo create >> $seqres.full
> -$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
> +$here/src/t_open_tmpfiles $SCRATCH_MNT $(_scratch_shutdown_handle) >> $seqres.full
>
>  # Unmount to prove that we can clean it all
>  echo umount >> $seqres.full
> diff --git a/tests/xfs/501 b/tests/xfs/501
> index 974f3414..4be03b31 100755
> --- a/tests/xfs/501
> +++ b/tests/xfs/501
> @@ -54,7 +54,7 @@ ulimit -n $max_files
>
>  # Open a lot of unlinked files
>  echo create >> $seqres.full
> -$here/src/t_open_tmpfiles $SCRATCH_MNT shutdown >> $seqres.full
> +$here/src/t_open_tmpfiles $SCRATCH_MNT $(_scratch_shutdown_handle) >> $seqres.full
>
>  # Unmount to prove that we can clean it all
>  echo umount >> $seqres.full
>

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

end of thread, other threads:[~2019-05-21  5:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20 22:30 [PATCH 0/3] fstests: various fixes Darrick J. Wong
2019-05-20 22:30 ` [PATCH 1/3] generic/530: revert commit f8f57747222 Darrick J. Wong
2019-05-21  5:01   ` Amir Goldstein
2019-05-20 22:31 ` [PATCH 2/3] generic/530, xfs/501: pass fs shutdown handle to t_open_tmpfiles Darrick J. Wong
2019-05-21  5:12   ` Amir Goldstein
2019-05-20 22:31 ` [PATCH 3/3] generic, xfs: use _scratch_shutdown instead of calling src/godown Darrick J. Wong
2019-05-21  5:04   ` Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).