| Message ID | 1508997466-12859-1-git-send-email-gujx@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >
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 >
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 >> >
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 fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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"