xfs
[Top] [All Lists]

Re: [PATCH] xfstests: add test for btrfs send issuing premature rmdir op

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfstests: add test for btrfs send issuing premature rmdir operations
From: Filipe David Manana <fdmanana@xxxxxxxxx>
Date: Mon, 24 Feb 2014 11:58:32 +0000
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>, "linux-btrfs@xxxxxxxxxxxxxxx" <linux-btrfs@xxxxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:reply-to:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=/JeZGl4rdK4bztVZLE0o+nPuOAKX8tqr8zeK4gddHy0=; b=LLrE2k9rpdVeamGV8EGgM7JZ1CGPcxknNYuZf3QWyd0YIUoe9UEZ6xzUkpWJKq1LZf T6ONYmZWCMbUw1c1U+oD9h48KsyLYL8VF0QlU4FyTz9MvUNtsEHX9os2uvPAc+POLg1E dWyfubyu6wlD8My1c60Y9PZB6CJ+3CuVO0VTZJWIvzpPPiujNHElP2Ouc+6W3tgo5Hae wsZj+6bn7UOKP/K4A2OM/QElWXfm+TwUlIM4xmoWF+PxXDr5ET//ESd7vxmkGl+YejAY RTrwCDD89vcNKYu8n4AqYHZIALr0qD3VRt0gh8r4icjKJwRlx6CDttGSJuIWq++aCV91 PjPQ==
In-reply-to: <20140224052553.GJ4317@dastard>
References: <1392820352-8550-1-git-send-email-fdmanana@xxxxxxxxx> <20140224052553.GJ4317@dastard>
Reply-to: fdmanana@xxxxxxxxx
On Mon, Feb 24, 2014 at 5:25 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> On Wed, Feb 19, 2014 at 02:32:32PM +0000, Filipe David Borba Manana wrote:
>> Regression test for btrfs incremental send issue where a rmdir instruction
>> is sent against an orphan directory inode which is not empty yet, causing
>> btrfs receive to fail when it attempts to remove the directory.
>>
>> This issue is fixed by the following linux kernel btrfs patch:
>>
>>    Btrfs: fix send attempting to rmdir non-empty directories
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@xxxxxxxxx>
>> ---
>>  tests/btrfs/041     |  153 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/btrfs/041.out |    1 +
>>  tests/btrfs/group   |    1 +
>>  3 files changed, 155 insertions(+)
>>  create mode 100755 tests/btrfs/041
>>  create mode 100644 tests/btrfs/041.out
>>
>> diff --git a/tests/btrfs/041 b/tests/btrfs/041
>> new file mode 100755
>> index 0000000..9de9326
>> --- /dev/null
>> +++ b/tests/btrfs/041
>> @@ -0,0 +1,153 @@
>> +#! /bin/bash
>> +# FS QA Test No. btrfs/041
>> +#
>> +# Regression test for btrfs incremental send issue where a rmdir instruction
>> +# is sent against an orphan directory inode which is not empty yet, causing
>> +# btrfs receive to fail when it attempts to remove the directory.
>> +#
>> +# This issue is fixed by the following linux kernel btrfs patch:
>> +#
>> +#   Btrfs: fix send attempting to rmdir non-empty directories
>> +#
>> +#-----------------------------------------------------------------------
>> +# 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"
>> +
>> +here=`pwd`
>> +tmp=`mktemp -d`
>> +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
>> +_need_to_be_root
>> +
>> +FSSUM_PROG=$here/src/fssum
>> +[ -x $FSSUM_PROG ] || _notrun "fssum not built"
>
> This is duplicated across several tests now. Perhaps this should be
> factored now into a _requires_fssum helper (separate patch is
> fine)?
>
>> +
>> +_scratch_unmount
>> +_check_btrfs_filesystem $SCRATCH_DEV
>
> you should be able to use _check_scratch_fs() here. I note that the
> btrfs path does not unmount the scratch device, so you should update
> it to do so (like _check_xfs_filesytem does) and then
> _check_scratch_fs() will just Do The Right Thing.
>
>> +_scratch_mkfs >/dev/null 2>&1
>> +_scratch_mount
>> +
>> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/1.snap
>> +run_check $FSSUM_PROG -r $tmp/1.fssum $SCRATCH_MNT/mysnap1 2>> $seqres.full
>> +
>> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/2.snap
>> +run_check $FSSUM_PROG -r $tmp/2.fssum $SCRATCH_MNT/mysnap2 2>> $seqres.full
>
> Hasn't run_check already redirected everything to $seqres.full?
>
>> +_scratch_unmount
>> +_check_btrfs_filesystem $SCRATCH_DEV
>
> _check_scratch_fs() here too.
>
> Cheers,

All done in V2 (same for the other test).
For the require_fssum() function, I made a patch on top of these 2 so
that it updates all existing btrfs tests that need fssum ("[PATCH]
xfstests: add function _require_fssum()").

Thanks for the good suggestions Dave.

>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

<Prev in Thread] Current Thread [Next in Thread>