xfstests: Add message indicating btrfs-progs support FST in read-only mode
diff mbox

Message ID 1508997466-12859-1-git-send-email-gujx@cn.fujitsu.com
State New
Headers show

Commit Message

Gu Jinxiang Oct. 26, 2017, 5:57 a.m. UTC
From: Gu JinXiang <gujx@cn.fujitsu.com>

btrfs-progs now support FST in read-only mode, so when space_cache=v2
enabled, this test case will fail.
Add message to help user to understand this status.

Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com>
---
 tests/btrfs/012 | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eryu Guan Oct. 26, 2017, 6:52 a.m. UTC | #1
On Thu, Oct 26, 2017 at 01:57:46PM +0800, Gu Jinxiang wrote:
> From: Gu JinXiang <gujx@cn.fujitsu.com>
> 
> btrfs-progs now support FST in read-only mode, so when space_cache=v2
> enabled, this test case will fail.
> Add message to help user to understand this status.

Sorry, I don't quite understand the new 'FST' feature. But is it a bug
that we want to fix when mounting with space_cache=v2 option, or we just
couldn't do btrfs-convert in this case? If it's a real bug, I'd say let
the test fail as it is, and track bug in tools like bugzilla not
comments/messages in the test; if it's the latter case, then just
_notrun the test if space_cache=v2 option is specified, e.g.

_exclude_scratch_mount_option "space_cache=v2"

Thanks,
Eryu

> 
> Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com>
> ---
>  tests/btrfs/012 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/btrfs/012 b/tests/btrfs/012
> index 85c82f07..529e6eca 100755
> --- a/tests/btrfs/012
> +++ b/tests/btrfs/012
> @@ -96,6 +96,12 @@ cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
>  
>  _scratch_unmount
>  
> +space_cache_version=$(echo "$MOUNT_OPTIONS" | grep "space_cache=v2")
> +if [ -n "$space_cache_version" ]; then
> +	_fail "since used space_cache=v2 when mount," \
> +	       "and for FST btrfs-progs support is read-only."\
> +	      "so btrfs-convert rollback will fail"
> +fi
>  # Now restore the ext4 device
>  $BTRFS_CONVERT_PROG -r $SCRATCH_DEV >> $seqres.full 2>&1 || \
>  	_fail "btrfs-convert rollback failed"
> -- 
> 2.13.5
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Qu Wenruo Oct. 26, 2017, 7:13 a.m. UTC | #2
On 2017年10月26日 14:52, Eryu Guan wrote:
> On Thu, Oct 26, 2017 at 01:57:46PM +0800, Gu Jinxiang wrote:
>> From: Gu JinXiang <gujx@cn.fujitsu.com>
>>
>> btrfs-progs now support FST in read-only mode, so when space_cache=v2
>> enabled, this test case will fail.
>> Add message to help user to understand this status.
> 
> Sorry, I don't quite understand the new 'FST' feature.

FST is Free Space (cache) Tree, which is one of the internal methods to
manage the free space cache for btrfs.


> But is it a bug
> that we want to fix when mounting with space_cache=v2 option, or we just
> couldn't do btrfs-convert in this case? If it's a real bug, I'd say let
> the test fail as it is, and track bug in tools like bugzilla not
> comments/messages in the test; if it's the latter case, then just
> _notrun the test if space_cache=v2 option is specified, e.g.

The problem is not about the kernel, it's btrfs-convert to blame, which
can't handle free space cache tree.

However, since btrfs-convert is just rollbacking the btrfs to ext*, it
doesn't really need to do any write to *btrfs*, so the correct method
here is to fix btrfs-convert rollback, to do a *readonly* open other
than RW open, other than skipping special mount option.

I'll fix it soon.

Thanks,
Qu

> 
> _exclude_scratch_mount_option "space_cache=v2"
> 
> Thanks,
> Eryu
> 
>>
>> Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com>
>> ---
>>  tests/btrfs/012 | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/btrfs/012 b/tests/btrfs/012
>> index 85c82f07..529e6eca 100755
>> --- a/tests/btrfs/012
>> +++ b/tests/btrfs/012
>> @@ -96,6 +96,12 @@ cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
>>  
>>  _scratch_unmount
>>  
>> +space_cache_version=$(echo "$MOUNT_OPTIONS" | grep "space_cache=v2")
>> +if [ -n "$space_cache_version" ]; then
>> +	_fail "since used space_cache=v2 when mount," \
>> +	       "and for FST btrfs-progs support is read-only."\
>> +	      "so btrfs-convert rollback will fail"
>> +fi
>>  # Now restore the ext4 device
>>  $BTRFS_CONVERT_PROG -r $SCRATCH_DEV >> $seqres.full 2>&1 || \
>>  	_fail "btrfs-convert rollback failed"
>> -- 
>> 2.13.5
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Gu Jinxiang Oct. 26, 2017, 7:16 a.m. UTC | #3
Hi,

