xfstests: btrfs/316: cross-subvolume sparse copy

PATCH Superseded Koen De Wit 3 participants 7 messages Jul 2, 2013 – Jul 3, 2013 Btrfs file system

Share & tools

Koen De Wit

author

2013-07-02 09:29 UTC #0 lore raw

PATCH v3 1/1 xfstests: btrfs/316: cross-subvolume sparse copy

This testscript creates reflinks to files on different subvolumes, overwrites the original files and reflinks, and moves reflinked files between subvolumes.

Originally submitted as testcase 302, changes are made based on comments from Eric: http://oss.sgi.com/archives/xfs/2013-03/msg00231.html
Two new common/rc functions used in this script (_require_cp_reflink and _verify_reflink) have been submitted recently: http://oss.sgi.com/archives/xfs/2013-05/msg00745.html
Thanks to Eric Sandeen and Dave Chinner for the reviews.

Version 3: fixing wrapped patch.

Signed-off-by Koen De Wit <[email protected]>

---
 tests/btrfs/316     |  130 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/316.out |   39 +++++++++++++++
 tests/btrfs/group   |    1 +
 3 files changed, 170 insertions(+), 0 deletions(-)
 create mode 100755 tests/btrfs/316
 create mode 100644 tests/btrfs/316.out

tests/btrfs/316 new file +130 -0

