* [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode
@ 2017-10-26 5:57 Gu Jinxiang
2017-10-26 6:52 ` Eryu Guan
0 siblings, 1 reply; 6+ messages in thread
From: Gu Jinxiang @ 2017-10-26 5:57 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs, Gu JinXiang
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(+)
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode
2017-10-26 5:57 [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode Gu Jinxiang
@ 2017-10-26 6:52 ` Eryu Guan
2017-10-26 7:13 ` Qu Wenruo
2017-10-26 7:16 ` Gu, Jinxiang
0 siblings, 2 replies; 6+ messages in thread
From: Eryu Guan @ 2017-10-26 6:52 UTC (permalink / raw)
To: Gu Jinxiang; +Cc: fstests, linux-btrfs
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode
2017-10-26 6:52 ` Eryu Guan
@ 2017-10-26 7:13 ` Qu Wenruo
2017-10-26 7:32 ` Qu Wenruo
2017-10-26 7:16 ` Gu, Jinxiang
1 sibling, 1 reply; 6+ messages in thread
From: Qu Wenruo @ 2017-10-26 7:13 UTC (permalink / raw)
To: Eryu Guan, Gu Jinxiang; +Cc: fstests, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 2658 bytes --]
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
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode
2017-10-26 6:52 ` Eryu Guan
2017-10-26 7:13 ` Qu Wenruo
@ 2017-10-26 7:16 ` Gu, Jinxiang
2017-10-26 7:34 ` Eryu Guan
1 sibling, 1 reply; 6+ messages in thread
From: Gu, Jinxiang @ 2017-10-26 7:16 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode
2017-10-26 7:13 ` Qu Wenruo
@ 2017-10-26 7:32 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-10-26 7:32 UTC (permalink / raw)
To: Eryu Guan, Gu Jinxiang; +Cc: fstests, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 2910 bytes --]
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
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode
2017-10-26 7:16 ` Gu, Jinxiang
@ 2017-10-26 7:34 ` Eryu Guan
0 siblings, 0 replies; 6+ messages in thread
From: Eryu Guan @ 2017-10-26 7:34 UTC (permalink / raw)
To: Gu, Jinxiang; +Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-26 7:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-26 5:57 [PATCH] xfstests: Add message indicating btrfs-progs support FST in read-only mode Gu Jinxiang
2017-10-26 6:52 ` Eryu Guan
2017-10-26 7:13 ` Qu Wenruo
2017-10-26 7:32 ` Qu Wenruo
2017-10-26 7:16 ` Gu, Jinxiang
2017-10-26 7:34 ` Eryu Guan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox