Re: cross-fs copy support
On 10/1/18 9:48 AM, Qu Wenruo wrote: > > > On 2018/10/1 下午10:32, Joshi wrote: >> I was wondering about the cross-fs copy through copy_file_range. > > The term "cross-fs" looks pretty confusing. > > If you mean "cross-subvolume", then it should work without problem in btrfs. > > If you mean reflink across two different file systems (not matter the > same fs type or not). > Then it's impossible to work. I believe Joshi is talking about vfs_copy_file_range() not vfs_clone_file range(), although _copy_ does call _clone_ if it can. > Reflink (clone_file_range) works by inserting data pointers into the > filesystem other than really copying the data. > Thus if the source is outside of the fs, it's really impossible to work, > as the source pointer/data is completely out of control of the dest fs. Yes, I would expect there to be problems with his modified kernel for a filesystem that supports clone_file_range, because vfs_copy_file_range() will clone if possible, and this should fail across filesystems. In general, though, I don't know for sure why we don't fall back to do_splice_direct() across filesystems, although the filesystems that implement their own ->copy_file_range ops may have their own, further restrictions within their implementations. This call /is/ documented in the manpage as only being valid for files on the same filesystem, though: http://man7.org/linux/man-pages/man2/copy_file_range.2.html -Eric signature.asc Description: OpenPGP digital signature
Re: [patch] file dedupe (and maybe clone) data corruption (was Re: [PATCH] generic: test for deduplication between different files)
On 8/20/18 7:49 PM, Dave Chinner wrote: > Upon successful completion of this ioctl, the number of > bytes successfully deduplicated is returned in bytes_deduped > and a status code for the deduplication operation is > returned in status. If even a single byte in the range does > not match, the deduplication request will be ignored and > status set to FILE_DEDUPE_RANGE_DIFFERS. > > This implies we can dedupe less than the entire range as long as the > entire range matches. If the entire range does not match, we have > to return FILE_DEDUPE_RANGE_DIFFERS, but in this case it does match > so we can pick and choose how much we deduplicate. How much we > dedupe is then returned as a byte count. In this case, it will be a > few bytes short of the entire length requested because we aligned > the dedupe inwards > > Does that sound reasonable? I had hoped that dedupe was advisory as Darrick wished for, but TBH my reading of that is no, if you ask for a range to be deduped and any of it differs, "even a single byte," you fail it all. Why else would that last part be present, if the interface is free to ignore later parts that don't match and truncate the range to the matching portion? -Eric
[PATCH V4] test online label ioctl
This tests the online label ioctl that btrfs has, which has been recently proposed for XFS. To run, it requires an updated xfs_io with the label command and a filesystem that supports it A slight change here to _require_xfs_io_command as well, so that tests which simply fail with "Inappropriate ioctl" can be caught in the common case. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- Now with new and improved sequential V4 versioning! This passes on btrfs, _notruns on xfs/ext4 of yore, and passes on xfs w/ my online label patchset (as long as xfs_io has the new capability) V2: Add a max label length helper Set the proper btrfs max label length o_O oops Filter trailing whitespace from blkid output V3: lowercase local vars, simplify max label len function V4: use/test new -s / -c options to set and clear diff --git a/common/rc b/common/rc index ffe53236..333cfb82 100644 --- a/common/rc +++ b/common/rc @@ -2144,6 +2144,9 @@ _require_xfs_io_command() echo $testio | grep -q "Inappropriate ioctl" && \ _notrun "xfs_io $command support is missing" ;; + "label") + testio=`$XFS_IO_PROG -c "label" $TEST_DIR 2>&1` + ;; "open") # -c "open $f" is broken in xfs_io <= 4.8. Along with the fix, # a new -C flag was introduced to execute one shot commands. @@ -2182,7 +2185,7 @@ _require_xfs_io_command() rm -f $testfile 2>&1 > /dev/null echo $testio | grep -q "not found" && \ _notrun "xfs_io $command support is missing" - echo $testio | grep -q "Operation not supported" && \ + echo $testio | grep -q "Operation not supported\|Inappropriate ioctl" && \ _notrun "xfs_io $command failed (old kernel/wrong fs?)" echo $testio | grep -q "Invalid" && \ _notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)" @@ -3788,6 +3791,29 @@ _require_scratch_feature() esac } +# The maximum filesystem label length, /not/ including terminating NULL +_label_get_max() +{ + case $FSTYP in + xfs) + echo 12 + ;; + btrfs) + echo 255 + ;; + *) + _notrun "$FSTYP does not define maximum label length" + ;; + esac +} + +# Helper to check above early in a script +_require_label_get_max() +{ + # Just call _label_get_max which will notrun if appropriate + dummy=$(_label_get_max) +} + init_rc diff --git a/tests/generic/488 b/tests/generic/488 new file mode 100755 index ..3616522d --- /dev/null +++ b/tests/generic/488 @@ -0,0 +1,95 @@ +#! /bin/bash +# FS QA Test 488 +# +# Test the online filesystem label set/get ioctls +# +#--- +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# Author: Eric Sandeen <sand...@redhat.com> +# +# 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.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +_supported_fs generic +_supported_os Linux +_require_scratch +_require_xfs_io_command "label" +_require_label_get_max + +_scratch_mkfs > $seqres.full 2>&1 +_scratch_mount + +# Make sure we can set & clear the label +$XFS_IO_PROG -c "label -s label.$seq" $SCRATCH_MNT +$XFS_IO_PROG -c "label" $SCRATCH_MNT + +$XFS_IO_PROG -c "label -c" $SCRATCH_MNT +$XFS_IO_PROG -c "label" $SCRATCH_MNT + +# And that userspace can see it now, while mounted +# NB: some blkid has trailing whitespace, f
Re: [PATCH V3] test online label ioctl
On 5/15/18 7:51 PM, Dave Chinner wrote: On Tue, May 15, 2018 at 10:22:37AM -0500, Eric Sandeen wrote: This tests the online label ioctl that btrfs has, which has been recently proposed for XFS. To run, it requires an updated xfs_io with the label command and a filesystem that supports it A slight change here to _require_xfs_io_command as well, so that tests which simply fail with "Inappropriate ioctl" can be caught in the common case. Signed-off-by: Eric Sandeen<sand...@redhat.com> --- (urgh send as proper new thread, sorry) This passes on btrfs, _notruns on xfs/ext4 of yore, and passes on xfs w/ my online label patchset (as long as xfs_io has the new capability) V2: Add a max label length helper Set the proper btrfs max label length o_O oops Filter trailing whitespace from blkid output V3: lowercase local vars, simplify max label len function Looks good now, but I wondered about one thing the test doesn't cover: can you clear the label by setting it to a null string? i.e you check max length bounds, but don't check empty string behaviour... hohum, yes. I'll fix that, which will also require a change to xfs_io to be able to set a null string. Will send a V3 in a bit. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3] test online label ioctl
This tests the online label ioctl that btrfs has, which has been recently proposed for XFS. To run, it requires an updated xfs_io with the label command and a filesystem that supports it A slight change here to _require_xfs_io_command as well, so that tests which simply fail with "Inappropriate ioctl" can be caught in the common case. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- (urgh send as proper new thread, sorry) This passes on btrfs, _notruns on xfs/ext4 of yore, and passes on xfs w/ my online label patchset (as long as xfs_io has the new capability) V2: Add a max label length helper Set the proper btrfs max label length o_O oops Filter trailing whitespace from blkid output V3: lowercase local vars, simplify max label len function diff --git a/common/rc b/common/rc index ffe53236..333cfb82 100644 --- a/common/rc +++ b/common/rc @@ -2144,6 +2144,9 @@ _require_xfs_io_command() echo $testio | grep -q "Inappropriate ioctl" && \ _notrun "xfs_io $command support is missing" ;; + "label") + testio=`$XFS_IO_PROG -c "label" $TEST_DIR 2>&1` + ;; "open") # -c "open $f" is broken in xfs_io <= 4.8. Along with the fix, # a new -C flag was introduced to execute one shot commands. @@ -2182,7 +2185,7 @@ _require_xfs_io_command() rm -f $testfile 2>&1 > /dev/null echo $testio | grep -q "not found" && \ _notrun "xfs_io $command support is missing" - echo $testio | grep -q "Operation not supported" && \ + echo $testio | grep -q "Operation not supported\|Inappropriate ioctl" && \ _notrun "xfs_io $command failed (old kernel/wrong fs?)" echo $testio | grep -q "Invalid" && \ _notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)" @@ -3788,6 +3791,29 @@ _require_scratch_feature() esac } +# The maximum filesystem label length, /not/ including terminating NULL +_label_get_max() +{ + case $FSTYP in + xfs) + echo 12 + ;; + btrfs) + echo 255 + ;; + *) + _notrun "$FSTYP does not define maximum label length" + ;; + esac +} + +# Helper to check above early in a script +_require_label_get_max() +{ + # Just call _label_get_max which will notrun if appropriate + dummy=$(_label_get_max) +} + init_rc diff --git a/tests/generic/479 b/tests/generic/479 old mode 100644 new mode 100755 diff --git a/tests/generic/488 b/tests/generic/488 new file mode 100755 index ..9521c5ba --- /dev/null +++ b/tests/generic/488 @@ -0,0 +1,90 @@ +#! /bin/bash +# FS QA Test 488 +# +# Test the online filesystem label set/get ioctls +# +#--- +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# Author: Eric Sandeen <sand...@redhat.com> +# +# 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.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +_supported_fs generic +_supported_os Linux +_require_scratch +_require_xfs_io_command "label" +_require_label_get_max + +_scratch_mkfs > $seqres.full 2>&1 +_scratch_mount + +# Make sure we can set & clear the label +$XFS_IO_PROG -c "label label.$seq" $SCRATCH_MNT +$XFS_IO_PROG -c "label" $SCRATCH_MNT + +# And that userspace can see it now, while mounted +# NB: some blkid has trailing whitespace, filter it out here +blkid -s LABEL $SCRATCH_DEV | _filter_scratch | sed -e "s/ $
Re: [PATCH V2] test online label ioctl
On 5/14/18 6:11 PM, Dave Chinner wrote: > On Mon, May 14, 2018 at 12:09:16PM -0500, Eric Sandeen wrote: >> This tests the online label ioctl that btrfs has, which has been >> recently proposed for XFS. >> >> To run, it requires an updated xfs_io with the label command and a >> filesystem that supports it >> >> A slight change here to _require_xfs_io_command as well, so that tests >> which simply fail with "Inappropriate ioctl" can be caught in the >> common case. >> >> Signed-off-by: Eric Sandeen <sand...@redhat.com> >> --- >> +# The maximum filesystem label length, not including terminating NULL >> +_label_get_max() >> +{ >> +case $FSTYP in >> +xfs) >> +MAXLEN=12 >> +;; >> +btrfs) >> +MAXLEN=255 >> +;; >> +*) >> +MAXLEN=0 > > Why not just _notrun here? do we want to go through the other steps only to get here and notrun halfway through? >> +;; >> +esac >> + >> +echo $MAXLEN >> +} >> + >> +_require_label_get_max() >> +{ >> +if [ $(_label_get_max) -eq 0 ]; then >> +_notrun "$FSTYP does not define maximum label length" >> +fi > > And this check can go away? We'd like to know if all the validations in this can complete before we get started, right? That's why I did it this way. > Also, shouldn't it be _require_online_label_change() ? And then > maybe you can move the xfs_io label command check inside it? Well, we want to know a lot of things: 1) can the kernel code for this filesystem support online label 2) does xfs_io support the label command 3) does this test know the maximum label length to test for this fs the above stuff is for 3) >> +# remove previous $seqres.full before test >> +rm -f $seqres.full >> + >> +# real QA test starts here >> + >> +_supported_fs generic >> +_supported_os Linux >> +_require_scratch >> +_require_xfs_io_command "label" >> +_require_label_get_max >> + >> +_scratch_mkfs > $seqres.full 2>&1 >> +_scratch_mount >> + >> +# Make sure we can set & clear the label >> +$XFS_IO_PROG -c "label label.$seq" $SCRATCH_MNT >> +$XFS_IO_PROG -c "label" $SCRATCH_MNT >> + >> +# And that userspace can see it now, while mounted >> +# NB: some blkid has trailing whitespace, filter it out here >> +blkid -s LABEL $SCRATCH_DEV | _filter_scratch | sed -e "s/ $//g" >> + >> +# And that the it is still there when it's unmounted >> +_scratch_unmount >> +blkid -s LABEL $SCRATCH_DEV | _filter_scratch | sed -e "s/ $//g" > > Ok, so "LABEL" here is a special blkid match token yep >> +# And that it persists after a remount >> +_scratch_mount >> +$XFS_IO_PROG -c "label" $SCRATCH_MNT >> + >> +# And that a too-long label is rejected, beyond the interface max: >> +LABEL=$(perl -e "print 'l' x 257;") > > And now you use it as a variable. Nasty and confusing. Using lower > case for local variables is the norm, right? I thought we were only > supposed to use upper case for global test harness variables... I guess I didn't know about that convention (or forgot) ok, yeah, that's a little confusing I guess. *shrug* I can change it if you prefer. Obviously the "blkid -s LABEL" must remain "LABEL" > But even making it "label" is problematic: > >> +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT > > because "label" is an xfs_io command. Perhaps call it "fs_label"? ok -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2] test online label ioctl
This tests the online label ioctl that btrfs has, which has been recently proposed for XFS. To run, it requires an updated xfs_io with the label command and a filesystem that supports it A slight change here to _require_xfs_io_command as well, so that tests which simply fail with "Inappropriate ioctl" can be caught in the common case. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- (urgh send as proper new thread, sorry) This passes on btrfs, _notruns on xfs/ext4 of yore, and passes on xfs w/ my online label patchset (as long as xfs_io has the new capability) V2: Add a max label length helper Set the proper btrfs max label length o_O oops Filter trailing whitespace from blkid output diff --git a/common/rc b/common/rc index 9ffab7fd..88a99cff 100644 --- a/common/rc +++ b/common/rc @@ -2158,6 +2158,9 @@ _require_xfs_io_command() echo $testio | grep -q "Inappropriate ioctl" && \ _notrun "xfs_io $command support is missing" ;; + "label") + testio=`$XFS_IO_PROG -c "label" $TEST_DIR 2>&1` + ;; "open") # -c "open $f" is broken in xfs_io <= 4.8. Along with the fix, # a new -C flag was introduced to execute one shot commands. @@ -2196,7 +2199,7 @@ _require_xfs_io_command() rm -f $testfile 2>&1 > /dev/null echo $testio | grep -q "not found" && \ _notrun "xfs_io $command support is missing" - echo $testio | grep -q "Operation not supported" && \ + echo $testio | grep -q "Operation not supported\|Inappropriate ioctl" && \ _notrun "xfs_io $command failed (old kernel/wrong fs?)" echo $testio | grep -q "Invalid" && \ _notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)" @@ -3802,6 +3805,31 @@ _require_scratch_feature() esac } +# The maximum filesystem label length, not including terminating NULL +_label_get_max() +{ + case $FSTYP in + xfs) + MAXLEN=12 + ;; + btrfs) + MAXLEN=255 + ;; + *) + MAXLEN=0 + ;; + esac + + echo $MAXLEN +} + +_require_label_get_max() +{ + if [ $(_label_get_max) -eq 0 ]; then + _notrun "$FSTYP does not define maximum label length" + fi +} + init_rc diff --git a/tests/generic/479 b/tests/generic/479 old mode 100644 new mode 100755 diff --git a/tests/generic/485 b/tests/generic/485 new file mode 100755 index ..941afd3d --- /dev/null +++ b/tests/generic/485 @@ -0,0 +1,90 @@ +#! /bin/bash +# FS QA Test 485 +# +# Test the online filesystem label set/get ioctls +# +#--- +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# Author: Eric Sandeen <sand...@redhat.com> +# +# 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.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +_supported_fs generic +_supported_os Linux +_require_scratch +_require_xfs_io_command "label" +_require_label_get_max + +_scratch_mkfs > $seqres.full 2>&1 +_scratch_mount + +# Make sure we can set & clear the label +$XFS_IO_PROG -c "label label.$seq" $SCRATCH_MNT +$XFS_IO_PROG -c "label" $SCRATCH_MNT + +# And that userspace can see it now, while mounted +# NB: some blkid has trailing whitespace, filter it out here +blkid -s LABEL $SCRATCH_DEV | _filter_scratch | sed -e "s/ $//g" + +# And that the it is still there when it's unmounted +_scratch_unmount +bl
Re: [PATCH] test online label ioctl
This tests the online label ioctl that btrfs has, which has been recently proposed for XFS. To run, it requires an updated xfs_io with the label command and a filesystem that supports it A slight change here to _require_xfs_io_command as well, so that tests which simply fail with "Inappropriate ioctl" can be caught in the common case. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- This passes on btrfs, _notruns on xfs/ext4 of yore, and passes on xfs w/ my online label patchset (as long as xfs_io has the new capability) V2: Add a max label length helper Set the proper btrfs max label length o_O oops Filter trailing whitespace from blkid output diff --git a/common/rc b/common/rc index 9ffab7fd..88a99cff 100644 --- a/common/rc +++ b/common/rc @@ -2158,6 +2158,9 @@ _require_xfs_io_command() echo $testio | grep -q "Inappropriate ioctl" && \ _notrun "xfs_io $command support is missing" ;; + "label") + testio=`$XFS_IO_PROG -c "label" $TEST_DIR 2>&1` + ;; "open") # -c "open $f" is broken in xfs_io <= 4.8. Along with the fix, # a new -C flag was introduced to execute one shot commands. @@ -2196,7 +2199,7 @@ _require_xfs_io_command() rm -f $testfile 2>&1 > /dev/null echo $testio | grep -q "not found" && \ _notrun "xfs_io $command support is missing" - echo $testio | grep -q "Operation not supported" && \ + echo $testio | grep -q "Operation not supported\|Inappropriate ioctl" && \ _notrun "xfs_io $command failed (old kernel/wrong fs?)" echo $testio | grep -q "Invalid" && \ _notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)" @@ -3802,6 +3805,31 @@ _require_scratch_feature() esac } +# The maximum filesystem label length, not including terminating NULL +_label_get_max() +{ + case $FSTYP in + xfs) + MAXLEN=12 + ;; + btrfs) + MAXLEN=255 + ;; + *) + MAXLEN=0 + ;; + esac + + echo $MAXLEN +} + +_require_label_get_max() +{ + if [ $(_label_get_max) -eq 0 ]; then + _notrun "$FSTYP does not define maximum label length" + fi +} + init_rc diff --git a/tests/generic/479 b/tests/generic/479 old mode 100644 new mode 100755 diff --git a/tests/generic/485 b/tests/generic/485 new file mode 100755 index ..941afd3d --- /dev/null +++ b/tests/generic/485 @@ -0,0 +1,90 @@ +#! /bin/bash +# FS QA Test 485 +# +# Test the online filesystem label set/get ioctls +# +#--- +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# Author: Eric Sandeen <sand...@redhat.com> +# +# 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.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +_supported_fs generic +_supported_os Linux +_require_scratch +_require_xfs_io_command "label" +_require_label_get_max + +_scratch_mkfs > $seqres.full 2>&1 +_scratch_mount + +# Make sure we can set & clear the label +$XFS_IO_PROG -c "label label.$seq" $SCRATCH_MNT +$XFS_IO_PROG -c "label" $SCRATCH_MNT + +# And that userspace can see it now, while mounted +# NB: some blkid has trailing whitespace, filter it out here +blkid -s LABEL $SCRATCH_DEV | _filter_scratch | sed -e "s/ $//g" + +# And that the it is still there when it's unmounted +_scratch_unmount +blkid -s LABEL $SCRATCH_DEV | _filter_scrat
Re: [PATCH 1/2 V2] hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
On 5/11/18 9:32 AM, Chris Mason wrote: > On 11 May 2018, at 10:10, David Sterba wrote: > >> On Thu, May 10, 2018 at 08:16:09PM +0100, Al Viro wrote: >>> On Thu, May 10, 2018 at 01:13:57PM -0500, Eric Sandeen wrote: >>>> Move the btrfs label ioctls up to the vfs for general use. >>>> >>>> This retains 256 chars as the maximum size through the interface, which >>>> is the btrfs limit and AFAIK exceeds any other filesystem's maximum >>>> label size. >>>> >>>> Signed-off-by: Eric Sandeen <sand...@redhat.com> >>>> Reviewed-by: Andreas Dilger <adil...@dilger.ca> >>>> Reviewed-by: David Sterba <dste...@suse.com> >>> >>> No objections (and it obviously ought to go through btrfs tree). >> >> I can take it through my tree, but Eric mentioned that there's a patch >> for xfs that depends on it. In this case it would make sense to take >> both patches at once via the xfs tree. There are no pending conflicting >> changes in btrfs. > > Probably easiest to just have a separate pull dedicated just for this series. > That way it doesn't really matter which tree it goes through. Actually, I just realized that the changes to include/uapi/linux/fs.h are completely independent of any btrfs changes, right - there's nothing wrong w/ redefining the common ioctl under a different name in btrfs. So the fs.h patch could go first, through the xfs tree since it'll be using it. Once the common ioctl definition goes in, then btrfs can change to define its ioctls to the common ioctls, or act on them directly as my patch did, etc. Would that be a better plan? IOWs there's no urgent need to coordinate a btrfs change. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2 V2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
Move the btrfs label ioctls up to the vfs for general use. This retains 256 chars as the maximum size through the interface, which is the btrfs limit and AFAIK exceeds any other filesystem's maximum label size. Signed-off-by: Eric Sandeen <sand...@redhat.com> Reviewed-by: Andreas Dilger <adil...@dilger.ca> Reviewed-by: David Sterba <dste...@suse.com> --- V2: note that hoisted btrfs ioctls exist in ioctl-number.txt, new since reviews but I took a little license. diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt index 84bb74dcae12..1299df349b71 100644 --- a/Documentation/ioctl/ioctl-number.txt +++ b/Documentation/ioctl/ioctl-number.txt @@ -298,7 +298,8 @@ Code Seq#(hex) Include FileComments 0x90 00 drivers/cdrom/sbpcd.h 0x92 00-0F drivers/usb/mon/mon_bin.c 0x93 60-7F linux/auto_fs.h -0x94 all fs/btrfs/ioctl.h +0x94 all fs/btrfs/ioctl.hBtrfs filesystem + and linux/fs.h some lifted to vfs 0x97 00-7F fs/ceph/ioctl.h Ceph file system 0x99 00-0F 537-Addinboard driver <mailto:b...@buks.ipn.de> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 632e26d6f7ce..8feea790bd00 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_setflags(file, argp); case FS_IOC_GETVERSION: return btrfs_ioctl_getversion(file, argp); + case FS_IOC_GETFSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); + case FS_IOC_SETFSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); case FITRIM: return btrfs_ioctl_fitrim(file, argp); case BTRFS_IOC_SNAP_CREATE: @@ -,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_quota_rescan_wait(file, argp); case BTRFS_IOC_DEV_REPLACE: return btrfs_ioctl_dev_replace(fs_info, argp); - case BTRFS_IOC_GET_FSLABEL: - return btrfs_ioctl_get_fslabel(file, argp); - case BTRFS_IOC_SET_FSLABEL: - return btrfs_ioctl_set_fslabel(file, argp); case BTRFS_IOC_GET_SUPPORTED_FEATURES: return btrfs_ioctl_get_supported_features(argp); case BTRFS_IOC_GET_FEATURES: diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h index c8d99b9ca550..af29cc9032a2 100644 --- a/include/uapi/linux/btrfs.h +++ b/include/uapi/linux/btrfs.h @@ -823,10 +823,8 @@ enum btrfs_err_code { #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \ struct btrfs_ioctl_quota_rescan_args) #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46) -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ - char[BTRFS_LABEL_SIZE]) -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ - char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_GET_FSLABEL FS_IOC_GETFSLABEL +#define BTRFS_IOC_SET_FSLABEL FS_IOC_SETFSLABEL #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \ diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index d2a8313fabd7..9d132f8f2df8 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -242,6 +242,8 @@ struct fsxattr { #define FICLONERANGE _IOW(0x94, 13, struct file_clone_range) #define FIDEDUPERANGE _IOWR(0x94, 54, struct file_dedupe_range) +#define FSLABEL_MAX 256/* Max chars for the interface; each fs may differ */ + #defineFS_IOC_GETFLAGS _IOR('f', 1, long) #defineFS_IOC_SETFLAGS _IOW('f', 2, long) #defineFS_IOC_GETVERSION _IOR('v', 1, long) @@ -251,8 +253,10 @@ struct fsxattr { #define FS_IOC32_SETFLAGS _IOW('f', 2, int) #define FS_IOC32_GETVERSION_IOR('v', 1, int) #define FS_IOC32_SETVERSION_IOW('v', 2, int) -#define FS_IOC_FSGETXATTR _IOR ('X', 31, struct fsxattr) -#define FS_IOC_FSSETXATTR _IOW ('X', 32, struct fsxattr) +#define FS_IOC_FSGETXATTR _IOR('X', 31, struct fsxattr) +#define FS_IOC_FSSETXATTR _IOW('X', 32, struct fsxattr) +#define FS_IOC_GETFSLABEL _IOR(0x94, 49, char[FSLABEL_MAX]) +#define FS_IOC_SETFSLABEL _IOW(0x94, 50, char[FSLABEL_MAX]) /* * File system encryption support -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 V2] man2: New page documenting filesystem get/set label ioctls
On 5/10/18 12:29 PM, Eric Sandeen wrote: > This documents the proposed new vfs-level ioctls which can > get or set a mounted filesytem's label. > > Signed-off-by: Eric Sandeen <sand...@redhat.com> > --- > > V2: make primary file ioctl_getfslabel, link ioctl_setfslabel to it > note that getfslabel requires CAP_SYS_ADMIN *sigh* that should say "setfslabel" - man page is correct, patch changelog is not. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2 V2] man2: New page documenting filesystem get/set label ioctls
This documents the proposed new vfs-level ioctls which can get or set a mounted filesytem's label. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- V2: make primary file ioctl_getfslabel, link ioctl_setfslabel to it note that getfslabel requires CAP_SYS_ADMIN diff --git a/man2/ioctl_getfslabel.2 b/man2/ioctl_getfslabel.2 new file mode 100644 index 000..2c3375c --- /dev/null +++ b/man2/ioctl_getfslabel.2 @@ -0,0 +1,87 @@ +.\" Copyright (c) 2018, Red Hat, Inc. All rights reserved. +.\" +.\" %%%LICENSE_START(GPLv2+_DOC_FULL) +.\" This is free documentation; you can redistribute it and/or +.\" modify it under the terms of the GNU General Public License as +.\" published by the Free Software Foundation; either version 2 of +.\" the License, or (at your option) any later version. +.\" +.\" The GNU General Public License's references to "object code" +.\" and "executables" are to be interpreted as the output of any +.\" document formatting or typesetting system, including +.\" intermediate and printed output. +.\" +.\" This manual is distributed in the hope that it will 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 manual; if not, see +.\" <http://www.gnu.org/licenses/>. +.\" %%%LICENSE_END +.TH IOCTL-FSLABEL 2 2018-05-02 "Linux" "Linux Programmer's Manual" +.SH NAME +ioctl_fslabel \- get or set a filesystem label +.SH SYNOPSIS +.br +.B #include +.br +.B #include +.sp +.BI "int ioctl(int " fd ", FS_IOC_GETFSLABEL, char " label [FSLABEL_MAX]); +.br +.BI "int ioctl(int " fd ", FS_IOC_SETFSLABEL, char " label [FSLABEL_MAX]); +.SH DESCRIPTION +If a filesystem supports online label manipulation, these +.BR ioctl (2) +operations can be used to get or set the filesystem label for the filesystem +on which +.B fd +resides. +The +.B FS_IOC_SETFSLABEL +operation requires privilege +.RB ( CAP_SYS_ADMIN ). +.SH RETURN VALUE +On success zero is returned. On error, \-1 is returned, and +.I errno +is set to indicate the error. +.PP +.SH ERRORS +Error codes can be one of, but are not limited to, the following: +.TP +.B EINVAL +The specified label exceeds the maximum label length for the filesystem. +.TP +.B ENOTTY +This can appear if the filesystem does not support online label manipulation. +.TP +.B EPERM +The calling process does not have sufficient permissions to set the label. +.TP +.B EFAULT +.I label +references an inaccessible memory area. +.SH VERSIONS +These ioctl operations first appeared in Linux 4.18. +They were previously known as +.B BTRFS_IOC_GET_FSLABEL +and +.B BTRFS_IOC_SET_FSLABEL +and were private to Btrfs. +.SH CONFORMING TO +This API is Linux-specific. +.SH NOTES +The maximum string length for this interface is +.BR FSLABEL_MAX , +including the terminating null byte (\(aq\\0\(aq). +Filesystems have differing maximum label lengths, which may or +may not include the terminating null. The string provided to +.B FS_IOC_SETFSLABEL +must always be null-terminated, and the string returned by +.B FS_IOC_GETFSLABEL +will always be null-terminated. +.SH SEE ALSO +.BR ioctl (2), +.BR blkid (8) diff --git a/man2/ioctl_setfslabel.2 b/man2/ioctl_setfslabel.2 new file mode 100644 index 000..2119835 --- /dev/null +++ b/man2/ioctl_setfslabel.2 @@ -0,0 +1 @@ +.so man2/ioctl_getfslabel.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
On 5/9/18 12:35 PM, Randy Dunlap wrote: > On 05/09/2018 09:01 AM, Eric Sandeen wrote: >> Move the btrfs label ioctls up to the vfs for general use. >> >> This retains 256 chars as the maximum size through the interface, which >> is the btrfs limit and AFAIK exceeds any other filesystem's maximum >> label size. >> >> Signed-off-by: Eric Sandeen <sand...@redhat.com> >> --- >> >> Let the bikeshedding on the exact ioctl name begin ;) >> >> fs/btrfs/ioctl.c | 8 >> include/uapi/linux/btrfs.h | 6 ++ >> include/uapi/linux/fs.h| 8 ++-- >> 3 files changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c >> index 632e26d..2dd4cdf 100644 >> --- a/fs/btrfs/ioctl.c >> +++ b/fs/btrfs/ioctl.c >> @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int >> return btrfs_ioctl_setflags(file, argp); >> case FS_IOC_GETVERSION: >> return btrfs_ioctl_getversion(file, argp); >> +case FS_IOC_GETFSLABEL: >> +return btrfs_ioctl_get_fslabel(file, argp); >> +case FS_IOC_SETFSLABEL: >> +return btrfs_ioctl_set_fslabel(file, argp); >> case FITRIM: >> return btrfs_ioctl_fitrim(file, argp); >> case BTRFS_IOC_SNAP_CREATE: >> @@ -,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int >> return btrfs_ioctl_quota_rescan_wait(file, argp); >> case BTRFS_IOC_DEV_REPLACE: >> return btrfs_ioctl_dev_replace(fs_info, argp); >> -case BTRFS_IOC_GET_FSLABEL: >> -return btrfs_ioctl_get_fslabel(file, argp); >> -case BTRFS_IOC_SET_FSLABEL: >> -return btrfs_ioctl_set_fslabel(file, argp); >> case BTRFS_IOC_GET_SUPPORTED_FEATURES: >> return btrfs_ioctl_get_supported_features(argp); >> case BTRFS_IOC_GET_FEATURES: >> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h >> index c8d99b9..ec611c8 100644 >> --- a/include/uapi/linux/btrfs.h >> +++ b/include/uapi/linux/btrfs.h >> @@ -823,10 +823,8 @@ enum btrfs_err_code { >> #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \ >> struct btrfs_ioctl_quota_rescan_args) >> #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46) >> -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ >> - char[BTRFS_LABEL_SIZE]) >> -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ >> - char[BTRFS_LABEL_SIZE]) >> +#define BTRFS_IOC_GET_FSLABEL FS_IOC_GETFSLABEL >> +#define BTRFS_IOC_SET_FSLABEL FS_IOC_SETFSLABEL >> #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ >>struct btrfs_ioctl_get_dev_stats) >> #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \ >> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h >> index d2a8313..1df3707 100644 >> --- a/include/uapi/linux/fs.h >> +++ b/include/uapi/linux/fs.h >> @@ -242,6 +242,8 @@ struct fsxattr { >> #define FICLONERANGE_IOW(0x94, 13, struct file_clone_range) >> #define FIDEDUPERANGE _IOWR(0x94, 54, struct file_dedupe_range) >> >> +#define FSLABEL_MAX 256 /* Max chars for the interface; each fs may >> differ */ >> + >> #define FS_IOC_GETFLAGS _IOR('f', 1, long) >> #define FS_IOC_SETFLAGS _IOW('f', 2, long) >> #define FS_IOC_GETVERSION _IOR('v', 1, long) >> @@ -251,8 +253,10 @@ struct fsxattr { >> #define FS_IOC32_SETFLAGS _IOW('f', 2, int) >> #define FS_IOC32_GETVERSION _IOR('v', 1, int) >> #define FS_IOC32_SETVERSION _IOW('v', 2, int) >> -#define FS_IOC_FSGETXATTR _IOR ('X', 31, struct fsxattr) >> -#define FS_IOC_FSSETXATTR _IOW ('X', 32, struct fsxattr) >> +#define FS_IOC_FSGETXATTR _IOR('X', 31, struct fsxattr) >> +#define FS_IOC_FSSETXATTR _IOW('X', 32, struct fsxattr) >> +#define FS_IOC_GETFSLABEL _IOR(0x94, 49, char[FSLABEL_MAX]) >> +#define FS_IOC_SETFSLABEL _IOW(0x94, 50, char[FSLABEL_MAX]) > > Also update Documentation/ioctl/ioctl-number.txt, where it says that 0x94:all > are used for btrfs: > > 0x94 all fs/btrfs/ioctl.h > > AFAICT 0x94 is now split between vfs and btrfs. Please correct me if I > misunderstand. It is split, though it has been for a while now, see also: #define FICLONE _IOW(0x94, 9, int) #define FICLONERANGE_IOW(0x94, 13, struct file_clone_range) #define FIDEDUPERANGE _IOWR(0x94, 54, struct file_dedupe_range) but sure, I can send another patch for that on the next round. Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test online label ioctl
On 5/9/18 10:49 AM, Eryu Guan wrote: > On Mon, Apr 30, 2018 at 04:43:18PM -0500, Eric Sandeen wrote: >> This tests the online label ioctl that btrfs has, which has been >> recently proposed for XFS. >> >> To run, it requires an updated xfs_io with the label command and a >> filesystem that supports it >> >> A slight change here to _require_xfs_io_command as well, so that tests >> which simply fail with "Inappropriate ioctl" can be caught in the >> common case. >> >> Signed-off-by: Eric Sandeen <sand...@redhat.com> >> --- >> ... >> +# And that it succeeds right at the filesystem max: >> +case $FSTYP in >> +xfs) >> +MAXLEN=12; >> +;; >> +btrfs) >> +MAXLEN=256 > > Seems this should be 255, otherwise I got failure like: > > -label = "MAXLABEL" > +label: Invalid argument > > and MAXLEN=255 makes the test pass with btrfs. You are correct, I missed that they exclude the trailing null from the length. (Sorry, thought I tested this :( ) >> +;; >> +*) >> +MAXLEN=256 >> +echo "Your filesystem supports online label, please add max length" > > Perhaps we can introduce a new helper similar to _require_acl_get_max() > and _notrun the test if current $FSTYP doesn't define a maxlen on > filesystem label? Ok, sure. >> +;; >> +esac >> +LABEL=$(perl -e "print 'o' x $MAXLEN;") >> +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT | sed -e 's/o\+/MAXLABEL/' >> + >> +# And that it fails just past the filesystem max: >> +let TOOLONG=MAXLEN+1 >> +LABEL=$(perl -e "print 'o' x $TOOLONG;") >> +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT >> + >> +# success, all done >> +status=0 >> +exit >> diff --git a/tests/generic/485.out b/tests/generic/485.out >> new file mode 100644 >> index 000..bc54684 >> --- /dev/null >> +++ b/tests/generic/485.out >> @@ -0,0 +1,9 @@ >> +QA output created by 485 >> +label = "label.485" >> +label = "label.485" >> +SCRATCH_DEV: LABEL="label.485" >> +SCRATCH_DEV: LABEL="label.485" > > There're trailing whitespaces in above two lines, I thought they're the > output from xfs_io label command at first, but actually I have to remove > the spaces to make test pass. It might need a filter, this is output from blkid; it might have changed. I noticed the whitespace as well but IIRC it works here. Will look into these and fix stuff up. Thanks! -Eric > Thanks, > Eryu > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] man2: New page documenting filesystem get/set label ioctls
This documents the proposed new vfs-level ioctls which can get or set a mounted filesytem's label. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- btrfs folks, please verify that this accurately describes your current behavior, thanks. diff --git a/man2/ioctl_fslabel.2 b/man2/ioctl_fslabel.2 new file mode 100644 index 000..150aa53 --- /dev/null +++ b/man2/ioctl_fslabel.2 @@ -0,0 +1,83 @@ +.\" Copyright (c) 2018, Red Hat, Inc. All rights reserved. +.\" +.\" %%%LICENSE_START(GPLv2+_DOC_FULL) +.\" This is free documentation; you can redistribute it and/or +.\" modify it under the terms of the GNU General Public License as +.\" published by the Free Software Foundation; either version 2 of +.\" the License, or (at your option) any later version. +.\" +.\" The GNU General Public License's references to "object code" +.\" and "executables" are to be interpreted as the output of any +.\" document formatting or typesetting system, including +.\" intermediate and printed output. +.\" +.\" This manual is distributed in the hope that it will 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 manual; if not, see +.\" <http://www.gnu.org/licenses/>. +.\" %%%LICENSE_END +.TH IOCTL-FSLABEL 2 2018-05-02 "Linux" "Linux Programmer's Manual" +.SH NAME +ioctl_fslabel \- get or set a filesystem label +.SH SYNOPSIS +.br +.B #include +.br +.B #include +.sp +.BI "int ioctl(int " fd ", FS_IOC_GETFSLABEL, char " label [FSLABEL_MAX]); +.br +.BI "int ioctl(int " fd ", FS_IOC_SETFSLABEL, char " label [FSLABEL_MAX]); +.SH DESCRIPTION +If a filesystem supports online label manipulation, these +.BR ioctl (2) +operations can be used to get or set the filesystem label for the filesystem +on which +.B fd +resides. +.SH RETURN VALUE +On success zero is returned. On error, \-1 is returned, and +.I errno +is set to indicate the error. +.PP +.SH ERRORS +Error codes can be one of, but are not limited to, the following: +.TP +.B EINVAL +The specified label exceeds the maximum label length for the filesystem. +.TP +.B ENOTTY +This can appear if the filesystem does not support online label manipulation. +.TP +.B EPERM +The calling process does not have sufficient permissions to set the label. +.TP +.B EFAULT +.I label +references an inaccessible memory area. +.SH VERSIONS +These ioctl operations first appeared in Linux 4.18. +They were previously known as +.B BTRFS_IOC_GET_FSLABEL +and +.B BTRFS_IOC_SET_FSLABEL +and were private to Btrfs. +.SH CONFORMING TO +This API is Linux-specific. +.SH NOTES +The maximum string length for this interface is +.BR FSLABEL_MAX , +including the terminating null byte (\(aq\\0\(aq). +Filesystems have differing maximum label lengths, which may or +may not include the terminating null. The string provided to +.B FS_IOC_SETFSLABEL +must always be null-terminated, and the string returned by +.B FS_IOC_GETFSLABEL +will always be null-terminated. +.SH SEE ALSO +.BR ioctl (2), +.BR blkid (8) diff --git a/man2/ioctl_getfslabel.2 b/man2/ioctl_getfslabel.2 new file mode 100644 index 000..bfa8dca --- /dev/null +++ b/man2/ioctl_getfslabel.2 @@ -0,0 +1 @@ +.so man2/ioctl_fslabel.2 diff --git a/man2/ioctl_setfslabel.2 b/man2/ioctl_setfslabel.2 new file mode 100644 index 000..bfa8dca --- /dev/null +++ b/man2/ioctl_setfslabel.2 @@ -0,0 +1 @@ +.so man2/ioctl_fslabel.2 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] fs: hoist BTRFS_IOC_[SG]ET_FSLABEL to vfs
Move the btrfs label ioctls up to the vfs for general use. This retains 256 chars as the maximum size through the interface, which is the btrfs limit and AFAIK exceeds any other filesystem's maximum label size. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- Let the bikeshedding on the exact ioctl name begin ;) fs/btrfs/ioctl.c | 8 include/uapi/linux/btrfs.h | 6 ++ include/uapi/linux/fs.h| 8 ++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 632e26d..2dd4cdf 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -5444,6 +5444,10 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_setflags(file, argp); case FS_IOC_GETVERSION: return btrfs_ioctl_getversion(file, argp); + case FS_IOC_GETFSLABEL: + return btrfs_ioctl_get_fslabel(file, argp); + case FS_IOC_SETFSLABEL: + return btrfs_ioctl_set_fslabel(file, argp); case FITRIM: return btrfs_ioctl_fitrim(file, argp); case BTRFS_IOC_SNAP_CREATE: @@ -,10 +5559,6 @@ long btrfs_ioctl(struct file *file, unsigned int return btrfs_ioctl_quota_rescan_wait(file, argp); case BTRFS_IOC_DEV_REPLACE: return btrfs_ioctl_dev_replace(fs_info, argp); - case BTRFS_IOC_GET_FSLABEL: - return btrfs_ioctl_get_fslabel(file, argp); - case BTRFS_IOC_SET_FSLABEL: - return btrfs_ioctl_set_fslabel(file, argp); case BTRFS_IOC_GET_SUPPORTED_FEATURES: return btrfs_ioctl_get_supported_features(argp); case BTRFS_IOC_GET_FEATURES: diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h index c8d99b9..ec611c8 100644 --- a/include/uapi/linux/btrfs.h +++ b/include/uapi/linux/btrfs.h @@ -823,10 +823,8 @@ enum btrfs_err_code { #define BTRFS_IOC_QUOTA_RESCAN_STATUS _IOR(BTRFS_IOCTL_MAGIC, 45, \ struct btrfs_ioctl_quota_rescan_args) #define BTRFS_IOC_QUOTA_RESCAN_WAIT _IO(BTRFS_IOCTL_MAGIC, 46) -#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \ - char[BTRFS_LABEL_SIZE]) -#define BTRFS_IOC_SET_FSLABEL _IOW(BTRFS_IOCTL_MAGIC, 50, \ - char[BTRFS_LABEL_SIZE]) +#define BTRFS_IOC_GET_FSLABEL FS_IOC_GETFSLABEL +#define BTRFS_IOC_SET_FSLABEL FS_IOC_SETFSLABEL #define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \ struct btrfs_ioctl_get_dev_stats) #define BTRFS_IOC_DEV_REPLACE _IOWR(BTRFS_IOCTL_MAGIC, 53, \ diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index d2a8313..1df3707 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -242,6 +242,8 @@ struct fsxattr { #define FICLONERANGE _IOW(0x94, 13, struct file_clone_range) #define FIDEDUPERANGE _IOWR(0x94, 54, struct file_dedupe_range) +#define FSLABEL_MAX 256/* Max chars for the interface; each fs may differ */ + #defineFS_IOC_GETFLAGS _IOR('f', 1, long) #defineFS_IOC_SETFLAGS _IOW('f', 2, long) #defineFS_IOC_GETVERSION _IOR('v', 1, long) @@ -251,8 +253,10 @@ struct fsxattr { #define FS_IOC32_SETFLAGS _IOW('f', 2, int) #define FS_IOC32_GETVERSION_IOR('v', 1, int) #define FS_IOC32_SETVERSION_IOW('v', 2, int) -#define FS_IOC_FSGETXATTR _IOR ('X', 31, struct fsxattr) -#define FS_IOC_FSSETXATTR _IOW ('X', 32, struct fsxattr) +#define FS_IOC_FSGETXATTR _IOR('X', 31, struct fsxattr) +#define FS_IOC_FSSETXATTR _IOW('X', 32, struct fsxattr) +#define FS_IOC_GETFSLABEL _IOR(0x94, 49, char[FSLABEL_MAX]) +#define FS_IOC_SETFSLABEL _IOW(0x94, 50, char[FSLABEL_MAX]) /* * File system encryption support -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] hoist btrfs' label set/get ioctls to vfs, and document
I'm planning to add online label set/get support to xfs, and to do so I plan to re-use the existing btrfs ioctls, BTRFS_IOC_[SG]ET_FSLABEL We're still working out minor details on the xfs side, but I'd like to start the conversation regarding the new more generic interface ASAP, so here goes - patches to move the ioctls to the vfs and document them. (Other filesystems may wish to use this interface in the future as well) Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] test online label ioctl
This tests the online label ioctl that btrfs has, which has been recently proposed for XFS. To run, it requires an updated xfs_io with the label command and a filesystem that supports it A slight change here to _require_xfs_io_command as well, so that tests which simply fail with "Inappropriate ioctl" can be caught in the common case. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- this passes on btrfs, _notruns on xfs/ext4 of yore, and passes on xfs w/ my online label patchset (as long as xfs_io has the new capability) diff --git a/common/rc b/common/rc index 9ffab7f..c53a721 100644 --- a/common/rc +++ b/common/rc @@ -2158,6 +2158,9 @@ _require_xfs_io_command() echo $testio | grep -q "Inappropriate ioctl" && \ _notrun "xfs_io $command support is missing" ;; + "label") + testio=`$XFS_IO_PROG -c "label" $TEST_DIR 2>&1` + ;; "open") # -c "open $f" is broken in xfs_io <= 4.8. Along with the fix, # a new -C flag was introduced to execute one shot commands. @@ -2196,7 +2199,7 @@ _require_xfs_io_command() rm -f $testfile 2>&1 > /dev/null echo $testio | grep -q "not found" && \ _notrun "xfs_io $command support is missing" - echo $testio | grep -q "Operation not supported" && \ + echo $testio | grep -q "Operation not supported\|Inappropriate ioctl" && \ _notrun "xfs_io $command failed (old kernel/wrong fs?)" echo $testio | grep -q "Invalid" && \ _notrun "xfs_io $command failed (old kernel/wrong fs/bad args?)" diff --git a/tests/generic/485 b/tests/generic/485 new file mode 100755 index 000..79902c2 --- /dev/null +++ b/tests/generic/485 @@ -0,0 +1,99 @@ +#! /bin/bash +# FS QA Test 485 +# +# Test the online filesystem label set/get ioctls +# +#--- +# Copyright (c) 2018 Red Hat, Inc. All Rights Reserved. +# Author: Eric Sandeen <sand...@redhat.com> +# +# 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.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +_supported_fs generic +_supported_os Linux +_require_scratch +_require_xfs_io_command "label" + +_scratch_mkfs > $seqres.full 2>&1 +_scratch_mount + +# Make sure we can set & clear the label +$XFS_IO_PROG -c "label label.$seq" $SCRATCH_MNT +$XFS_IO_PROG -c "label" $SCRATCH_MNT + +# And that userspace can see it now, while mounted +blkid -s LABEL $SCRATCH_DEV | _filter_scratch + +# And that the it is still there when it's unmounted +_scratch_unmount +blkid -s LABEL $SCRATCH_DEV | _filter_scratch + +# And that it persists after a remount +_scratch_mount +$XFS_IO_PROG -c "label" $SCRATCH_MNT + +# And that a too-long label is rejected, beyond the interface max: +LABEL=$(perl -e "print 'l' x 257;") +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT + +# And that it succeeds right at the filesystem max: +case $FSTYP in +xfs) + MAXLEN=12; + ;; +btrfs) + MAXLEN=256 + ;; +*) + MAXLEN=256 + echo "Your filesystem supports online label, please add max length" + ;; +esac +LABEL=$(perl -e "print 'o' x $MAXLEN;") +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT | sed -e 's/o\+/MAXLABEL/' + +# And that it fails just past the filesystem max: +let TOOLONG=MAXLEN+1 +LABEL=$(perl -e "print 'o' x $TOOLONG;") +$XFS_IO_PROG -c "label $LABEL" $SCRATCH_MNT + +# success, all done +status=0 +exit diff --git a/tests/generic/485.out b/tests/generic/485.out new file mode 100644 index 000..bc5
Re: Does btrfs use crc32 for error correction?
On 9/19/17 10:35 AM, Timofey Titovets wrote: > Stupid question: > Does btrfs use crc32 for error correction? > If no, why? > > (AFAIK if using CRC that possible to fix 1 bit flip) > > P.S. I try check that (i create image, create text file, flip bit, try > read and btrfs show IO-error) > > Thanks! I wasn't aware that crc32 could (in general) be used for single bit correction; I've read up on that, and it seems pretty cool. However, I don't think that the generator polynomial used in crc32c /can/ be used for error correction. I just skimmed some reading, but that seems to be the case if I understand it correctly. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs-progs: fix cross-compile build
On 8/24/17 12:01 PM, David Sterba wrote: > On Wed, Aug 16, 2017 at 08:17:00AM +0800, Qu Wenruo wrote: >> >> >> On 2017年08月16日 02:11, Eric Sandeen wrote: >>> The mktables binary needs to be build with the host >>> compiler at built time, not the target compiler, because >>> it runs at build time to generate the raid tables. >>> >>> Copy auto-fu from xfsprogs and modify Makefile to >>> accomodate this. >>> >>> Reported-by: Hallo32 <hall...@gmx.net> >>> Signed-off-by: Eric Sandeen <sand...@redhat.com> >> >> Looks better than my previous patch. >> With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for >> native build environment. > > The contents of tables.c has practially not changed since its > introduction in 2000s. I see no sense in regenerating it each time > during build Make does know enough to generate it only as needed, of course. > and I don't want to introduce host CC build just for > this one file, standard practice or not. *shrug* it seems like the obviously correct fix to me, but I won't fight over it. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs-progs: fix cross-compile build
On 8/15/17 7:17 PM, Qu Wenruo wrote: > > > On 2017年08月16日 02:11, Eric Sandeen wrote: >> The mktables binary needs to be build with the host >> compiler at built time, not the target compiler, because >> it runs at build time to generate the raid tables. >> >> Copy auto-fu from xfsprogs and modify Makefile to >> accomodate this. >> >> Reported-by: Hallo32 <hall...@gmx.net> >> Signed-off-by: Eric Sandeen <sand...@redhat.com> > > Looks better than my previous patch. > With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for native > build environment. > > Reviewed-by: Qu Wenruo <quwenruo.bt...@gmx.com> Thanks - and sorry for missing your earlier patch, I didn't mean to ignore it. :) I just missed it. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs-progs-v4.12: cross compiling
On 8/15/17 12:28 PM, Eric Sandeen wrote: > I guess it's harder to do in btrfs-progs since it's not using autotools... Eh, I don't know why I thought that was still true :) patch sent. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs-progs: fix cross-compile build
The mktables binary needs to be build with the host compiler at built time, not the target compiler, because it runs at build time to generate the raid tables. Copy auto-fu from xfsprogs and modify Makefile to accomodate this. Reported-by: Hallo32 <hall...@gmx.net> Signed-off-by: Eric Sandeen <sand...@redhat.com> --- diff --git a/Makefile b/Makefile index b3e2b63..2647b95 100644 --- a/Makefile +++ b/Makefile @@ -323,7 +323,7 @@ version.h: version.sh version.h.in configure.ac mktables: kernel-lib/mktables.c @echo "[CC] $@" - $(Q)$(CC) $(CFLAGS) $< -o $@ + $(Q)$(BUILD_CC) $(BUILD_CFLAGS) $< -o $@ kernel-lib/tables.c: mktables @echo "[TABLE] $@" diff --git a/Makefile.inc.in b/Makefile.inc.in index 4e1b68c..0570bf8 100644 --- a/Makefile.inc.in +++ b/Makefile.inc.in @@ -4,6 +4,8 @@ export CC = @CC@ +BUILD_CC = @BUILD_CC@ +BUILD_CFLAGS = @BUILD_CFLAGS@ LN_S = @LN_S@ AR = @AR@ RM = @RM@ diff --git a/configure.ac b/configure.ac index 30055f8..bc590cc 100644 --- a/configure.ac +++ b/configure.ac @@ -26,6 +26,23 @@ AC_CONFIG_SRCDIR([btrfs.c]) AC_PREFIX_DEFAULT([/usr/local]) AC_PROG_CC +AC_ARG_VAR(BUILD_CC, [C compiler for build tools]) +if test "${BUILD_CC+set}" != "set"; then + if test $cross_compiling = no; then +BUILD_CC="$CC" + else +AC_CHECK_PROGS(BUILD_CC, gcc cc) + fi +fi +AC_ARG_VAR(BUILD_CFLAGS, [C compiler flags for build tools]) +if test "${BUILD_CFLAGS+set}" != "set"; then + if test $cross_compiling = no; then +BUILD_CFLAGS="$CFLAGS" + else +BUILD_CFLAGS="-g -O2" + fi +fi + AC_CANONICAL_HOST AC_C_CONST AC_C_VOLATILE -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs-progs-v4.12: cross compiling
On 8/15/17 7:48 AM, David Sterba wrote: > On Tue, Aug 15, 2017 at 02:44:07PM +0200, Hallo32 wrote: ... How the kernel deals with this kind of problem ? Looking at the source of btrfs Makefile, it is more simple to replace mktables: kernel-lib/mktables.c @echo "[CC] $@" $(Q)$(CC) $(CFLAGS) $< -o $@ with mktables: kernel-lib/mktables.c @echo "[HOSTCC] $@" $(Q)$(HOSTCC) $(CFLAGS) $< -o $@ where HOSTCC is defined as HOSTCC=gcc (may be the same applied also to CFLAGS <-> HOSTCFLAGS ?) >>> >>> If using HOSTCC then I'm fine with it. >> >> CFLAGS needs also be replaced by something like HOSTCFLAGS, because if >> you use something like mips/architecture specific CFLAGS, they may be >> not applicably on the host system. > > Good point. Without a regular/automated cross-compilation build testing > I think we could break it quite easily. I'm going to keep the > pregenerated file in git. Isn't using the host compiler for some binaries during a cross-compile a very standard thing to do? The kernel manages it, as shown above. xfsprogs does it (see libxfs/Makefile for the crc32table.h and crc32selftest targets), e2fsprogs does it (see gen_crc32ctable target in lib/ext2fs/Makefile), etc. Seems that checking in a generated file would be more prone to eventual errors, no? I guess it's harder to do in btrfs-progs since it's not using autotools... -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result
On 4/7/17 10:42 AM, Darrick J. Wong wrote: > On Fri, Apr 07, 2017 at 01:02:58PM +0800, Eryu Guan wrote: >> On Thu, Apr 06, 2017 at 11:28:01AM -0500, Eric Sandeen wrote: >>> On 4/6/17 11:26 AM, Theodore Ts'o wrote: >>>> On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >>>>> >>>>> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >>>>> after umount/mount cycle, then changed back to original result after >>>>> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) >>>> >>>> I haven't had time to look at this, but I'm not sure this test is a >>>> reasonable one on the face of it. >>>> >>>> A file system may choose to optimize a file's extent tree for whatever >>>> reason it wants, whenever it wants, including on an unmount --- and >>>> that would not be an invalid thing to do. So to have an xfstests that >>>> causes a test failure if a file system were to, say, do some cleanup >>>> at mount or unmount time, or when the file is next opened, to merge >>>> adjacent extents together (and hence change what is returned by >>>> FIEMAP) might be strange, or even weird --- but is this any of user >>>> space's business? Or anything we want to enforce as wrong wrong wrong >>>> by xfstests? >> >> So I was asking for a review from ext4 side instead of queuing it for >> next xfstests update :) > > In general FIEMAP can return pretty much whatever it wants, which > usually means that it won't report extents larger than the underlying > block mapping extents, though as we've seen it can split a single > on-disk extent into multiple FIEMAP records for the purpose of reporting > sharedness. > > For ext3 I'm wondering if it's the case that the first time we FIEMAP an > indirect map file we see a possibly-merged version of whatever's in the > particular leaf node we land in; then that information gets cached & > merged with other records in the extent status tree, such that > subsequent FIEMAP calls see longer extents than the first time around. > >>> I had the same question. If the exact behavior isn't defined anywhere, >>> I don't know what we can be testing, TBH. >> >> Agreed, I was about to ask for the expected behavior today if there was >> no new review comments on this patch. > > I think the expected behavior is that any behavior is expected. :( I suppose that if any particular filesystem wants to enforce stricter rules for its own fiemap interface, that could be done in a filesystem-specific test... -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result
On 4/6/17 11:26 AM, Theodore Ts'o wrote: > On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >> >> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >> after umount/mount cycle, then changed back to original result after >> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) > > I haven't had time to look at this, but I'm not sure this test is a > reasonable one on the face of it. > > A file system may choose to optimize a file's extent tree for whatever > reason it wants, whenever it wants, including on an unmount --- and > that would not be an invalid thing to do. So to have an xfstests that > causes a test failure if a file system were to, say, do some cleanup > at mount or unmount time, or when the file is next opened, to merge > adjacent extents together (and hence change what is returned by > FIEMAP) might be strange, or even weird --- but is this any of user > space's business? Or anything we want to enforce as wrong wrong wrong > by xfstests? I had the same question. If the exact behavior isn't defined anywhere, I don't know what we can be testing, TBH. -Eric > - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Thoughts on 'btrfs device stats' and security.
On 3/17/17 11:25 AM, Austin S. Hemmelgarn wrote: > I'm currently working on a plugin for colllectd [1] to track per-device > per-filesystem error rates for BTRFS volumes. Overall, this is actually > going quite well (I've got most of the secondary logic like matching > filesystems to watch and parsing the data done already), but I've come across > a rather nasty caveat on the actual data collection part. > > As of right now, there are only two ways I can see to get this data: > 1. Parse the output of `btrfs device stats` for the filesystem. > 2. Make the same ioctl() call that `btrfs device stats` does and compose the > data yourself. > > In both cases, one of the following has to be the case: > 1. You're running as root. > 2. You're running SUID root. > 3. You're running with CAP_SYS_ADMIN (I'm not 100% certain that this is the > correct capability, but it appears to be the case from my testing). > > In other words, you have to reduce the overall security of your system to be > able to get this data which is itself not security sensitive for most intents > and purposes. As one datapoint, xfs stats are ugo+r - see /proc/fs/xfs/stat or /sys/fs/xfs//stats/stats -r--r--r--. 1 root root 4096 Mar 17 13:58 stats However, the stats_clear file is only writable by root --w---. 1 root root 4096 Mar 17 13:58 stats_clear Stats & other info for ext4 are also ugo+r, other than an error trigger which is only writable by root, and for which a read is meaningless. /sys/fs/ext4/sda1/ -r--r--r--. 1 root root 4096 Mar 17 14:00 delayed_allocation_blocks -r--r--r--. 1 root root 4096 Mar 17 14:00 errors_count -rw-r--r--. 1 root root 4096 Mar 17 14:00 err_ratelimit_burst -rw-r--r--. 1 root root 4096 Mar 17 14:00 err_ratelimit_interval_ms -rw-r--r--. 1 root root 4096 Mar 17 14:00 extent_max_zeroout_kb -r--r--r--. 1 root root 4096 Mar 17 14:00 first_error_time -rw-r--r--. 1 root root 4096 Mar 17 14:00 inode_goal -rw-r--r--. 1 root root 4096 Mar 17 14:00 inode_readahead_blks -r--r--r--. 1 root root 4096 Mar 17 14:00 last_error_time -r--r--r--. 1 root root 4096 Mar 17 14:00 lifetime_write_kbytes -r--r--r--. 1 root root 4096 Mar 17 14:00 max_writeback_mb_bump -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_group_prealloc -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_max_to_scan -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_min_to_scan -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_order2_req -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_stats -rw-r--r--. 1 root root 4096 Mar 17 14:00 mb_stream_req -rw-r--r--. 1 root root 4096 Mar 17 14:00 msg_ratelimit_burst -rw-r--r--. 1 root root 4096 Mar 17 14:00 msg_ratelimit_interval_ms -rw-r--r--. 1 root root 4096 Mar 17 14:00 reserved_clusters -r--r--r--. 1 root root 4096 Mar 17 14:00 session_write_kbytes --w---. 1 root root 4096 Mar 17 14:00 trigger_fs_error -rw-r--r--. 1 root root 4096 Mar 17 14:00 warning_ratelimit_burst -rw-r--r--. 1 root root 4096 Mar 17 14:00 warning_ratelimit_interval_ms -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Generic/389 xfs_io segfault on btrfs and ext4
On 1/10/17 8:45 PM, Qu Wenruo wrote: > > > At 01/11/2017 10:40 AM, Eric Sandeen wrote: ... >> What version of kernel and xfsprogs were you testing, and what was the >> segfault? > > Kernel is mainline v4.10-rc1. > No debuginfo installed, so no backtrace, but since there is so many > > So the problem is the old xfsprogs, I'll update or compile xfsprogs. > Thanks for your kind info. Just FWIW, I always have latest xfsprogs in rawhide, but I don't usually push to other fedoras. It should be easy to grab https://kojipkgs.fedoraproject.org/packages/xfsprogs/4.9.0/1.fc26/src/xfsprogs-4.9.0-1.fc26.src.rpm and rebuild & install. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Generic/389 xfs_io segfault on btrfs and ext4
On 1/10/17 8:08 PM, Qu Wenruo wrote: > Hi, Andreas and Eryu, > > I found test case generic/389 fails on btrfs and ext4, and it fails with > xfs_io segfault. Details, please? Works ok here: # ./check generic/389 FSTYP -- ext4 PLATFORM -- Linux/x86_64 bp-05 4.9.0-rc1+ MKFS_OPTIONS -- /dev/sdb2 MOUNT_OPTIONS -- -o acl,user_xattr -o context=system_u:object_r:nfs_t:s0 /dev/sdb2 /mnt/scratch generic/389 1s ... 1s Ran: generic/389 Passed all 1 tests # rpm -q xfsprogs xfsprogs-4.9.0-1.el6.x86_64 What version of kernel and xfsprogs were you testing, and what was the segfault? Thanks, -Eric > So it seems to be a bug in xfsprogs, but I'm already using the latest release > from Fedora. > > Is this a known bug? > > Thanks, > Qu > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Converging userspace and kernel code
On 1/8/17 8:11 PM, Qu Wenruo wrote: > > > At 01/08/2017 09:16 PM, Goldwyn Rodrigues wrote: >> >> 1. Motivation >> While fixing user space tools for btrfs-progs, I found a couple of bugs >> which are already solved in kernel space but were not ported to user >> space. User space is a little ignored when it comes to fixing bugs in >> the core functionality. XFS developers have already performed this and >> the userspace and kernel code walks in lockstep for libxfs. > > Personally speaking, I'm not a fan of re-using kernel code in btrfs-progs. But it already does re-use kernel code, it's just that the re-use is extremely stale, with unfixed bugs in both directions as a result (at least last time I looked.) > In fact, in btrfs-progs, we don't need a lot of kernel facilities, > like page/VFS/lock(btrfs-progs works in single thread under most > case). > > And that should make btrfs-progs easier to maintain. But as Goldwyn already pointed out, many bugs have gone un-fixed in userspace, in code which was forked long ago from kernelspace. For things like locking it's trivial to define that away. xfsprogs does i.e. - /* miscellaneous kernel routines not in user space */ #define down_read(a)((void) 0) #define up_read(a) ((void) 0) #define spin_lock_init(a) ((void) 0) #define spin_lock(a)((void) 0) #define spin_unlock(a) ((void) 0) #define likely(x) (x) #define unlikely(x) (x) #define rcu_read_lock() ((void) 0) #define rcu_read_unlock() ((void) 0) #define WARN_ON_ONCE(expr) ((void) 0) > Furthermore, there are cases while kernel is doing things wrong while > btrfs-progs does it right. All the more reason to sync it up, fixes should always be in both places, right? I had looked at this a few years ago, and started trying to sync things up, but got daunted and busy and never completed anything. :( I sent a few fixups back in April 2013 to get things /slightly/ closer. The libxfs sync in xfs has borne fruit; I'm of the opinion that similar work would help btrfs too, though it can be a long road. (e2fsprogs has gone the other way, and has a completely separate re-implementation in userspace; it works, I guess, but I have to say that I really like the code commonality in xfs.) Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs-progs: fix build for programs including ioctl.h
On 10/27/16 9:54 PM, Eric Sandeen wrote: > On 10/13/16 12:36 PM, Eric Sandeen wrote: >> This was reported when docker failed to build against >> btrfs-progs v4.8.1. >> >> It includes ioctl.h which now calls BUILD_ASSERT(), which >> is defined in kerncompat.h, which was not included in the >> ioctl.h header file. > > Ping? Oh sorry I see it's in, I had searched by author and missed it. Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs-progs: fix build for programs including ioctl.h
On 10/13/16 12:36 PM, Eric Sandeen wrote: > This was reported when docker failed to build against > btrfs-progs v4.8.1. > > It includes ioctl.h which now calls BUILD_ASSERT(), which > is defined in kerncompat.h, which was not included in the > ioctl.h header file. Ping? -Eric > Signed-off-by: Eric Sandeen <sand...@redhat.com> > --- > > diff --git a/ioctl.h b/ioctl.h > index a7235c0..abea7ed 100644 > --- a/ioctl.h > +++ b/ioctl.h > @@ -26,6 +26,12 @@ extern "C" { > #include > #include > > +#if BTRFS_FLAT_INCLUDES > +#include "kerncompat.h" > +#else > +#include > +#endif /* BTRFS_FLAT_INCLUDES */ > + > #ifndef __user > #define __user > #endif > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs-progs: define FIEMAP_EXTENT_SHARED locally if needed
As it stands today, btrfs-progs won't build if the installed kernel headers don't define FIEMAP_EXTENT_SHARED. But that doesn't tell us anything about the capabilities of the /running/ kernel - so just define it locally if not found in the fiemap header, and allow the build to proceed. If run against an old kernel, worst case scenario is that no shared extents will be reported via the du command. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- you can take it or leave it, but I had this locally anyway, so if it's helpful here you go :) diff --git a/cmds-fi-du.c b/cmds-fi-du.c index a5b66e6..fa8f421 100644 --- a/cmds-fi-du.c +++ b/cmds-fi-du.c @@ -29,6 +29,11 @@ #include #include +/* Appeared upstream in 2.6.33 */ +#ifndef FIEMAP_EXTENT_SHARED +#define FIEMAP_EXTENT_SHARED 0x2000 +#endif + #include "utils.h" #include "commands.h" #include "kerncompat.h" diff --git a/configure.ac b/configure.ac index 8fd8f42..9b87b24 100644 --- a/configure.ac +++ b/configure.ac @@ -144,8 +144,6 @@ if test "$DISABLE_BTRFSCONVERT" = 0 && test "x$convertfs" = "x"; then AC_MSG_ERROR([no filesystems for convert, use --disable-convert instead]) fi -AX_CHECK_DEFINE([linux/fiemap.h], [FIEMAP_EXTENT_SHARED], [], - [AC_MSG_ERROR([no definition of FIEMAP_EXTENT_SHARED found])]) dnl Define _LIBS= and _CFLAGS= by pkg-config dnl -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Btrfs progs build fails for 4.8
On 10/14/16 10:49 AM, David Sterba wrote: > On Thu, Oct 13, 2016 at 03:25:57PM +0800, Qu Wenruo wrote: >> At 10/13/2016 01:26 AM, David Sterba wrote: >>> On Wed, Oct 12, 2016 at 10:01:27PM +0800, Qu Wenruo wrote: On 10/12/2016 09:58 PM, Abhay Sachan wrote: > Hi, > I tried building latest btrfs progs on CentOS 6, "./configure" failed > with the error: > > checking for FIEMAP_EXTENT_SHARED defined in linux/fiemap.h... no > configure: error: no definition of FIEMAP_EXTENT_SHARED found > > Any ideas what am I lacking in the system? Your kernel is too old and its header doesn't support FIEMAP_EXTENT_SHARED flag for fiemap ioctl. >>> >>> As this is not the first time, I wonder if we could provide a defintion >>> of the macro in progs, regardless of the installed headers. Note that >>> this does not mean the running kernel is old. Previously the user said >>> he was running a 4.4 kernel [1] (or it could be any other kernel >>> version). For that combination of headers and running kernel, I think >>> it's ok to add a fallback definition. >> >> Yes, fallback definition is good for case like running kernel supports >> SHARED_EXTENT flag, but not kernel header. >> >> But I'm a little concerned about such fallback definition will lead to >> corrupted result for "fi du". > > Well, not corrupted bug wrong. The shared data will be reported as > exclusive. > And since that flag is very important for tools like "btrfs filesystem du", for old kernel doesn't support EXTENT_SHARED flag, we have no choice but abort configuration. >>> >>> The check was added to configure so it's caught earlier than during >>> build. The 'fi du' command is useful, but not IMO critical to stop the >>> build. >> >> What about just disabling subcommands using SHARED_EXTENT flag? >> Like "fi du". > > This would need to be checked at runtime, based only on kernel version. > I think we can do that, print a warning in 'fi du' but otherwise let it > continue. Checking kernel version is pretty unreliable - distro kernels that backport will quite possibly fail the version test. Sure, it's possible to patch the check back out of userspace in the distro, but one must first know that such things exist, and look out for them... Maybe warn on kernel version < 2.6.33 /and/ no shared extents found. Or, why not just: 1) #ifndef FIEMAP_EXTENT_SHARED / #define FIEMAP_EXTENT_SHARED 0x2000 / #endif 2) carry on as usual. Worst case is that you don't get shared accounting on very old kernels. And FIEMAP_EXTENT_SHARED has been around since 2.6.33 so it's not going to bite too many people, right? (If you really wanted to, you could export has_fiemap_shared in the btrfs features sysfs file, and runtime could see if shared extents are detectable and if not, print a warning or something ... but given that this would be a new feature much newer than the fiemap flag I'm not sure that would do much good.) -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs-progs: fix build for programs including ioctl.h
This was reported when docker failed to build against btrfs-progs v4.8.1. It includes ioctl.h which now calls BUILD_ASSERT(), which is defined in kerncompat.h, which was not included in the ioctl.h header file. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- diff --git a/ioctl.h b/ioctl.h index a7235c0..abea7ed 100644 --- a/ioctl.h +++ b/ioctl.h @@ -26,6 +26,12 @@ extern "C" { #include #include +#if BTRFS_FLAT_INCLUDES +#include "kerncompat.h" +#else +#include +#endif /* BTRFS_FLAT_INCLUDES */ + #ifndef __user #define __user #endif -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Btrfs progs build fails for 4.8
On 10/12/16 9:45 AM, Abhay Sachan wrote: > Hi Qu, > I am running latest 4.8.1, which I compiled on the machine itself. You likely still have the fiemap.h from Centos' kernel-headers rpm, which is from an older kernel and does not define it. -Eric > Linux vm88 4.8.1 #1 SMP Thu Oct 13 10:33:08 IST 2016 x86_64 x86_64 > x86_64 GNU/Linux > > Regards, > Abhay > > On Wed, Oct 12, 2016 at 7:31 PM, Qu Wenruowrote: >> >> >> On 10/12/2016 09:58 PM, Abhay Sachan wrote: >>> >>> Hi, >>> I tried building latest btrfs progs on CentOS 6, "./configure" failed >>> with the error: >>> >>> checking for FIEMAP_EXTENT_SHARED defined in linux/fiemap.h... no >>> configure: error: no definition of FIEMAP_EXTENT_SHARED found >>> >>> Any ideas what am I lacking in the system? >> >> >> Your kernel is too old and its header doesn't support FIEMAP_EXTENT_SHARED >> flag for fiemap ioctl. >> >> And since that flag is very important for tools like "btrfs filesystem du", >> for old kernel doesn't support EXTENT_SHARED flag, we have no choice but >> abort configuration. >> >> Thanks, >> Qu >> >>> >>> Regards, >>> Abhay >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >>> the body of a message to majord...@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> > > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mkfs+mount failure of small fs on ppc64
On 9/13/16 4:44 PM, Eric Sandeen wrote: > on ppc64, 4.7-rc kernel, git btrfs-progs, v4.7.2: > > # truncate --size=500m testfile > # ./mkfs.btrfs testfile > # mkdir -p mnt > # mount -o loop testfile mnt Same failure on aarch64 if that makes it any more interesting. ;) # mount -o loop testfile mnt mount: mount /dev/loop0 on /root/mnt failed: No space left on device Sector size issue I guess, driven by page size. -Eric > btrfs-progs v4.7.2 > See http://btrfs.wiki.kernel.org for more information. > > Label: (null) > UUID: c531b759-a491-4c9f-a954-4787cea9106d > Node size: 65536 > Sector size:65536 > Filesystem size:500.00MiB > Block group profiles: > Data: single8.00MiB > Metadata: DUP 32.00MiB > System: DUP 8.00MiB > SSD detected: no > Incompat features: extref, skinny-metadata > Number of devices: 1 > Devices: >IDSIZE PATH > 1 500.00MiB testfile > > > # dmesg -c > [ 61.210287] loop: module loaded > [ 61.247105] BTRFS: device fsid a8d79cd0-977f-4b93-8410-246dc08b3683 devid > 1 transid 5 /dev/loop0 > [ 61.247391] BTRFS info (device loop0): disk space caching is enabled > [ 61.247397] BTRFS info (device loop0): has skinny extents > [ 61.270492] BTRFS info (device loop0): creating UUID tree > [ 61.312149] BTRFS warning (device loop0): failed to create the UUID tree: > -28 > [ 61.483028] BTRFS: open_ctree failed > > 2nd mount works: > > # mount -o loop testfile mnt > # dmesg -c > [ 87.504564] BTRFS info (device loop0): disk space caching is enabled > [ 87.504579] BTRFS info (device loop0): has skinny extents > [ 87.506979] BTRFS info (device loop0): creating UUID tree > > Any ideas? This seems to have regressed since 3.9.1, but there are a couple > other mkfs breakages in between, and my bisect was not fruitful. > > Thanks, > -Eric > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
mkfs+mount failure of small fs on ppc64
on ppc64, 4.7-rc kernel, git btrfs-progs, v4.7.2: # truncate --size=500m testfile # ./mkfs.btrfs testfile # mkdir -p mnt # mount -o loop testfile mnt btrfs-progs v4.7.2 See http://btrfs.wiki.kernel.org for more information. Label: (null) UUID: c531b759-a491-4c9f-a954-4787cea9106d Node size: 65536 Sector size:65536 Filesystem size:500.00MiB Block group profiles: Data: single8.00MiB Metadata: DUP 32.00MiB System: DUP 8.00MiB SSD detected: no Incompat features: extref, skinny-metadata Number of devices: 1 Devices: IDSIZE PATH 1 500.00MiB testfile # dmesg -c [ 61.210287] loop: module loaded [ 61.247105] BTRFS: device fsid a8d79cd0-977f-4b93-8410-246dc08b3683 devid 1 transid 5 /dev/loop0 [ 61.247391] BTRFS info (device loop0): disk space caching is enabled [ 61.247397] BTRFS info (device loop0): has skinny extents [ 61.270492] BTRFS info (device loop0): creating UUID tree [ 61.312149] BTRFS warning (device loop0): failed to create the UUID tree: -28 [ 61.483028] BTRFS: open_ctree failed 2nd mount works: # mount -o loop testfile mnt # dmesg -c [ 87.504564] BTRFS info (device loop0): disk space caching is enabled [ 87.504579] BTRFS info (device loop0): has skinny extents [ 87.506979] BTRFS info (device loop0): creating UUID tree Any ideas? This seems to have regressed since 3.9.1, but there are a couple other mkfs breakages in between, and my bisect was not fruitful. Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: create example debugfs file only in debugging build
On 9/1/16 7:44 AM, David Sterba wrote: > Signed-off-by: David Sterba <dste...@suse.com> Reviewed-by: Eric Sandeen <sand...@redhat.com> > --- > fs/btrfs/sysfs.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index 804bd1c42e47..31f09564e170 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -836,9 +836,18 @@ static int btrfs_init_debugfs(void) > if (!btrfs_debugfs_root_dentry) > return -ENOMEM; > > + /* > + * Example code, how export data through debugfs. > + * > + * file:/sys/kernel/debug/btrfs/test > + * contents of: btrfs_debugfs_test > + */ > +#ifdef CONFIG_BTRFS_DEBUG > debugfs_create_u64("test", S_IRUGO | S_IWUSR, btrfs_debugfs_root_dentry, > _debugfs_test); > #endif > + > +#endif > return 0; > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: remove pointless debugfs interface
On 8/31/16 2:08 PM, David Sterba wrote: > On Wed, Aug 31, 2016 at 10:13:49AM -0500, Eric Sandeen wrote: >> A /sys/kernel/debug/btrfs/test file was added nearly >> two and a half years ago, but it serves no purpose; > > It does. Introduced in 1bae30982bc86ab66d61ccb6e22792593b45d44d says > something about helping developers to easily export information from the > filesystem, to aid debugging. Writing the debugfs support code is not > obviously trivial, so it's idling in the source. Exporing a new value is > as easy as copy and update 3 lines of code. If you have no use for it, > fine. I had thought that Documentation/filesystems/debugfs.txt would suffice, but if you keep stuff lying around in btrfs just in case somebody needs to export a global variable in the future, I suppose that's cool too. ;) >> it stores and returns a value, but nothing in the btrfs >> code uses this value in any way. There are no other btrfs >> files in this debugfs dir. >> >> This was brought to my attention because it is world-writable; >> it is the only such file under /sys/kernel/debug, and without >> knowledge of its purpose, some users were alarmed by this. > > So let's fix the permissions. *shrug* ok. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2] btrfs: fix perms on demonstration debugfs interface
btrfs provides a helpful demonstration of how to export a global variable via debugfs; however, it is unique among other debugfs files in that it is world-writable, which causes some concern to people who are not familiar with its purpose. Fix it so that it is only user-writable. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 4879656..fb84685 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -834,7 +834,7 @@ static int btrfs_init_debugfs(void) if (!btrfs_debugfs_root_dentry) return -ENOMEM; - debugfs_create_u64("test", S_IRUGO | S_IWUGO, btrfs_debugfs_root_dentry, + debugfs_create_u64("test", S_IRUGO | S_IWUSR, btrfs_debugfs_root_dentry, _debugfs_test); #endif return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: remove pointless debugfs interface
A /sys/kernel/debug/btrfs/test file was added nearly two and a half years ago, but it serves no purpose; it stores and returns a value, but nothing in the btrfs code uses this value in any way. There are no other btrfs files in this debugfs dir. This was brought to my attention because it is world-writable; it is the only such file under /sys/kernel/debug, and without knowledge of its purpose, some users were alarmed by this. Another option would be to change the perms, but given that there is no point to it as far as I can tell, it seems best to simply remove it. Signed-off-by: Eric Sandeen <sand...@redhat.com> --- diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 4879656..8c3ffb9 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -24,7 +24,6 @@ #include #include #include -#include #include "ctree.h" #include "disk-io.h" @@ -728,12 +727,6 @@ int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices, /* /sys/fs/btrfs/ entry */ static struct kset *btrfs_kset; -/* /sys/kernel/debug/btrfs */ -static struct dentry *btrfs_debugfs_root_dentry; - -/* Debugging tunables and exported data */ -u64 btrfs_debugfs_test; - /* * Can be called by the device discovery thread. * And parent can be specified for seed device @@ -827,19 +820,6 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info, ret = sysfs_create_group(fsid_kobj, _feature_attr_group); } -static int btrfs_init_debugfs(void) -{ -#ifdef CONFIG_DEBUG_FS - btrfs_debugfs_root_dentry = debugfs_create_dir("btrfs", NULL); - if (!btrfs_debugfs_root_dentry) - return -ENOMEM; - - debugfs_create_u64("test", S_IRUGO | S_IWUGO, btrfs_debugfs_root_dentry, - _debugfs_test); -#endif - return 0; -} - int btrfs_init_sysfs(void) { int ret; @@ -848,28 +828,19 @@ int btrfs_init_sysfs(void) if (!btrfs_kset) return -ENOMEM; - ret = btrfs_init_debugfs(); - if (ret) - goto out1; - init_feature_attrs(); ret = sysfs_create_group(_kset->kobj, _feature_attr_group); - if (ret) - goto out2; + if (ret) { + kset_unregister(btrfs_kset); + return ret; + } return 0; -out2: - debugfs_remove_recursive(btrfs_debugfs_root_dentry); -out1: - kset_unregister(btrfs_kset); - - return ret; } void btrfs_exit_sysfs(void) { sysfs_remove_group(_kset->kobj, _feature_attr_group); kset_unregister(btrfs_kset); - debugfs_remove_recursive(btrfs_debugfs_root_dentry); } diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h index d7da1a4..45bad52 100644 --- a/fs/btrfs/sysfs.h +++ b/fs/btrfs/sysfs.h @@ -1,11 +1,6 @@ #ifndef _BTRFS_SYSFS_H_ #define _BTRFS_SYSFS_H_ -/* - * Data exported through sysfs - */ -extern u64 btrfs_debugfs_test; - enum btrfs_feature_set { FEAT_COMPAT, FEAT_COMPAT_RO, -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs-progs: du: fix to skip not btrfs dir/file
On 7/6/16 8:35 AM, Holger Hoffstätte wrote: > On 07/06/16 14:25, Wang Shilong wrote: ... >> After patch, it will look like: >>Total Exclusive Set shared Filename >> skipping not btrfs dir/file: boot >> skipping not btrfs dir/file: dev >> skipping not btrfs dir/file: proc >> skipping not btrfs dir/file: run >> skipping not btrfs dir/file: sys >> 0.00B 0.00B - //root/.bash_logout >> 0.00B 0.00B - //root/.bash_profile >> 0.00B 0.00B - //root/.bashrc >> 0.00B 0.00B - //root/.cshrc >> 0.00B 0.00B - //root/.tcshrc >> >> This works for me to analysis system usage and analysis >> performaces. > > This is great, but can we please skip the "skipping .." messages? > Maybe it's just me but I really don't see the value of printing them > when they don't contribute to the result. > They also mess up the display. :) I agree, those messages add no value. -Eric > thanks, > Holger -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs-progs 4.6 won't build on CentOS6
On 6/23/16 8:49 PM, Steven Haigh wrote: > I've tried to build the new tools for CentOS 6 / Scientific Linux 6 / RHEL 6 > etc. > > During the build process, I see: > cmds-fi-du.c: In function 'du_calc_file_space': > cmds-fi-du.c:330: error: 'FIEMAP_EXTENT_SHARED' undeclared (first use in this > function) > cmds-fi-du.c:330: error: (Each undeclared identifier is reported only once > cmds-fi-du.c:330: error: for each function it appears in.) > make: *** [cmds-fi-du.o] Error 1 > > I'm guessing this is probably due to a different GCC version used? I'm > guessing this is a simple fix for someone with knowhow... :) Fair warning, the btrfs.ko in centos6 is positively ancient, and it won't be updated. Just in case that matters to you ... not sure if you just want to build there, or use actually btrfs under a centos6 kernel -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Idea on compatibility for old distributions
On 5/9/16 8:16 PM, Qu Wenruo wrote: > Hi David, Mark, > > In the recent test for new btrfs-convert backward compatibility, I > found that cmds-fi-du.c uses FIEMAP_EXTENT_SHARED bits, which is not > present in kernel of old distributions like RHEL6 (Sorry, didn't test > on openSUSE equivalent). There is really no need to invest in compatibility with such old distros IMHO. As Phoronix breathlessly points out today, the tech preview of btrfs in RHEL6 has ended, and it is now deprecated in RHEL6. RHEL6-era btrfs was an interesting experiment, but it is one that will not continue. Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ideas on unified real-ro mount option across all filesystems
On 12/17/15 8:01 PM, Christoph Anton Mitterer wrote: > On Fri, 2015-12-18 at 09:29 +0800, Qu Wenruo wrote: >> Given that nothing in the documentation implies that the block >>> device itself >>> must remain unchanged on a read-only mount, I don't see any problem >>> which >>> needs fixing. MS_RDONLY rejects user IO; that's all. >> >> And thanks for the info provided by Karel, it's clear that at least >> mount(8) itself already has explain on what ro will do and what it >> won't do. > > I wouldn't really agree, here. At least not from the non-developer side > (and one should hope filesystems and their manpages aren't only made > for fs-devlopers). > > The manpage says: >> ro Mount the filesystem read-only. >> rw Mount the filesystem read-write. > > IMHO, that leaves absolutely unclear, what this actually means, > especially given that most end-users will probably consider the > filesystem and its device being basically "the same". Karel pointed out that recent mount(8) says: >-r, --read-only >Mount the filesystem read-only. A synonym is -o ro. > >Note that, depending on the filesystem type, state and >kernel behavior, the system may still write to the device. For >example, ext3 and ext4 will replay the journal if the >filesystem is dirty. To prevent this kind of write access, you >may want to mount an ext3 or ext4 filesystem with the ro,noload >mount options or set the block device itself to read-only >mode, see the blockdev(8) command. which should leave nothing to the imagination. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ideas on unified real-ro mount option across all filesystems
On 12/16/15 7:41 PM, Qu Wenruo wrote: > Hi, > > In a recent btrfs patch, it is going to add a mount option to disable > log replay for btrfs, just like "norecovery" for ext4/xfs. > > But in the discussion on the mount option name and use case, it seems > better to have an unified and fs independent mount option alias for > real RO mount > > Reasons: > 1) Some file system may have already used [no]"recovery" mount option >In fact, btrfs has already used "recovery" mount option. >Using "norecovery" mount option will be quite confusing for btrfs. Too bad btrfs picked those semantics when "norecovery" has existed on other filesystems for quite some time with a different meaning... :( > 2) More straight forward mount option >Currently, to get real RO mount, for ext4/xfs, user must use -o >ro,norecovery. >Just ro won't ensure real RO, and norecovery can't be used alone. >If we have a simple alias, it would be much better for user to use. >(it maybe done just in user space mount) mount(8) simply says: ro Mount the filesystem read-only. and mount(2) is no more illustrative: MS_RDONLY Mount file system read-only. kernel code is no help, either: #define MS_RDONLY1 /* Mount read-only */ They say nothing about what, exactly, "read-only" means. But since at least the early ext3 days, it means that you cannot write through the filesystem, not that the filesystem will leave the block device unmodified when it mounts. I have always interpreted it as simply "no user changes to the filesystem," and that is clearly what the vfs does with the flag... >Not to mention some fs (yeah, btrfs again) doesn't have "norecovery" >but "nologreplay". well, again, btrfs picked unfortunate semantics, given the precedent set by other filesystems. f2fs, ext4, gfs2, nilfs2, and xfs all support "norecovery" - xfs since forever, ext4 & f2fs since 2009, etc. > 3) A lot of user even don't now mount ro can still modify device >Yes, I didn't know this point until I checked the log replay code of >btrfs. >Adding such mount option alias may raise some attention of users. Given that nothing in the documentation implies that the block device itself must remain unchanged on a read-only mount, I don't see any problem which needs fixing. MS_RDONLY rejects user IO; that's all. If you want to be sure your block device rejects all IO for forensics or what have you, I'd suggest # blockdev --setro /dev/whatever prior to mount, and take it out of the filesystem's control. Or better yet, making an image and not touching the original. -Eric > Any ideas about this? -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: attacking btrfs filesystems via UUID collisions?
On 12/11/15 4:21 PM, Christoph Anton Mitterer wrote: >> Note that Btrfs is >> > not unique, XFS v5 does a very similar thing with volume UUID as >> > well, >> > and resulted in this change: >> > http://oss.sgi.com/pipermail/xfs/2015-April/041267.html > Do you mean that xfs may suffer from the same issues that we're talking > about here? If so, one should probably give them a notice. That was disabled temporarily because changing the fs UUID meant that every piece of checksummed metadata with an embedded UUID would then mismatch. It was fixed (re-allowed) with ce748ea xfs: create new metadata UUID field and incompat flag in the kernel and 9c4e12f xfsprogs: Add new sb_meta_uuid field, update userspace tools to manipulate it in xfsprogs. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay
On 12/7/15 12:06 AM, Qu Wenruo wrote: > Introduce a new mount option "nologreplay" to co-operate with "ro" mount > option to get real readonly mount, like "norecovery" in ext* and xfs. > > Since the new parse_options() need to check new flags at remount time, > so add a new parameter for parse_options(). > > Signed-off-by: Qu Wenruo> --- > Documentation/filesystems/btrfs.txt | 5 + > fs/btrfs/ctree.h| 4 +++- > fs/btrfs/disk-io.c | 7 --- > fs/btrfs/super.c| 20 +--- > 4 files changed, 29 insertions(+), 7 deletions(-) > > diff --git a/Documentation/filesystems/btrfs.txt > b/Documentation/filesystems/btrfs.txt > index c772b47..ac4ed68 100644 > --- a/Documentation/filesystems/btrfs.txt > +++ b/Documentation/filesystems/btrfs.txt > @@ -168,6 +168,11 @@ Options with (*) are default options and will not show > in the mount options. >notreelog > Enable/disable the tree logging used for fsync and O_SYNC writes. > > + nologreplay > + Disable the log tree replay at mount time for real read-only mount. > + Must be use with "ro" mount option and can't be disabled by mount > + option. This documentation is not clear to me - "can't be disabled by mount option?" I think you mean to talk about remount here? Perhaps something like: "... Must be used with 'ro' mount option. A filesystem mounted with the 'nologreplay' option cannot transition to a read-write mount via remount,rw - the filesystem must be unmounted and remounted if read-write access is desired." Thanks, -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay
On 12/7/15 10:52 AM, Chandan Rajendra wrote: > On Monday 07 Dec 2015 10:27:05 Eric Sandeen wrote: >> On 12/7/15 12:06 AM, Qu Wenruo wrote: >>> Introduce a new mount option "nologreplay" to co-operate with "ro" mount >>> option to get real readonly mount, like "norecovery" in ext* and xfs. >>> >>> Since the new parse_options() need to check new flags at remount time, >>> so add a new parameter for parse_options(). >>> >>> Signed-off-by: Qu Wenruo <quwen...@cn.fujitsu.com> >>> --- >>> >>> Documentation/filesystems/btrfs.txt | 5 + >>> fs/btrfs/ctree.h| 4 +++- >>> fs/btrfs/disk-io.c | 7 --- >>> fs/btrfs/super.c| 20 +--- >>> 4 files changed, 29 insertions(+), 7 deletions(-) >>> >>> diff --git a/Documentation/filesystems/btrfs.txt >>> b/Documentation/filesystems/btrfs.txt index c772b47..ac4ed68 100644 >>> --- a/Documentation/filesystems/btrfs.txt >>> +++ b/Documentation/filesystems/btrfs.txt >>> @@ -168,6 +168,11 @@ Options with (*) are default options and will not >>> show in the mount options.> >>>notreelog >>> >>> Enable/disable the tree logging used for fsync and O_SYNC writes. >>> >>> + nologreplay >>> + Disable the log tree replay at mount time for real read-only mount. >>> + Must be use with "ro" mount option and can't be disabled by mount >>> + option. >> >> This documentation is not clear to me - "can't be disabled by mount option?" >> >> I think you mean to talk about remount here? Perhaps something like: >> >> "... Must be used with 'ro' mount option. A filesystem mounted with the >> 'nologreplay' option cannot transition to a read-write mount via >> remount,rw - the filesystem must be unmounted and remounted if read-write >> access is desired." >> > Eric, I had assumed the same logic with respect to the transition from 'ro' to > 'rw' via remount. But when doing so, btrfs_remount() flags an error only when > a valid 'tree log' tree is present in the filesystem > i.e. btrfs_super_block->log_root has a non-zero value. Otherwise, > btrfs_remount() does not seem to have any problem with the transition from > 'ro' to 'rw'. Ok, I don't know if that's intended - but I think the docs should be clarified to explicitly state the expected behavior in any case. FWIW, new mount options and their descriptions should be added to BTRFS-MOUNT(5) as well. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Introduce new mount option to disable tree log replay
On 12/7/15 2:54 PM, Christoph Anton Mitterer wrote: ... > 2) a section that describes "ro" in btrfs-mount(5) which describes that > normal "ro" alone may cause changes on the device and which then refers > to hard-ro and/or the list of options (currently nologreplay) which are > required right now to make it truly ro. > > > I think this is important as an end-user probably expects "ro" to be > truly ro, Yeah, I don't know that this is true. It hasn't been true for over a decade (2?), with the most widely-used filesystem in linux history, i.e. ext3. So if btrfs wants to go on this re-education crusade, more power to you, but I don't know that it's really a fight worth fighting. ;) > so if he looks it up in the documentation (2) he should find > enough information that this isn't the case and what to do instead. > Further, as one might expect that in the future, other places (than > just the log) may cause changes to a device, even though mounted ro... > I think it's better for the end user to also have a real "hard-ro" like > option, than a possibly growing list of "noXYZ" where the end-user may > have no clue that something else is now also required to get the truly > read-only behaviour. # blockdev --setro ... -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug/regression: Read-only mount not read-only
On 12/2/15 3:23 AM, Qu Wenruo wrote: > > > Qu Wenruo wrote on 2015/12/02 17:06 +0800: >> >> >> Russell Coker wrote on 2015/12/02 17:25 +1100: >>> On Wed, 2 Dec 2015 06:05:09 AM Eric Sandeen wrote: >>>> yes, xfs does; we have "-o norecovery" if you don't want that, or need >>>> to mount a filesystem with a dirty log on a readonly device. >>> >>> That option also works with Ext3/4 so it seems to be a standard way of >>> dealing >>> with this. I think that BTRFS should do what Ext3/4 and XFS do in this >>> regard. >>> >> BTW, does -o norecovery implies -o ro? >> >> If not, how does it keep the filesystem consistent? >> >> I'd like to follow that ext2/xfs behavior, but I'm not familiar with >> those filesystems. >> >> Thanks, >> Qu >> > > OK, norecovery implies ro. For XFS, it doesn't imply it, it requires it; i.e. both must be stated explicitly: /* * no recovery flag requires a read-only mount */ if ((mp->m_flags & XFS_MOUNT_NORECOVERY) && !(mp->m_flags & XFS_MOUNT_RDONLY)) { xfs_warn(mp, "no-recovery mounts must be read-only."); return -EINVAL; } ext4 is the same, I believe: } else if (test_opt(sb, NOLOAD) && !(sb->s_flags & MS_RDONLY) && ext4_has_feature_journal_needs_recovery(sb)) { ext4_msg(sb, KERN_ERR, "required journal recovery " "suppressed and not mounted read-only"); goto failed_mount_wq; so if you'd like btrfs to be consistent with these, I would not make norecovery imply ro; rather, make I would make it require an explicit ro, i.e. mount -o ro,norecovery -Eric > So I think it's possible to do the same thing for btrfs. > I'll try to do it soon. > > Thanks, > Qu > >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug/regression: Read-only mount not read-only
On 12/2/15 11:48 AM, Austin S Hemmelgarn wrote: > On a side note, do either XFS or ext4 support removing the norecovery > option from the mount flags through mount -o remount? Even if they > don't, that might be a nice feature to have in BTRFS if we can safely > support it. It's not remountable today on xfs: /* ro -> rw */ if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(*flags & MS_RDONLY)) { if (mp->m_flags & XFS_MOUNT_NORECOVERY) { xfs_warn(mp, "ro->rw transition prohibited on norecovery mount"); return -EINVAL; } not sure about ext4. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug/regression: Read-only mount not read-only
On 12/1/15 1:00 PM, Chris Mason wrote: > On Mon, Nov 30, 2015 at 05:06:00PM +, Hugo Mills wrote: >> On Mon, Nov 30, 2015 at 11:48:01AM -0500, Chris Mason wrote: >>> On Sat, Nov 28, 2015 at 01:46:34PM +, Hugo Mills wrote: We've just had someone on IRC with a problem mounting their FS. The main problem is that they've got a corrupt log tree. That isn't the subject of this email, though. The issue I'd like to raise is that even with -oro as a point option, the FS is trying to replay the log tree. The dmesg output from mount -oro is at the end of the email. Now, my memory, experience and understanding is that the FS doesn't, and shouldn't replay the log tree on a RO mount, because the FS should still be consistent even without the reply, and RO-means-actually-RO is possible and desirable. (Compared to a journalling FS, where journal replay is required for a consistent, usable FS). So, this looks to me like a regression that's come in somewhere. (Just for completeness, the system in question usually runs 4.2.5, but the live CD the OP is using is 4.2.3). >>> >>> We do need to replay the log tree, even on readonly mounts. Otherwise >>> files created and fsunk before crashing may not even exist. >> >>I'm actually happy with that, as long as the log tree is retained >> until it _can_ be played back. I think it's much more important that >> read-only actually means read-only *as much as is possible* (if for no >> other reason than being able to test the status of the log tree). >> Obviously, for journalling FSes, a journal reply is required by the >> design of the FS, but with a CoW FS, the FS should be consistent if >> possibly outdated with a RO mount. > > Normally I'd agree, but we have a long tradition of mounting root > readonly at first for no good reason at all. This is why reiserfs/ext > (and I think xfs) all replay logs on readonly mounts. It's not an > admin initiated action but an early stage of boot. yes, xfs does; we have "-o norecovery" if you don't want that, or need to mount a filesystem with a dirty log on a readonly device. TBH I think it comes down to semantics: does a readonly mount mean that the filesystem will not write to the block device, or does it mean that you cannot write to the block device through the filesystem? Subtle difference. I think most filesystems treat it as "you cannot write to the filesystem" but will still replay the log for consistency, because that's what is normally expected. If you're doing forensics, blkdev --setro /dev/blah to be sure; use fs-specific mount options to bypass any log replay that would otherwise be done, and have at it ... -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: shall distros run btrfsck on boot?
On 11/24/15 12:56 AM, Duncan wrote: > Duncan posted on Tue, 24 Nov 2015 06:46:18 + as excerpted: > >> That wouldn't be entirely uncommon, because as Eric mentions, btrfs >> check is intended to be thorough, where the kernel mount-time check is >> intended to be fast. >> >> But of course, as Eric also mentions, that's yet another reason you >> don't want btrfs check running at boot... it's *SSLLLOOW*, because >> it's being thorough. > > Oops! Mis-attribution. Qu not Eric. > > (I had read both replies in my email but only saw Eric's on the list, > which I read in my news client via gmane's list2news service, when I > composed the above. So I presumed the points I remembered being made > were from Eric's post, when it was Qu's.) Yeah, I don't think that being thorough requires being slow. ;) In a nutshell, though, I think a filesystem repair should be an admin-initiated action, not something that surprises you on a boot, at least for a journaling filesystem which is designed to maintain its integrity even in the face of a power loss or crash. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: shall distros run btrfsck on boot?
On 11/24/15 2:38 PM, Austin S Hemmelgarn wrote: > if the system was > shut down cleanly, you're fine barring software bugs, but if it > crashed, you should be running a check on the FS. Um, no... The *entire point* of having a journaling filesystem is that after a crash or power loss, a journal replay on next mount will bring the metadata into a consistent state. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: shall distros run btrfsck on boot?
On 11/23/15 10:35 PM, Duncan wrote: > Christoph Anton Mitterer posted on Tue, 24 Nov 2015 05:02:34 +0100 as > excerpted: > >> Hey. >> >> Short question since that came up on debian-devel. >> >> Now that btrfs check get's more and more useful, are the developers >> going to recommend running it periodically on boot (of course that >> wouldn't work right now, as it would *always* check)? > > I'm a list regular and btrfs user, not a dev, but all the indications > continue to point to _not_ running it automatically at boot, nobody even > _suggesting_ otherwise. The btrfs kernel code itself detects and often > corrects many problems, and btrfs check is simply not recommended for > automatic at-boot scheduling -- if the kernel code can't fix it without > intervention, then the problem is too serious to be fixed without > intervention by some scheduled btrfs check run, as well. > > In fact, take a look at the shipped fsck.btrfs shell-script, based upon > the xfs one. FWIW, fsck.xfs is a no-op because xfs is a *journaling filesystem* and an unclean shutdown is not a reason to force a filesystem repair on the next boot. ...that's what the journal was for...! The filesystem should be 100% consistent after a mount & a log replay. The only reason fsck.xfs came into existence was to satisfy initscripts, IIRC. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] btrfs-progs: introduce framework to check kernel supported features
On 10/21/15 4:09 AM, Qu Wenruo wrote: >> +static int get_kernel_code() >> +{ >> +int ret; >> +struct utsname utsbuf; >> +char *version; >> + >> +ret = uname(); >> +if (ret) >> +return -ret; >> + >> +version = strtok(utsbuf.release, "-"); >> + >> +return version_to_code(version); >> +} > > The only problem is, kernel version is never reliable. > If someone wants, uname output may even contain no numeric value. yep, I agree. This will be misery for any custom kernel. > IIRC, I suggest to maintain similar feature matrix in fstests, but Dave > pointed out the above problem. > > So I'm not fan of reading kernel version and generate supported features for > that. > > IMHO, just use /sys/fs/btrfs/features is good enough. *nod* > And if there is no such file, just ignore it, user is responsible for > such case. Yep, 3.14 was over a year and a half ago, I don't see much point in hardcoding kernel versions for such old kernels in the current upstream codebase. The only kernels that old still running are likely distro kernels, and they can solve this problem by backporting the /sys/fs/btrfs/features patch. -Eric > Thanks, > Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions about FIEMAP
On 10/11/15 11:37 PM, Wang, Zhiye wrote: > Hello everyone, > > After googled a bit, I got information that btrfs supports FIEMAP (as "cp" > needs it), but it's not valid for "write" operation. cp should not be using fiemap any more. It was for a while, until they realized that copying based on fiemap output could lead to corruption because things changed between the fiemap call and the actual copy... > I guess we cannot write to block device directly after get block list using > FIEMAP. This is because: > > 1. COW feature of btrfs (but this can be disabled using NOCOW) > 2. File system rebalance > 3. Defragmentation > > Aren't item #2 and #3 also a problem for "read" operation? For example, after > "cp" get block list using FIEMAP, file system rebalance occurs, So, previous > result of FIEMAP is not valid anymore. > > Or maybe I misunderstood something. Please correct me. That all may be true for btrfs, but more fundamentally as dsterba said, nothing guarantees that the layout won't change *immediately* after your fiemap call. This is the case on any filesystem, not just btrfs. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] fstests: generic: Test that fsync works on file in overlayfs merged directory
On 9/30/15 4:56 PM, Dave Chinner wrote: > On Wed, Sep 30, 2015 at 10:57:45PM +0300, Roman Lebedev wrote: >> As per overlayfs documentation, any activity on a merged directory >> for a application that is doing such activity should work exactly >> as if that would be a normal, non overlayfs-merged directory. >> >> That is, e.g. simple fopen-fwrite-fsync-fclose sequence should >> work just fine. > > We have plenty of tests that do things like that. > >> But apparently it does not. Add a simple generic test to check that. >> As of right now (linux-4.2.1) this test fails at least on btrfs. >> >> PS: An alternative (and probably better approach) would be to run >> fstests test suite with TEST_DIR set to overlayfs work directory. > > Much better is to run xfstests directly on overlayfs. THere have > been some patches to do that posted in the past, but those patches > and discussions kinda ended up going nowhere: > > http://www.mail-archive.com/fstests@vger.kernel.org/msg00474.html > > Perhaps you'd like to pick this up, and then overlay will by much > easier to test and hence likely not to have bugs like this... Yeah, that could still be used for fun, but Zach's POV was that we should just have a specific overlayfs config (dictating paths to over/under/merge/around/through/whatever directories), a special mount_overlayfs helper, etc, ala NFS & CIFS. It may actually be easier than what I proposed. If you want to take a stab at it I'm happy to help, answer questions, etc - I'm not sure when I'll get back to it... -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fstests: generic/018: expand write backwards sync but contiguous to test regression in btrfs
On 8/13/15 3:47 AM, Liu Bo wrote: Btrfs has a problem when defraging a file which has a large fragment'ed range, it'd leave the tail extent as a seperate extent instead of merging it with previous extents. This makes generic/018 recognize the above regression. Sorry for the late review, but here it is ;) In 2 years (heck, even now) we'll have no idea why this change was made. What regression is that? Can you describe it? Is there already an upstream fix/commit you can refer to? I see 3 changes here: 1) You change xfs_io's for loop from seq 9 -1 0 to seq 64 -1 0 - presumably this matters to btrfs. Why does this matter? Meanwhile, I find that in the case of 'write backwards sync but contiguous, ext4 doesn't produce fragments like btrfs and xfs, so I modify 018.out a little bit to let ext4 pass. 2) You stop expecting 10 extents initially in the backwards-write test for the above reason, I guess. I'm a little unsure about this. For me, this passes as-is. If it isn't working for you, we should understand why, instead of making the test ignore it. (And bundling this ext4 change into a btrfs-specific commit isn't great, anyway) Moreover, I follow Filipe's suggestion to filter xfs_io's output in order to check these writes actually succeed. 3) You stop redirecting xfs_io to /dev/null, and save it to the golden output file instead. Honestly, I find hundreds of extra xfs_io output lines to be rather unhelpful, because the old output file used to be quite easy to read, to see what's going on. Today it only redirects stdout: $XFS_IO_PROG -f -c pwrite -b $((4 * bsize)) 0 $((4 * bsize)) $fragfile \ /dev/null so if a write fails, I *think* stderr will get output, and the test *should* fail as a result.[1] You could add a || _fail xfs_io failed for good measure... -Eric [1] oh, maybe not, I guess xfs_io is kind of notorious for not returning errors... Signed-off-by: Liu Bo bo.li@oracle.com --- v2: fix typo in title, s/expend/expand/g tests/generic/018 | 16 ++-- tests/generic/018.out | 198 +- 2 files changed, 203 insertions(+), 11 deletions(-) diff --git a/tests/generic/018 b/tests/generic/018 index d97bb88..3693874 100755 --- a/tests/generic/018 +++ b/tests/generic/018 @@ -68,28 +68,24 @@ $XFS_IO_PROG -f -c truncate 1m $fragfile _defrag --before 0 --after 0 $fragfile echo Contiguous file: | tee -a $seqres.full -$XFS_IO_PROG -f -c pwrite -b $((4 * bsize)) 0 $((4 * bsize)) $fragfile \ - /dev/null +$XFS_IO_PROG -f -c pwrite -b $((4 * bsize)) 0 $((4 * bsize)) $fragfile | _filter_xfs_io _defrag --before 1 --after 1 $fragfile echo Write backwards sync, but contiguous - should defrag to 1 extent | tee -a $seqres.full -for i in `seq 9 -1 0`; do - $XFS_IO_PROG -fs -c pwrite -b $bsize $((i * bsize)) $bsize $fragfile \ - /dev/null +for i in `seq 64 -1 0`; do + $XFS_IO_PROG -fd -c pwrite -b $bsize $((i * bsize)) $bsize $fragfile | _filter_xfs_io done -_defrag --before 10 --after 1 $fragfile +_defrag --after 1 $fragfile echo Write backwards sync leaving holes - defrag should do nothing | tee -a $seqres.full for i in `seq 31 -2 0`; do - $XFS_IO_PROG -fs -c pwrite -b $bsize $((i * bsize)) $bsize $fragfile \ - /dev/null + $XFS_IO_PROG -fs -c pwrite -b $bsize $((i * bsize)) $bsize $fragfile | _filter_xfs_io done _defrag --before 16 --after 16 $fragfile echo Write forwards sync leaving holes - defrag should do nothing | tee -a $seqres.full for i in `seq 0 2 31`; do - $XFS_IO_PROG -fs -c pwrite -b $bsize $((i * bsize)) $bsize $fragfile \ - /dev/null + $XFS_IO_PROG -fs -c pwrite -b $bsize $((i * bsize)) $bsize $fragfile | _filter_xfs_io done _defrag --before 16 --after 16 $fragfile diff --git a/tests/generic/018.out b/tests/generic/018.out index 5f265d1..0886a9a 100644 --- a/tests/generic/018.out +++ b/tests/generic/018.out @@ -6,14 +6,210 @@ Sparse file (no blocks): Before: 0 After: 0 Contiguous file: +wrote 16384/16384 bytes at offset 0 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) Before: 1 After: 1 Write backwards sync, but contiguous - should defrag to 1 extent -Before: 10 +wrote 4096/4096 bytes at offset 262144 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 4096/4096 bytes at offset 258048 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 4096/4096 bytes at offset 253952 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 4096/4096 bytes at offset 249856 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 4096/4096 bytes at offset 245760 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 4096/4096 bytes at offset
Re: Is there a nossd option ?
On Jun 21, 2015, at 1:11 PM, Swâmi Petaramesh sw...@petaramesh.org wrote: Le dimanche 21 juin 2015 22:31:21 Roman Mamedov a écrit : Yes the nossd option (written literally like that) does in fact exist. It would have taken you less time to try if it works, than to write this long-winded message. :) Possibly. Unless trying a non-existing mount option causes the mount to fail, thus causing the system boot to fail, thus needing me to boot from a live rescue CD... that would take much longer than writing an half-page email ;-) Man pages work well too :) http://manpages.ubuntu.com/manpages/utopic/man8/mount.8.html EricN�r��yb�X��ǧv�^�){.n�+{�n�߲)
Re: [PATCH] btrfs-progs: fix btrfs quota rescan failed on PPC64 arch
On 4/20/15 12:33 AM, xuw2...@gmail.com wrote: From: George Wang xuw2...@gmail.com PPC64 arch use such following IOC values \#define _IOC_NONE 1U \#define _IOC_READ 2U \#define _IOC_WRITE 4U comparing to the default IOC values \#define _IOC_NONE 0U \#define _IOC_READ 2U \#define _IOC_WRITE 1U This means the value _IOW* will be negative when we store it in the int variables. Such as the BTRFS_IOC_QGROUP_CREATE, it will be 0x4010942e on X86_64, but 0x8010942e on PPC64. Notice that the IOC values are the unsigned long type, so we use the unsigned long to store it, and this can insure the comparison between the variable and BTRFS_IOC_* valid. Looks good - very strange that the manpage states that the interface takes an int. :( But - an unsigned int would be enough, right? I don't think it needs to be an unsigned long. *shrug* Thanks, -Eric Signed-off-by: George Wang xuw2...@gmail.com --- cmds-quota.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmds-quota.c b/cmds-quota.c index 89cc89c..f6a1cfa 100644 --- a/cmds-quota.c +++ b/cmds-quota.c @@ -109,7 +109,7 @@ static int cmd_quota_rescan(int argc, char **argv) int e; char *path = NULL; struct btrfs_ioctl_quota_rescan_args args; - int ioctlnum = BTRFS_IOC_QUOTA_RESCAN; + unsigned long ioctlnum = BTRFS_IOC_QUOTA_RESCAN; DIR *dirstream = NULL; int wait_for_completion = 0; -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2] fstests: fix _filter_mkfs regression on btrfs
commit: 5e8b9e6 btrfs: add regression test for remount with thread_pool resized did weird things to _filter_mkfs; aside from broken indentation, it also short-circuited the default non-xfs behavior, which was to emit a default block inode size. And that was all because btrfs/082 was using _filter_mkfs not redirecting output away as per normal. Granted, it's not super clear that _filter_mkfs serves this rather unique purpose, but anyway... And, while having this default seems to be of questionable value, not emitting *anything* led to this on btrfs: +./tests/generic/204: line 76: space / (isize + dbsize): division by 0 (error token is )) because those variables don't get set for btrfs, thanks to the above commit. So take out the use of _filter_mkfs in btrfs/082, and take out the munging of _filter_mkfs which broke generic/204, and get things back to something semi-sane. Signed-off-by: Eric Sandeen sand...@redhat.com --- V2: truncate seqres.full on first write, thanks Eryu! diff --git a/common/filter b/common/filter index 71ef2e2..05dbae6 100644 --- a/common/filter +++ b/common/filter @@ -137,10 +137,6 @@ _filter_mkfs() case $FSTYP in xfs) ;; - btrfs) - sed -e /Performing full device TRIM/d \ - -e /Turning ON incompat feature/d - return ;; *) cat - /dev/null perl -e 'print STDERR dbsize=4096\nisize=256\n' diff --git a/tests/btrfs/082 b/tests/btrfs/082 index dd3c87e..1324d27 100755 --- a/tests/btrfs/082 +++ b/tests/btrfs/082 @@ -55,7 +55,7 @@ _supported_fs btrfs _supported_os Linux _require_scratch -_scratch_mkfs | _filter_mkfs +_scratch_mkfs $seqres.full 21 _scratch_mount -o thread_pool=6 _scratch_mount -o remount,thread_pool=10 diff --git a/tests/generic/204 b/tests/generic/204 index d6bb094..13069d8 100755 --- a/tests/generic/204 +++ b/tests/generic/204 @@ -60,6 +60,7 @@ _scratch_mkfs_sized $SIZE $dbsize 2 /dev/null \ | _filter_mkfs 2 $tmp.mkfs /dev/null _scratch_mount +# Source $tmp.mkfs to get geometry . $tmp.mkfs # fix the reserve block pool to a known size so that the enospc calculations -- To unsubscribe from this list: send the line unsubscribe fstests in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I think btrfs: fix leak of path in btrfs_find_item broke stable trees ...
On 3/26/15 10:34 PM, Eryu Guan wrote: Just FYI. I think generic/204 is a test case issue, _filter_mkfs failed to print isize and dbsize for btrfs and test failed because of divide by zero error. --- /dev/fd/632015-03-25 12:17:05.987107715 -0400 +++ results/generic/204.out.bad 2015-03-25 12:17:05.423101244 -0400 @@ -1,2 +1,3 @@ QA output created by 204 +./tests/generic/204: line 76: space / (isize + dbsize): division by 0 (error token is )) *** done I'm working a patch for fstests. Sorry, I beat you to it :) -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I think btrfs: fix leak of path in btrfs_find_item broke stable trees ...
On 3/26/15 9:48 AM, Chris Mason wrote: On Thu, Mar 26, 2015 at 10:11 AM, Eric Sandeen sand...@redhat.com wrote: ... 9c4f61f btrfs: simplify insert_orphan_item made the whole path alloc/free go away. so I think there's no need for my patch; may as well just send the above to stable and fix it that way, as long as 9c4f61f is deemed safe correct, I think. Nice catch, thanks Eric. 9c4f61f looks fine for stable to me, but since he's already testing on stable, I talked Eric into giving it a pass through xfstests before I send it up. -chris ./check -g auto on 3.19-stable-ish seems fine-ish. Certainly no worse w/ the patch added :) Failures: btrfs/010 btrfs/017 btrfs/078 generic/015 generic/039 generic/040 generic/041 generic/065 generic/066 generic/071 generic/204 Failed 11 of 202 tests I'd say ship it! -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fstests: fix _filter_mkfs regression on btrfs
commit: 5e8b9e6 btrfs: add regression test for remount with thread_pool resized did weird things to _filter_mkfs; aside from broken indentation, it also short-circuited the default non-xfs behavior, which was to emit a default block inode size. And that was all because btrfs/082 was using _filter_mkfs not redirecting output away as per normal. Granted, it's not super clear that _filter_mkfs serves this rather unique purpose, but anyway... And, while having this default seems to be of questionable value, not emitting *anything* led to this on btrfs: +./tests/generic/204: line 76: space / (isize + dbsize): division by 0 (error token is )) because those variables don't get set for btrfs, thanks to the above commit. So take out the use of _filter_mkfs in btrfs/082, and take out the munging of _filter_mkfs which broke generic/204, and get things back to something semi-sane. Signed-off-by: Eric Sandeen sand...@redhat.com --- diff --git a/common/filter b/common/filter index 71ef2e2..05dbae6 100644 --- a/common/filter +++ b/common/filter @@ -137,10 +137,6 @@ _filter_mkfs() case $FSTYP in xfs) ;; - btrfs) - sed -e /Performing full device TRIM/d \ - -e /Turning ON incompat feature/d - return ;; *) cat - /dev/null perl -e 'print STDERR dbsize=4096\nisize=256\n' diff --git a/tests/btrfs/082 b/tests/btrfs/082 index dd3c87e..1324d27 100755 --- a/tests/btrfs/082 +++ b/tests/btrfs/082 @@ -55,7 +55,7 @@ _supported_fs btrfs _supported_os Linux _require_scratch -_scratch_mkfs | _filter_mkfs +_scratch_mkfs $seqres.full 21 _scratch_mount -o thread_pool=6 _scratch_mount -o remount,thread_pool=10 diff --git a/tests/generic/204 b/tests/generic/204 index d6bb094..13069d8 100755 --- a/tests/generic/204 +++ b/tests/generic/204 @@ -60,6 +60,7 @@ _scratch_mkfs_sized $SIZE $dbsize 2 /dev/null \ | _filter_mkfs 2 $tmp.mkfs /dev/null _scratch_mount +# Source $tmp.mkfs to get geometry . $tmp.mkfs # fix the reserve block pool to a known size so that the enospc calculations -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I think btrfs: fix leak of path in btrfs_find_item broke stable trees ...
On 3/26/15 5:23 AM, Filipe David Manana wrote: On Thu, Mar 26, 2015 at 3:24 AM, Eric Sandeen sand...@redhat.com wrote: Looks like btrfs: fix leak of path in btrfs_find_item got sent to stable trees, but in my testing, it causes deadlocks on mount: [23379.359246] mount D 0 22541 22274 0x0080 [23379.366326] 8803ebadf6c8 0086 88027ff10230 00013680 [23379.373770] 00013680 8803ebadffd8 8803ebadc010 00013680 [23379.381208] 8803ebadffd8 00013680 880261c78b60 8802140a0b60 [23379.388648] Call Trace: [23379.391106] [816182b9] schedule+0x29/0x70 [23379.396091] [a06b82f5] btrfs_tree_lock+0xb5/0x290 [btrfs] [23379.402444] [8109b470] ? wake_up_bit+0x40/0x40 [23379.407855] [a064f7a5] ? generic_bin_search+0xf5/0x180 [btrfs] [23379.414643] [a065041b] btrfs_lock_root_node+0x3b/0x50 [btrfs] [23379.421345] [a065936b] btrfs_search_slot+0x63b/0x800 [btrfs] [23379.427956] [a064fa49] ? btrfs_set_path_blocking+0x39/0x80 [btrfs] [23379.435088] [a0659ede] btrfs_insert_empty_items+0x7e/0xe0 [btrfs] [23379.442125] [a065051a] ? btrfs_alloc_path+0x1a/0x20 [btrfs] [23379.448655] [a06b8989] btrfs_insert_orphan_item+0x69/0x90 [btrfs] [23379.455696] [a06ba938] insert_orphan_item+0x68/0x90 [btrfs] [23379.462251] [a06bf772] replay_one_buffer+0x372/0x380 [btrfs] [23379.468878] [a069a4c1] ? mark_extent_buffer_accessed+0x51/0x70 [btrfs] [23379.476372] [a06ba54b] walk_up_log_tree+0x1cb/0x250 [btrfs] [23379.482910] [a06ba68f] walk_log_tree+0xbf/0x1b0 [btrfs] [23379.489098] [a06bd51c] btrfs_recover_log_trees+0x1ec/0x4c0 [btrfs] ... I could hit this by running ./check generic/015 generic/039 in fstests, with a SCRATCH_DEV_POOL defined (not sure it matters, it's just what I have...) This fixes it, though I'm not totally sure why. Refcounts? Hi Eric, Your patch seems correct to me. The problem is that btrfs_insert_orphan_item tries to get a write lock on the same node/leaf for which its caller (insert_orphan_item) is already holding a read lock. If you plan to submit a proper patch, feel free to add my Reviewed-by: Filipe Manana fdman...@suse.com Thanks; well - but it never likely showed up upstream, because 9c4f61f btrfs: simplify insert_orphan_item made the whole path alloc/free go away. so I think there's no need for my patch; may as well just send the above to stable and fix it that way, as long as 9c4f61f is deemed safe correct, I think. -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I think btrfs: fix leak of path in btrfs_find_item broke stable trees ...
Looks like btrfs: fix leak of path in btrfs_find_item got sent to stable trees, but in my testing, it causes deadlocks on mount: [23379.359246] mount D 0 22541 22274 0x0080 [23379.366326] 8803ebadf6c8 0086 88027ff10230 00013680 [23379.373770] 00013680 8803ebadffd8 8803ebadc010 00013680 [23379.381208] 8803ebadffd8 00013680 880261c78b60 8802140a0b60 [23379.388648] Call Trace: [23379.391106] [816182b9] schedule+0x29/0x70 [23379.396091] [a06b82f5] btrfs_tree_lock+0xb5/0x290 [btrfs] [23379.402444] [8109b470] ? wake_up_bit+0x40/0x40 [23379.407855] [a064f7a5] ? generic_bin_search+0xf5/0x180 [btrfs] [23379.414643] [a065041b] btrfs_lock_root_node+0x3b/0x50 [btrfs] [23379.421345] [a065936b] btrfs_search_slot+0x63b/0x800 [btrfs] [23379.427956] [a064fa49] ? btrfs_set_path_blocking+0x39/0x80 [btrfs] [23379.435088] [a0659ede] btrfs_insert_empty_items+0x7e/0xe0 [btrfs] [23379.442125] [a065051a] ? btrfs_alloc_path+0x1a/0x20 [btrfs] [23379.448655] [a06b8989] btrfs_insert_orphan_item+0x69/0x90 [btrfs] [23379.455696] [a06ba938] insert_orphan_item+0x68/0x90 [btrfs] [23379.462251] [a06bf772] replay_one_buffer+0x372/0x380 [btrfs] [23379.468878] [a069a4c1] ? mark_extent_buffer_accessed+0x51/0x70 [btrfs] [23379.476372] [a06ba54b] walk_up_log_tree+0x1cb/0x250 [btrfs] [23379.482910] [a06ba68f] walk_log_tree+0xbf/0x1b0 [btrfs] [23379.489098] [a06bd51c] btrfs_recover_log_trees+0x1ec/0x4c0 [btrfs] ... I could hit this by running ./check generic/015 generic/039 in fstests, with a SCRATCH_DEV_POOL defined (not sure it matters, it's just what I have...) This fixes it, though I'm not totally sure why. Refcounts? diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index 906934e..d37e6d1 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -1265,11 +1265,11 @@ static int insert_orphan_item(struct btrfs_trans_handle *trans, ret = btrfs_find_item(root, path, BTRFS_ORPHAN_OBJECTID, offset, BTRFS_ORPHAN_ITEM_KEY, NULL); + btrfs_free_path(path); + if (ret 0) ret = btrfs_insert_orphan_item(trans, root, offset); - btrfs_free_path(path); - return ret; } but it never likely showed up upstream, because 9c4f61f btrfs: simplify insert_orphan_item made the whole path alloc/free go away. -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fstests: fix _filter_transcation_commit_default
btrfs has started emitting new information from cmd_subvol_delete(), so filter that out or it breaks btrfs/001: -Delete subvolume 'SCRATCH_MNT/snap' +Delete subvolume (no-commit): 'SCRATCH_MNT/snap' (Spell transaction correctly while we're at it.) Signed-off-by: Eric Sandeen sand...@redhat.com --- diff --git a/common/filter.btrfs b/common/filter.btrfs index ea047c5..9bb6479 100644 --- a/common/filter.btrfs +++ b/common/filter.btrfs @@ -57,13 +57,14 @@ _filter_btrfs_device_stats() sed -e s/ *$NUMDEVS /NUMDEVS /g } -_filter_transcation_commit_default() { - sed -e /Transaction commit: none (default)/d +_filter_transaction_commit() { + sed -e /Transaction commit: none (default)/d | \ + sed -e s/Delete subvolume (.*commit):/Delete subvolume/g } _filter_btrfs_subvol_delete() { - _filter_scratch | _filter_transcation_commit_default + _filter_scratch | _filter_transaction_commit } -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fstests: update _filter_btrfs_version
The btrfs cmd now sometimes emits btrfs-progs not Btrfs-progs as it used to, so update the filter accordingly. (This fixes a failure on btrfs/006 w/ btrfs-progs v3.19) Signed-off-by: Eric Sandeen sand...@redhat.com --- diff --git a/common/filter.btrfs b/common/filter.btrfs index c9b3f3a..ea047c5 100644 --- a/common/filter.btrfs +++ b/common/filter.btrfs @@ -5,7 +5,7 @@ # Some, but not all, commands emit Btrfs version _filter_btrfs_version() { - sed -e s/^Btrfs.*//g + sed -e s/^[Bb]trfs.*//g } _filter_devid() -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/5] btrfs-progs: Record and report every file extent hole.
On 1/2/15 1:12 AM, Qu Wenruo wrote: Record every file extent discontinuous hole in inode_record using a rb_tree member. Before the patch, btrfsck will only record the first file extent hole by using first_extent_gap, that's good for detecting error, but not suitable for fixing it. This patch provides the ability to record every file extent hole and report it. This is causing use after free and segfaults in my testing, running xfstests btrfs/078 with multiple devices defined: SCRATCH_DEV_POOL=/dev/sdc5 /dev/sdc6 /dev/sdc7 /dev/sdc8 /dev/sdc9 /dev/sdc10 /dev/sdc11 /dev/sdc12 -Eric # valgrind ./btrfsck /dev/sdc5 ==31620== Memcheck, a memory error detector ==31620== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. ==31620== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info ==31620== Command: ./btrfsck /dev/sdc5 ==31620== Checking filesystem on /dev/sdc5 UUID: ab91fc96-549b-4048-a68b-73c5190e6265 checking extents checking free space cache checking fs roots ==31620== Invalid read of size 8 ==31620==at 0x4C257C3: rb_first (rbtree.c:420) ==31620==by 0x41E609: first_extent_gap (cmds-check.c:182) ==31620==by 0x427D43: merge_inode_recs (cmds-check.c:950) ==31620==by 0x42827B: splice_shared_node (cmds-check.c:1032) ==31620==by 0x428827: enter_shared_node (cmds-check.c:1138) ==31620==by 0x428BCF: walk_down_tree (cmds-check.c:1745) ==31620==by 0x42CA64: check_fs_root (cmds-check.c:3360) ==31620==by 0x42CE2D: check_fs_roots (cmds-check.c:3496) ==31620==by 0x42E342: cmd_check (cmds-check.c:9161) ==31620==by 0x40C089: main (btrfs.c:245) ==31620== Address 0x4e5dc60 is 16 bytes inside a block of size 40 free'd ==31620==at 0x4A063F0: free (vg_replace_malloc.c:446) ==31620==by 0x421887: free_file_extent_holes (cmds-check.c:359) ==31620==by 0x4218FB: free_inode_rec (cmds-check.c:718) ==31620==by 0x42753E: maybe_free_inode_rec (cmds-check.c:786) ==31620==by 0x4282A5: splice_shared_node (cmds-check.c:1038) ==31620==by 0x42849E: leave_shared_node (cmds-check.c:1170) ==31620==by 0x42869F: walk_up_tree (cmds-check.c:1817) ==31620==by 0x42CA82: check_fs_root (cmds-check.c:3366) ==31620==by 0x42CE2D: check_fs_roots (cmds-check.c:3496) ==31620==by 0x42E342: cmd_check (cmds-check.c:9161) ==31620==by 0x40C089: main (btrfs.c:245) ==31620== ==31620== Invalid read of size 8 ==31620==at 0x41E60A: first_extent_gap (cmds-check.c:183) ==31620==by 0x427D43: merge_inode_recs (cmds-check.c:950) ==31620==by 0x42827B: splice_shared_node (cmds-check.c:1032) ==31620==by 0x428827: enter_shared_node (cmds-check.c:1138) ==31620==by 0x428BCF: walk_down_tree (cmds-check.c:1745) ==31620==by 0x42CA64: check_fs_root (cmds-check.c:3360) ==31620==by 0x42CE2D: check_fs_roots (cmds-check.c:3496) ==31620==by 0x42E342: cmd_check (cmds-check.c:9161) ==31620==by 0x40C089: main (btrfs.c:245) ==31620== Address 0x4e5dc68 is 24 bytes inside a block of size 40 free'd ==31620==at 0x4A063F0: free (vg_replace_malloc.c:446) ==31620==by 0x421887: free_file_extent_holes (cmds-check.c:359) ==31620==by 0x4218FB: free_inode_rec (cmds-check.c:718) ==31620==by 0x42753E: maybe_free_inode_rec (cmds-check.c:786) ==31620==by 0x4282A5: splice_shared_node (cmds-check.c:1038) ==31620==by 0x42849E: leave_shared_node (cmds-check.c:1170) ==31620==by 0x42869F: walk_up_tree (cmds-check.c:1817) ==31620==by 0x42CA82: check_fs_root (cmds-check.c:3366) ==31620==by 0x42CE2D: check_fs_roots (cmds-check.c:3496) ==31620==by 0x42E342: cmd_check (cmds-check.c:9161) ==31620==by 0x40C089: main (btrfs.c:245) ==31620== ==31620== Invalid read of size 8 ==31620==at 0x4C257C3: rb_first (rbtree.c:420) ==31620==by 0x41E609: first_extent_gap (cmds-check.c:182) ==31620==by 0x427421: maybe_free_inode_rec (cmds-check.c:768) ==31620==by 0x4282A5: splice_shared_node (cmds-check.c:1038) ==31620==by 0x428827: enter_shared_node (cmds-check.c:1138) ==31620==by 0x428BCF: walk_down_tree (cmds-check.c:1745) ==31620==by 0x42CA64: check_fs_root (cmds-check.c:3360) ==31620==by 0x42CE2D: check_fs_roots (cmds-check.c:3496) ==31620==by 0x42E342: cmd_check (cmds-check.c:9161) ==31620==by 0x40C089: main (btrfs.c:245) ==31620== Address 0x4e5dc60 is 16 bytes inside a block of size 40 free'd ==31620==at 0x4A063F0: free (vg_replace_malloc.c:446) ==31620==by 0x421887: free_file_extent_holes (cmds-check.c:359) ==31620==by 0x4218FB: free_inode_rec (cmds-check.c:718) ==31620==by 0x42753E: maybe_free_inode_rec (cmds-check.c:786) ==31620==by 0x4282A5: splice_shared_node (cmds-check.c:1038) ==31620==by 0x42849E: leave_shared_node (cmds-check.c:1170) ==31620==by 0x42869F: walk_up_tree (cmds-check.c:1817) ==31620==by 0x42CA82: check_fs_root (cmds-check.c:3366) ==31620==by 0x42CE2D: check_fs_roots (cmds-check.c:3496) ==31620==by 0x42E342: cmd_check
Re: btrfs oops while mounting fuzzed btrfs image
On 3/6/15 4:01 AM, Omar Sandoval wrote: Hi, Qu, I'm not seeing that in the code I'm looking at :( In fsfuzz:447, I see the mangle executable called with an offset starting at 0, which would mean that the superblock isn't safe. (Semi-wild guess follows): He may be using a hacked version of mangle which I had been using for btrfsck testing; I had neutered it a lot because if I let it hit the front of the filesystem, there was no hope at all for the repair, ever. Qu, if you're using it for fsck testing, I'd eventually make the mangling more severe, and go back to the upstream version of mangle.c. (I keep meaning to make mangle.c and fsfuzzer in general more flexible and useful, but for now I just have local hacks, sorry). -Eric (Btw, that line also indicates that we potentially write to the entire file system image, not just the beginning. My understanding from mangle.c is that up to 10% of the file contents are modified, not the first 10% of the file by length. Someone please correct me if I'm wrong!). Indeed, Eryu's dmesg shows: [ 309.384037] BTRFS: super block crcs don't match, older mkfs detected This commit is relevant: commit 667e7d94a1683661cff5fe9a0fa0d7f8fdd2c007 Author: Chris Mason chris.ma...@fusionio.com Date: Tue May 7 11:00:13 2013 -0400 Btrfs: allow superblock mismatch from older mkfs We've added new checks to make sure the super block crc is correct during mount. A fresh filesystem from an older mkfs won't have the crc set. This adds a warning when it finds a newly created filesystem but doesn't fail the mount. Signed-off-by: Chris Mason chris.ma...@fusionio.com diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index bc423f7e..4e9ebe1 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -383,6 +383,11 @@ static int btrfs_check_super_csum(char *raw_disk_sb) if (memcmp(raw_disk_sb, result, csum_size)) ret = 1; + + if (ret btrfs_super_generation(disk_sb) 10) { + printk(KERN_WARNING btrfs: super block crcs don't match, older mkfs detected\n); + ret = 0; + } } if (csum_type = ARRAY_SIZE(btrfs_csum_sizes)) { So, it looks like the super block is corrupted, but we ignore it because this is a fresh filesystem. I can easily trigger a related panic with this: while true; do dd if=/dev/urandom of=btrfs.img bs=1M count=16 mkfs.btrfs btrfs.img dd if=/dev/urandom of=btrfs.img bs=1 seek=$((64 * 1024 + 88)) count=8 conv=notrunc mount -o loop btrfs.img /mnt umount /mnt done I'm not sure that this is exactly what's happening with Eryu's image, but it's definitely an issue. I also don't know whether it's safe to get rid of that special case. It looks like it's needed for btrfs-progs before v3.12 (November 2013). Chris? David? Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs oops while mounting fuzzed btrfs image
On 3/5/15 3:46 AM, Liu Bo wrote: On Thu, Mar 05, 2015 at 03:09:33PM +0800, Eryu Guan wrote: Hi, I was testing btrfs with fsfuzzer and encountered a divide error on mount, kernel version 3.19 and 4.0-rc1. I found a similar bug on kernel bugzilla https://bugzilla.kernel.org/show_bug.cgi?id=88611 Please find the fuzzed btrfs image in the buzilla, and the following command will reproduce: mount -o loop btrfs.img /mnt/btrfs A divide by 0 oops. My printk shows that a raid56 chunk has a negative map-length, so we need to find out how fsfuzzer made that. Can you share your script so that we can reproduce the oops? All you need to reproduce the oops is the image Eryu provided. fsfuzzer intentionally damages the filesystem, simulating what might happen if hardware goes bad, disks fail, admins dd to the wrong disk, memory corrupts, bugs happen, etc. The point is that filesystems need to be robust in the face of unexpected data on the disk, and Eryu has uncovered a case where btrfs is not. :) -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] fstests: generic test for directory fsync after adding hard links
On 2/24/15 6:51 PM, Filipe Manana wrote: This test is motivated by an fsync issue discovered in btrfs. The issue was that after adding a new hard link to an existing file (one that was created in a past transaction) and fsync'ing the parent directory of the new hard link, after the fsync log replay the file's inode link count did not get its link count incremented, while the new directory entry was visible. Also, unlike xfs and ext4, new files under the directory we fsync were not being written to the fsync log, nor were any child directories and new files and links under the children directories. So this test verifies too that btrfs has the same behaviour as xfs and ext4. The btrfs issue was fixed by the following linux kernel patch: Btrfs: fix metadata inconsistencies after directory fsync Signed-off-by: Filipe Manana fdman...@suse.com --- V2: Make use of the new function '_require_metadata_journaling' added by Eric. Make the test pass on ext3 - unlike ext4 (and xfs), the file hello gets all its data synced, so we don't get an empty file after the fsync log is replayed. V3: Make our file 'foo' not empty and verify that after log replay its content remains unchanged. Motivated by an issue found during development of the btrfs fix. V4: Updated test description to be more concise. Now it mentions more directly what the test does rather than the btrfs issue, which forced the reader to infer it and read the whole test. Thanks. Ok, verified that it passes on ext4 and xfs, too - Reviewed-by: Eric Sandeen sand...@redhat.com tests/generic/060 | 172 ++ tests/generic/060.out | 11 tests/generic/group | 1 + 3 files changed, 184 insertions(+) create mode 100755 tests/generic/060 create mode 100644 tests/generic/060.out diff --git a/tests/generic/060 b/tests/generic/060 new file mode 100755 index 000..2b80078 --- /dev/null +++ b/tests/generic/060 @@ -0,0 +1,172 @@ +#! /bin/bash +# FS QA Test No. 060 +# +# Test fsync on directories that got new hardlinks added to them and that point +# to existing inodes. The goal is to verify that after the fsync log is replayed +# the new hardlinks exist and the inodes have a correct link count. +# Also test that new hardlinks pointing to new inodes are logged and exist as +# well after the fsync log is replayed. +# +# This test is motivated by an issue discovered in btrfs, where the inode link +# counts were incorrect after the fsync log was replayed and the hardlinks for +# new inodes were not logged. +# +#--- +# Copyright (C) 2015 SUSE Linux Products GmbH. All Rights Reserved. +# Author: Filipe Manana fdman...@suse.com +# +# 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! + +_cleanup() +{ + _cleanup_flakey + rm -f $tmp.* +} +trap _cleanup; exit \$status 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter +. ./common/dmflakey + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_need_to_be_root +_require_scratch +_require_dm_flakey +_require_metadata_journaling $SCRATCH_DEV + +rm -f $seqres.full + +_scratch_mkfs $seqres.full 21 +_init_flakey +_mount_flakey + +# Create our main test file and directory. +$XFS_IO_PROG -f -c pwrite -S 0xaa 0 8K $SCRATCH_MNT/foo | _filter_xfs_io +mkdir $SCRATCH_MNT/mydir + +# Make sure all metadata and data are durably persisted. +sync + +# Add a hard link to 'foo' inside our test directory and fsync only the +# directory. The btrfs fsync implementation had a bug that caused the new +# directory entry to be visible after the fsync log replay but, the inode +# of our file remained with a link count of 1. +ln $SCRATCH_MNT/foo $SCRATCH_MNT/mydir/foo_2 + +# Add a few more links and new files. +# This is just to verify nothing breaks or gives incorrect results after the +# fsync log is replayed. +ln $SCRATCH_MNT/foo $SCRATCH_MNT/mydir/foo_3 +$XFS_IO_PROG -f -c pwrite -S 0xff 0
Re: [PATCH v3] fstests: generic test for directory fsync after adding hard links
On 2/24/15 5:29 PM, Filipe Manana wrote: This test is motivated by an fsync issue discovered in btrfs. The issue was that after adding a new hard link to an existing file (one that was created in a past transaction) and fsync'ing the parent directory of the new hard link, after the fsync log replay the file's inode link count did not get its link count incremented, while the new directory entry was visible. Also, unlike xfs and ext4, new files under the directory we fsync were not being written to the fsync log, nor were any child directories and new files and links under the children directories. So this test verifies too that btrfs has the same behaviour as xfs and ext4. The btrfs issue was fixed by the following linux kernel patch: Btrfs: fix metadata inconsistencies after directory fsync Signed-off-by: Filipe Manana fdman...@suse.com --- V2: Make use of the new function '_require_metadata_journaling' added by Eric. Make the test pass on ext3 - unlike ext4 (and xfs), the file hello gets all its data synced, so we don't get an empty file after the fsync log is replayed. V3: Make our file 'foo' not empty and verify that after log replay its content remains unchanged. Motivated by an issue found during development of the btrfs fix. tests/generic/060 | 175 ++ tests/generic/060.out | 11 tests/generic/group | 1 + 3 files changed, 187 insertions(+) create mode 100755 tests/generic/060 create mode 100644 tests/generic/060.out diff --git a/tests/generic/060 b/tests/generic/060 new file mode 100755 index 000..0d459fa --- /dev/null +++ b/tests/generic/060 @@ -0,0 +1,175 @@ +#! /bin/bash +# FS QA Test No. 060 +# +# This test is motivated by an fsync issue discovered in btrfs. +# The issue was that after adding a new hard link to an existing file (one that +# was created in a past transaction) and fsync'ing the parent directory of the +# new hard link, after the fsync log replay the file's inode link count did not +# get its link count incremented, while the new directory entry was visible. +# Also, unlike xfs and ext4, new files under the directory we fsync were not +# being written to the fsync log, nor were any child directories and new files +# and links under the children directories. So this test verifies too that +# btrfs has the same behaviour as xfs and ext3/4. +# +# The btrfs issue was fixed by the following linux kernel patch: +# +# Btrfs: fix metadata inconsistencies after directory fsync I still would like to know *what this test does* - not some narrative about btrfs's troubled past. ;) Could you please add that line or two, and feel free to keep all the detail about the btrfs-specific bug later? We're getting a lot of these tests, and a short description of what a test does is just How We Do It(tm). It saves having to read a lot of bash code just to get some idea of what is under test. i.e. like this, or whatever is accurate: # Test that link counts remain correct after fsyncing a parent directory # containing hardlinks, and subsequent log recovery # # insert fascinating btrfs story here Please just do this; it'll save people time, down the line, if/when this test fails in the future, or needs to be maintained by someone else. If btrfs folks don't want simple test descriptions under tests/btrfs, your choice, but I really would like to have this clarity on the generic tests. Thanks, -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fstests: generic test for fsync after removing xattrs
On 2/23/15 2:24 PM, Filipe David Manana wrote: On Mon, Feb 23, 2015 at 8:14 PM, Eric Sandeen sand...@sandeen.net wrote: On 2/23/15 1:55 PM, Filipe Manana wrote: This test is motivated by an fsync issue discovered in btrfs. The issue was that the fsync log replay code did not remove xattrs that were deleted before the inode was fsynced. The result was unexpected and differed from xfs and ext3/4 for example. The btrfs issue was fixed by the following linux kernel patch: Btrfs: remove deleted xattrs on fsync log replay Signed-off-by: Filipe Manana fdman...@suse.com --- tests/generic/061 | 115 ++ tests/generic/061.out | 10 + tests/generic/group | 1 + 3 files changed, 126 insertions(+) create mode 100755 tests/generic/061 create mode 100644 tests/generic/061.out diff --git a/tests/generic/061 b/tests/generic/061 new file mode 100755 index 000..a5eb668 --- /dev/null +++ b/tests/generic/061 @@ -0,0 +1,115 @@ +#! /bin/bash +# FS QA Test No. 061 Could you describe what the test actually does first in the header comment? You have the btrfs-specific flaw, but (I say this after having looked at 034 just this morning) sometimes it's nice to have a concise description of the test. i.e.: # Test that log replay properly handles deleted xattrs after an fsync or whatever is the proper operational description at the very top, so it's clear from a glance what the test *does* without having to read through it. Hi Eric, I thought the initial summary was clear enough: # FS QA Test No. 061 # # This test is motivated by an fsync issue discovered in btrfs. # The issue was that the fsync log replay code did not remove xattrs that # were deleted before the inode was fsynced. How more detailed/clear would you put it? Eh, it's ok; it describes why you decided to write the test, not what the test does. Subtle difference? ... I think maybe a _requires_metadata_journaling or similar might be a good idea. Thoughts? Unfortunately that would need some special-casing for ext4, to see if it has jbd2 turned on or not, but I can help with that. (dumpe2fs -h $TEST_DEV | grep has_journal) Thanks for the heads up on that detail. For the ext4 case, since the test creates the fs, if FSTYP == ext4, could we force passing -O has_journal to mkfs (or make sure -O ^has_journal is filtered out). Well, if someone is explicitly testing nojournal ext4, I wouldn't turn it on behind their backs just for this test; that'd be a bit odd. Let me take a stab at a _requires_metadata_journaling() helper. -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fstests: generic test for fsync after removing xattrs
On 2/23/15 1:55 PM, Filipe Manana wrote: This test is motivated by an fsync issue discovered in btrfs. The issue was that the fsync log replay code did not remove xattrs that were deleted before the inode was fsynced. The result was unexpected and differed from xfs and ext3/4 for example. The btrfs issue was fixed by the following linux kernel patch: Btrfs: remove deleted xattrs on fsync log replay Signed-off-by: Filipe Manana fdman...@suse.com --- tests/generic/061 | 115 ++ tests/generic/061.out | 10 + tests/generic/group | 1 + 3 files changed, 126 insertions(+) create mode 100755 tests/generic/061 create mode 100644 tests/generic/061.out diff --git a/tests/generic/061 b/tests/generic/061 new file mode 100755 index 000..a5eb668 --- /dev/null +++ b/tests/generic/061 @@ -0,0 +1,115 @@ +#! /bin/bash +# FS QA Test No. 061 Could you describe what the test actually does first in the header comment? You have the btrfs-specific flaw, but (I say this after having looked at 034 just this morning) sometimes it's nice to have a concise description of the test. i.e.: # Test that log replay properly handles deleted xattrs after an fsync or whatever is the proper operational description at the very top, so it's clear from a glance what the test *does* without having to read through it. Also - for this and 034 and possibly others, anything which tests log replay really can't be wholly generic. ext2 doesn't have a journal, for example, and ext4 can run without one. I was going to make an: _excluded_fs ext2 type helper, but that doesn't solve the ext4 problem, so I think maybe a _requires_metadata_journaling or similar might be a good idea. Thoughts? Unfortunately that would need some special-casing for ext4, to see if it has jbd2 turned on or not, but I can help with that. (dumpe2fs -h $TEST_DEV | grep has_journal) Thanks, -Eric +# +# This test is motivated by an fsync issue discovered in btrfs. +# The issue was that the fsync log replay code did not remove xattrs that +# were deleted before the inode was fsynced. +# +# The btrfs issue was fixed by the following linux kernel patch: +# +# Btrfs: remove deleted xattrs on fsync log replay +# +#--- +# Copyright (C) 2015 SUSE Linux Products GmbH. All Rights Reserved. +# Author: Filipe Manana fdman...@suse.com +# +# 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! + +_cleanup() +{ + _cleanup_flakey + rm -f $tmp.* +} +trap _cleanup; exit \$status 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter +. ./common/dmflakey +. ./common/attr + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_need_to_be_root +_require_scratch +_require_dm_flakey +_require_attrs + +_crash_and_mount() +{ + # Simulate a crash/power loss. + _load_flakey_table $FLAKEY_DROP_WRITES + _unmount_flakey + _load_flakey_table $FLAKEY_ALLOW_WRITES + _mount_flakey +} + +rm -f $seqres.full + +_scratch_mkfs $seqres.full 21 +_init_flakey +_mount_flakey + +# Create out test file and add 3 xattrs to it. +touch $SCRATCH_MNT/foobar +$SETFATTR_PROG -n user.attr1 -v val1 $SCRATCH_MNT/foobar +$SETFATTR_PROG -n user.attr2 -v val2 $SCRATCH_MNT/foobar +$SETFATTR_PROG -n user.attr3 -v val3 $SCRATCH_MNT/foobar + +# Make sure everything is durably persisted. +sync + +# Now delete the second xattr and fsync the inode. +$SETFATTR_PROG -x user.attr2 $SCRATCH_MNT/foobar +$XFS_IO_PROG -c fsync $SCRATCH_MNT/foobar + +_crash_and_mount + +# After the fsync log is replayed, the file should have only 2 xattrs, the ones +# named user.attr1 and user.attr3. The btrfs fsync log replay bug left the file +# with the 3 xattrs that we had before deleting the second one and fsyncing the +# file. +echo xattr names and values after first fsync log replay: +$GETFATTR_PROG --absolute-names --dump $SCRATCH_MNT/foobar | _filter_scratch + +# Now write some data to our file, fsync
Re: Documenting MS_LAZYTIME
On 2/20/15 2:50 AM, Michael Kerrisk wrote: Hello Ted, Based on your commit message 0ae45f63d4e, I I wrote the documentation below for MS_LAZYTIME, to go into the mount(2) man page. Could you please check it over and let me know if it's accurate. In particular, I added pieces marked with * below that were not part of the commit message and I'd like confirmation that they're accurate. Thanks, Michael [[ MS_LAZYTIME (since Linux 3.20) Only update filetimes (atime, mtime, ctime) on the in- memory version of the file inode. The on-disk time‐ stamps are updated only when: filetimes and file inode seems a bit awkward. How about: MS_LAZYTIME (since Linux 3.20) Reduce on-disk updates of inode timestamps (atime, mtime, ctime) by maintaining these changes only in memory, unless: (maybe I'm bike-shedding too much, if so, sorry). (a) the inode needs to be updated for some change unre‐ lated to file timestamps; (b) the application employs fsync(2), syncfs(2), or sync(2); (c) an undeleted inode is evicted from memory; or * (d) more than 24 hours have passed since the i-node was * written to disk. Please don't use i-node - simply inode is much more common in the manpages AFAICT. This mount option significantly reduces writes to the inode table for workloads that perform frequent random writes to preallocated files. This seems like an overly specific description of a single workload out of many which may benefit, but what do others think? inode table is also fairly extN-specific. -Eric * As at Linux 3.20, this option is supported only on ext4. ]] -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs-progs: allow CFLAGS specification in new build
Disclaimer: I am not an auto$FOO expert by any means. But I could find no way to specify additional CFLAGS, and I think that this is because they are hard-coded in configure.ac. The below works for me, but I don't know if it's the right solution... Signed-off-by: Eric Sandeen sand...@redhat.com --- diff --git a/Makefile.in b/Makefile.in index 93b6a3c..4a36f6a 100644 --- a/Makefile.in +++ b/Makefile.in @@ -12,6 +12,7 @@ DISABLE_BTRFSCONVERT = @DISABLE_BTRFSCONVERT@ # Non-static compilation flags CFLAGS = @CFLAGS@ \ +-g -O1 \ -include config.h -Wall \ -D_FILE_OFFSET_BITS=64 -DBTRFS_FLAT_INCLUDES \ -D_XOPEN_SOURCE=700 \ diff --git a/configure.ac b/configure.ac index cbad099..f6ebb7d 100644 --- a/configure.ac +++ b/configure.ac @@ -8,8 +8,6 @@ LIBBTRFS_MAJOR=0 LIBBTRFS_MINOR=1 LIBBTRFS_PATCHLEVEL=1 -CFLAGS=-g -O1 - AC_PREREQ([2.60]) AC_CONFIG_AUX_DIR([config]) -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: should I use btrfs on Centos 7 for a new production server?
On 12/30/14 10:03 PM, Wang Shilong wrote: Hello, I have a well tested and working fine Centos5-Xen system. Accumulated cruft from various development efforts make it desirable to redo the install. Currently a RAID-10 ext4 filesystem with LVM and 750G of storage. There's a hot spare 750 drive in the system. I'm thinking of migrating the web sites (almost the only use of the server) to a spare then installing Centos-7 and btrfs, then migrating the sites back. I see RH marks btrfs in C7 as a technology preview but don't understand what that implies for future support and a suitably stable basis for storage. Red Hat's statement on tech preview is here (I sure hope it doesn't require a login ...) https://access.redhat.com/support/offerings/techpreview The demand on the system is low and not likely to change in the near future, storage access speeds are not likely to be dealbreakers and it would be nice to not need to use LVM, btrfs seems to have a better feature set and more intuitive command set. But I'm uncertain about stability. Anyone have an opinion? I used CentOS7 btrfs myself, just doing some tests..it crashed easily. I don’t know how much efforts that Redhat do on btrfs for 7 series. RHEL7.0 GA (released last May) has btrfs kernel code from v3.13. RHEL7.1 will have btrfs code from around v3.16. The stability of btrfs in RHEL7 releases depends heavily on the maturity and stability of upstream btrfs at the time of the release. IOWS, if btrfs around v3.13 crashed easily; there is nothing magical in RHEL7.0 to fix that. ;) -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: should I use btrfs on Centos 7 for a new production server?
On 12/30/14 10:06 PM, Wang Shilong wrote: Hello, I have a well tested and working fine Centos5-Xen system. Accumulated cruft from various development efforts make it desirable to redo the install. Currently a RAID-10 ext4 filesystem with LVM and 750G of storage. There's a hot spare 750 drive in the system. I'm thinking of migrating the web sites (almost the only use of the server) to a spare then installing Centos-7 and btrfs, then migrating the sites back. I see RH marks btrfs in C7 as a technology preview but don't understand what that implies for future support and a suitably stable basis for storage. The demand on the system is low and not likely to change in the near future, storage access speeds are not likely to be dealbreakers and it would be nice to not need to use LVM, btrfs seems to have a better feature set and more intuitive command set. But I'm uncertain about stability. Anyone have an opinion? I used CentOS7 btrfs myself, just doing some tests..it crashed easily. I don’t know how much efforts that Redhat do on btrfs for 7 series. Maybe use SUSE enterprise for btrfs will be a better choice, they offered better support for btrfs as far as i know. I believe SuSE's most recent support statement on btrfs is here, I think. https://www.suse.com/releasenotes/x86_64/SUSE-SLES/12/#fate-317221 -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] btrfs-progs: corrupt block, add break after option U
On 12/19/14 10:06 AM, David Sterba wrote: Resolves-Coverity-CID: 1258793 Signed-off-by: David Sterba dste...@suse.cz Reviewed-by: Eric Sandeen sand...@redhat.com --- btrfs-corrupt-block.c | 1 + 1 file changed, 1 insertion(+) diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index aeeb1b298f66..b477e878376b 100644 --- a/btrfs-corrupt-block.c +++ b/btrfs-corrupt-block.c @@ -1068,6 +1068,7 @@ int main(int ac, char **av) break; case 'U': chunk_tree = 1; + break; case 'i': inode = arg_strtou64(optarg); break; -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] btrfs-progs: corrupt block, add missing break to option I
On 12/19/14 10:06 AM, David Sterba wrote: Using -I would imply -d. Resolves-Coverity-CID: 1258792 Signed-off-by: David Sterba dste...@suse.cz Reviewed-by: Eric Sandeen sand...@redhat.com --- btrfs-corrupt-block.c | 1 + 1 file changed, 1 insertion(+) diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c index af9ae4d4047c..aeeb1b298f66 100644 --- a/btrfs-corrupt-block.c +++ b/btrfs-corrupt-block.c @@ -1096,6 +1096,7 @@ int main(int ac, char **av) break; case 'I': corrupt_item = 1; + break; case 'd': delete = 1; break; -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] btrfs-progs: fragments, close output file on error
On 12/19/14 10:06 AM, David Sterba wrote: Resolves-Coverity-CID: 1258794 Signed-off-by: David Sterba dste...@suse.cz Reviewed-by: Eric Sandeen sand...@redhat.com --- btrfs-fragments.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/btrfs-fragments.c b/btrfs-fragments.c index d03c2c3e7319..360f10f87bfa 100644 --- a/btrfs-fragments.c +++ b/btrfs-fragments.c @@ -233,7 +233,7 @@ list_fragments(int fd, u64 flags, char *dir) ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, args); if (ret 0) { fprintf(stderr, ERROR: can't perform the search\n); - return ret; + goto out_close; } /* the ioctl returns the number of item it found in nr_items */ if (sk-nr_items == 0) @@ -373,7 +373,10 @@ skip:; fprintf(html, /p); } fprintf(html, /body/html\n); - + +out_close: + fclose(html); + return ret; } -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] btrfs-progs: check result of first_cache_extent
On 12/19/14 10:06 AM, David Sterba wrote: If the tree's empty, we'll get NULL and dereference it. Hm, but this is under an explicit check for not empty: while (!cache_tree_empty(roots_info_cache)) { sooo? Maybe it's just defensive? Nothing really wrong with being defensive, I suppose, so: Reviewed-by: Eric Sandeen sand...@redhat.com Resolves-Coverity-CID: 1248828 Signed-off-by: David Sterba dste...@suse.cz --- cmds-check.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmds-check.c b/cmds-check.c index 6eea36c2f52c..3e7a4ebdce44 100644 --- a/cmds-check.c +++ b/cmds-check.c @@ -8075,6 +8075,8 @@ static void free_roots_info_cache(void) struct root_item_info *rii; entry = first_cache_extent(roots_info_cache); + if (!entry) + break; remove_cache_extent(roots_info_cache, entry); rii = container_of(entry, struct root_item_info, cache_extent); free(rii); -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] btrfs-progs: check allocation result in add_clone_source
On 12/19/14 10:06 AM, David Sterba wrote: Resolves-Coverity-CID: 1054894 Signed-off-by: David Sterba dste...@suse.cz Reviewed-by: Eric Sandeen sand...@redhat.com --- cmds-send.c | 25 + 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/cmds-send.c b/cmds-send.c index b17b5e2ca666..9b32c1f0e624 100644 --- a/cmds-send.c +++ b/cmds-send.c @@ -172,11 +172,16 @@ out: return ret; } -static void add_clone_source(struct btrfs_send *s, u64 root_id) +static int add_clone_source(struct btrfs_send *s, u64 root_id) { s-clone_sources = realloc(s-clone_sources, sizeof(*s-clone_sources) * (s-clone_sources_count + 1)); + + if (!s-clone_sources) + return -ENOMEM; s-clone_sources[s-clone_sources_count++] = root_id; + + return 0; } static int write_buf(int fd, const void *buf, int size) @@ -475,7 +480,11 @@ int cmd_send(int argc, char **argv) goto out; } - add_clone_source(send, root_id); + ret = add_clone_source(send, root_id); + if (ret 0) { + fprintf(stderr, ERROR: not enough memory\n); + goto out; + } subvol_uuid_search_finit(send.sus); free(subvol); subvol = NULL; @@ -575,7 +584,11 @@ int cmd_send(int argc, char **argv) goto out; } - add_clone_source(send, parent_root_id); + ret = add_clone_source(send, parent_root_id); + if (ret 0) { + fprintf(stderr, ERROR: not enough memory\n); + goto out; + } } for (i = optind; i argc; i++) { @@ -671,7 +684,11 @@ int cmd_send(int argc, char **argv) goto out; /* done with this subvol, so add it to the clone sources */ - add_clone_source(send, root_id); + ret = add_clone_source(send, root_id); + if (ret 0) { + fprintf(stderr, ERROR: not enough memory\n); + goto out; + } parent_root_id = 0; full_send = 0; -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] btrfs-progs: let btrfs_free_path accept NULL
On 12/19/14 10:06 AM, David Sterba wrote: Same in kernel and matches semantics of free(). Resolves-Coverity-CID: 1054881 Signed-off-by: David Sterba dste...@suse.cz Reviewed-by: Eric Sandeen sand...@redhat.com --- ctree.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ctree.c b/ctree.c index bd6cb125b2a2..589efa3db17e 100644 --- a/ctree.c +++ b/ctree.c @@ -48,6 +48,8 @@ struct btrfs_path *btrfs_alloc_path(void) void btrfs_free_path(struct btrfs_path *p) { + if (!p) + return; btrfs_release_path(p); kfree(p); } -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs-prog: improve build-system by autoconf
On 12/18/14 7:31 AM, Karel Zak wrote: On Wed, Dec 17, 2014 at 03:07:26PM +0100, David Sterba wrote: On Fri, Dec 12, 2014 at 01:35:14PM +0100, Karel Zak wrote: This is first step to make btrfs-progs build system more conventional for userspace users and developers. All is implemented by small incremental patches to keep things review-able. Thanks. I went through the patches and haven't found major problems. The changes are affecting build system and this will need a longer period before all distros have a chance to adapt to that, so I'm postponing it to 3.19. Cool, I'll try to prepare next set of patches with automake. BTW, I have good experience with build-system changes -- downstream distributions (maintainers) are usually pretty flexible :-) Note that there is also strange unused btrfs_convert_libs, btrfs_image_libs and btrfs_fragments_libs variables with things like -lgd -lpng -ljpeg -lfreetype. I guess it's some legacy, right? I didn't touch these variables as I have no clue about sense of this stuff. No, it's part of the macro magic. There are pattern rules that accept any source in the form btrfs-something.c and also pick the libraries for that from variable btrfs_something_libs: btrfs-%: $(objects) $(libs) btrfs-%.o @echo [LD] $@ $(Q)$(CC) $(CFLAGS) -o $@ $(objects) $@.o $(LDFLAGS) $(LIBS) $($(subst -,_,$@-libs)) This is for convenience, if this turns out to be hard to do with in combination with autotools, I don't insist on keeping it but it has simplified the Makefile significantly. Yeah, I did those long ago IIRC. It obfuscates things a bit, but cut down on tons of cut and paste in the Makefile... OK, so -ljpeg in the Makefile is just example, right? It's been specified for the btrfs-framgnets tool since .. forever. commit 6d37fbfc1f83c34f00df7c9d8e5b60e49d9db48d Author: Arne Jansen sensi...@gmx.net Date: Mon Nov 28 17:12:30 2011 +0100 Btrfs-progs: tool to visualize fragmentation ... +btrfs-fragments: $(objects) $(libs) fragments.o + @echo [LD] $@ + $(Q)$(CC) $(CFLAGS) -o btrfs-fragments $(objects) fragments.o $(LDFLAGS) $(LIBS) -lgd -lpng -ljpeg -lfreetype but the code only does: gdImagePng(im, pngout); so it's probably not needed... Anyway, for these things is better to introduce extra autoconf AC_ARG_VAR() variables, keep is empty by default and use it in Makefile. The advantage is that the variables are documented and visible by ./configure --help. For example in util-linux we have many {SUID,DAEMON,SOLIB,...}_CFLAGS and LDFLAGS for distributions that require extensions like -fPIE, -Wl,-z,relro etc. The same is possible to do with $LIBS. Once auto$FOO is implemented, I am sure there are better ways to do it than what I did. :) Thanks, -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Announcements for btrfs-progs?
On 12/11/14 6:37 AM, Holger Hoffstätte wrote: David, I was wondering if you could please send out announcements for btrfs-progs when you tag a release or -rc? There doesn't seem to be a good mechanism to track releases and IMHO the more people are notified, the more testing we can get, not to mention faster propagation into distros. Not that it matters for non-Fedora users, but FWIW Fedora has release monitoring scripts which notify me when a new tarball appears. So Fedora, at least, gets a prompt update even if there's no formal announcement (assuming I'm not behind the curve). ;) Still, announcements would be great, esp. if they have a summary of what's contained in the update. -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fstests: mark replace tests in btrfs/group
On Nov 20, 2014, at 10:44 PM, Eryu Guan eg...@redhat.com wrote: On Wed, Nov 19, 2014 at 09:33:31AM -0600, Eric Sandeen wrote: A couple tests exercise replace but were not marked as such in the group file. Hi Eric, I have a patch sitting in my git tree that adds most of btrfs tests in one or more groups, I'll send it out for review soon. Thanks, much more complete than mine. -Eric Thanks, Eryu Signed-off-by: Eric Sandeen sand...@redhat.com --- diff --git a/tests/btrfs/group b/tests/btrfs/group index 9adf862..1f23979 100644 --- a/tests/btrfs/group +++ b/tests/btrfs/group @@ -13,7 +13,7 @@ 008 auto quick 009 auto quick 010 auto quick -011 auto +011 auto replace 012 auto 013 auto quick 014 auto @@ -22,7 +22,7 @@ 017 auto quick 018 auto quick 019 auto quick -020 auto quick +020 auto quick replace 021 auto quick 022 auto 023 auto -- To unsubscribe from this list: send the line unsubscribe fstests in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe fstests in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fstests: mark replace tests in btrfs/group
A couple tests exercise replace but were not marked as such in the group file. Signed-off-by: Eric Sandeen sand...@redhat.com --- diff --git a/tests/btrfs/group b/tests/btrfs/group index 9adf862..1f23979 100644 --- a/tests/btrfs/group +++ b/tests/btrfs/group @@ -13,7 +13,7 @@ 008 auto quick 009 auto quick 010 auto quick -011 auto +011 auto replace 012 auto 013 auto quick 014 auto @@ -22,7 +22,7 @@ 017 auto quick 018 auto quick 019 auto quick -020 auto quick +020 auto quick replace 021 auto quick 022 auto 023 auto -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [bug] df reports wrong Size and Avail on raid1, 3.18rc2
On 10/28/14 9:19 PM, Chris Murphy wrote: 3.18.0-0.rc2.git1.1.fc22.x86_64 btrfs-progs-3.17-1.fc21.x86_64 # btrfs fi show /mnt Label: 'btrfs1' uuid: 0f1c615f-30a0-4166-8a3c-987849551513 Total devices 2 FS bytes used 233.54GiB devid1 size 465.76GiB used 236.03GiB path /dev/sdb devid2 size 298.09GiB used 236.03GiB path /dev/sdc # df -h /mnt Filesystem Size Used Avail Use% Mounted on /dev/sdb382G 234G 65G 79% /mnt a. df -h should report Size as 298GiB rather than as 382GiB. Because this is 2 device raid1, the limiting factor is devid 2 @ 298GiB. b. df -h should report Avail as 62GiB or less, rather than as 65GiB. 298.09 - 236.03 = 62.06 Is there an fstest for btrfs disk space reporting? ext2/3/4 used to keep getting overhead wrong for various filesystem types ... until we wrote a regression test. Just sayin' :) -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] some btrfs-progs coverity fixes
On 10/16/14 6:46 AM, David Sterba wrote: On Wed, Oct 15, 2014 at 04:14:17PM -0700, Zach Brown wrote: Here's another set of coverity fixes for btrfs-progs against David's integration-20141007 branch. Thanks, I've fished 2 patches for 3.17, the rest is queued. I got tired of adding error checking after a few so I moved on to the other warnings. Maybe we should subscribe linux-btrfs to the reports that coverity can send out? I'm not sure if this is allowed by the coverity service and I was not able to any info about that. We could, in theory, invite the list, and then I suppose we'd get a confirmation email that 100 people would click on. ;) Could maybe just email the scan folks and ask. I'm a little on the fence about immediately broadcasting all defects, though. I doubt there should be security implications, but ... I wish scan were better integrated with git, and then something like email the author of the commit that introduced a new defect would be pretty cool. -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the vision for btrfs fs repair?
On 10/10/14 2:35 PM, Austin S Hemmelgarn wrote: On 2014-10-10 13:43, Bob Marley wrote: On 10/10/2014 16:37, Chris Murphy wrote: The fail safe behavior is to treat the known good tree root as the default tree root, and bypass the bad tree root if it cannot be repaired, so that the volume can be mounted with default mount options (i.e. the ones in fstab). Otherwise it's a filesystem that isn't well suited for general purpose use as rootfs let alone for boot. A filesystem which is suited for general purpose use is a filesystem which honors fsync, and doesn't *ever* auto-roll-back without user intervention. Anything different is not suited for database transactions at all. Any paid service which has the users database on btrfs is going to be at risk of losing payments, and probably without the company even knowing. If btrfs goes this way I hope a big warning is written on the wiki and on the manpages telling that this filesystem is totally unsuitable for hosting databases performing transactions. If they need reliability, they should have some form of redundancy in-place and/or run the database directly on the block device; because even ext4, XFS, and pretty much every other filesystem can lose data sometimes, Not if i.e. fsync returns. If the data is gone later, it's a hardware problem, or occasionally a bug - bugs that are usually found fixed pretty quickly. the difference being that those tend to give worse results when hardware is misbehaving than BTRFS does, because BTRFS usually has a old copy of whatever data structure gets corrupted to fall back on. I'm curious, is that based on conjecture or real-world testing? -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What is the vision for btrfs fs repair?
On 10/9/14 8:49 AM, Duncan wrote: Austin S Hemmelgarn posted on Thu, 09 Oct 2014 09:18:22 -0400 as excerpted: On 2014-10-09 08:34, Duncan wrote: The only way a read-only mount should be writable is if it's mounted (bind-mounted or btrfs-subvolume-mounted) read-write elsewhere, and the write occurs to that mount, not the read-only mounted location. In theory yes, but there are caveats to this, namely: * atime updates still happen unless you have mounted the fs with noatime Getting off the topic a bit, but that really shouldn't happen: #define IS_NOATIME(inode) __IS_FLG(inode, MS_RDONLY|MS_NOATIME) and in touch_atime(): if (IS_NOATIME(inode)) return; -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What is the vision for btrfs fs repair?
I was looking at Marc's post: http://marc.merlins.org/perso/btrfs/post_2014-03-19_Btrfs-Tips_-Btrfs-Scrub-and-Btrfs-Filesystem-Repair.html and it feels like there isn't exactly a cohesive, overarching vision for repair of a corrupted btrfs filesystem. In other words - I'm an admin cruising along, when the kernel throws some fs corruption error, or for whatever reason btrfs fails to mount. What should I do? Marc lays out several steps, but to me this highlights that there seem to be a lot of disjoint mechanisms out there to deal with these problems; mostly from Marc's blog, with some bits of my own: * btrfs scrub Errors are corrected along if possible (what *is* possible?) * mount -o recovery Enable autorecovery attempts if a bad tree root is found at mount time. * mount -o degraded Allow mounts to continue with missing devices. (This isn't really a way to recover from corruption, right?) * btrfs-zero-log remove the log tree if log tree is corrupt * btrfs rescue Recover a damaged btrfs filesystem chunk-recover super-recover How does this relate to btrfs check? * btrfs check repair a btrfs filesystem --repair --init-csum-tree --init-extent-tree How does this relate to btrfs rescue? * btrfs restore try to salvage files from a damaged filesystem (not really repair, it's disk-scraping) What's the vision for, say, scrub vs. check vs. rescue? Should they repair the same errors, only online vs. offline? If not, what class of errors does one fix vs. the other? How would an admin know? Can btrfs check recover a bad tree root in the same way that mount -o recovery does? How would I know if I should use --init-*-tree, or chunk-recover, and what are the ramifications of using these options? It feels like recovery tools have been badly splintered, and if there's an overarching design or vision for btrfs fs repair, I can't tell what it is. Can anyone help me? Thanks, -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.
On 9/23/14 7:49 AM, Chris Mason wrote: On 09/23/2014 01:40 AM, Qu Wenruo wrote: [BUG] Originally when mount btrfs with -o subvol= mount option, btrfs will lose all security lable. And if the btrfs fs is mounted somewhere else, due to the lost of security lable, SELinux will refuse to mount since the same super block is being mounted using different security lable. [REPRODUCER] With SELinux enabled: #mkfs -t btrfs /dev/sda5 #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs #btrfs subvolume create /mnt/btrfs/subvol #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/test kernel message: SELinux: mount invalid. Same superblock, different security settings for (dev sda5, type btrfs) [REASON] This happens because btrfs will call vfs_kern_mount() and then mount_subtree() to handle subvolume name lookup. First mount will cut off all the security lables and when it comes to the second vfs_kern_mount(), it has no security label now. [FIX] This patch will makes btrfs behavior much more like nfs, which has the type flag FS_BINARY_MOUNTDATA, making btrfs handles the security label internally. So security label will be set in the real mount time and won't lose label when use with subvol= mount option. Thanks for working on this. Eric Sandeen (cc'd) was trying out something similar recently, so I want to make sure this doesn't conflict with his ideas. My ideas didn't get very far. ;) What I was after was a way for multiple subvolumes to have unique contexts. It looks like this might do the trick, as long as they are mounted on a unique mount point. Would this allow subvolume create to take a context, so that everything under /mnt/btrfs/subvol/ has a unique subvol-wide context? thanks, -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label.
On 9/23/14 7:31 PM, Qu Wenruo wrote: Original Message Subject: Re: [PATCH] btrfs: Make btrfs handle security mount options internally to avoid losing security label. From: Eric Sandeen sand...@redhat.com To: Chris Mason c...@fb.com, Qu Wenruo quwen...@cn.fujitsu.com, linux-btrfs@vger.kernel.org Date: 2014年09月24日 02:51 On 9/23/14 7:49 AM, Chris Mason wrote: On 09/23/2014 01:40 AM, Qu Wenruo wrote: [BUG] Originally when mount btrfs with -o subvol= mount option, btrfs will lose all security lable. And if the btrfs fs is mounted somewhere else, due to the lost of security lable, SELinux will refuse to mount since the same super block is being mounted using different security lable. [REPRODUCER] With SELinux enabled: #mkfs -t btrfs /dev/sda5 #mount -o context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/btrfs #btrfs subvolume create /mnt/btrfs/subvol #mount -o subvol=subvol,context=system_u:object_r:nfs_t:s0 /dev/sda5 /mnt/test kernel message: SELinux: mount invalid. Same superblock, different security settings for (dev sda5, type btrfs) [REASON] This happens because btrfs will call vfs_kern_mount() and then mount_subtree() to handle subvolume name lookup. First mount will cut off all the security lables and when it comes to the second vfs_kern_mount(), it has no security label now. [FIX] This patch will makes btrfs behavior much more like nfs, which has the type flag FS_BINARY_MOUNTDATA, making btrfs handles the security label internally. So security label will be set in the real mount time and won't lose label when use with subvol= mount option. Thanks for working on this. Eric Sandeen (cc'd) was trying out something similar recently, so I want to make sure this doesn't conflict with his ideas. My ideas didn't get very far. ;) What I was after was a way for multiple subvolumes to have unique contexts. It looks like this might do the trick, as long as they are mounted on a unique mount point. Would this allow subvolume create to take a context, so that everything under /mnt/btrfs/subvol/ has a unique subvol-wide context? thanks, -Eric Did you mean the following situation? /dev/sdb default subvol(FS_TREE) mounted on /mnt/default with context A /dev/sdb subvol=subvol mounted on /mnt/subvol with context B If that's your goal, I am afraid that my patch can't achieve it and even worse, will even forbid it. :( SELinux doesn't allow same superblock mounted with different context, and the patch follows it. If SELinux is modified to allow same superblock different context, then my patch also needs to be modified. oh, ok, I see. I don't think that my wish should disallow your patch. For the problem I was looking at, I think the only way forward would require some significant selinux modification, and treating a subvol root essentially like a superblock. So ... don't let me slow you down, at least for now. ;) -Eric -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs-progs: super-recover: fix double free fs_devices memory
On 9/18/14 4:01 AM, Wang Shilong wrote: super-recover collects btrfs devices infomation using existed functions scan_one_devices(). Problem is fs_devices is freed twice in close_ctree() and free_recover_superblock() for super correction path. Fix this problem by checking whether fs_devices memory have been freed before we free it. Cc: Eric Sandeen sand...@redhat.com Cc: Chris Murphy li...@colorremedies.com Signed-off-by: Wang Shilong wangshilong1...@gmail.com That does seem to fix the testcase. Thanks! Acked-by: Eric Sandeen sand...@redhat.com --- super-recover.c | 13 +++-- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/super-recover.c b/super-recover.c index 767de4b..419b86a 100644 --- a/super-recover.c +++ b/super-recover.c @@ -69,21 +69,11 @@ void init_recover_superblock(struct btrfs_recover_superblock *recover) static void free_recover_superblock(struct btrfs_recover_superblock *recover) { - struct btrfs_device *device; struct super_block_record *record; if (!recover-fs_devices) return; - while (!list_empty(recover-fs_devices-devices)) { - device = list_entry(recover-fs_devices-devices.next, - struct btrfs_device, dev_list); - list_del_init(device-dev_list); - free(device-name); - free(device); - } - free(recover-fs_devices); - while (!list_empty(recover-good_supers)) { record = list_entry(recover-good_supers.next, struct super_block_record, list); @@ -341,6 +331,9 @@ int btrfs_recover_superblocks(const char *dname, no_recover: recover_err_str(ret); free_recover_superblock(recover); + /* check if we have freed fs_deivces in close_ctree() */ + if (!root) + btrfs_close_devices(recover.fs_devices); return ret; } -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html