[PATCH 0/2] RFC: O_TMPFILE tests

[PATCH 0/2] RFC: O_TMPFILE tests

[Christoph Hellwig]

This series adds a generic and one xfs-specific test for the O_TMPFILE
functionality.  For now it's purely RFC as there is no infrastructure
to test the filesystem and xfs_io supports O_TMPFILE yet, as I plan to
implement that ontop of Lukas' refactoring of the related feature
checks.

Even with these tests not going in ASAP having them out and waiting to
be merged is enough that I'd be fine with merging the tmpfile branch
into for-next so that it can go into the 3.15 merge window.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] generic: add a basic O_TMPFILE test

[Christoph Hellwig]

diff --git a/tests/generic/004 b/tests/generic/004
new file mode 100755
index 0000000..2848d36
--- /dev/null
+++ b/tests/generic/004
@@ -0,0 +1,70 @@
+#! /bin/bash
+# FS QA Test No. 004
+#
+# Test O_TMPFILE opens
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Christoph Hellwig.  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
+#-----------------------------------------------------------------------
+#
+
+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 ${testfile}
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+#_require_xfs_io_falloc_flink
+
+testfile="${TEST_DIR}/tst-tmpfile-flink"
+
+# test creating a r/w tmpfile, do I/O and link it into the namespace
+$XFS_IO_PROG -T \
+	-c "pwrite 0 4096" \
+	-c "pread 0 4096" \
+	-c "flink ${testfile}" \
+	${TEST_DIR} | _filter_xfs_io
+
+if [ ! -f "${testfile}" ]; then
+    echo "failed to link testfile into place"
+    exit 1
+fi
+
+rm -f ${tmpfile}
+
+# test creating a r/o tmpfile.  Should fail
+$XFS_IO_PROG -Tr ${TEST_DIR} -c "close" | _filter_test_dir
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/004.out b/tests/generic/004.out
new file mode 100644
index 0000000..b85c11c
--- /dev/null
+++ b/tests/generic/004.out
@@ -0,0 +1,6 @@
+QA output created by 004
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+/mnt/test: Invalid argument
diff --git a/tests/generic/group b/tests/generic/group
index 34bd118..cf2e5ee 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -6,6 +6,7 @@
 001 rw dir udf auto quick
 002 metadata udf auto quick
 003 atime auto quick
+004 auto quick
 005 dir udf auto quick
 006 dir udf auto quick
 007 dir udf auto quick

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfs: add a test for unlinked inodes due to O_TMPFILE

[Christoph Hellwig]

