diff mbox series

[RESEND] Revert "btrfs-progs: convert: add raid-stripe-tree to allowed features"

Message ID 62b84fee8124d455689f8c2ab151eafb136a04d9.1718008470.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series [RESEND] Revert "btrfs-progs: convert: add raid-stripe-tree to allowed features" | expand

Commit Message

Qu Wenruo June 10, 2024, 8:35 a.m. UTC 
This reverts commit 346a3819237b319985ad6042d6302f67b040a803.

The RST feature (at least for now) is mostly for zoned devices to
support extra data profiles.

Thus btrfs-convert is never designed to handle RST, because there is no
way an ext4/reiserfs can even be created on a zoned device, or it would
create the correct RST tree root at all

Revert this unsupported feature to prevent false alerts.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
This was included in the btrfs-convert unwritten extent patchset/pull
request but is not merged.
---
 common/fsfeatures.h                           |  3 +-
 tests/common.convert                          | 44 ++++---------------
 tests/convert-tests/001-ext2-basic/test.sh    |  2 +-
 tests/convert-tests/003-ext4-basic/test.sh    |  2 +-
 .../005-delete-all-rollback/test.sh           |  6 +--
 .../convert-tests/010-reiserfs-basic/test.sh  |  2 +-
 .../011-reiserfs-delete-all-rollback/test.sh  |  6 +--
 tests/convert-tests/024-ntfs-basic/test.sh    |  2 +-
 8 files changed, 15 insertions(+), 52 deletions(-)

--
2.45.2

Comments

David Sterba June 12, 2024, 7:48 p.m. UTC | #1 
On Mon, Jun 10, 2024 at 06:05:10PM +0930, Qu Wenruo wrote:
> This reverts commit 346a3819237b319985ad6042d6302f67b040a803.
> 
> The RST feature (at least for now) is mostly for zoned devices to
> support extra data profiles.
> 
> Thus btrfs-convert is never designed to handle RST, because there is no
> way an ext4/reiserfs can even be created on a zoned device, or it would
> create the correct RST tree root at all
> 
> Revert this unsupported feature to prevent false alerts.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> This was included in the btrfs-convert unwritten extent patchset/pull
> request but is not merged.

For the record, RST first use is for zoned device but will allso fix the
raid56 problems once implemented. It can be used independently, I don't
see a reason to remove it's support from convert. Any related test
failures will be fixed separately.
Qu Wenruo June 12, 2024, 9:11 p.m. UTC | #2 
在 2024/6/13 05:18, David Sterba 写道:
> On Mon, Jun 10, 2024 at 06:05:10PM +0930, Qu Wenruo wrote:
>> This reverts commit 346a3819237b319985ad6042d6302f67b040a803.
>>
>> The RST feature (at least for now) is mostly for zoned devices to
>> support extra data profiles.
>>
>> Thus btrfs-convert is never designed to handle RST, because there is no
>> way an ext4/reiserfs can even be created on a zoned device, or it would
>> create the correct RST tree root at all
>>
>> Revert this unsupported feature to prevent false alerts.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> This was included in the btrfs-convert unwritten extent patchset/pull
>> request but is not merged.
> 
> For the record, RST first use is for zoned device but will allso fix the
> raid56 problems once implemented.

*ONCE* implemented, while it's still hiding behind CONFIG_BTRFS_DEBUG.

> It can be used independently, I don't
> see a reason to remove it's support from convert. Any related test
> failures will be fixed separately.

Nope, I have already argued it again and again, only DEBUG kernel can 
even recognize the RST feature, while you still insists to enable it for 
regular mkfs.

I can not understand your eager to push such an experimental features to 
btrfs-progs meanwhile we have so many things to fix for RST.

Let me to be honest on this, I do not understand why you refuse to even 
move RST to experimental features, when no distro enables 
CONFIG_BTRFS_DEBUG by default.
Thus no 6.9 kernel can even mount a RST btrfs.

I think you're doing an immature decision on RST.

Thanks,
Qu
diff mbox series

Patch