> -----Original Message-----

> From: Eryu Guan [mailto:eguan@redhat.com]

> Sent: Thursday, October 26, 2017 2:52 PM

> To: Gu, Jinxiang/顾 金香 <gujx@cn.fujitsu.com>

> Cc: fstests@vger.kernel.org; linux-btrfs@vger.kernel.org

> Subject: Re: [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode

> 

> On Thu, Oct 26, 2017 at 01:57:46PM +0800, Gu Jinxiang wrote:

> > From: Gu JinXiang <gujx@cn.fujitsu.com>

> >

> > btrfs-progs now support FST in read-only mode, so when space_cache=v2

> > enabled, this test case will fail.

> > Add message to help user to understand this status.

> 

> Sorry, I don't quite understand the new 'FST' feature. But is it a bug that we want to fix when mounting with space_cache=v2

> option, or we just couldn't do btrfs-convert in this case? If it's a real bug, I'd say let the test fail as it is, and

> track bug in tools like bugzilla not comments/messages in the test; if it's the latter case, then just _notrun the test

> if space_cache=v2 option is specified, e.g.

> 

> _exclude_scratch_mount_option "space_cache=v2"

Thank you for your suggestion.
As an known behavior when use btrfs-convert when space_cache=v2 enabled, I prefer to modify this case not to be run.
I all send a new patch for this.

> 

> Thanks,

> Eryu

> 

> >

> > Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com>

> > ---

> >  tests/btrfs/012 | 6 ++++++

> >  1 file changed, 6 insertions(+)

> >

> > diff --git a/tests/btrfs/012 b/tests/btrfs/012 index

> > 85c82f07..529e6eca 100755

> > --- a/tests/btrfs/012

> > +++ b/tests/btrfs/012

> > @@ -96,6 +96,12 @@ cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new

> >

> >  _scratch_unmount

> >

> > +space_cache_version=$(echo "$MOUNT_OPTIONS" | grep "space_cache=v2")

> > +if [ -n "$space_cache_version" ]; then

> > +	_fail "since used space_cache=v2 when mount," \

> > +	       "and for FST btrfs-progs support is read-only."\

> > +	      "so btrfs-convert rollback will fail"

> > +fi

> >  # Now restore the ext4 device

> >  $BTRFS_CONVERT_PROG -r $SCRATCH_DEV >> $seqres.full 2>&1 || \

> >  	_fail "btrfs-convert rollback failed"

> > --

> > 2.13.5

> >

> >

> >

> > --

> > To unsubscribe from this list: send the line "unsubscribe fstests" in

> > the body of a message to majordomo@vger.kernel.org More majordomo info

> > at  http://vger.kernel.org/majordomo-info.html

>
Qu Wenruo Oct. 26, 2017, 7:32 a.m. UTC | #4
On 2017年10月26日 15:13, Qu Wenruo wrote:
> 
> 
> On 2017年10月26日 14:52, Eryu Guan wrote:
>> On Thu, Oct 26, 2017 at 01:57:46PM +0800, Gu Jinxiang wrote:
>>> From: Gu JinXiang <gujx@cn.fujitsu.com>
>>>
>>> btrfs-progs now support FST in read-only mode, so when space_cache=v2
>>> enabled, this test case will fail.
>>> Add message to help user to understand this status.
>>
>> Sorry, I don't quite understand the new 'FST' feature.
> 
> FST is Free Space (cache) Tree, which is one of the internal methods to
> manage the free space cache for btrfs.
> 
> 
>> But is it a bug
>> that we want to fix when mounting with space_cache=v2 option, or we just
>> couldn't do btrfs-convert in this case? If it's a real bug, I'd say let
>> the test fail as it is, and track bug in tools like bugzilla not
>> comments/messages in the test; if it's the latter case, then just
>> _notrun the test if space_cache=v2 option is specified, e.g.
> 
> The problem is not about the kernel, it's btrfs-convert to blame, which
> can't handle free space cache tree.
> 
> However, since btrfs-convert is just rollbacking the btrfs to ext*, it
> doesn't really need to do any write to *btrfs*, so the correct method
> here is to fix btrfs-convert rollback, to do a *readonly* open other
> than RW open, other than skipping special mount option.
> 
> I'll fix it soon.

Here comes the fix for btrfs-progs.
https://patchwork.kernel.org/patch/10027529/

Thanks,
Qu

> 
> Thanks,
> Qu
> 
>>
>> _exclude_scratch_mount_option "space_cache=v2"
>>
>> Thanks,
>> Eryu
>>
>>>
>>> Signed-off-by: Gu JinXiang <gujx@cn.fujitsu.com>
>>> ---
>>>  tests/btrfs/012 | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tests/btrfs/012 b/tests/btrfs/012
>>> index 85c82f07..529e6eca 100755
>>> --- a/tests/btrfs/012
>>> +++ b/tests/btrfs/012
>>> @@ -96,6 +96,12 @@ cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
>>>  
>>>  _scratch_unmount
>>>  
>>> +space_cache_version=$(echo "$MOUNT_OPTIONS" | grep "space_cache=v2")
>>> +if [ -n "$space_cache_version" ]; then
>>> +	_fail "since used space_cache=v2 when mount," \
>>> +	       "and for FST btrfs-progs support is read-only."\
>>> +	      "so btrfs-convert rollback will fail"
>>> +fi
>>>  # Now restore the ext4 device
>>>  $BTRFS_CONVERT_PROG -r $SCRATCH_DEV >> $seqres.full 2>&1 || \
>>>  	_fail "btrfs-convert rollback failed"
>>> -- 
>>> 2.13.5
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>>> the body of a message to majordomo@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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
Eryu Guan Oct. 26, 2017, 7:34 a.m. UTC | #5
On Thu, Oct 26, 2017 at 07:16:02AM +0000, Gu, Jinxiang wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Eryu Guan [mailto:eguan@redhat.com]
> > Sent: Thursday, October 26, 2017 2:52 PM
> > To: Gu, Jinxiang/顾 金香 <gujx@cn.fujitsu.com>
> > Cc: fstests@vger.kernel.org; linux-btrfs@vger.kernel.org
> > Subject: Re: [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode
> > 
> > On Thu, Oct 26, 2017 at 01:57:46PM +0800, Gu Jinxiang wrote:
> > > From: Gu JinXiang <gujx@cn.fujitsu.com>
> > >
> > > btrfs-progs now support FST in read-only mode, so when space_cache=v2
> > > enabled, this test case will fail.
> > > Add message to help user to understand this status.
> > 
> > Sorry, I don't quite understand the new 'FST' feature. But is it a bug that we want to fix when mounting with space_cache=v2
> > option, or we just couldn't do btrfs-convert in this case? If it's a real bug, I'd say let the test fail as it is, and
> > track bug in tools like bugzilla not comments/messages in the test; if it's the latter case, then just _notrun the test
> > if space_cache=v2 option is specified, e.g.
> > 
> > _exclude_scratch_mount_option "space_cache=v2"
> Thank you for your suggestion.
> As an known behavior when use btrfs-convert when space_cache=v2 enabled, I prefer to modify this case not to be run.
> I all send a new patch for this.

But from what Qu said in his reply, it looks like this is a real bug in
btrfs-convert, so I don't think _notrun is a good idea. I'd leave the
test as it is, because that's a bug and this test finds it.

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/tests/btrfs/012 b/tests/btrfs/012
index 85c82f07..529e6eca 100755
--- a/tests/btrfs/012
+++ b/tests/btrfs/012
@@ -96,6 +96,12 @@  cp -aR /lib/modules/`uname -r`/ $SCRATCH_MNT/new
 
 _scratch_unmount
 
+space_cache_version=$(echo "$MOUNT_OPTIONS" | grep "space_cache=v2")
+if [ -n "$space_cache_version" ]; then
+	_fail "since used space_cache=v2 when mount," \
+	       "and for FST btrfs-progs support is read-only."\
+	      "so btrfs-convert rollback will fail"
+fi
 # Now restore the ext4 device
 $BTRFS_CONVERT_PROG -r $SCRATCH_DEV >> $seqres.full 2>&1 || \
 	_fail "btrfs-convert rollback failed"