[PATCH 0/4] overlay/017: fix false negatives

[PATCH 0/4] overlay/017: fix false negatives

[Amir Goldstein]

Eryu,

With current mainline master branch, overlay/017 may pass on some systems
(e.g. kvm-xfstests) due to a bug in the test, which I already reported.

This series fixes the test back to failing by using the auxiliary program
t_dir_type instead of 'find' whose behavior is system dependent.

I also removed the exception of testing hardlinks, so now the test really
tests what it was meant to test (constant inode number across copy up)
without any discounts for special cases.

Amir.

Amir Goldstein (4):
  src/t_dir_type: support filtering by inode number
  overlay/017: use t_dir_type to find file by d_ino
  overlay/017: test consistent st_ino/d_ino for hardlinks
  overlay: tag tests 016-018 as experimental

 doc/auxiliary-programs.txt |  7 +++++++
 src/t_dir_type.c           | 11 ++++++++---
 tests/overlay/017          |  9 +++++----
 tests/overlay/group        |  6 +++---
 4 files changed, 23 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH 1/4] src/t_dir_type: support filtering by inode number

[Amir Goldstein]

usage: t_dir_type <dir> <inode number>

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 doc/auxiliary-programs.txt |  7 +++++++
 src/t_dir_type.c           | 11 ++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/doc/auxiliary-programs.txt b/doc/auxiliary-programs.txt
index 2e2060a..21ef118 100644
--- a/doc/auxiliary-programs.txt
+++ b/doc/auxiliary-programs.txt
@@ -18,6 +18,7 @@ Contents:
  - af_unix		-- Create an AF_UNIX socket
  - open_by_handle	-- open_by_handle_at syscall exercise
  - stat_test		-- statx syscall exercise
+ - t_dir_type		-- print directory entries and their file type
  - xfs_io		-- General I/O operation exercise
 
 
@@ -48,6 +49,12 @@ stat_test
 		_require_statx
 
 
+t_dir_type
+
+	The t_dir_type program exercises the getdents64() system call.
+	It prints directory entry names returned from getdents64() and
+	thier d_type, optionally filtered by type or by inode number.
+
 xfs_io
 
 	The xfs_io program can be found in the xfsprogs package and can be used
diff --git a/src/t_dir_type.c b/src/t_dir_type.c
index 344bef8..76aaa9b 100644
--- a/src/t_dir_type.c
+++ b/src/t_dir_type.c
@@ -19,9 +19,10 @@
 /*
  * t_dir_type
  *
- * print directory entries, optionally filtered by d_type
+ * print directory entries and their file type, optionally filtered by d_type
+ * or by inode number.
  *
- * ./t_dir_type <path> [u|f|d|c|b|l|p|s|w]
+ * ./t_dir_type <path> [u|f|d|c|b|l|p|s|w|<ino>]
  */
 
 #include <fcntl.h>
@@ -67,6 +68,7 @@ main(int argc, char *argv[])
 	struct linux_dirent64 *d;
 	int bpos;
 	int type = -1; /* -1 means all types */
+	uint64_t ino = 0;
 	int ret = 1;
 
 	fd = open(argv[1], O_RDONLY | O_DIRECTORY);
@@ -82,6 +84,8 @@ main(int argc, char *argv[])
 			if (DT_CHAR(type) == t)
 				break;
 		/* no match ends up with type = -1 */