diff --git a/common/fsfeatures.h b/common/fsfeatures.h
index 5b241bdf9122..f636171f6bd9 100644
--- a/common/fsfeatures.h
+++ b/common/fsfeatures.h
@@ -68,8 +68,7 @@  static const struct btrfs_mkfs_features btrfs_convert_allowed_features = {
 			   BTRFS_FEATURE_INCOMPAT_BIG_METADATA |
 			   BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF |
 			   BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA |
-			   BTRFS_FEATURE_INCOMPAT_NO_HOLES |
-			   BTRFS_FEATURE_INCOMPAT_RAID_STRIPE_TREE,
+			   BTRFS_FEATURE_INCOMPAT_NO_HOLES,
 	.runtime_flags   = BTRFS_FEATURE_RUNTIME_QUOTA,
 };

diff --git a/tests/common.convert b/tests/common.convert
index 06eec9b2a853..50d49a0d32da 100644
--- a/tests/common.convert
+++ b/tests/common.convert
@@ -214,47 +214,19 @@  convert_test_post_check_checksums() {
 		grep -q 'FAILED' && _fail "file validation failed"
 }

-# Check if convert can mount the image based on features
-# $1: name of the feature
-convert_can_mount() {
-	local features="$1"
-
-	if [ "$features" = 'block-group-tree' ]; then
-		if ! [ -f "/sys/fs/btrfs/features/block_group_tree" ]; then
-			_log "Skip mount checks due to missing support of block-group-tree"
-			return 1
-		fi
-	fi
-	if [ "$features" = 'raid-stripe-tree' ]; then
-		if ! [ -f "/sys/fs/btrfs/features/raid_stripe_tree" ]; then
-			_log "Skip mount checks due to missing support of raid-stripe-tree"
-			return 1
-		fi
-	fi
-
-	return 0
-}
-
 # post conversion checks, all three in one call, on an unmounted image