new file mode 100755
index 0000000..7c9c368
@@ -0,0 +1,130 @@
+#! /bin/bash
+# FS QA Test No. btrfs/316
+#
+# Testing cross-subvolume sparse copy on btrfs
+#    - Create two subvolumes, mount one of them
+#    - Create a file on each (sub/root)volume,
+#      reflink them on the other volumes
+#    - Change one original and two reflinked files
+#    - Move reflinked files between subvolumes
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2013, Oracle and/or its affiliates.  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`
+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()
+{
+    umount $SCRATCH_MNT
+    rm -rf $TESTDIR1
+    rm -rf $TESTDIR2
+    btrfs subvolume delete $SUBVOL1 >> $seqres.full
+    btrfs subvolume delete $SUBVOL2 >> $seqres.full
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+
+_require_scratch
+_require_cp_reflink
+
+_checksum_files() {
+    for F in file1 file2 file3
+    do
+        for D in $TESTDIR1 $SCRATCH_MNT $SUBVOL2
+        do
+            md5sum $D/$F | _filter_test_dir | _filter_scratch
+        done
+    done
+}
+
+TESTDIR1=$TEST_DIR/test-$seq-1
+TESTDIR2=$TEST_DIR/test-$seq-2
+SUBVOL1=$TEST_DIR/subvol-$seq-1
+SUBVOL2=$TEST_DIR/subvol-$seq-2
+
+_scratch_unmount 2>/dev/null
+rm -rf $seqres.full
+rm -rf $TESTDIR1 $TESTDIR2
+btrfs subvol delete $SUBVOL1 >/dev/null 2>&1
+btrfs subvol delete $SUBVOL2 >/dev/null 2>&1
+
+mkdir $TESTDIR1
+mkdir $TESTDIR2
+btrfs subvolume create $SUBVOL1 >> $seqres.full
+btrfs subvolume create $SUBVOL2 >> $seqres.full
+_mount -t btrfs -o subvol=subvol-$seq-1 $TEST_DEV $SCRATCH_MNT
+
+echo "Create initial files"
+# TESTDIR1/file1 is very small and will be inlined
+$XFS_IO_PROG -f -c 'pwrite -S 0x61 0 10' $TESTDIR1/file1 >> $seqres.full
+$XFS_IO_PROG -f -c 'pwrite -S 0x62 0 13000' $SCRATCH_MNT/file2 >> $seqres.full
+$XFS_IO_PROG -f -c 'pwrite -S 0x63 0 17000' $SUBVOL2/file3 >> $seqres.full
+
+echo "Create reflinks to the initial files on other subvolumes"
+cp --reflink $TESTDIR1/file1 $SUBVOL1
+cp --reflink $TESTDIR1/file1 $SUBVOL2
+cp --reflink $SUBVOL1/file2 $TESTDIR1/
+cp --reflink $SUBVOL1/file2 $SUBVOL2
+cp --reflink $SUBVOL2/file3 $TESTDIR1/
+cp --reflink $SUBVOL2/file3 $SUBVOL1
+
+echo "Verify the reflinks"
+_verify_reflink $SCRATCH_MNT/file2 $TESTDIR1/file2
+_verify_reflink $SCRATCH_MNT/file2 $SUBVOL2/file2
+_verify_reflink $SUBVOL2/file3 $TESTDIR1/file3
+_verify_reflink $SUBVOL2/file3 $SCRATCH_MNT/file3
+echo "Verify the file contents:"
+_checksum_files
+
+echo -e "---\nOverwrite some files with new content"
+$XFS_IO_PROG -c 'pwrite -S 0x64 0 20' $TESTDIR1/file1 >> $seqres.full
+$XFS_IO_PROG -c 'pwrite -S 0x66 0 21000' $SUBVOL2/file2 >> $seqres.full
+$XFS_IO_PROG -c 'pwrite -S 0x65 5000 5000' $SCRATCH_MNT/file3 >> $seqres.full
+
+echo "Verify that non-overwritten reflinks still have the same data blocks"
+_verify_reflink $TESTDIR1/file2 $SCRATCH_MNT/file2
+_verify_reflink $TESTDIR1/file3 $SUBVOL2/file3
+echo "Verify the file contents:"
+_checksum_files
+
+echo -e "---\nShuffle files between directories"
+mv $TESTDIR1/file* $TESTDIR2
+mv $SCRATCH_MNT/file* $TESTDIR1/
+mv $SUBVOL2/file* $SCRATCH_MNT/
+mv $TESTDIR2/file* $SUBVOL2/
+
+# No _verify_reflink here as data is copied when moving files between subvols
+echo "Verify the file contents:"
+_checksum_files
+
+# success, all done
+status=0
+exit

tests/btrfs/316.out new file +39 -0

new file mode 100644
index 0000000..fbfa550
@@ -0,0 +1,39 @@
+QA output created by 316
+Create initial files
+Create reflinks to the initial files on other subvolumes
+Verify the reflinks
+Verify the file contents:
+e09c80c42fda55f9d992e59ca6b3307d  TEST_DIR/test-302-1/file1
+e09c80c42fda55f9d992e59ca6b3307d  SCRATCH_MNT/file1
+e09c80c42fda55f9d992e59ca6b3307d  TEST_DIR/subvol-302-2/file1
+d7402b46310fbbfbc5e466b1dccb043b  TEST_DIR/test-302-1/file2
+d7402b46310fbbfbc5e466b1dccb043b  SCRATCH_MNT/file2
+d7402b46310fbbfbc5e466b1dccb043b  TEST_DIR/subvol-302-2/file2
+5a95800e4c04b11117aa4e4de057721f  TEST_DIR/test-302-1/file3
+5a95800e4c04b11117aa4e4de057721f  SCRATCH_MNT/file3
+5a95800e4c04b11117aa4e4de057721f  TEST_DIR/subvol-302-2/file3
+---
+Overwrite some files with new content
+Verify that non-overwritten reflinks still have the same data blocks
+Verify the file contents:
+00d620f69f30327f0f8946b95c12de44  TEST_DIR/test-302-1/file1
+e09c80c42fda55f9d992e59ca6b3307d  SCRATCH_MNT/file1
+e09c80c42fda55f9d992e59ca6b3307d  TEST_DIR/subvol-302-2/file1
+d7402b46310fbbfbc5e466b1dccb043b  TEST_DIR/test-302-1/file2
+d7402b46310fbbfbc5e466b1dccb043b  SCRATCH_MNT/file2
+917619ae44b38bb9968af261c3c45440  TEST_DIR/subvol-302-2/file2
+5a95800e4c04b11117aa4e4de057721f  TEST_DIR/test-302-1/file3
+b9f275cd638cb784c9e61def94c622a8  SCRATCH_MNT/file3
+5a95800e4c04b11117aa4e4de057721f  TEST_DIR/subvol-302-2/file3
+---
+Shuffle files between directories
+Verify the file contents:
+e09c80c42fda55f9d992e59ca6b3307d  TEST_DIR/test-302-1/file1
+e09c80c42fda55f9d992e59ca6b3307d  SCRATCH_MNT/file1
+00d620f69f30327f0f8946b95c12de44  TEST_DIR/subvol-302-2/file1
+d7402b46310fbbfbc5e466b1dccb043b  TEST_DIR/test-302-1/file2
+917619ae44b38bb9968af261c3c45440  SCRATCH_MNT/file2
+d7402b46310fbbfbc5e466b1dccb043b  TEST_DIR/subvol-302-2/file2
+b9f275cd638cb784c9e61def94c622a8  TEST_DIR/test-302-1/file3
+5a95800e4c04b11117aa4e4de057721f  SCRATCH_MNT/file3
+5a95800e4c04b11117aa4e4de057721f  TEST_DIR/subvol-302-2/file3

tests/btrfs/group +1 -0

index bc6c256..a6d8de1 100644
@@ -9,3 +9,4 @@
 276 auto rw metadata
 284 auto
 307 auto quick
+316 auto quick

Signature · 1 line

1.7.2.5

Dave Chinner

2013-07-02 10:15 UTC #1 lore raw

Quoted 7 lines · Koen De Wit

This testscript creates reflinks to files on different subvolumes, overwrites the original files and reflinks, and moves reflinked files between subvolumes.
Originally submitted as testcase 302, changes are made based on comments from Eric: http://oss.sgi.com/archives/xfs/2013-03/msg00231.html
Two new common/rc functions used in this script (_require_cp_reflink and _verify_reflink) have been submitted recently: http://oss.sgi.com/archives/xfs/2013-05/msg00745.html
Thanks to Eric Sandeen and Dave Chinner for the reviews.
Version 3: fixing wrapped patch.

Needs a proper commit message - wrap it at 72 columns, change log should be placed below the --- devider, not be part of hte commit message.

Test numbers do not need to be unique across all test directories, just unique within the tests/btrfs directory.

tests/btrfs/316 line 63

Quoted 7 lines

tests/btrfs/316 new file +7 -0

+
+_checksum_files() {
+    for F in file1 file2 file3
+    do
+        for D in $TESTDIR1 $SCRATCH_MNT $SUBVOL2
+        do
+            md5sum $D/$F | _filter_test_dir | _filter_scratch

Just cut the file name out. No need for filtering at that point...

tests/btrfs/316 line 71

Quoted 8 lines

tests/btrfs/316 new file +8 -0

+        done
+    done
+}
+
+TESTDIR1=$TEST_DIR/test-$seq-1
+TESTDIR2=$TEST_DIR/test-$seq-2
+SUBVOL1=$TEST_DIR/subvol-$seq-1
+SUBVOL2=$TEST_DIR/subvol-$seq-2

....-316-.....

Review

Quoted 2 lines

(quoted diff) +2 -0

+Verify the file contents:
+e09c80c42fda55f9d992e59ca6b3307d  TEST_DIR/test-302-1/file1

which means the golden output is broken, despite the filtering

You should test your patches before posting ;)

FWIW, this is why you should simply cut the filename completely out like we do elsewhere....

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Koen De Wit

author

2013-07-02 14:28 UTC #2 lore raw

Dave,

Thanks for the review. I will clean up the commit message and do a full mail-to-myself-and-test-patch round trip to avoid errors like the wrong test numbers in the golden output. I'm sorry for this.

About cutting out file names from the output. I did this in the first version of the patch:

       md5sum $TESTDIR1/$F | $AWK_PROG 'END {print $1}'

but Eric Sandeen suggested to include them in order to provide more context in the output. (See http://oss.sgi.com/archives/xfs/2013-03/msg00231.html and http://oss.sgi.com/archives/xfs/2013-03/msg00220.html) That sounds like a good idea to me, it makes debugging failures easier. Whose opinion should I follow?

Koen.

Quoted 50 lines · Dave Chinner

Quoted 7 lines · Koen De Wit
This testscript creates reflinks to files on different subvolumes, overwrites the original files and reflinks, and moves reflinked files between subvolumes.
Originally submitted as testcase 302, changes are made based on comments from Eric: http://oss.sgi.com/archives/xfs/2013-03/msg00231.html
Two new common/rc functions used in this script (_require_cp_reflink and _verify_reflink) have been submitted recently: http://oss.sgi.com/archives/xfs/2013-05/msg00745.html
Thanks to Eric Sandeen and Dave Chinner for the reviews.
Version 3: fixing wrapped patch.
Needs a proper commit message - wrap it at 72 columns, change log should be placed below the --- devider, not be part of hte commit message.
Test numbers do not need to be unique across all test directories, just unique within the tests/btrfs directory.
tests/btrfs/316 line 63
Quoted 7 lines
tests/btrfs/316 new file +7 -0
+
+_checksum_files() {
+    for F in file1 file2 file3
+    do
+        for D in $TESTDIR1 $SCRATCH_MNT $SUBVOL2
+        do
+            md5sum $D/$F | _filter_test_dir | _filter_scratch
Just cut the file name out. No need for filtering at that point...
tests/btrfs/316 line 71
Quoted 8 lines
tests/btrfs/316 new file +8 -0
+        done
+    done
+}
+
+TESTDIR1=$TEST_DIR/test-$seq-1
+TESTDIR2=$TEST_DIR/test-$seq-2
+SUBVOL1=$TEST_DIR/subvol-$seq-1
+SUBVOL2=$TEST_DIR/subvol-$seq-2
....-316-.....
Review
Quoted 2 lines
(quoted diff) +2 -0
+Verify the file contents:
+e09c80c42fda55f9d992e59ca6b3307d  TEST_DIR/test-302-1/file1
which means the golden output is broken, despite the filtering
You should test your patches before posting ;)
FWIW, this is why you should simply cut the filename completely out like we do elsewhere....
Cheers,
Dave.

Eric Sandeen

2013-07-02 15:51 UTC #3 lore raw

Quoted 10 lines · Koen De Wit

Dave,
Thanks for the review. I will clean up the commit message and do a full mail-to-myself-and-test-patch round trip to avoid errors like the wrong test numbers in the golden output. I'm sorry for this.
About cutting out file names from the output. I did this in the first version of the patch:
      md5sum $TESTDIR1/$F | $AWK_PROG 'END {print $1}'
but Eric Sandeen suggested to include them in order to provide more context in the output. (See http://oss.sgi.com/archives/xfs/2013-03/msg00231.html and http://oss.sgi.com/archives/xfs/2013-03/msg00220.html) That sounds like a good idea to me, it makes debugging failures easier. Whose opinion should I follow?

Heh sorry. IMHO maybe a middle ground; not bare md5sum but show only the base name? In the end up to you; it seems Dave and I have different opinions on this. :)

Eric

Quoted 57 lines

Koen.
Quoted 50 lines · Dave Chinner
Quoted 7 lines · Koen De Wit
This testscript creates reflinks to files on different subvolumes, overwrites the original files and reflinks, and moves reflinked files between subvolumes.
Originally submitted as testcase 302, changes are made based on comments from Eric: http://oss.sgi.com/archives/xfs/2013-03/msg00231.html
Two new common/rc functions used in this script (_require_cp_reflink and _verify_reflink) have been submitted recently: http://oss.sgi.com/archives/xfs/2013-05/msg00745.html
Thanks to Eric Sandeen and Dave Chinner for the reviews.
Version 3: fixing wrapped patch.
Needs a proper commit message - wrap it at 72 columns, change log should be placed below the --- devider, not be part of hte commit message.
Test numbers do not need to be unique across all test directories, just unique within the tests/btrfs directory.
tests/btrfs/316 line 63
Quoted 7 lines
tests/btrfs/316 new file +7 -0
+
+_checksum_files() {
+    for F in file1 file2 file3
+    do
+        for D in $TESTDIR1 $SCRATCH_MNT $SUBVOL2
+        do
+            md5sum $D/$F | _filter_test_dir | _filter_scratch
Just cut the file name out. No need for filtering at that point...
tests/btrfs/316 line 71
Quoted 8 lines
tests/btrfs/316 new file +8 -0
+        done
+    done
+}
+
+TESTDIR1=$TEST_DIR/test-$seq-1
+TESTDIR2=$TEST_DIR/test-$seq-2
+SUBVOL1=$TEST_DIR/subvol-$seq-1
+SUBVOL2=$TEST_DIR/subvol-$seq-2
....-316-.....
Review
Quoted 2 lines
(quoted diff) +2 -0
+Verify the file contents:
+e09c80c42fda55f9d992e59ca6b3307d  TEST_DIR/test-302-1/file1
which means the golden output is broken, despite the filtering
You should test your patches before posting ;)
FWIW, this is why you should simply cut the filename completely out like we do elsewhere....
Cheers,
Dave.

Dave Chinner

2013-07-03 06:37 UTC #4 lore raw

Quoted 22 lines · Eric Sandeen

Quoted 10 lines · Koen De Wit
Dave,
Thanks for the review. I will clean up the commit message and do a full mail-to-myself-and-test-patch round trip to avoid errors like the wrong test numbers in the golden output. I'm sorry for this.
About cutting out file names from the output. I did this in the first version of the patch:
Quoted 9 lines
      md5sum $TESTDIR1/$F | $AWK_PROG 'END {print $1}'
but Eric Sandeen suggested to include them in order to provide more context in the output. (See
http://oss.sgi.com/archives/xfs/2013-03/msg00231.html and
http://oss.sgi.com/archives/xfs/2013-03/msg00220.html) That sounds like a good idea to me, it makes debugging failures easier. Whose opinion should I follow?

Quoted 3 lines

Heh sorry. IMHO maybe a middle ground; not bare md5sum but show only the base name? In the end up to you; it seems Dave and I have different opinions on this. :)

I was just going by current xfstests convention. i.e, in common/rc:

# Prints the md5 checksum of a given file
_md5_checksum()

{
        md5sum $1 | cut -d ' ' -f1
}

Which is used by all the hole punch tests and generic/311.

Make of that what you will, but I'd prefer to see consistency of implementation across tests... ;)

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Koen De Wit

author

2013-07-03 10:04 UTC #5 lore raw

Quoted 29 lines · Dave Chinner

Quoted 22 lines · Eric Sandeen
Quoted 10 lines · Koen De Wit
Dave,
Thanks for the review. I will clean up the commit message and do a full mail-to-myself-and-test-patch round trip to avoid errors like the wrong test numbers in the golden output. I'm sorry for this.
About cutting out file names from the output. I did this in the first version of the patch:
Quoted 9 lines
      md5sum $TESTDIR1/$F | $AWK_PROG 'END {print $1}'
but Eric Sandeen suggested to include them in order to provide more context in the output. (See
http://oss.sgi.com/archives/xfs/2013-03/msg00231.html and
http://oss.sgi.com/archives/xfs/2013-03/msg00220.html) That sounds like a good idea to me, it makes debugging failures easier. Whose opinion should I follow?
Quoted 3 lines
Heh sorry. IMHO maybe a middle ground; not bare md5sum but show only the base name? In the end up to you; it seems Dave and I have different opinions on this. :)
I was just going by current xfstests convention. i.e, in common/rc:

Quoted 7 lines

# Prints the md5 checksum of a given file
_md5_checksum()
{
        md5sum $1 | cut -d ' ' -f1
}
Which is used by all the hole punch tests and generic/311.

That's true, but these tests generate other context information in the output. They don't just print a bunch of checksums.

For example, the output of generic/255 looks like:

1. into a hole
daa100df6e6711906b61c9ab5aa16032
2. into allocated space
0: [0..7]: extent
1: [8..23]: hole
2: [24..39]: extent
cc58a7417c2d7763adc45b6fcd3fa024
3. into unwritten space
0: [0..7]: extent
1: [8..23]: hole
2: [24..39]: extent
daa100df6e6711906b61c9ab5aa16032
(...)

The output of generic/311 looks like:

Running test 1 buffered, normal suspend
Random seed is 1
ee6103415276cde95544b11b2675f132
ee6103415276cde95544b11b2675f132
Running test 1 direct, normal suspend
Random seed is 1
ee6103415276cde95544b11b2675f132
ee6103415276cde95544b11b2675f132
(...)

Quoted 2 lines

Make of that what you will, but I'd prefer to see consistency of implementation across tests... ;)

Do I agree if I change _checksum_files() to:

   _checksum_files() {
       for F in file1 file2 file3
       do
           echo "$F:"
           for D in $TESTDIR1 $SCRATCH_MNT $SUBVOL2
           do
               _md5_checksum $D/$F
           done
       done
   }

It produces this in the output:

(...)
file1:
00d620f69f30327f0f8946b95c12de44
e09c80c42fda55f9d992e59ca6b3307d
e09c80c42fda55f9d992e59ca6b3307d
file2:
d7402b46310fbbfbc5e466b1dccb043b
d7402b46310fbbfbc5e466b1dccb043b
917619ae44b38bb9968af261c3c45440
file3:
5a95800e4c04b11117aa4e4de057721f
b9f275cd638cb784c9e61def94c622a8
5a95800e4c04b11117aa4e4de057721f
(...)

Thanks,
Koen.

Dave Chinner

2013-07-03 10:17 UTC #6 lore raw

Quoted 37 lines · Koen De Wit

Quoted 35 lines · Dave Chinner
Quoted 22 lines · Eric Sandeen
Quoted 10 lines · Koen De Wit
Dave,
Thanks for the review. I will clean up the commit message and do a full mail-to-myself-and-test-patch round trip to avoid errors like the wrong test numbers in the golden output. I'm sorry for this.
About cutting out file names from the output. I did this in the first version of the patch:
Quoted 9 lines
      md5sum $TESTDIR1/$F | $AWK_PROG 'END {print $1}'
but Eric Sandeen suggested to include them in order to provide more context in the output. (See
http://oss.sgi.com/archives/xfs/2013-03/msg00231.html and
http://oss.sgi.com/archives/xfs/2013-03/msg00220.html) That sounds like a good idea to me, it makes debugging failures easier. Whose opinion should I follow?
Quoted 3 lines
Heh sorry. IMHO maybe a middle ground; not bare md5sum but show only the base name? In the end up to you; it seems Dave and I have different opinions on this. :)
I was just going by current xfstests convention. i.e, in common/rc:
# Prints the md5 checksum of a given file
_md5_checksum()
{
        md5sum $1 | cut -d ' ' -f1
}

Quoted 5 lines

Quoted 1 line
Which is used by all the hole punch tests and generic/311.
That's true, but these tests generate other context information in the output. They don't just print a bunch of checksums.

Sure, but it's pretty trivial to work out which sum in output belongs to which file in this test - there's only a handful of them.

Quoted 5 lines

   (...)
   file1:
   00d620f69f30327f0f8946b95c12de44
   e09c80c42fda55f9d992e59ca6b3307d
   e09c80c42fda55f9d992e59ca6b3307d

That's fine.

Cheers,

Dave,

Signature · 2 lines

Dave Chinner
[email protected]