Re: [PATCH] xfstests: generic: test for discard properly discarding unused extents
On Mon, 30 Mar 2015, Jeff Mahoney wrote: Date: Mon, 30 Mar 2015 15:11:06 -0400 From: Jeff Mahoney je...@suse.com To: linux-btrfs linux-btrfs@vger.kernel.org, fste...@vger.kernel.org Subject: [PATCH] xfstests: generic: test for discard properly discarding unused extents This tests tests four conditions where discard can potentially not discard unused extents completely. We test, with -o discard and with fstrim, scenarios of removing many relatively small files and removing several large files. The important part of the two scenarios is that the large files must be large enough to span a blockgroup alone. It's possible for an entire block group to be emptied and dropped without an opportunity to discard individual extents as would happen with smaller files. The test confirms the discards have occured by using a sparse file mounted via loopback to punch holes and then check how many blocks are still allocated within the file. Signed-off-by: Jeff Mahoney je...@suse.com --- tests/generic/326 | 164 ++ tests/generic/326.out | 5 ++ tests/generic/group | 1 + 3 files changed, 170 insertions(+) create mode 100644 tests/generic/326 create mode 100644 tests/generic/326.out diff --git a/tests/generic/326 b/tests/generic/326 new file mode 100644 index 000..923a27f --- /dev/null +++ b/tests/generic/326 @@ -0,0 +1,164 @@ +#! /bin/bash +# FSQA Test No. 326 +# +# This test uses a loopback mount with PUNCH_HOLE support to test +# whether discard operations are working as expected. +# +# It tests both -odiscard and fstrim. +# +# Copyright (C) 2015 SUSE. All Rights Reserved. +# Author: Jeff Mahoney je...@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 + +tmp=/tmp/$$ +status=1 # failure is the default! +trap _cleanup; exit \$status 0 1 2 3 15 + +loopdev= +tmpdir= +_cleanup() +{ + [ -n $tmpdir ] umount $tmpdir + [ -n $loopdev ] losetup -d $loopdev +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_need_to_be_root +_supported_fs generic +_supported_os Linux +_require_scratch +_require_fstrim + +rm -f $seqres.full + +_scratch_mkfs $seqres.full +_require_fs_space $SCRATCH_MNT $(( 10 * 1024 * 1024 )) +_scratch_mount + +test_discard() +{ + discard=$1 + files=$2 + + tmpfile=$SCRATCH_MNT/testfs.img.$$ + tmpdir=$SCRATCH_MNT/testdir.$$ + mkdir -p $tmpdir || _fail !!! failed to create temp mount dir + + # Create a sparse file to host the file system + dd if=/dev/zero of=$tmpfile bs=1M count=1 seek=10240 $seqres.full \ + || _fail !!! failed to create fs image file You can just use truncate here. + + opts= + if [ $discard = discard ]; then + opts=-o discard + fi + losetup -f $tmpfile + loopdev=$(losetup -j $tmpfile|awk -F: '{print $1}') you can just do loopdev=$(losetup --show -f $tmpfile) + _mkfs_dev $loopdev $seqres.full + $MOUNT_PROG $opts $loopdev $tmpdir \ + || _fail !!! failed to loopback mount + + if [ $files = large ]; then + # Create files larger than 1GB so each one occupies + # more than one block group Why it needs to be that big ? Can't you make btrfs groups smaller ? + for n in $(seq 1 8); do + dd if=/dev/zero of=$tmpdir/file$n bs=1M count=1200 \ + $seqres.full + done + else + # Create up to 40k files sized 32k-1GB. + mkdir -p $tmpdir/testdir + for ((i = 1; i = 4; i++)); do + SIZE=$(( $((1024*1024*1024)) / $(( $RANDOM + 1 )) )) + $XFS_IO_PROG -f -c pwrite -S 0xaa 0 $SIZE \ + $tmpdir/testdir/${prefix}_$i /dev/null + if [ $? -ne 0 ]; then + echo Failed creating file ${prefix}_$i \ + $seqres.full + break + fi + done + fi + + sync
Re: [PATCH] fstests: generic test for fallocate
On Thu, 12 Mar 2015, Filipe Manana wrote: Date: Thu, 12 Mar 2015 15:38:56 + From: Filipe Manana fdman...@suse.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe Manana fdman...@suse.com Subject: [PATCH] fstests: generic test for fallocate Test extent pre-allocation (using fallocate) into a region that already has a pre-allocated extent that ends beyond the file's size. Verify that if the fs is unmounted immediately after, the file's size and content are not lost. This is motivated by a minor issue found in btrfs where the second fallocate wouldn't update the inode's i_size on disk, fixed by the following btrfs patch: Btrfs: add missing inode item update in fallocate(). Looks good. Reviewed-by: Lukas Czerner lczer...@redhat.com Thanks! -Lukas Signed-off-by: Filipe Manana fdman...@suse.com --- tests/generic/071 | 79 +++ tests/generic/071.out | 9 ++ tests/generic/group | 1 + 3 files changed, 89 insertions(+) create mode 100755 tests/generic/071 create mode 100644 tests/generic/071.out diff --git a/tests/generic/071 b/tests/generic/071 new file mode 100755 index 000..7aaaed7 --- /dev/null +++ b/tests/generic/071 @@ -0,0 +1,79 @@ +#! /bin/bash +# FS QA Test No. 071 +# +# Test extent pre-allocation (using fallocate) into a region that already has a +# pre-allocated extent that ends beyond the file's size. Verify that if the fs +# is unmounted immediately after, the file's size and content are not lost. +# +#--- +# 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() +{ + rm -f $tmp.* +} +trap _cleanup; exit \$status 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_need_to_be_root +_require_scratch +_require_xfs_io_command falloc + +rm -f $seqres.full + +_scratch_mkfs $seqres.full 21 +_scratch_mount + +# Create our test file with a pre-allocated extent that doesn't increase the +# file's size. +$XFS_IO_PROG -f -c falloc -k 0 1M $SCRATCH_MNT/foo + +# Write some data to our file. +$XFS_IO_PROG -c pwrite -S 0xaa 0 256K $SCRATCH_MNT/foo | _filter_xfs_io + +# Now call fallocate again, but allowing it to increase the file's size and +# cover a range that is entirely covered by the extent that we previously +# pre-allocated. +$XFS_IO_PROG -c falloc 0 512K $SCRATCH_MNT/foo + +# Now ummount and mount again the fs. After this we expect the file's size to +# be 512Kb. +_scratch_remount + +# Now check that all data we wrote before are available and the file size is +# 512Kb. +echo File content after remount: +od -t x1 $SCRATCH_MNT/foo + +status=0 +exit diff --git a/tests/generic/071.out b/tests/generic/071.out new file mode 100644 index 000..c12fd00 --- /dev/null +++ b/tests/generic/071.out @@ -0,0 +1,9 @@ +QA output created by 071 +wrote 262144/262144 bytes at offset 0 +XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +File content after remount: +000 aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa +* +100 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 +* +200 diff --git a/tests/generic/group b/tests/generic/group index 05cc6a9..b4e5cc6 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -73,6 +73,7 @@ 068 other auto freeze dangerous stress 069 rw udf auto quick 070 attr udf auto quick stress +071 auto quick prealloc 074 rw udf auto 075 rw udf auto quick 076 metadata rw udf auto quick stress -- 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] Generic test for file fsync after moving files across directories
On Thu, 26 Feb 2015, Filipe Manana wrote: Date: Thu, 26 Feb 2015 19:53:14 + From: Filipe Manana fdman...@suse.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe Manana fdman...@suse.com Subject: [PATCH] Generic test for file fsync after moving files across directories Test file fsync after moving one file between directories and fsyncing the old parent directory before fsyncing the file. Verify that after a crash the file data we fsynced is available. This test is motivated by an issue discovered in btrfs which caused the file data to be lost (despite fsync returning success to user space). That btrfs bug is fixed by the following linux kernel patch: Btrfs: fix data loss in the fast fsync path Hi, some comments below. Signed-off-by: Filipe Manana fdman...@suse.com --- tests/generic/067 | 113 ++ tests/generic/067.out | 11 + tests/generic/group | 1 + 3 files changed, 125 insertions(+) create mode 100755 tests/generic/067 create mode 100644 tests/generic/067.out diff --git a/tests/generic/067 b/tests/generic/067 new file mode 100755 index 000..90f26b2 --- /dev/null +++ b/tests/generic/067 @@ -0,0 +1,113 @@ +#! /bin/bash +# FS QA Test No. 067 +# +# Test file fsync after moving one file between directories and fsyncing the +# old parent directory before fsyncing the file. Verify that after a crash the +# file data we fsynced is available. +# +# This test is motivated by an issue discovered in btrfs which caused the file +# data to be lost (despite fsync returning success to user space). That btrfs +# bug was fixed by the following linux kernel patch: +# +# Btrfs: fix data loss in the fast fsync path +# +#--- +# 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 'foo', the one we check for data loss. +# By doing an fsync against our file, it makes btrfs clear the 'needs_full_sync' +# bit from its flags (btrfs inode specific flags). I am not sure we need that last sentence, it's btrfs specific and I am sure it has been already explained in detail in the btrfs commit description. +$XFS_IO_PROG -f -c pwrite -S 0xaa 0 8K \ + -c fsync $SCRATCH_MNT/foo | _filter_xfs_io + +# Now create one other file and 2 directories. We will move this second file +# from one directory to the other later because it forces btrfs to commit its +# currently open transaction if we fsync the old parent directory. This is +# necessary to trigger the data loss bug that affected btrfs. +mkdir $SCRATCH_MNT/testdir_1 +touch $SCRATCH_MNT/testdir_1/bar +mkdir $SCRATCH_MNT/testdir_2 + +# Make sure everything is durably persisted. +sync + +# Write more 8Kb of data to our file. +$XFS_IO_PROG -c pwrite -S 0xbb 8K 8K $SCRATCH_MNT/foo | _filter_xfs_io + +# Move our 'bar' file into a new directory. +mv $SCRATCH_MNT/testdir_1/bar $SCRATCH_MNT/testdir_2/bar + +# Fsync our first directory. Because it had a file moved into some other +# directory, this made btrfs commit the currently open transaction. This is +# a condition necessary to trigger the data loss bug. +$XFS_IO_PROG -c fsync $SCRATCH_MNT/testdir_1 + +# Now fsync our main test file. If the fsync succeeds, we expect the 8Kb of +# data we wrote previously to be persisted and available if a crash happens. +# This did not happen with btrfs, because of the transaction commit that +#
Re: [PATCH] Generic test for file fsync after moving files across directories
On Fri, 27 Feb 2015, Filipe David Manana wrote: Date: Fri, 27 Feb 2015 10:54:43 + From: Filipe David Manana fdman...@gmail.com To: Lukáš Czerner lczer...@redhat.com Cc: Filipe Manana fdman...@suse.com, fste...@vger.kernel.org, linux-btrfs@vger.kernel.org linux-btrfs@vger.kernel.org Subject: Re: [PATCH] Generic test for file fsync after moving files across directories On Fri, Feb 27, 2015 at 9:40 AM, Lukáš Czerner lczer...@redhat.com wrote: On Thu, 26 Feb 2015, Filipe Manana wrote: Date: Thu, 26 Feb 2015 19:53:14 + From: Filipe Manana fdman...@suse.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe Manana fdman...@suse.com Subject: [PATCH] Generic test for file fsync after moving files across directories Test file fsync after moving one file between directories and fsyncing the old parent directory before fsyncing the file. Verify that after a crash the file data we fsynced is available. This test is motivated by an issue discovered in btrfs which caused the file data to be lost (despite fsync returning success to user space). That btrfs bug is fixed by the following linux kernel patch: Btrfs: fix data loss in the fast fsync path Hi, some comments below. Signed-off-by: Filipe Manana fdman...@suse.com --- tests/generic/067 | 113 ++ tests/generic/067.out | 11 + tests/generic/group | 1 + 3 files changed, 125 insertions(+) create mode 100755 tests/generic/067 create mode 100644 tests/generic/067.out diff --git a/tests/generic/067 b/tests/generic/067 new file mode 100755 index 000..90f26b2 --- /dev/null +++ b/tests/generic/067 @@ -0,0 +1,113 @@ +#! /bin/bash +# FS QA Test No. 067 +# +# Test file fsync after moving one file between directories and fsyncing the +# old parent directory before fsyncing the file. Verify that after a crash the +# file data we fsynced is available. +# +# This test is motivated by an issue discovered in btrfs which caused the file +# data to be lost (despite fsync returning success to user space). That btrfs +# bug was fixed by the following linux kernel patch: +# +# Btrfs: fix data loss in the fast fsync path +# +#--- +# 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 'foo', the one we check for data loss. +# By doing an fsync against our file, it makes btrfs clear the 'needs_full_sync' +# bit from its flags (btrfs inode specific flags). I am not sure we need that last sentence, it's btrfs specific and I am sure it has been already explained in detail in the btrfs commit description. Right, I left it because without this fsync the bug isn't triggered and the reader could find it weird because shortly after there's a 'sync' call. So I wanted to emphasize that fsync's importance. +$XFS_IO_PROG -f -c pwrite -S 0xaa 0 8K \ + -c fsync $SCRATCH_MNT/foo | _filter_xfs_io + +# Now create one other file and 2 directories. We will move this second file +# from one directory to the other later because it forces btrfs to commit its +# currently open transaction if we fsync the old parent directory. This is +# necessary to trigger the data loss bug that affected btrfs. +mkdir $SCRATCH_MNT/testdir_1 +touch $SCRATCH_MNT
Re: [PATCH v3] fstests: generic test for directory fsync after adding hard links
On Wed, 25 Feb 2015, Dave Chinner wrote: Date: Wed, 25 Feb 2015 11:06:25 +1100 From: Dave Chinner da...@fromorbit.com To: Eric Sandeen sand...@redhat.com Cc: Filipe Manana fdman...@suse.com, fste...@vger.kernel.org, linux-btrfs@vger.kernel.org Subject: Re: [PATCH v3] fstests: generic test for directory fsync after adding hard links On Tue, Feb 24, 2015 at 05:40:05PM -0600, Eric Sandeen wrote: On 2/24/15 5:29 PM, Filipe Manana wrote: 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. ;) Right, the description in the test is supposed to be a consise description of what the test does. It is parsed by lsqa.pl to inform the reader of what the test exercises and that's why we actually care about the quality of the description. Right, but if the test was written specifically to address a bug found in the code I'd love to see the commit id, or name preferably directly in the test description or at least in the commit description. It's very useful information to have and often we forget to include it. Thanks! -Lukas 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 The fascinating btrfs story belongs in the commit message, not the test itself. If people need to know what btrfs commit the test exercises, the history, motivations and commentary on the code, then they can look it up in the git history. 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. Precisely. Cheers, Dave. -- 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
2014 Linux Plumbers Call for File and Storage Systems Microconference Proposals
Submission for Microconferences discussion topics and BOFs are now open [1] and as a File and Storage Systems [2] microconference leads we'd like to invite you to submit your proposals. You can find the instructions on how to propose a discussion topic here: http://www.linuxplumbersconf.org/2014/how-to-submit-microconference-discussions-topics/ The Linux Plumbers Conference has officially sold out all of the places, so we encourage submissions for our file and storage microconference from people who have already registered. We will work to get exceptionally compelling session leaders registered, but our ability is limited, so please submit your topics early. Deadline for submissions is Friday, August 30th at midnight and we will announce the topics on Monday, September 1st. If you have any questions, please let us know. Thanks! -Lukas -- [1] http://www.linuxplumbersconf.org/2014/submissions-for-microconferences-discussion-topics-and-bofs-are-now-open/ [2] http://www.linuxplumbersconf.org/2014/ocw/events/LPC2014/tracks/303 [3] http://www.linuxplumbersconf.org/2014/how-to-submit-microconference-discussions-topics/ [4] http://wiki.linuxplumbersconf.org/2014:file_and_storage_systems -- 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] generic/017: skip invalid block sizes for btrfs
On Mon, 23 Jun 2014, Filipe David Borba Manana wrote: Date: Mon, 23 Jun 2014 11:28:00 +0100 From: Filipe David Borba Manana fdman...@gmail.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe David Borba Manana fdman...@gmail.com Subject: [PATCH] generic/017: skip invalid block sizes for btrfs In btrfs the block size (called sector size in btrfs) can not be smaller then the page size. Therefore skip block sizes smaller then page size if the fs is btrfs, so that the test can succeed on btrfs (testing only with block sizes of 4kb on systems with a page size of 4Kb). The test itself is wrong, it's trying to do _scratch_mkfs with different block size, but the block size might already be specified by the user (in fact it should be user responsibility to test different block sizes). In the case that mkfs can not handle multiple of the same option like mkfs.xfs for example it will fail, but the test will go on with the original file system. The test needs to be fixed to just test the file system with options specified by the user. Also we should change _scratch_mkfs() to fail the test if the mkfs failed (no one is actually testing mkfs_status variable anyway. Once we do that this patch will no longer be needed. Thanks! -Lukas Signed-off-by: Filipe David Borba Manana fdman...@gmail.com --- tests/generic/017 | 8 1 file changed, 8 insertions(+) diff --git a/tests/generic/017 b/tests/generic/017 index 13b7254..6495be5 100755 --- a/tests/generic/017 +++ b/tests/generic/017 @@ -51,6 +51,14 @@ BLOCKS=10240 for (( BSIZE = 1024; BSIZE = 4096; BSIZE *= 2 )); do + # btrfs doesn't support block size smaller then page size + if [ $FSTYP == btrfs ]; then + if (( $BSIZE `getconf PAGE_SIZE` )); then + echo 80 + continue + fi + fi + length=$(($BLOCKS * $BSIZE)) case $FSTYP in xfs) -- 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] generic/017: skip invalid block sizes for btrfs
On Mon, 23 Jun 2014, Lukáš Czerner wrote: Date: Mon, 23 Jun 2014 14:35:50 +0200 (CEST) From: Lukáš Czerner lczer...@redhat.com To: Filipe David Borba Manana fdman...@gmail.com Cc: fste...@vger.kernel.org, linux-btrfs@vger.kernel.org Subject: Re: [PATCH] generic/017: skip invalid block sizes for btrfs On Mon, 23 Jun 2014, Filipe David Borba Manana wrote: Date: Mon, 23 Jun 2014 11:28:00 +0100 From: Filipe David Borba Manana fdman...@gmail.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe David Borba Manana fdman...@gmail.com Subject: [PATCH] generic/017: skip invalid block sizes for btrfs In btrfs the block size (called sector size in btrfs) can not be smaller then the page size. Therefore skip block sizes smaller then page size if the fs is btrfs, so that the test can succeed on btrfs (testing only with block sizes of 4kb on systems with a page size of 4Kb). The test itself is wrong, it's trying to do _scratch_mkfs with different block size, but the block size might already be specified by the user (in fact it should be user responsibility to test different block sizes). In the case that mkfs can not handle multiple of the same option like mkfs.xfs for example it will fail, but the test will go on with the original file system. The test needs to be fixed to just test the file system with options specified by the user. Also we should change _scratch_mkfs() to fail the test if the mkfs failed (no one is actually testing mkfs_status variable anyway. Correction, _scratch_mkfs_xfs() is actually testing mkfs_status and will attempt to re-run mkfs only with provided options if it failed before. But my point remains the same, block size to test should be in users hands and we should run all tests with different block sizes, if supported. Thanks! -Lukas Once we do that this patch will no longer be needed. Thanks! -Lukas Signed-off-by: Filipe David Borba Manana fdman...@gmail.com --- tests/generic/017 | 8 1 file changed, 8 insertions(+) diff --git a/tests/generic/017 b/tests/generic/017 index 13b7254..6495be5 100755 --- a/tests/generic/017 +++ b/tests/generic/017 @@ -51,6 +51,14 @@ BLOCKS=10240 for (( BSIZE = 1024; BSIZE = 4096; BSIZE *= 2 )); do + # btrfs doesn't support block size smaller then page size + if [ $FSTYP == btrfs ]; then + if (( $BSIZE `getconf PAGE_SIZE` )); then + echo 80 + continue + fi + fi + length=$(($BLOCKS * $BSIZE)) case $FSTYP in xfs) -- 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
Re: [PATCH] generic/017: skip invalid block sizes for btrfs
On Tue, 24 Jun 2014, Dave Chinner wrote: Date: Tue, 24 Jun 2014 14:26:46 +1000 From: Dave Chinner da...@fromorbit.com To: Lukáš Czerner lczer...@redhat.com Cc: Filipe David Borba Manana fdman...@gmail.com, fste...@vger.kernel.org, linux-btrfs@vger.kernel.org Subject: Re: [PATCH] generic/017: skip invalid block sizes for btrfs On Mon, Jun 23, 2014 at 04:09:18PM +0200, Lukáš Czerner wrote: On Mon, 23 Jun 2014, Lukáš Czerner wrote: Date: Mon, 23 Jun 2014 14:35:50 +0200 (CEST) From: Lukáš Czerner lczer...@redhat.com To: Filipe David Borba Manana fdman...@gmail.com Cc: fste...@vger.kernel.org, linux-btrfs@vger.kernel.org Subject: Re: [PATCH] generic/017: skip invalid block sizes for btrfs On Mon, 23 Jun 2014, Filipe David Borba Manana wrote: Date: Mon, 23 Jun 2014 11:28:00 +0100 From: Filipe David Borba Manana fdman...@gmail.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe David Borba Manana fdman...@gmail.com Subject: [PATCH] generic/017: skip invalid block sizes for btrfs In btrfs the block size (called sector size in btrfs) can not be smaller then the page size. Therefore skip block sizes smaller then page size if the fs is btrfs, so that the test can succeed on btrfs (testing only with block sizes of 4kb on systems with a page size of 4Kb). The test itself is wrong, it's trying to do _scratch_mkfs with different block size, but the block size might already be specified by the user (in fact it should be user responsibility to test different block sizes). In the case that mkfs can not handle multiple of the same option like mkfs.xfs for example it will fail, but the test will go on with the original file system. The test needs to be fixed to just test the file system with options specified by the user. Also we should change _scratch_mkfs() to fail the test if the mkfs failed (no one is actually testing mkfs_status variable anyway. Correction, _scratch_mkfs_xfs() is actually testing mkfs_status and will attempt to re-run mkfs only with provided options if it failed before. But my point remains the same, block size to test should be in users hands and we should run all tests with different block sizes, if supported. Follow that line of reasoning to other options. If we take that argument to it's logical conclusion, no test should be able to set any mount or mkfs option because that's for the user to control. This implies we can't even have quota specific tests because they need to override the mount options (and perhaps mkfs options) the user has specified to be properly tested. I acknowledge that there are tests that actually needs to set its own options. That's usually in the nature of the test, but it's not the case with this particular test. Here, it's not needed and in my view it's wrong because it brings unnecessary limitations and problems. Some tests only work on specific configurations and therefore they need to be able to control the execution environment directly. Hence the behaviour of _scratch_mkfs_xfs(), where it will *attempt* to use the user provided options. However, if they conflict with what the test requires it will drop the user options and use what the test requires. It's better that the test overrides user provided options than fail due to incompatible configuration. It makes maintenance of the tests much easier because we don't have to declare and maintain the supported list of user options for every test. Either the test works for all configurations (i.e. whatever the user sets in MKFS/MOUNT_OPTIONS), or it specifically defines the configuration it is testing. I do agree with that, I do not think I've ever said otherwise. I was addressing this particular test where the specific configuration is not needed. -Lukas Cheers, Dave.
Re: [PATCH v2] xfstests: add test for btrfs cloning with file holes
On Sat, 31 May 2014, Filipe David Borba Manana wrote: Date: Sat, 31 May 2014 15:12:45 +0100 From: Filipe David Borba Manana fdman...@gmail.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe David Borba Manana fdman...@gmail.com Subject: [PATCH v2] xfstests: add test for btrfs cloning with file holes Regression test for the btrfs ioctl clone operation when the source range contains hole(s) and the FS has the NO_HOLES feature enabled (file holes don't need file extent items in the btree to represent them). This issue is fixed by the following linux kernel btrfs patch: Btrfs: fix clone to deal with holes when NO_HOLES feature is enabled Signed-off-by: Filipe David Borba Manana fdman...@gmail.com --- V2: Increased test coverage by testing the cases where a hole overlaps the start and end of the cloning range. tests/btrfs/055 | 112 + tests/btrfs/055.out | 117 tests/btrfs/group | 1 + 3 files changed, 230 insertions(+) create mode 100755 tests/btrfs/055 create mode 100644 tests/btrfs/055.out diff --git a/tests/btrfs/055 b/tests/btrfs/055 new file mode 100755 index 000..4a1614b --- /dev/null +++ b/tests/btrfs/055 @@ -0,0 +1,112 @@ +#! /bin/bash +# FS QA Test No. btrfs/055 +# +# Regression test for the btrfs ioctl clone operation when the source range +# contains hole(s) and the FS has the NO_HOLES feature enabled (file holes +# don't need file extent items in the btree to represent them). +# +# This issue is fixed by the following linux kernel btrfs patch: +# +#Btrfs: fix clone to deal with holes when NO_HOLES feature is enabled +# +#--- +# Copyright (c) 2014 Filipe Manana. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#--- +# + +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo QA output created by $seq + +tmp=/tmp/$$ +status=1 # failure is the default! +trap _cleanup; exit \$status 0 1 2 3 15 + +_cleanup() +{ +rm -fr $tmp +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_fs btrfs +_supported_os Linux +_require_scratch +_require_btrfs_cloner +_need_to_be_root + +rm -f $seqres.full + +test_btrfs_clone_with_holes() +{ + _scratch_mkfs $1 /dev/null 21 + _scratch_mount + + # Create a file with 4 extents and 1 hole, all with a size of 8Kb each. + $XFS_IO_PROG -f -c pwrite -S 0x01 -b 8192 0 8192 $SCRATCH_MNT/foo \ + | _filter_xfs_io + sync + $XFS_IO_PROG -c pwrite -S 0x02 -b 8192 8192 8192 $SCRATCH_MNT/foo \ + | _filter_xfs_io + sync + # After the following write we get a hole in the range [16384, 24576[ + $XFS_IO_PROG -c pwrite -S 0x04 -b 8192 24576 8192 $SCRATCH_MNT/foo \ + | _filter_xfs_io + sync + $XFS_IO_PROG -c pwrite -S 0x05 -b 8192 32768 8192 $SCRATCH_MNT/foo \ + | _filter_xfs_io + sync + + # Clone destination file, 1 extent of 96kb. + $XFS_IO_PROG -f -c pwrite -S 0xff -b 98304 0 98304 $SCRATCH_MNT/bar \ + | _filter_xfs_io + sync you could merge all the XFS_IO_PROG calls into one. and use -W to sync it. Otherwise it looks good to me. Reviewed-by: Lukas Czerner lczer...@redhat.com + + # Clone 2nd extent, 8Kb hole and 3rd extent of foo into bar. + $CLONER_PROG -s 8192 -d 0 -l 24576 $SCRATCH_MNT/foo $SCRATCH_MNT/bar + + # Verify both extents and the hole were cloned. + od -t x1 $SCRATCH_MNT/bar + + # Cloning range starts at the middle of a hole. + $CLONER_PROG -s 20480 -d 32768 -l 12288 $SCRATCH_MNT/foo $SCRATCH_MNT/bar + + # Verify that half of the hole and the following 8Kb extent were cloned. + od -t x1 $SCRATCH_MNT/bar + + # Cloning range ends at the middle of a hole. + $CLONER_PROG -s 0 -d 65536 -l 20480 $SCRATCH_MNT/foo $SCRATCH_MNT/bar + + # Verify that 2 extents of 8kb and a 4kb hole were cloned. + od -t x1 $SCRATCH_MNT/bar + + # Verify that there are no consistency errors. + _check_scratch_fs +} + +echo
Re: [PATCH v4] xfstests: add test for btrfs cloning with file holes
On Sun, 1 Jun 2014, Filipe David Borba Manana wrote: Date: Sun, 1 Jun 2014 14:06:06 +0100 From: Filipe David Borba Manana fdman...@gmail.com To: fste...@vger.kernel.org Cc: linux-btrfs@vger.kernel.org, Filipe David Borba Manana fdman...@gmail.com Subject: [PATCH v4] xfstests: add test for btrfs cloning with file holes Regression test for the btrfs ioctl clone operation when the source range contains hole(s) and the FS has the NO_HOLES feature enabled (file holes don't need file extent items in the btree to represent them). This issue is fixed by the following linux kernel btrfs patch: Btrfs: fix clone to deal with holes when NO_HOLES feature is enabled Signed-off-by: Filipe David Borba Manana fdman...@gmail.com --- V2: Increased test coverage by testing the cases where a hole overlaps the start and end of the cloning range. V3: Test the case where the cloning range includes an hole at the end of the source file and might increase the size of the target file. V4: Added test for the case where the clone range covers only a hole at the beginning of the source file. Made the test be skipped if the available version of mkfs.btrfs doesn't support the no-holes feature. And when testing the case where the no-holes feature isn't enabled, explicitly ask mkfs.btrfs to disable no-holes (future versions of mkfs.btrfs might enable this feature by default). common/rc | 13 ++ tests/btrfs/055 | 165 ++ tests/btrfs/055.out | 333 tests/btrfs/group | 1 + 4 files changed, 512 insertions(+) create mode 100755 tests/btrfs/055 create mode 100644 tests/btrfs/055.out diff --git a/common/rc b/common/rc index f27ee53..0ef0ece 100644 --- a/common/rc +++ b/common/rc @@ -2177,6 +2177,19 @@ _require_btrfs_send_stream_version() fi } +_require_btrfs_fs_feature() +{ + if [ -z $1 ]; then + echo Missing feature name argument for _require_btrfs_fs_feature + exit 1 + fi + feat=$1 + $MKFS_BTRFS_PROG -O list-all 21 | \ + grep '^[ \t]*'$feat'\b' /dev/null 21 + [ $? -eq 0 ] || \ + _notrun Feature $feat not supported in the available version of mkfs.btrfs And what if kernel does not support this ? +} + init_rc() { if [ $iam == new ] diff --git a/tests/btrfs/055 b/tests/btrfs/055 new file mode 100755 index 000..0e67c88 --- /dev/null +++ b/tests/btrfs/055 @@ -0,0 +1,165 @@ +#! /bin/bash +# FS QA Test No. btrfs/055 +# +# Regression test for the btrfs ioctl clone operation when the source range +# contains hole(s) and the FS has the NO_HOLES feature enabled (file holes +# don't need file extent items in the btree to represent them). +# +# This issue is fixed by the following linux kernel btrfs patch: +# +#Btrfs: fix clone to deal with holes when NO_HOLES feature is enabled +# +#--- +# Copyright (c) 2014 Filipe Manana. All Rights Reserved. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +#--- +# + +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo QA output created by $seq + +tmp=/tmp/$$ +status=1 # failure is the default! +trap _cleanup; exit \$status 0 1 2 3 15 + +_cleanup() +{ +rm -fr $tmp +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_fs btrfs +_supported_os Linux +_require_scratch +_require_btrfs_cloner +_require_btrfs_fs_feature no-holes +_need_to_be_root + +rm -f $seqres.full + +test_btrfs_clone_with_holes() +{ + _scratch_mkfs $1 /dev/null 21 + _scratch_mount + + # Create a file with 4 extents and 1 hole, all with a size of 8Kb each. + $XFS_IO_PROG -f -c pwrite -S 0x01 -b 8192 0 8192 $SCRATCH_MNT/foo \ + | _filter_xfs_io + sync + $XFS_IO_PROG -c pwrite -S 0x02 -b 8192 8192 8192 $SCRATCH_MNT/foo \ + | _filter_xfs_io + sync + # After the following write we get a hole in the range [16384, 24576[ + $XFS_IO_PROG -c pwrite -S 0x04 -b 8192 24576 8192 $SCRATCH_MNT/foo \ + | _filter_xfs_io +
Re: [ANNOUNCE] xfstests: new mailing list
On Thu, 15 May 2014, Christoph Hellwig wrote: Date: Thu, 15 May 2014 22:40:09 -0700 From: Christoph Hellwig h...@infradead.org To: Dave Chinner da...@fromorbit.com Cc: fste...@vger.kernel.org, linux-fsde...@vger.kernel.org, linux-e...@vger.kernel.org, linux-btrfs@vger.kernel.org, x...@oss.sgi.com Subject: Re: [ANNOUNCE] xfstests: new mailing list On Fri, May 16, 2014 at 02:46:11PM +1000, Dave Chinner wrote: Hi folks, As requested I've created a new mailing list for xfstests development and discussion. Reflecting the fact that the test harness is not really XFS specific anymore, the list is: fste...@vger.kernel.org Isn't there an x missing somewhere? It's intentional and it is Reflecting the fact that the test harness is not really XFS specific anymore, even though the test suite itself keep the name xfstests. This way it's more obvious to people that this is in fact not xfs specific. -Lukas -- To unsubscribe from this list: send the line unsubscribe linux-ext4 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: [ANNOUNCE] xfstests: new mailing list
On Fri, 16 May 2014, Christoph Hellwig wrote: Date: Fri, 16 May 2014 01:53:20 -0700 From: Christoph Hellwig h...@infradead.org To: Luk?? Czerner lczer...@redhat.com Cc: Christoph Hellwig h...@infradead.org, Dave Chinner da...@fromorbit.com, fste...@vger.kernel.org, linux-fsde...@vger.kernel.org, linux-e...@vger.kernel.org, linux-btrfs@vger.kernel.org, x...@oss.sgi.com Subject: Re: [ANNOUNCE] xfstests: new mailing list On Fri, May 16, 2014 at 10:48:42AM +0200, Luk?? Czerner wrote: As requested I've created a new mailing list for xfstests development and discussion. Reflecting the fact that the test harness is not really XFS specific anymore, the list is: fste...@vger.kernel.org Isn't there an x missing somewhere? It's intentional and it is Reflecting the fact that the test harness is not really XFS specific anymore, even though the test suite itself keep the name xfstests. This way it's more obvious to people that this is in fact not xfs specific. Having the name different from the project it is for is stupid. Either rename the test suite, or use the same name for the mailing list. Renaming the test suite is a good option if we can agree on this. -Lukas -- 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: [ANNOUNCE] xfstests: updated to cf1ed54
On Sat, 5 Apr 2014, Dave Chinner wrote: Date: Sat, 5 Apr 2014 08:12:25 +1100 From: Dave Chinner da...@fromorbit.com To: Filipe David Manana fdman...@gmail.com Cc: x...@oss.sgi.com x...@oss.sgi.com, linux-btrfs@vger.kernel.org linux-btrfs@vger.kernel.org, linux-e...@vger.kernel.org, lczer...@redhat.com Subject: Re: [ANNOUNCE] xfstests: updated to cf1ed54 On Fri, Apr 04, 2014 at 02:07:16PM +0100, Filipe David Manana wrote: On Fri, Apr 4, 2014 at 10:03 AM, Dave Chinner da...@fromorbit.com wrote: Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. The new head of the master branch is commit: cf1ed54 check: fix RESULT_BASE typo in check script The major new functionality worth mentioning in this update is the new config file format from Lukas. The existing format config files should continue to work without change, but the new format is much richer and allows specification of multiple different configurations to run test under. Hence testing multiple mount an dmkfs configurations becomes as simple as iterating the configurations in the config file. Hi, I might be missing something, but after checking out these changes, I am no longer able to run btrfs tests. Example: $ ./check btrfs/041 common/config: Error: $SCRATCH_DEV should be unset when $SCRATCH_DEV_POOL is set Passed all 0 tests $ cat local.config export TEST_DEV=/dev/sdb export TEST_DIR=/home/fdmanana/btrfs-tests/dev export SCRATCH_MNT=/home/fdmanana/btrfs-tests/scratch_1 export SCRATCH_DEV_POOL=/dev/sdc /dev/sdd OK, that'll be a bug in the new config file parsing code. Lukas, can you have a look at this? Oh, yes. I'll take a look at this. Honestly I was not aware of SCRATCH_DEV_POOL. Thanks! -Lukas Cheers, Dave. -- 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
How btrfs resize should work ?
Hi, I am working on system storage manager (ssm) trying to implement btrfs resize correctly, however I have some troubles with it. # mkfs.btrfs /dev/sda /dev/sdb # mount /dev/sda /mnt/test # btrfs filesystem show failed to open /dev/sr0: No medium found Label: none uuid: 8dce5578-a2bc-416e-96fd-16a2f4f770b7 Total devices 2 FS bytes used 28.00KB devid2 size 50.00GB used 2.01GB path /dev/sdb devid1 size 100.00GB used 2.03GB path /dev/sda Btrfs v0.20-rc1 # btrfs filesystem resize 3G /mnt/test # sync # btrfs filesystem show failed to open /dev/sr0: No medium found Label: none uuid: 8dce5578-a2bc-416e-96fd-16a2f4f770b7 Total devices 2 FS bytes used 796.00KB devid2 size 50.00GB used 2.01GB path /dev/sdb devid1 size 3.00GB used 2.03GB path /dev/sda So it appears that it only resized part of the file system which lies on /dev/sda to 3G. This behavior is confusing as hell, is this really desirable behaviour ? I do understand that you're able to specify devid to resize, however without devid specification this kind of does not make any sense. dm get it right, and it also has several different policies on handling this stuff and it was not easy to get this right. Is btrfs going to fix this somehow, it does it expect user to take care of this (I hope not). Because currently ssm is not able to resize btrfs correctly. Thanks! -Lukas -- 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/3] Btrfs-progs: move path modification to filters
On Thu, 31 Jan 2013, Lukáš Czerner wrote: Date: Thu, 31 Jan 2013 07:40:37 +0100 (CET) From: Lukáš Czerner lczer...@redhat.com To: Gene Czarcinski g...@czarc.net Cc: Lukáš Czerner lczer...@redhat.com, linux-btrfs linux-btrfs@vger.kernel.org Subject: Re: [PATCH 1/3] Btrfs-progs: move path modification to filters On Wed, 30 Jan 2013, Gene Czarcinski wrote: Date: Wed, 30 Jan 2013 13:03:30 -0500 From: Gene Czarcinski g...@czarc.net To: Lukáš Czerner lczer...@redhat.com Cc: linux-btrfs linux-btrfs@vger.kernel.org Subject: Re: [PATCH 1/3] Btrfs-progs: move path modification to filters Ignoring for the moment whether these patches are a good idea or not, what is the base upon which these patches were built. You might want to consider rebasing them to David Sterba's integration-20130130 Gene Hi, yet it has been a while since I've posted it so I guess I have to rebase. Just to be sure sure I have to ask. Which is the btrfs-progs repository/branch I should base my work on ? Currently I am using repostitory git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-progs.git and master branch. Is that ok ? Thanks! -Lukas Thanks! -Lukas On 01/30/2013 08:32 AM, Lukáš Czerner wrote: On Thu, 10 Jan 2013, Lukáš Czerner wrote: Date: Thu, 10 Jan 2013 13:02:42 +0100 (CET) From: Lukáš Czerner lczer...@redhat.com To: Lukas Czerner lczer...@redhat.com Cc: linux-btrfs@vger.kernel.org, chris.ma...@fusionio.com, cwi...@cwillu.com Subject: Re: [PATCH 1/3] Btrfs-progs: move path modification to filters On Tue, 11 Dec 2012, Lukas Czerner wrote: Date: Tue, 11 Dec 2012 15:24:58 +0100 From: Lukas Czerner lczer...@redhat.com To: linux-btrfs@vger.kernel.org Cc: chris.ma...@fusionio.com, cwi...@cwillu.com, Lukas Czerner lczer...@redhat.com Subject: [PATCH 1/3] Btrfs-progs: move path modification to filters Commit 8e8e019e910f20947fea7eff5da40753639d8870 introduces -a option which will list all subvolumes with distinguishing between relative and absolute by prepending absolute patch with FS_TREE. This commit moves the path modification to a filter code rather than doing so in path construction in resolve_root(). This gives us more flexibility in formatting path output. ping any comments on this ? -Lukas ping Signed-off-by: Lukas Czerner lczer...@redhat.com --- btrfs-list.c | 32 +++- btrfs-list.h |1 + cmds-subvolume.c | 11 +-- man/btrfs.8.in |3 ++- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/btrfs-list.c b/btrfs-list.c index e5f0f96..77d99f8 100644 --- a/btrfs-list.c +++ b/btrfs-list.c @@ -628,15 +628,6 @@ static int resolve_root(struct root_lookup *rl, struct root_info *ri, } if (next == BTRFS_FS_TREE_OBJECTID) { - char p[] = FS_TREE; - add_len = strlen(p); - len = strlen(full_path); - tmp = malloc(len + add_len + 2); - memcpy(tmp + add_len + 1, full_path, len); - tmp[add_len] = '/'; - memcpy(tmp, p, add_len); - free(full_path); - full_path = tmp; ri-top_id = next; break; } @@ -1176,6 +1167,28 @@ static int filter_topid_equal(struct root_info *ri, u64 data) return ri-top_id == data; } +static int filter_full_path(struct root_info *ri, u64 data) +{ + if (ri-full_path ri-top_id != data) { + char *tmp; + char p[] = FS_TREE; + int add_len = strlen(p); + int len = strlen(ri-full_path); + + tmp = malloc(len + add_len + 2); + if (!tmp) { + fprintf(stderr, memory allocation failed\n); + exit(1); + } + memcpy(tmp + add_len + 1, ri-full_path, len); + tmp[add_len] = '/'; + memcpy(tmp, p, add_len); + free(ri-full_path); + ri-full_path = tmp; + } + return 1; +} + static btrfs_list_filter_func all_filter_funcs[] = { [BTRFS_LIST_FILTER_ROOTID] = filter_by_rootid, [BTRFS_LIST_FILTER_SNAPSHOT_ONLY] = filter_snapshot, @@ -1187,6 +1200,7 @@ static btrfs_list_filter_func all_filter_funcs[] = { [BTRFS_LIST_FILTER_CGEN_LESS] = filter_cgen_less, [BTRFS_LIST_FILTER_CGEN_EQUAL] = filter_cgen_equal
Re: [PATCH 1/3] Btrfs-progs: move path modification to filters
On Thu, 10 Jan 2013, Lukáš Czerner wrote: Date: Thu, 10 Jan 2013 13:02:42 +0100 (CET) From: Lukáš Czerner lczer...@redhat.com To: Lukas Czerner lczer...@redhat.com Cc: linux-btrfs@vger.kernel.org, chris.ma...@fusionio.com, cwi...@cwillu.com Subject: Re: [PATCH 1/3] Btrfs-progs: move path modification to filters On Tue, 11 Dec 2012, Lukas Czerner wrote: Date: Tue, 11 Dec 2012 15:24:58 +0100 From: Lukas Czerner lczer...@redhat.com To: linux-btrfs@vger.kernel.org Cc: chris.ma...@fusionio.com, cwi...@cwillu.com, Lukas Czerner lczer...@redhat.com Subject: [PATCH 1/3] Btrfs-progs: move path modification to filters Commit 8e8e019e910f20947fea7eff5da40753639d8870 introduces -a option which will list all subvolumes with distinguishing between relative and absolute by prepending absolute patch with FS_TREE. This commit moves the path modification to a filter code rather than doing so in path construction in resolve_root(). This gives us more flexibility in formatting path output. ping any comments on this ? -Lukas ping Signed-off-by: Lukas Czerner lczer...@redhat.com --- btrfs-list.c | 32 +++- btrfs-list.h |1 + cmds-subvolume.c | 11 +-- man/btrfs.8.in |3 ++- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/btrfs-list.c b/btrfs-list.c index e5f0f96..77d99f8 100644 --- a/btrfs-list.c +++ b/btrfs-list.c @@ -628,15 +628,6 @@ static int resolve_root(struct root_lookup *rl, struct root_info *ri, } if (next == BTRFS_FS_TREE_OBJECTID) { - char p[] = FS_TREE; - add_len = strlen(p); - len = strlen(full_path); - tmp = malloc(len + add_len + 2); - memcpy(tmp + add_len + 1, full_path, len); - tmp[add_len] = '/'; - memcpy(tmp, p, add_len); - free(full_path); - full_path = tmp; ri-top_id = next; break; } @@ -1176,6 +1167,28 @@ static int filter_topid_equal(struct root_info *ri, u64 data) return ri-top_id == data; } +static int filter_full_path(struct root_info *ri, u64 data) +{ + if (ri-full_path ri-top_id != data) { + char *tmp; + char p[] = FS_TREE; + int add_len = strlen(p); + int len = strlen(ri-full_path); + + tmp = malloc(len + add_len + 2); + if (!tmp) { + fprintf(stderr, memory allocation failed\n); + exit(1); + } + memcpy(tmp + add_len + 1, ri-full_path, len); + tmp[add_len] = '/'; + memcpy(tmp, p, add_len); + free(ri-full_path); + ri-full_path = tmp; + } + return 1; +} + static btrfs_list_filter_func all_filter_funcs[] = { [BTRFS_LIST_FILTER_ROOTID] = filter_by_rootid, [BTRFS_LIST_FILTER_SNAPSHOT_ONLY] = filter_snapshot, @@ -1187,6 +1200,7 @@ static btrfs_list_filter_func all_filter_funcs[] = { [BTRFS_LIST_FILTER_CGEN_LESS] = filter_cgen_less, [BTRFS_LIST_FILTER_CGEN_EQUAL] = filter_cgen_equal, [BTRFS_LIST_FILTER_TOPID_EQUAL] = filter_topid_equal, + [BTRFS_LIST_FILTER_FULL_PATH] = filter_full_path, }; struct btrfs_list_filter_set *btrfs_list_alloc_filter_set(void) diff --git a/btrfs-list.h b/btrfs-list.h index cde4b3c..f7fbea6 100644 --- a/btrfs-list.h +++ b/btrfs-list.h @@ -71,6 +71,7 @@ enum btrfs_list_filter_enum { BTRFS_LIST_FILTER_CGEN_LESS, BTRFS_LIST_FILTER_CGEN_MORE, BTRFS_LIST_FILTER_TOPID_EQUAL, + BTRFS_LIST_FILTER_FULL_PATH, BTRFS_LIST_FILTER_MAX, }; diff --git a/cmds-subvolume.c b/cmds-subvolume.c index ac39f7b..37cb8cc 100644 --- a/cmds-subvolume.c +++ b/cmds-subvolume.c @@ -277,7 +277,9 @@ static const char * const cmd_subvol_list_usage[] = { List subvolumes (and snapshots), , -p print parent ID, - -a print all the subvolumes in the filesystem., + -a print all the subvolumes in the filesystem and, +distinguish absolute and relative path with respect, +to the given path, -u print the uuid of subvolumes (and snapshots), -t print the result as a table, -s list snapshots only in the filesystem, @@ -400,7 +402,12 @@ static int cmd_subvol_list(int argc, char **argv) } top_id = btrfs_list_get_path_rootid(fd); - if (!is_list_all) + + if (is_list_all) + btrfs_list_setup_filter(filter_set, + BTRFS_LIST_FILTER_FULL_PATH, + top_id); + else
Re: [PATCH 1/3] Btrfs-progs: move path modification to filters
On Tue, 11 Dec 2012, Lukas Czerner wrote: Date: Tue, 11 Dec 2012 15:24:58 +0100 From: Lukas Czerner lczer...@redhat.com To: linux-btrfs@vger.kernel.org Cc: chris.ma...@fusionio.com, cwi...@cwillu.com, Lukas Czerner lczer...@redhat.com Subject: [PATCH 1/3] Btrfs-progs: move path modification to filters Commit 8e8e019e910f20947fea7eff5da40753639d8870 introduces -a option which will list all subvolumes with distinguishing between relative and absolute by prepending absolute patch with FS_TREE. This commit moves the path modification to a filter code rather than doing so in path construction in resolve_root(). This gives us more flexibility in formatting path output. ping any comments on this ? -Lukas Signed-off-by: Lukas Czerner lczer...@redhat.com --- btrfs-list.c | 32 +++- btrfs-list.h |1 + cmds-subvolume.c | 11 +-- man/btrfs.8.in |3 ++- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/btrfs-list.c b/btrfs-list.c index e5f0f96..77d99f8 100644 --- a/btrfs-list.c +++ b/btrfs-list.c @@ -628,15 +628,6 @@ static int resolve_root(struct root_lookup *rl, struct root_info *ri, } if (next == BTRFS_FS_TREE_OBJECTID) { - char p[] = FS_TREE; - add_len = strlen(p); - len = strlen(full_path); - tmp = malloc(len + add_len + 2); - memcpy(tmp + add_len + 1, full_path, len); - tmp[add_len] = '/'; - memcpy(tmp, p, add_len); - free(full_path); - full_path = tmp; ri-top_id = next; break; } @@ -1176,6 +1167,28 @@ static int filter_topid_equal(struct root_info *ri, u64 data) return ri-top_id == data; } +static int filter_full_path(struct root_info *ri, u64 data) +{ + if (ri-full_path ri-top_id != data) { + char *tmp; + char p[] = FS_TREE; + int add_len = strlen(p); + int len = strlen(ri-full_path); + + tmp = malloc(len + add_len + 2); + if (!tmp) { + fprintf(stderr, memory allocation failed\n); + exit(1); + } + memcpy(tmp + add_len + 1, ri-full_path, len); + tmp[add_len] = '/'; + memcpy(tmp, p, add_len); + free(ri-full_path); + ri-full_path = tmp; + } + return 1; +} + static btrfs_list_filter_func all_filter_funcs[] = { [BTRFS_LIST_FILTER_ROOTID] = filter_by_rootid, [BTRFS_LIST_FILTER_SNAPSHOT_ONLY] = filter_snapshot, @@ -1187,6 +1200,7 @@ static btrfs_list_filter_func all_filter_funcs[] = { [BTRFS_LIST_FILTER_CGEN_LESS] = filter_cgen_less, [BTRFS_LIST_FILTER_CGEN_EQUAL] = filter_cgen_equal, [BTRFS_LIST_FILTER_TOPID_EQUAL] = filter_topid_equal, + [BTRFS_LIST_FILTER_FULL_PATH] = filter_full_path, }; struct btrfs_list_filter_set *btrfs_list_alloc_filter_set(void) diff --git a/btrfs-list.h b/btrfs-list.h index cde4b3c..f7fbea6 100644 --- a/btrfs-list.h +++ b/btrfs-list.h @@ -71,6 +71,7 @@ enum btrfs_list_filter_enum { BTRFS_LIST_FILTER_CGEN_LESS, BTRFS_LIST_FILTER_CGEN_MORE, BTRFS_LIST_FILTER_TOPID_EQUAL, + BTRFS_LIST_FILTER_FULL_PATH, BTRFS_LIST_FILTER_MAX, }; diff --git a/cmds-subvolume.c b/cmds-subvolume.c index ac39f7b..37cb8cc 100644 --- a/cmds-subvolume.c +++ b/cmds-subvolume.c @@ -277,7 +277,9 @@ static const char * const cmd_subvol_list_usage[] = { List subvolumes (and snapshots), , -p print parent ID, - -a print all the subvolumes in the filesystem., + -a print all the subvolumes in the filesystem and, + distinguish absolute and relative path with respect, + to the given path, -u print the uuid of subvolumes (and snapshots), -t print the result as a table, -s list snapshots only in the filesystem, @@ -400,7 +402,12 @@ static int cmd_subvol_list(int argc, char **argv) } top_id = btrfs_list_get_path_rootid(fd); - if (!is_list_all) + + if (is_list_all) + btrfs_list_setup_filter(filter_set, + BTRFS_LIST_FILTER_FULL_PATH, + top_id); + else btrfs_list_setup_filter(filter_set, BTRFS_LIST_FILTER_TOPID_EQUAL, top_id); diff --git a/man/btrfs.8.in b/man/btrfs.8.in index 9222580..d6d8e94 100644 --- a/man/btrfs.8.in +++ b/man/btrfs.8.in @@ -124,7 +124,8 @@ and top level. The parent's ID may be used
Re: [PATCH] btrfs: Return EINVAL when length to trim is less than FSB
On Tue, 16 Oct 2012, Lukas Czerner wrote: Date: Tue, 16 Oct 2012 11:34:36 +0200 From: Lukas Czerner lczer...@redhat.com To: linux-btrfs@vger.kernel.org Cc: jba...@fusionio.com, Lukas Czerner lczer...@redhat.com Subject: [PATCH] btrfs: Return EINVAL when length to trim is less than FSB Currently if len argument in btrfs_ioctl_fitrim() is smaller than one FSB we will continue and finally return 0 bytes discarded. However if the length to discard is smaller then file system block we should really return EINVAL. ping Signed-off-by: Lukas Czerner lczer...@redhat.com --- fs/btrfs/ioctl.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6116880..3b8b509 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -343,7 +343,8 @@ static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg) return -EOPNOTSUPP; if (copy_from_user(range, arg, sizeof(range))) return -EFAULT; - if (range.start total_bytes) + if (range.start total_bytes || + range.len fs_info-sb-s_blocksize) return -EINVAL; range.len = min(range.len, total_bytes - range.start); -- 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 v3 01/13] btrfs: add one new mount option '-o hot_track'
On Wed, 10 Oct 2012, Zhi Yong Wu wrote: Date: Wed, 10 Oct 2012 20:21:48 +0800 From: Zhi Yong Wu zwu.ker...@gmail.com To: Lukáš Czerner lczer...@redhat.com Cc: linux-fsde...@vger.kernel.org, linux-e...@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-ker...@vger.kernel.org, linux...@linux.vnet.ibm.com, v...@zeniv.linux.org.uk, da...@fromorbit.com, d...@jikos.cz, ty...@mit.edu, c...@us.ibm.com, Zhi Yong Wu wu...@linux.vnet.ibm.com Subject: Re: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track' On Wed, Oct 10, 2012 at 7:59 PM, Lukáš Czerner lczer...@redhat.com wrote: On Wed, 10 Oct 2012, zwu.ker...@gmail.com wrote: Date: Wed, 10 Oct 2012 18:07:23 +0800 From: zwu.ker...@gmail.com To: linux-fsde...@vger.kernel.org Cc: linux-e...@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-ker...@vger.kernel.org, linux...@linux.vnet.ibm.com, v...@zeniv.linux.org.uk, da...@fromorbit.com, d...@jikos.cz, ty...@mit.edu, c...@us.ibm.com, Zhi Yong Wu wu...@linux.vnet.ibm.com Subject: [RFC v3 01/13] btrfs: add one new mount option '-o hot_track' From: Zhi Yong Wu wu...@linux.vnet.ibm.com Introduce one new mount option '-o hot_track', and add its parsing support. Its usage looks like: mount -o hot_track mount -o nouser,hot_track mount -o nouser,hot_track,loop mount -o hot_track,nouser This patch should probably be at the end of the series. Can you let me know your reason? I think that it is not necessary to be at the end of the series. Simply because you're adding the mount option which does not do anything yet. Moreover you change the implementation of the hot track as you go. You should enable this once it is ready to use, not the other way around. So, please move this at the end of the patch set when the feature is supposed to be ready to use. Thanks! -Lukas -Lukas Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com --- fs/btrfs/ctree.h |1 + fs/btrfs/super.c |7 ++- 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 9821b67..094bec6 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1726,6 +1726,7 @@ struct btrfs_ioctl_defrag_range_args { #define BTRFS_MOUNT_CHECK_INTEGRITY (1 20) #define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 21) #define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR (1 22) +#define BTRFS_MOUNT_HOT_TRACK(1 23) #define btrfs_clear_opt(o, opt) ((o) = ~BTRFS_MOUNT_##opt) #define btrfs_set_opt(o, opt)((o) |= BTRFS_MOUNT_##opt) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 83d6f9f..00be9e3 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -41,6 +41,7 @@ #include linux/slab.h #include linux/cleancache.h #include linux/ratelimit.h +#include linux/hot_tracking.h #include compat.h #include delayed-inode.h #include ctree.h @@ -303,7 +304,7 @@ enum { Opt_notreelog, Opt_ratio, Opt_flushoncommit, Opt_discard, Opt_space_cache, Opt_clear_cache, Opt_user_subvol_rm_allowed, Opt_enospc_debug, Opt_subvolrootid, Opt_defrag, Opt_inode_cache, - Opt_no_space_cache, Opt_recovery, Opt_skip_balance, + Opt_no_space_cache, Opt_recovery, Opt_skip_balance, Opt_hot_track, Opt_check_integrity, Opt_check_integrity_including_extent_data, Opt_check_integrity_print_mask, Opt_fatal_errors, Opt_err, @@ -342,6 +343,7 @@ static match_table_t tokens = { {Opt_no_space_cache, nospace_cache}, {Opt_recovery, recovery}, {Opt_skip_balance, skip_balance}, + {Opt_hot_track, hot_track}, {Opt_check_integrity, check_int}, {Opt_check_integrity_including_extent_data, check_int_data}, {Opt_check_integrity_print_mask, check_int_print_mask=%d}, @@ -553,6 +555,9 @@ int btrfs_parse_options(struct btrfs_root *root, char *options) case Opt_skip_balance: btrfs_set_opt(info-mount_opt, SKIP_BALANCE); break; + case Opt_hot_track: + btrfs_set_opt(info-mount_opt, HOT_TRACK); + break; #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY case Opt_check_integrity_including_extent_data: printk(KERN_INFO btrfs: enabling check integrity