Make sure that we see unlinked inodes when shutting a filesystem
down that has an open O_TMPFILE descriptor, and make sure that it
has been deleted after a mount/umount cycle.

Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/tests/xfs/006 b/tests/xfs/006
new file mode 100755
index 0000000..6be8418
--- /dev/null
+++ b/tests/xfs/006
@@ -0,0 +1,86 @@
+#! /bin/bash
+# XFS QA Test No. 006
+#
+# Test O_TMPFILE interaction with log recovery and repair.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Christoph Hellwig.  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
+#-----------------------------------------------------------------------
+#
+
+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.*
+    rm -f ${testfile}
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/repair
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+_require_scratch
+#_require_xfs_io_falloc_flink
+
+testfile="${SCRATCH_MNT}/tst-tmpfile-flink"
+
+_scratch_mount
+
+# test creating a r/w tmpfile, do I/O and link it into the namespace
+$XFS_IO_PROG -x -T \
+	-c "pwrite 0 4096" \
+	-c "pread 0 4096" \
+	-c "freeze" \
+	-c "thaw" \
+	-c "shutdown" \
+	${SCRATCH_MNT} | _filter_xfs_io
+
+_scratch_unmount
+
+# repair should find an unlinked inode and an incorrect icount
+_scratch_xfs_repair -n 2>&1 | \
+	_filter_repair > $tmp.repair
+
+ino=`grep 'disconnected inode ' $tmp.repair | \
+	head -1 | \
+	awk '{print $3}' | \
+	sed 's/,//'`
+
+cat $tmp.repair | sed "s/$ino/XXX/g"
+
+# filesystem should be clean after an unmount/mount cycle
+_scratch_mount
+_scratch_unmount
+
+_scratch_xfs_repair -n 2>&1 | _filter_repair
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/006.out b/tests/xfs/006.out
new file mode 100644
index 0000000..46f3b73
--- /dev/null
+++ b/tests/xfs/006.out
@@ -0,0 +1,43 @@
+QA output created by 006
+wrote 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 0
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Phase 1 - find and verify superblock...
+Phase 2 - using <TYPEOF> log
+        - scan filesystem freespace and inode maps...
+        - found root inode chunk
+Phase 3 - for each AG...
+        - scan (but don't clear) agi unlinked lists...
+        - process known inodes and perform inode discovery...
+        - process newly discovered inodes...
+Phase 4 - check for duplicate blocks...
+        - setting up duplicate extent list...
+        - check for inodes claiming duplicate blocks...
+No modify flag set, skipping phase 5
+Phase 6 - check inode connectivity...
+        - traversing filesystem ...
+        - traversal finished ...
+        - moving disconnected inodes to lost+found ...
+disconnected inode XXX, would move to lost+found
+Phase 7 - verify link counts...
+would have reset inode XXX nlinks from 0 to 1
+No modify flag set, skipping filesystem flush and exiting.
+Phase 1 - find and verify superblock...
+Phase 2 - using <TYPEOF> log
+        - scan filesystem freespace and inode maps...
+        - found root inode chunk
+Phase 3 - for each AG...
+        - scan (but don't clear) agi unlinked lists...
+        - process known inodes and perform inode discovery...
+        - process newly discovered inodes...
+Phase 4 - check for duplicate blocks...
+        - setting up duplicate extent list...
+        - check for inodes claiming duplicate blocks...
+No modify flag set, skipping phase 5
+Phase 6 - check inode connectivity...
+        - traversing filesystem ...
+        - traversal finished ...
+        - moving disconnected inodes to lost+found ...
+Phase 7 - verify link counts...
+No modify flag set, skipping filesystem flush and exiting.
diff --git a/tests/xfs/group b/tests/xfs/group
index ba34650..2f896e1 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -3,6 +3,7 @@
 003 db auto quick
 004 db auto quick
 005 auto quick
+006 auto quick
 008 rw ioctl auto quick
 009 rw ioctl auto prealloc quick
 012 rw auto quick

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfs: add a test for unlinked inodes due to O_TMPFILE

[Dave Chinner]

On Tue, Mar 04, 2014 at 09:21:48AM -0800, Christoph Hellwig wrote:
> Make sure that we see unlinked inodes when shutting a filesystem
> down that has an open O_TMPFILE descriptor, and make sure that it
> has been deleted after a mount/umount cycle.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> diff --git a/tests/xfs/006 b/tests/xfs/006
> new file mode 100755
> index 0000000..6be8418
> --- /dev/null
> +++ b/tests/xfs/006
> @@ -0,0 +1,86 @@
> +#! /bin/bash
> +# XFS QA Test No. 006
> +#
> +# Test O_TMPFILE interaction with log recovery and repair.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2014 Christoph Hellwig.  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
> +#-----------------------------------------------------------------------
> +#
> +
> +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.*
> +    rm -f ${testfile}
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/repair
> +
> +# real QA test starts here
> +_supported_fs generic

XFS only.

> +_supported_os Linux
> +
> +_require_scratch
> +#_require_xfs_io_falloc_flink
> +
> +testfile="${SCRATCH_MNT}/tst-tmpfile-flink"
> +
> +_scratch_mount
> +
> +# test creating a r/w tmpfile, do I/O and link it into the namespace
> +$XFS_IO_PROG -x -T \
> +	-c "pwrite 0 4096" \
> +	-c "pread 0 4096" \
> +	-c "freeze" \
> +	-c "thaw" \
> +	-c "shutdown" \
> +	${SCRATCH_MNT} | _filter_xfs_io

$testfile?

Also, I don't see the file being linked into the namespace, so the
comment is probably wrong. Also, please add a comment as to
why the freeze/thaw is necessary.

> +# repair should find an unlinked inode and an incorrect icount
> +_scratch_xfs_repair -n 2>&1 | \
> +	_filter_repair > $tmp.repair
> +
> +ino=`grep 'disconnected inode ' $tmp.repair | \
> +	head -1 | \
> +	awk '{print $3}' | \
> +	sed 's/,//'`
> +
> +cat $tmp.repair | sed "s/$ino/XXX/g"

Ok, so we should see that line in the golden output...

....
> diff --git a/tests/xfs/006.out b/tests/xfs/006.out
> new file mode 100644
> index 0000000..46f3b73
> --- /dev/null
> +++ b/tests/xfs/006.out
> @@ -0,0 +1,43 @@
> +QA output created by 006
> +wrote 4096/4096 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 0
> +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Phase 1 - find and verify superblock...
> +Phase 2 - using <TYPEOF> log
> +        - scan filesystem freespace and inode maps...
> +        - found root inode chunk
> +Phase 3 - for each AG...
> +        - scan (but don't clear) agi unlinked lists...
> +        - process known inodes and perform inode discovery...
> +        - process newly discovered inodes...
> +Phase 4 - check for duplicate blocks...
> +        - setting up duplicate extent list...
> +        - check for inodes claiming duplicate blocks...
> +No modify flag set, skipping phase 5
> +Phase 6 - check inode connectivity...
> +        - traversing filesystem ...
> +        - traversal finished ...
> +        - moving disconnected inodes to lost+found ...
> +disconnected inode XXX, would move to lost+found

There it is, but is moving it to lost+found the right thing to do,
given that it was on the unlinked list and should have had a zero
link count? i.e. aren't we supposed to free unlinked inodes with a
zero link count, not recover them to lost+found?


> +Phase 7 - verify link counts...
> +would have reset inode XXX nlinks from 0 to 1

Yeah, that seems like the wrong behaviour to have for an anonymous
O_TMPFILE file - it's making it visible because we moved it to
lost+found in phase 6....

Also, I don't see any icount mismatch, so the comment above in the
test is probably wrong.

> diff --git a/tests/xfs/group b/tests/xfs/group
> index ba34650..2f896e1 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -3,6 +3,7 @@
>  003 db auto quick
>  004 db auto quick
>  005 auto quick
> +006 auto quick

and repair.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfs: add a test for unlinked inodes due to O_TMPFILE

[Christoph Hellwig]

On Wed, Mar 05, 2014 at 07:39:27AM +1100, Dave Chinner wrote:
> > +_supported_fs generic
> 
> XFS only.

Hmm.  I remember fixing this up, but for some reason it didn't make it
into the final patch.

> > +# test creating a r/w tmpfile, do I/O and link it into the namespace
> > +$XFS_IO_PROG -x -T \
> > +	-c "pwrite 0 4096" \
> > +	-c "pread 0 4096" \
> > +	-c "freeze" \
> > +	-c "thaw" \
> > +	-c "shutdown" \
> > +	${SCRATCH_MNT} | _filter_xfs_io
> 
> $testfile?

No, O_TMPFILE doesn't take an actual file name but a "virtual" parent
directory.  It's a reall creative abuse of the open ABI..

> Also, I don't see the file being linked into the namespace, so the
> comment is probably wrong. Also, please add a comment as to
> why the freeze/thaw is necessary.

Yes, we don't want to link it so that we have it on the unlinke dinode
list.  The freeze/thaw is to make sure the log has been cleaned, I'll
add a comment explaining it.

> There it is, but is moving it to lost+found the right thing to do,
> given that it was on the unlinked list and should have had a zero
> link count? i.e. aren't we supposed to free unlinked inodes with a
> zero link count, not recover them to lost+found?

> Yeah, that seems like the wrong behaviour to have for an anonymous
> O_TMPFILE file - it's making it visible because we moved it to
> lost+found in phase 6....

Good question.  I thought about this a little and decided that it wasn't
worth special casing O_TMPFILE inodes in repair, but thinking about it a
bit more this also happens for normal unlinked but open files.  I can
look into this if you want, and would create another test for that case.

> 
> Also, I don't see any icount mismatch, so the comment above in the
> test is probably wrong.

We do have an icount mismatch, but _filter_repair filters it away.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs