[PATCH] btrfs/294: reject zoned devices for now

[PATCH] btrfs/294: reject zoned devices for now

[Qu Wenruo]

The test case itself is utilizing RAID5/6, which is not yet supported on
zoned device.

In the future we would use raid-stripe-tree (RST) feature, but for now
just reject zoned devices completely.

And since we're here, also update the _fixed_by_kernel_commit lines, as
the proper fix is already merged upstream.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/294 | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/btrfs/294 b/tests/btrfs/294
index 61ce7d97..d7d13646 100755
--- a/tests/btrfs/294
+++ b/tests/btrfs/294
@@ -16,11 +16,15 @@ _begin_fstest auto raid volume
 
 # Modify as appropriate.
 _supported_fs btrfs
+
+# No zoned support for RAID56 yet.
+_require_non_zoned_device "${SCRATCH_DEV}"
+
 _require_scratch_dev_pool 8
 _fixed_by_kernel_commit a7299a18a179 \
 	"btrfs: fix u32 overflows when left shifting @stripe_nr"
-_fixed_by_kernel_commit xxxxxxxxxxxx \
-	"btrfs: use a dedicated helper to convert stripe_nr to offset"
+_fixed_by_kernel_commit cb091225a538 \
+	"btrfs: fix remaining u32 overflows when left shifting stripe_nr"
 
 _scratch_dev_pool_get 8
 
-- 
2.41.0

Re: [PATCH] btrfs/294: reject zoned devices for now

[Zorro Lang]

On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
> The test case itself is utilizing RAID5/6, which is not yet supported on
> zoned device.
> 
> In the future we would use raid-stripe-tree (RST) feature, but for now
> just reject zoned devices completely.
> 
> And since we're here, also update the _fixed_by_kernel_commit lines, as
> the proper fix is already merged upstream.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---

Oh, good, you've sent this patch, ignore my reply to last patch.
Looks good to me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

Thanks,
Zorro

>  tests/btrfs/294 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/294 b/tests/btrfs/294
> index 61ce7d97..d7d13646 100755
> --- a/tests/btrfs/294
> +++ b/tests/btrfs/294
> @@ -16,11 +16,15 @@ _begin_fstest auto raid volume
>  
>  # Modify as appropriate.
>  _supported_fs btrfs
> +
> +# No zoned support for RAID56 yet.
> +_require_non_zoned_device "${SCRATCH_DEV}"
> +
>  _require_scratch_dev_pool 8
>  _fixed_by_kernel_commit a7299a18a179 \
>  	"btrfs: fix u32 overflows when left shifting @stripe_nr"
> -_fixed_by_kernel_commit xxxxxxxxxxxx \
> -	"btrfs: use a dedicated helper to convert stripe_nr to offset"
> +_fixed_by_kernel_commit cb091225a538 \
> +	"btrfs: fix remaining u32 overflows when left shifting stripe_nr"
>  
>  _scratch_dev_pool_get 8
>  
> -- 
> 2.41.0
> 

Re: [PATCH] btrfs/294: reject zoned devices for now

[Naohiro Aota]

On Mon, Jul 24, 2023 at 12:55:30PM +0800, Zorro Lang wrote:
> On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
> > The test case itself is utilizing RAID5/6, which is not yet supported on
> > zoned device.
> > 
> > In the future we would use raid-stripe-tree (RST) feature, but for now
> > just reject zoned devices completely.
> > 
> > And since we're here, also update the _fixed_by_kernel_commit lines, as
> > the proper fix is already merged upstream.
> > 
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > ---
> 
> Oh, good, you've sent this patch, ignore my reply to last patch.
> Looks good to me.
> 
> Reviewed-by: Zorro Lang <zlang@redhat.com>

Thank you. I didn't notice the patch had been merged.

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>

> 
> Thanks,
> Zorro
> 
> >  tests/btrfs/294 | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/btrfs/294 b/tests/btrfs/294
> > index 61ce7d97..d7d13646 100755
> > --- a/tests/btrfs/294
> > +++ b/tests/btrfs/294
> > @@ -16,11 +16,15 @@ _begin_fstest auto raid volume
> >  
> >  # Modify as appropriate.
> >  _supported_fs btrfs
> > +
> > +# No zoned support for RAID56 yet.
> > +_require_non_zoned_device "${SCRATCH_DEV}"
> > +
> >  _require_scratch_dev_pool 8
> >  _fixed_by_kernel_commit a7299a18a179 \
> >  	"btrfs: fix u32 overflows when left shifting @stripe_nr"
> > -_fixed_by_kernel_commit xxxxxxxxxxxx \
> > -	"btrfs: use a dedicated helper to convert stripe_nr to offset"
> > +_fixed_by_kernel_commit cb091225a538 \
> > +	"btrfs: fix remaining u32 overflows when left shifting stripe_nr"
> >  
> >  _scratch_dev_pool_get 8
> >  
> > -- 
> > 2.41.0
> > 
> 

Re: [PATCH] btrfs/294: reject zoned devices for now

[Christoph Hellwig]

On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
> The test case itself is utilizing RAID5/6, which is not yet supported on
> zoned device.
> 
> In the future we would use raid-stripe-tree (RST) feature, but for now
> just reject zoned devices completely.
> 
> And since we're here, also update the _fixed_by_kernel_commit lines, as
> the proper fix is already merged upstream.

Hmm, instead of spreading these checks, shouldn't we check that the
RAID level is supported, and have one single nob for that based off
it, similar to _btrfs_get_profile_configs()?

Re: [PATCH] btrfs/294: reject zoned devices for now

[Naohiro Aota]

On Mon, Jul 24, 2023 at 07:10:51AM -0700, Christoph Hellwig wrote:
> On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
> > The test case itself is utilizing RAID5/6, which is not yet supported on
> > zoned device.
> > 
> > In the future we would use raid-stripe-tree (RST) feature, but for now
> > just reject zoned devices completely.
> > 
> > And since we're here, also update the _fixed_by_kernel_commit lines, as
> > the proper fix is already merged upstream.
> 
> Hmm, instead of spreading these checks, shouldn't we check that the
> RAID level is supported, and have one single nob for that based off
> it, similar to _btrfs_get_profile_configs()?
> 

That's beneficial. We need to declare which profile the test going to use,
something like this?

  _btrfs_require_profile raid5 raid6

or

  _btrfs_require_profile data:dup

as the zoned mode cannot work with the DUP profile for data.

Re: [PATCH] btrfs/294: reject zoned devices for now

[Qu Wenruo]

On 2023/7/25 08:22, Naohiro Aota wrote:
> On Mon, Jul 24, 2023 at 07:10:51AM -0700, Christoph Hellwig wrote:
>> On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
>>> The test case itself is utilizing RAID5/6, which is not yet supported on
>>> zoned device.
>>>
>>> In the future we would use raid-stripe-tree (RST) feature, but for now
>>> just reject zoned devices completely.
>>>
>>> And since we're here, also update the _fixed_by_kernel_commit lines, as
>>> the proper fix is already merged upstream.
>>
>> Hmm, instead of spreading these checks, shouldn't we check that the
>> RAID level is supported, and have one single nob for that based off
>> it, similar to _btrfs_get_profile_configs()?
>>
> 
> That's beneficial. We need to declare which profile the test going to use,
> something like this?
> 
>    _btrfs_require_profile raid5 raid6
> 
> or
> 
>    _btrfs_require_profile data:dup
> 
> as the zoned mode cannot work with the DUP profile for data.

The latter one sounds good, considering zoned support differs for data 
and metadata profiles.

Another thing is, do we have any sysfs files to indicate what profiles 
we support?
For now we only have a "zoned" features, but no dedicated zoned specific 
sysfs files.

Would that be a good thing to consider?

Thanks,
Qu

Re: [PATCH] btrfs/294: reject zoned devices for now

[David Sterba]

On Tue, Jul 25, 2023 at 08:58:34AM +0800, Qu Wenruo wrote:
> 
> 
> On 2023/7/25 08:22, Naohiro Aota wrote:
> > On Mon, Jul 24, 2023 at 07:10:51AM -0700, Christoph Hellwig wrote:
> >> On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
> >>> The test case itself is utilizing RAID5/6, which is not yet supported on
> >>> zoned device.
> >>>
> >>> In the future we would use raid-stripe-tree (RST) feature, but for now
> >>> just reject zoned devices completely.
> >>>
> >>> And since we're here, also update the _fixed_by_kernel_commit lines, as
> >>> the proper fix is already merged upstream.
> >>
> >> Hmm, instead of spreading these checks, shouldn't we check that the
> >> RAID level is supported, and have one single nob for that based off
> >> it, similar to _btrfs_get_profile_configs()?
> >>
> > 
> > That's beneficial. We need to declare which profile the test going to use,
> > something like this?
> > 
> >    _btrfs_require_profile raid5 raid6
> > 
> > or
> > 
> >    _btrfs_require_profile data:dup
> > 
> > as the zoned mode cannot work with the DUP profile for data.
> 
> The latter one sounds good, considering zoned support differs for data 
> and metadata profiles.
> 
> Another thing is, do we have any sysfs files to indicate what profiles 
> we support?
> For now we only have a "zoned" features, but no dedicated zoned specific 
> sysfs files.
> 
> Would that be a good thing to consider?

We don't have it but I think it's a good idea not just for testing, we
export other information as the "features/supported_..." files. The
profiles don't change often but with the incremental support for zoned
this would make it easier for tests to keep up.

Re: [PATCH] btrfs/294: reject zoned devices for now

[Zorro Lang]

On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
> The test case itself is utilizing RAID5/6, which is not yet supported on
> zoned device.
> 
> In the future we would use raid-stripe-tree (RST) feature, but for now
> just reject zoned devices completely.
> 
> And since we're here, also update the _fixed_by_kernel_commit lines, as
> the proper fix is already merged upstream.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---

Just check before next fstests release. This patch has been reviewed, but looks
like there're still more review points from btrfs list. So will you send a new
version to replace this patch?

Thanks,
Zorro

>  tests/btrfs/294 | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/btrfs/294 b/tests/btrfs/294
> index 61ce7d97..d7d13646 100755
> --- a/tests/btrfs/294
> +++ b/tests/btrfs/294
> @@ -16,11 +16,15 @@ _begin_fstest auto raid volume
>  
>  # Modify as appropriate.
>  _supported_fs btrfs
> +
> +# No zoned support for RAID56 yet.
> +_require_non_zoned_device "${SCRATCH_DEV}"
> +
>  _require_scratch_dev_pool 8
>  _fixed_by_kernel_commit a7299a18a179 \
>  	"btrfs: fix u32 overflows when left shifting @stripe_nr"
> -_fixed_by_kernel_commit xxxxxxxxxxxx \
> -	"btrfs: use a dedicated helper to convert stripe_nr to offset"
> +_fixed_by_kernel_commit cb091225a538 \
> +	"btrfs: fix remaining u32 overflows when left shifting stripe_nr"
>  
>  _scratch_dev_pool_get 8
>  
> -- 
> 2.41.0
> 

Re: [PATCH] btrfs/294: reject zoned devices for now

[Qu Wenruo]

On 2023/7/27 23:50, Zorro Lang wrote:
> On Mon, Jul 24, 2023 at 11:04:23AM +0800, Qu Wenruo wrote:
>> The test case itself is utilizing RAID5/6, which is not yet supported on
>> zoned device.
>>
>> In the future we would use raid-stripe-tree (RST) feature, but for now
>> just reject zoned devices completely.
>>
>> And since we're here, also update the _fixed_by_kernel_commit lines, as
>> the proper fix is already merged upstream.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>
> Just check before next fstests release. This patch has been reviewed, but looks
> like there're still more review points from btrfs list. So will you send a new
> version to replace this patch?

IMHO the patch is fine to be merged for now.

The new comments are mostly a future work idea, which needs some time to
develop and extra kernel features.

Thanks,
Qu
>
> Thanks,
> Zorro
>
>>   tests/btrfs/294 | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/btrfs/294 b/tests/btrfs/294
>> index 61ce7d97..d7d13646 100755
>> --- a/tests/btrfs/294
>> +++ b/tests/btrfs/294
>> @@ -16,11 +16,15 @@ _begin_fstest auto raid volume
>>
>>   # Modify as appropriate.
>>   _supported_fs btrfs
>> +
>> +# No zoned support for RAID56 yet.
>> +_require_non_zoned_device "${SCRATCH_DEV}"
>> +
>>   _require_scratch_dev_pool 8
>>   _fixed_by_kernel_commit a7299a18a179 \
>>   	"btrfs: fix u32 overflows when left shifting @stripe_nr"
>> -_fixed_by_kernel_commit xxxxxxxxxxxx \
>> -	"btrfs: use a dedicated helper to convert stripe_nr to offset"
>> +_fixed_by_kernel_commit cb091225a538 \
>> +	"btrfs: fix remaining u32 overflows when left shifting stripe_nr"
>>
>>   _scratch_dev_pool_get 8
>>
>> --
>> 2.41.0
>>
>