+		if (type < 0)
+			ino = atoll(argv[2]);
 	}
 
 	for ( ; ; ) {
@@ -96,7 +100,8 @@ main(int argc, char *argv[])
 
 		for (bpos = 0; bpos < nread;) {
 			d = (struct linux_dirent64 *) (buf + bpos);
-			if (type < 0 || type == (int)d->d_type) {
+			if ((type < 0 || type == (int)d->d_type) &&
+			    (!ino || ino == d->d_ino)) {
 				ret = 0;
 				printf("%s %c\n", d->d_name, DT_CHAR(d->d_type));
 			}
-- 
2.7.4

[PATCH 2/4] overlay/017: use t_dir_type to find file by d_ino

[Amir Goldstein]

'find -ino' is this test was supposed to filter files by inode number
that was recorded with 'ls -i' to compare st_ino returned by stat(2)
with d_ino returned by getdents64(2).

It turns out that on some systems, 'find -ino' uses stat(2) for
filtering by inode number, which is not what we want.

Use the auxiliary program t_dir_type to filter files by inode number
instead.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index fabfbb5..bb467f7 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -56,6 +56,7 @@ _supported_fs overlay
 _supported_os Linux
 _require_scratch
 _require_test_program "af_unix"
+_require_test_program "t_dir_type"
 
 rm -f $seqres.full
 
@@ -107,7 +108,7 @@ function check_inode_numbers()
 	# Test constant readdir(3)/getdents(2) d_ino -
 	#   Expect to find file by inode number
 	cat $before | while read ino f; do
-		find $dir/ -maxdepth 1 -inum $ino | grep -q $f || \
+		$here/src/t_dir_type $dir $ino | grep -q $f || \
 			echo "$f not found by ino $ino (from $before)"
 	done
 }
-- 
2.7.4

[PATCH 3/4] overlay/017: test consistent st_ino/d_ino for hardlinks

[Amir Goldstein]

Currently hardlinks do not preserve the inode number across copy up,
so hardlinks did not participate in this test so far.

Stay honest and let the test verify what is was meant to verify and
let it fail because of the fact that hardlinks inode numbers are not
constant across copy up.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index bb467f7..e9c8525 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -63,8 +63,6 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 
 # Create our test files.
-# Not dealing with hardlinks here, when hardlinks are broken they
-# should not preserve the inode number.
 lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 mkdir $lowerdir/dir
@@ -74,8 +72,10 @@ mknod $lowerdir/chrdev c 1 1
 mknod $lowerdir/blkdev b 1 1
 mknod $lowerdir/fifo p
 $here/src/af_unix $lowerdir/socket
+touch $lowerdir/hardlink1
+ln $lowerdir/hardlink1 $lowerdir/hardlink2
 
-FILES="dir file symlink chrdev blkdev fifo socket"
+FILES="dir file symlink chrdev blkdev fifo socket hardlink1 hardlink2"
 
 # Record inode numbers in format <ino> <basename>
 function record_inode_numbers()
-- 
2.7.4

[PATCH 4/4] overlay: tag tests 016-018 as experimental

[Amir Goldstein]

The tests 016-018 were added to track overlayfs 'Non-standard behavior'
as are documented in Documentation/filesystems/overlayfs.txt, with the
intention of fixing those behaviors 'some day'.

While that day seems to be seen in the horizon, at least for test 017,
it may take a while longer for all those tests to pass.
Add those tests to group 'experimental' so at least this special status
is published somehow.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/group | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/overlay/group b/tests/overlay/group
index c5048c4..39169d3 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -18,9 +18,9 @@
 013 auto quick copyup
 014 auto quick copyup
 015 auto quick whiteout
-016 auto quick copyup
-017 auto quick copyup
-018 auto quick copyup
+016 auto quick copyup experimental
+017 auto quick copyup experimental
+018 auto quick copyup experimental
 019 auto stress
 020 auto quick copyup perms
 021 auto quick copyup
-- 
2.7.4

Re: [PATCH 4/4] overlay: tag tests 016-018 as experimental

[Eryu Guan]

On Thu, May 11, 2017 at 09:55:10AM +0300, Amir Goldstein wrote:
> The tests 016-018 were added to track overlayfs 'Non-standard behavior'
> as are documented in Documentation/filesystems/overlayfs.txt, with the
> intention of fixing those behaviors 'some day'.
> 
> While that day seems to be seen in the horizon, at least for test 017,
> it may take a while longer for all those tests to pass.
> Add those tests to group 'experimental' so at least this special status
> is published somehow.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

The first three patches look good to me, but I'm a bit reluctant on
merging this one.

To me, this new 'experimental' group serves as an indication of "this
test is known to fail". But this info already can be found in commit
logs and comments of these tests. Also fstests had been pushing back
attempts to add "known issue" support in the past. So I'd rather drop
this patch.

Thanks,
Eryu

> ---
>  tests/overlay/group | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/overlay/group b/tests/overlay/group
> index c5048c4..39169d3 100644
> --- a/tests/overlay/group
> +++ b/tests/overlay/group
> @@ -18,9 +18,9 @@
>  013 auto quick copyup
>  014 auto quick copyup
>  015 auto quick whiteout
> -016 auto quick copyup
> -017 auto quick copyup
> -018 auto quick copyup
> +016 auto quick copyup experimental
> +017 auto quick copyup experimental
> +018 auto quick copyup experimental
>  019 auto stress
>  020 auto quick copyup perms
>  021 auto quick copyup
> -- 
> 2.7.4
> 

Re: [PATCH 4/4] overlay: tag tests 016-018 as experimental

[Amir Goldstein]

On Fri, May 12, 2017 at 7:01 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Thu, May 11, 2017 at 09:55:10AM +0300, Amir Goldstein wrote:
>> The tests 016-018 were added to track overlayfs 'Non-standard behavior'
>> as are documented in Documentation/filesystems/overlayfs.txt, with the
>> intention of fixing those behaviors 'some day'.
>>
>> While that day seems to be seen in the horizon, at least for test 017,
>> it may take a while longer for all those tests to pass.
>> Add those tests to group 'experimental' so at least this special status
>> is published somehow.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> The first three patches look good to me, but I'm a bit reluctant on
> merging this one.
>
> To me, this new 'experimental' group serves as an indication of "this
> test is known to fail". But this info already can be found in commit
> logs and comments of these tests. Also fstests had been pushing back
> attempts to add "known issue" support in the past. So I'd rather drop
> this patch.
>
Perfectly fine by me.
I have no problem remembering that those tests fail and why ;)