-# $1: features for mount compatibility checks
-# $2: file with checksums
-# $3: file with permissions.
-# $4: file with acl entries.
+# $1: file with checksums
+# $2: file with permissions.
+# $3: file with acl entries.
 convert_test_post_checks_all() {
-	local features="$1"
-
+	_assert_path "$1"
 	_assert_path "$2"
 	_assert_path "$3"
-	_assert_path "$4"
-
-	if ! convert_can_mount "$features"; then
-		return 0
-	fi

 	run_check_mount_test_dev
-	convert_test_post_check_checksums "$2"
-	convert_test_post_check_permissions "$3"
-	convert_test_post_check_acl "$4"
+	convert_test_post_check_checksums "$1"
+	convert_test_post_check_permissions "$2"
+	convert_test_post_check_acl "$3"

 	# Create a large file to trigger data chunk allocation
 	generate_dataset "large"
@@ -313,7 +285,7 @@  convert_test() {
 	run_check_umount_test_dev

 	convert_test_do_convert "$features" "$nodesize" "$fstype"
-	convert_test_post_checks_all "$features" "$CHECKSUMTMP" "$EXT_PERMTMP" "$EXT_ACLTMP"
+	convert_test_post_checks_all "$CHECKSUMTMP" "$EXT_PERMTMP" "$EXT_ACLTMP"
 	rm -- "$CHECKSUMTMP"
 	rm -- "$EXT_PERMTMP"
 	rm -- "$EXT_ACLTMP"
diff --git a/tests/convert-tests/001-ext2-basic/test.sh b/tests/convert-tests/001-ext2-basic/test.sh
index 461ff4a5f1a9..6dc105b55e27 100755
--- a/tests/convert-tests/001-ext2-basic/test.sh
+++ b/tests/convert-tests/001-ext2-basic/test.sh
@@ -12,7 +12,7 @@  check_kernel_support_acl

 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices
-for feature in '' 'block-group-tree' 'raid-stripe-tree'; do
+for feature in '' 'block-group-tree' ; do
 	convert_test ext2 "$feature" "ext2 4k nodesize" 4096 mke2fs -b 4096
 	convert_test ext2 "$feature" "ext2 16k nodesize" 16384 mke2fs -b 4096
 	convert_test ext2 "$feature" "ext2 64k nodesize" 65536 mke2fs -b 4096
diff --git a/tests/convert-tests/003-ext4-basic/test.sh b/tests/convert-tests/003-ext4-basic/test.sh
index 14637fc852db..8ae5264cb340 100755
--- a/tests/convert-tests/003-ext4-basic/test.sh
+++ b/tests/convert-tests/003-ext4-basic/test.sh
@@ -12,7 +12,7 @@  check_kernel_support_acl

 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices
-for feature in '' 'block-group-tree' 'raid-stripe-tree' ; do
+for feature in '' 'block-group-tree' ; do
 	convert_test ext4 "$feature" "ext4 4k nodesize" 4096 mke2fs -t ext4 -b 4096
 	convert_test ext4 "$feature" "ext4 16k nodesize" 16384 mke2fs -t ext4 -b 4096
 	convert_test ext4 "$feature" "ext4 64k nodesize" 65536 mke2fs -t ext4 -b 4096
diff --git a/tests/convert-tests/005-delete-all-rollback/test.sh b/tests/convert-tests/005-delete-all-rollback/test.sh
index 5603d3078bc8..fa56ca292bfc 100755
--- a/tests/convert-tests/005-delete-all-rollback/test.sh
+++ b/tests/convert-tests/005-delete-all-rollback/test.sh
@@ -38,10 +38,6 @@  do_test() {

 	convert_test_do_convert "$features" "$nodesize"

-	if ! convert_can_mount "$features"; then
-		return 0
-	fi
-
 	run_check_mount_test_dev
 	convert_test_post_check_checksums "$CHECKSUMTMP"

@@ -68,7 +64,7 @@  do_test() {

 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices
-for feature in '' 'block-group-tree' 'raid-stripe-tree' ; do
+for feature in '' 'block-group-tree' ; do
 	do_test "$feature" "ext4 4k nodesize" 4096 mke2fs -t ext4 -b 4096
 	do_test "$feature" "ext4 16k nodesize" 16384 mke2fs -t ext4 -b 4096
 	do_test "$feature" "ext4 64k nodesize" 65536 mke2fs -t ext4 -b 4096
diff --git a/tests/convert-tests/010-reiserfs-basic/test.sh b/tests/convert-tests/010-reiserfs-basic/test.sh
index 6ab02b31d176..86b1826783b3 100755
--- a/tests/convert-tests/010-reiserfs-basic/test.sh
+++ b/tests/convert-tests/010-reiserfs-basic/test.sh
@@ -15,7 +15,7 @@  prepare_test_dev

 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices
-for feature in '' 'block-group-tree' 'raid-stripe-tree'; do
+for feature in '' 'block-group-tree' ; do
 	convert_test reiserfs "$feature" "reiserfs 4k nodesize" 4096 mkreiserfs -b 4096
 	convert_test reiserfs "$feature" "reiserfs 16k nodesize" 16384 mkreiserfs -b 4096
 	convert_test reiserfs "$feature" "reiserfs 64k nodesize" 65536 mkreiserfs -b 4096
diff --git a/tests/convert-tests/011-reiserfs-delete-all-rollback/test.sh b/tests/convert-tests/011-reiserfs-delete-all-rollback/test.sh
index 688613c2c6ad..e1c3e02eb7a0 100755
--- a/tests/convert-tests/011-reiserfs-delete-all-rollback/test.sh
+++ b/tests/convert-tests/011-reiserfs-delete-all-rollback/test.sh
@@ -40,10 +40,6 @@  do_test() {

 	convert_test_do_convert "$features" "$nodesize"

-	if ! convert_can_mount "$features"; then
-		return 0
-	fi
-
 	run_check_mount_test_dev
 	convert_test_post_check_checksums "$CHECKSUMTMP"

@@ -70,7 +66,7 @@  do_test() {

 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices
-for feature in '' 'block-group-tree' 'raid-stripe-tree'; do
+for feature in '' 'block-group-tree' ; do
 	do_test "$feature" "reiserfs 4k nodesize" 4096 mkreiserfs -b 4096
 	do_test "$feature" "reiserfs 16k nodesize" 16384 mkreiserfs -b 4096
 	do_test "$feature" "reiserfs 64k nodesize" 65536 mkreiserfs -b 4096
diff --git a/tests/convert-tests/024-ntfs-basic/test.sh b/tests/convert-tests/024-ntfs-basic/test.sh
index a93f60070674..d2e1be79ef6b 100755
--- a/tests/convert-tests/024-ntfs-basic/test.sh
+++ b/tests/convert-tests/024-ntfs-basic/test.sh
@@ -17,6 +17,6 @@  prepare_test_dev

 # Iterate over defaults and options that are not tied to hardware capabilities
 # or number of devices. Test only 4K block size as minimum.
-for feature in '' 'block-group-tree' 'raid-stripe-tree'; do
+for feature in '' 'block-group-tree' ; do
 	convert_test ntfs "$feature" "ntfs 4k nodesize" 4096 mkfs.ntfs -s 4096
 done