| Message ID | 1382120790-31060-2-git-send-email-jbacik@fusionio.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 10/18/13 1:26 PM, Josef Bacik wrote: > There was a problem with send trying to overwrite a file that wasn't actually > the same. This is a test to check this particular case where receive fails when > it should succeed properly. I tested this to verify it fails without my fix and > passes with my fix. Thanks, 2 things - Why does the selinux context break things? That seems like a problem w/ send if it can't work on a context-mounted fs? (disabling it for now doesn't bother me, but I'm surprised that it's required). ((I also wonder if I should get rid of that context in general and use it only for tests which fail without it)) Rather than all the cd'ing around (to /) what if you just do something like: SEND_TEST_DIR=$TEST_DIR/$tmp_dir/send mkdir $SEND_TEST_DIR touch $SEND_TEST_DIR/baz touch $SEND_TEST_DIR/blah mkdir $SEND_TEST_DIR/foo touch $SEND_TEST_DIR/foo/bar that seems a bit cleaner to me vs. the cd back and forth. -Eric > Signed-off-by: Josef Bacik <jbacik@fusionio.com> > --- > tests/btrfs/015 | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/015.out | 2 + > tests/btrfs/group | 1 + > 3 files changed, 113 insertions(+) > create mode 100644 tests/btrfs/015 > create mode 100644 tests/btrfs/015.out > > diff --git a/tests/btrfs/015 b/tests/btrfs/015 > new file mode 100644 > index 0000000..f35600f > --- /dev/null > +++ b/tests/btrfs/015 > @@ -0,0 +1,110 @@ > +#! /bin/bash > +# FS QA Test No. btrfs/015 > +# > +# btrfs send ENOENT regression test, kernel bugzilla 60673 > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2013 Fusion IO. 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/$$ > +tmp_dir=send_temp_$seq > + > +status=1 # failure is the default! > + > +_cleanup() > +{ > + $BTRFS_UTIL_PROG subvol del $TEST_DIR/$tmp_dir/snap1 > /dev/null 2>&1 > + $BTRFS_UTIL_PROG subvol del $TEST_DIR/$tmp_dir/snap2 > /dev/null 2>&1 > + $BTRFS_UTIL_PROG subvol del $TEST_DIR/$tmp_dir/send > /dev/null 2>&1 > + rm -rf $TEST_DIR/$tmp_dir > + rm -f $tmp.* > +} > + > +trap "_cleanup ; exit \$status" 0 1 2 3 15 > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# real QA test starts here > +_supported_fs btrfs > +_supported_os Linux > +_require_scratch > + > +_scratch_mkfs > /dev/null 2>&1 > + > +#receive needs to be able to setxattrs, including the selinux context, if we use > +#the normal nfs context thing it screws up our ability to set the > +#security.selinux xattrs so we need to disable this for this test > +export SELINUX_MOUNT_OPTIONS="" > + > +_scratch_mount > + > +mkdir $TEST_DIR/$tmp_dir > +$BTRFS_UTIL_PROG subvol create $TEST_DIR/$tmp_dir/send \ > + > $seqres.full 2>&1 || _fail "failed subvol create" > + > +cd $TEST_DIR/$tmp_dir/send > + > +mkdir test > +touch test/baz > +touch test/blah > +mkdir test/foo > +touch test/foo/bar > + > +# cd out in case any of this fails > +cd / > + > +$BTRFS_UTIL_PROG subvol snap -r $TEST_DIR/$tmp_dir/send \ > + $TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap1" > + > +$BTRFS_UTIL_PROG send -f $TEST_DIR/$tmp_dir/send1.dump \ > + $TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed send" > + > +$BTRFS_UTIL_PROG receive -f $TEST_DIR/$tmp_dir/send1.dump $SCRATCH_MNT \ > + >> $seqres.full 2>&1 || _fail "failed receive" > + > +#recreate everything exactly the way it was exceptn in a different order so we > +#get different inode numbers > +cd $TEST_DIR/$tmp_dir/send > +rm -rf test > +mkdir test > +touch test/baz > +mkdir test/foo > +touch test/foo/bar > +touch test/blah > +cd / > + > +$BTRFS_UTIL_PROG subvol snap -r $TEST_DIR/$tmp_dir/send \ > + $TEST_DIR/$tmp_dir/snap2 >> $seqres.full 2>&1 || _fail "failed snap2" > + > +$BTRFS_UTIL_PROG send -f $TEST_DIR/$tmp_dir/send2.dump \ > + $TEST_DIR/$tmp_dir/snap2 -p $TEST_DIR/$tmp_dir/snap1 \ > + >> $seqres.full 2>&1 || _fail "failed second send" > + > +$BTRFS_UTIL_PROG receive -f $TEST_DIR/$tmp_dir/send2.dump $SCRATCH_MNT \ > + >> $seqres.full 2>&1 || _fail "failed second receive" > + > +echo "Silence is golden" > +status=0 ; exit > diff --git a/tests/btrfs/015.out b/tests/btrfs/015.out > new file mode 100644 > index 0000000..fee0fcf > --- /dev/null > +++ b/tests/btrfs/015.out > @@ -0,0 +1,2 @@ > +QA output created by 015 > +Silence is golden > diff --git a/tests/btrfs/group b/tests/btrfs/group > index 07df957..a6f1820 100644 > --- a/tests/btrfs/group > +++ b/tests/btrfs/group > @@ -17,3 +17,4 @@ > 012 auto > 013 auto quick > 014 auto > +015 auto quick > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 21, 2013 at 10:03:10AM -0500, Eric Sandeen wrote: > On 10/18/13 1:26 PM, Josef Bacik wrote: > > There was a problem with send trying to overwrite a file that wasn't actually > > the same. This is a test to check this particular case where receive fails when > > it should succeed properly. I tested this to verify it fails without my fix and > > passes with my fix. Thanks, > > 2 things - > > Why does the selinux context break things? That seems like a problem w/ send > if it can't work on a context-mounted fs? (disabling it for now doesn't bother > me, but I'm surprised that it's required). > So it is the context that xfstests is using, not contexts itself. Xfstests is using the nfs context, and using selinux contexts intercepts all getxattr calls, so when send tries to copy the xattrs for the file it calls getxattr, and because we are using the nfs context it returns EOPNOTSUPP from selinux, it never makes it down to btrfs. When using the actual real context it works fine because it calls down into the file system. > ((I also wonder if I should get rid of that context in general and use it only > for tests which fail without it)) > > Rather than all the cd'ing around (to /) what if you just do something like: > > SEND_TEST_DIR=$TEST_DIR/$tmp_dir/send > > mkdir $SEND_TEST_DIR > touch $SEND_TEST_DIR/baz > touch $SEND_TEST_DIR/blah > mkdir $SEND_TEST_DIR/foo > touch $SEND_TEST_DIR/foo/bar > > that seems a bit cleaner to me vs. the cd back and forth. > Yeah I can do that, thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/21/13 10:09 AM, Josef Bacik wrote: > On Mon, Oct 21, 2013 at 10:03:10AM -0500, Eric Sandeen wrote: >> > On 10/18/13 1:26 PM, Josef Bacik wrote: >>> > > There was a problem with send trying to overwrite a file that wasn't actually >>> > > the same. This is a test to check this particular case where receive fails when >>> > > it should succeed properly. I tested this to verify it fails without my fix and >>> > > passes with my fix. Thanks, >> > >> > 2 things - >> > >> > Why does the selinux context break things? That seems like a problem w/ send >> > if it can't work on a context-mounted fs? (disabling it for now doesn't bother >> > me, but I'm surprised that it's required). >> > > So it is the context that xfstests is using, not contexts itself. Xfstests is > using the nfs context, and using selinux contexts intercepts all getxattr calls, > so when send tries to copy the xattrs for the file it calls getxattr, and > because we are using the nfs context it returns EOPNOTSUPP from selinux, it > never makes it down to btrfs. When using the actual real context it works fine > because it calls down into the file system. > This still sounds weird. Is btrfs send trying to copy the selinux attrs directly? Shouldn't they be skipped, and be left up to the receiving end to set the selinux xattrs (or not) per the policy for the destination? -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/21/13 10:14 AM, Eric Sandeen wrote: > On 10/21/13 10:09 AM, Josef Bacik wrote: >> On Mon, Oct 21, 2013 at 10:03:10AM -0500, Eric Sandeen wrote: >>>> On 10/18/13 1:26 PM, Josef Bacik wrote: >>>>>> There was a problem with send trying to overwrite a file that wasn't actually >>>>>> the same. This is a test to check this particular case where receive fails when >>>>>> it should succeed properly. I tested this to verify it fails without my fix and >>>>>> passes with my fix. Thanks, >>>> >>>> 2 things - >>>> >>>> Why does the selinux context break things? That seems like a problem w/ send >>>> if it can't work on a context-mounted fs? (disabling it for now doesn't bother >>>> me, but I'm surprised that it's required). >>>> >> So it is the context that xfstests is using, not contexts itself. Xfstests is >> using the nfs context, and using selinux contexts intercepts all getxattr calls, >> so when send tries to copy the xattrs for the file it calls getxattr, and >> because we are using the nfs context it returns EOPNOTSUPP from selinux, it >> never makes it down to btrfs. When using the actual real context it works fine >> because it calls down into the file system. >> > > This still sounds weird. Is btrfs send trying to copy the selinux attrs directly? > > Shouldn't they be skipped, and be left up to the receiving end to set the selinux > xattrs (or not) per the policy for the destination? Eh, ok, Josef pointed out that "cp -a" does exactly the same thing. So I'll retract the concern & go learn more about selinux. ;) -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/btrfs/015 b/tests/btrfs/015 new file mode 100644 index 0000000..f35600f --- /dev/null +++ b/tests/btrfs/015 @@ -0,0 +1,110 @@ +#! /bin/bash +# FS QA Test No. btrfs/015 +# +# btrfs send ENOENT regression test, kernel bugzilla 60673 +# +#----------------------------------------------------------------------- +# Copyright (c) 2013 Fusion IO. 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/$$ +tmp_dir=send_temp_$seq + +status=1 # failure is the default! + +_cleanup() +{ + $BTRFS_UTIL_PROG subvol del $TEST_DIR/$tmp_dir/snap1 > /dev/null 2>&1 + $BTRFS_UTIL_PROG subvol del $TEST_DIR/$tmp_dir/snap2 > /dev/null 2>&1 + $BTRFS_UTIL_PROG subvol del $TEST_DIR/$tmp_dir/send > /dev/null 2>&1 + rm -rf $TEST_DIR/$tmp_dir + rm -f $tmp.* +} + +trap "_cleanup ; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_fs btrfs +_supported_os Linux +_require_scratch + +_scratch_mkfs > /dev/null 2>&1 + +#receive needs to be able to setxattrs, including the selinux context, if we use +#the normal nfs context thing it screws up our ability to set the +#security.selinux xattrs so we need to disable this for this test +export SELINUX_MOUNT_OPTIONS="" + +_scratch_mount + +mkdir $TEST_DIR/$tmp_dir +$BTRFS_UTIL_PROG subvol create $TEST_DIR/$tmp_dir/send \ + > $seqres.full 2>&1 || _fail "failed subvol create" + +cd $TEST_DIR/$tmp_dir/send + +mkdir test +touch test/baz +touch test/blah +mkdir test/foo +touch test/foo/bar + +# cd out in case any of this fails +cd / + +$BTRFS_UTIL_PROG subvol snap -r $TEST_DIR/$tmp_dir/send \ + $TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed snap1" + +$BTRFS_UTIL_PROG send -f $TEST_DIR/$tmp_dir/send1.dump \ + $TEST_DIR/$tmp_dir/snap1 >> $seqres.full 2>&1 || _fail "failed send" + +$BTRFS_UTIL_PROG receive -f $TEST_DIR/$tmp_dir/send1.dump $SCRATCH_MNT \ + >> $seqres.full 2>&1 || _fail "failed receive" + +#recreate everything exactly the way it was exceptn in a different order so we +#get different inode numbers +cd $TEST_DIR/$tmp_dir/send +rm -rf test +mkdir test +touch test/baz +mkdir test/foo +touch test/foo/bar +touch test/blah +cd / + +$BTRFS_UTIL_PROG subvol snap -r $TEST_DIR/$tmp_dir/send \ + $TEST_DIR/$tmp_dir/snap2 >> $seqres.full 2>&1 || _fail "failed snap2" + +$BTRFS_UTIL_PROG send -f $TEST_DIR/$tmp_dir/send2.dump \ + $TEST_DIR/$tmp_dir/snap2 -p $TEST_DIR/$tmp_dir/snap1 \ + >> $seqres.full 2>&1 || _fail "failed second send" + +$BTRFS_UTIL_PROG receive -f $TEST_DIR/$tmp_dir/send2.dump $SCRATCH_MNT \ + >> $seqres.full 2>&1 || _fail "failed second receive" + +echo "Silence is golden" +status=0 ; exit diff --git a/tests/btrfs/015.out b/tests/btrfs/015.out new file mode 100644 index 0000000..fee0fcf --- /dev/null +++ b/tests/btrfs/015.out @@ -0,0 +1,2 @@ +QA output created by 015 +Silence is golden diff --git a/tests/btrfs/group b/tests/btrfs/group index 07df957..a6f1820 100644 --- a/tests/btrfs/group +++ b/tests/btrfs/group @@ -17,3 +17,4 @@ 012 auto 013 auto quick 014 auto +015 auto quick
There was a problem with send trying to overwrite a file that wasn't actually the same. This is a test to check this particular case where receive fails when it should succeed properly. I tested this to verify it fails without my fix and passes with my fix. Thanks, Signed-off-by: Josef Bacik <jbacik@fusionio.com> --- tests/btrfs/015 | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/btrfs/015.out | 2 + tests/btrfs/group | 1 + 3 files changed, 113 insertions(+) create mode 100644 tests/btrfs/015 create mode 100644 tests/btrfs/015.out