Re: cross-fs copy support

[Eric Sandeen]

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)

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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?

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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.

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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 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.
>>
>> 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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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?

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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?

[Eric Sandeen]

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?

[Eric Sandeen]

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?

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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 ?

[Eric Sandeen]

 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

[Eric Sandeen]

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

[Eric Sandeen]

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 ...

[Eric Sandeen]

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 ...

[Eric Sandeen]

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

[Eric Sandeen]

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 ...

[Eric Sandeen]

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 ...

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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.

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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?

[Eric Sandeen]

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?

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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?

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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

[Eric Sandeen]

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?

[Eric Sandeen]

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?

[Eric Sandeen]

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?

[Eric Sandeen]

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.

[Eric Sandeen]

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.

[Eric Sandeen]

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

[Eric Sandeen]

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