[PATCH 0/2] btrfs-progs: fix the conflicting super flags

[PATCH 0/2] btrfs-progs: fix the conflicting super flags

[Qu Wenruo]

There is a github issue that a canceled csum conversion leaves a
CHANGING_FSID_V2 flag, which turns out to be conflicting super flags
(CHANGING_FSID_V2 and CHANGING_DATA_CSUM).

Fix the problem and output all supported flags for print-tree.

Qu Wenruo (2):
  btrfs-progs: fix the conflicting super flags
  btrfs-progs: print-tree: handle all supported flags

 kernel-shared/ctree.h           | 10 ----------
 kernel-shared/print-tree.c      | 35 ++++++++++++++++++---------------
 kernel-shared/uapi/btrfs_tree.h |  8 ++++++++
 3 files changed, 27 insertions(+), 26 deletions(-)

--
2.45.2

[PATCH 1/2] btrfs-progs: fix the conflicting super flags

[Qu Wenruo]

[BUG]
There is a bug report that a canceled csum conversion (still
experimental feature) resulted unexpected super flags:

csum_type		0 (crc32c)
csum_size		4
csum			0x14973811 [match]
bytenr			65536
flags			0x1000000001
			( WRITTEN |
			  CHANGING_FSID_V2 )
magic			_BHRfS_M [match]

Meanwhile for a fs under csum conversion it should have either
CHANGING_DATA_CSUM or CHANGING_META_CSUM.

[CAUSE]
It turns out that, due to btrfs-progs keeps its own extra flags inside
its own ctree.h headers, not the shared uapi headers, we have
conflicting super flags:

kernel-shared/uapi/btrfs_tree.h:#define BTRFS_SUPER_FLAG_METADUMP_V2	(1ULL << 34)
kernel-shared/uapi/btrfs_tree.h:#define BTRFS_SUPER_FLAG_CHANGING_FSID	(1ULL << 35)
kernel-shared/uapi/btrfs_tree.h:#define BTRFS_SUPER_FLAG_CHANGING_FSID_V2 (1ULL << 36)
kernel-shared/ctree.h:#define BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM	(1ULL << 36)
kernel-shared/ctree.h:#define BTRFS_SUPER_FLAG_CHANGING_META_CSUM	(1ULL << 37)

Note that CHANGING_FSID_V2 is conflicting with CHANGING_DATA_CSUM.

[FIX]
Cross port the proper updated uapi headers into btrfs-progs, and remove
the definition from ctree.h.

This would change the value for CHANGING_DATA_CSUM and
CHANGING_META_CSUM, but considering they are experimental features, and
kernel would reject them anyway, the damage is not that huge and we can
accept such change before exposing it to end users.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/ctree.h           | 10 ----------
 kernel-shared/uapi/btrfs_tree.h |  8 ++++++++
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index 444d206e0a2c..1341a418726b 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -59,16 +59,6 @@ static inline unsigned long btrfs_chunk_item_size(int num_stripes)
 		sizeof(struct btrfs_stripe) * (num_stripes - 1);
 }
 
-#define BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM	(1ULL << 36)
-#define BTRFS_SUPER_FLAG_CHANGING_META_CSUM	(1ULL << 37)
-
-/*
- * The fs is undergoing block group tree feature change.
- * If no BLOCK_GROUP_TREE compat ro flag, it's changing from regular
- * bg item in extent tree to new bg tree.
- */
-#define BTRFS_SUPER_FLAG_CHANGING_BG_TREE	(1ULL << 38)
-
 static inline u32 __BTRFS_LEAF_DATA_SIZE(u32 nodesize)
 {
 	return nodesize - sizeof(struct btrfs_header);
diff --git a/kernel-shared/uapi/btrfs_tree.h b/kernel-shared/uapi/btrfs_tree.h
index e2ac228bcccc..23e18e735959 100644
--- a/kernel-shared/uapi/btrfs_tree.h
+++ b/kernel-shared/uapi/btrfs_tree.h
@@ -758,6 +758,14 @@ struct btrfs_free_space_header {
 #define BTRFS_SUPER_FLAG_CHANGING_FSID	(1ULL << 35)
 #define BTRFS_SUPER_FLAG_CHANGING_FSID_V2 (1ULL << 36)
 
+/*
+ * Those are temporaray flags utilized by btrfs-progs to do offline conversion.
+ * They are rejected by kernel.
+ * But still keep them all here to avoid conflicts.
+ */
+#define BTRFS_SUPER_FLAG_CHANGING_BG_TREE	(1ULL << 38)
+#define BTRFS_SUPER_FLAG_CHANGING_DATA_CSUM	(1ULL << 39)
+#define BTRFS_SUPER_FLAG_CHANGING_META_CSUM	(1ULL << 40)
 
 /*
  * items in the extent btree are used to record the objectid of the
-- 
2.45.2

[PATCH 2/2] btrfs-progs: print-tree: handle all supported flags

[Qu Wenruo]

Although we already have a pretty good array defined for all
super/compat_ro/incompat flags, we still rely on hand defined mask to do
the print.

This can lead to easy de-sync between the definition and the flags.

Change it to automatically iterate through the array to calculate the
flags, and add the remaining super flags.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/print-tree.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
index a7018cb724fd..23cd225a9a50 100644
--- a/kernel-shared/print-tree.c
+++ b/kernel-shared/print-tree.c
@@ -1950,18 +1950,13 @@ static struct readable_flag_entry super_flags_array[] = {
 	DEF_SUPER_FLAG_ENTRY(CHANGING_FSID_V2),
 	DEF_SUPER_FLAG_ENTRY(SEEDING),
 	DEF_SUPER_FLAG_ENTRY(METADUMP),
-	DEF_SUPER_FLAG_ENTRY(METADUMP_V2)
+	DEF_SUPER_FLAG_ENTRY(METADUMP_V2),
+	DEF_SUPER_FLAG_ENTRY(CHANGING_BG_TREE),
+	DEF_SUPER_FLAG_ENTRY(CHANGING_DATA_CSUM),
+	DEF_SUPER_FLAG_ENTRY(CHANGING_META_CSUM),
 };
 static const int super_flags_num = ARRAY_SIZE(super_flags_array);
 
-#define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN |\
-				 BTRFS_HEADER_FLAG_RELOC |\
-				 BTRFS_SUPER_FLAG_CHANGING_FSID |\
-				 BTRFS_SUPER_FLAG_CHANGING_FSID_V2 |\
-				 BTRFS_SUPER_FLAG_SEEDING |\
-				 BTRFS_SUPER_FLAG_METADUMP |\
-				 BTRFS_SUPER_FLAG_METADUMP_V2)
-
 static void __print_readable_flag(u64 flag, struct readable_flag_entry *array,
 				  int array_size, u64 supported_flags)
 {
@@ -1995,26 +1990,34 @@ static void __print_readable_flag(u64 flag, struct readable_flag_entry *array,
 
 static void print_readable_compat_ro_flag(u64 flag)
 {
-	/*
-	 * We know about the FREE_SPACE_TREE{,_VALID} bits, but we don't
-	 * actually support them yet.
-	 */
+	u64 print_flags = 0;
+
+	for (int i = 0; i < compat_ro_flags_num; i++)
+		print_flags |= compat_ro_flags_array[i].bit;
 	return __print_readable_flag(flag, compat_ro_flags_array,
 				     compat_ro_flags_num,
-				     BTRFS_FEATURE_COMPAT_RO_SUPP);
+				     print_flags);
 }
 
 static void print_readable_incompat_flag(u64 flag)
 {
+	u64 print_flags = 0;
+
+	for (int i = 0; i < incompat_flags_num; i++)
+		print_flags |= incompat_flags_array[i].bit;
 	return __print_readable_flag(flag, incompat_flags_array,
 				     incompat_flags_num,
-				     BTRFS_FEATURE_INCOMPAT_SUPP);
+				     print_flags);
 }
 
 static void print_readable_super_flag(u64 flag)
 {
+	int print_flags = 0;
+
+	for (int i = 0; i < super_flags_num; i++)
+		print_flags |= super_flags_array[i].bit;
 	return __print_readable_flag(flag, super_flags_array,
-				     super_flags_num, BTRFS_SUPER_FLAG_SUPP);
+				     super_flags_num, print_flags);
 }
 
 static void print_sys_chunk_array(struct btrfs_super_block *sb)
-- 
2.45.2

Re: [PATCH 2/2] btrfs-progs: print-tree: handle all supported flags

[Qu Wenruo]

在 2024/6/8 13:43, Qu Wenruo 写道:
> Although we already have a pretty good array defined for all
> super/compat_ro/incompat flags, we still rely on hand defined mask to do
> the print.
> 
> This can lead to easy de-sync between the definition and the flags.
> 
> Change it to automatically iterate through the array to calculate the
> flags, and add the remaining super flags.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   kernel-shared/print-tree.c | 35 +++++++++++++++++++----------------
>   1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
> index a7018cb724fd..23cd225a9a50 100644
> --- a/kernel-shared/print-tree.c
> +++ b/kernel-shared/print-tree.c
> @@ -1950,18 +1950,13 @@ static struct readable_flag_entry super_flags_array[] = {
>   	DEF_SUPER_FLAG_ENTRY(CHANGING_FSID_V2),
>   	DEF_SUPER_FLAG_ENTRY(SEEDING),
>   	DEF_SUPER_FLAG_ENTRY(METADUMP),
> -	DEF_SUPER_FLAG_ENTRY(METADUMP_V2)
> +	DEF_SUPER_FLAG_ENTRY(METADUMP_V2),
> +	DEF_SUPER_FLAG_ENTRY(CHANGING_BG_TREE),
> +	DEF_SUPER_FLAG_ENTRY(CHANGING_DATA_CSUM),
> +	DEF_SUPER_FLAG_ENTRY(CHANGING_META_CSUM),
>   };
>   static const int super_flags_num = ARRAY_SIZE(super_flags_array);
>   
> -#define BTRFS_SUPER_FLAG_SUPP	(BTRFS_HEADER_FLAG_WRITTEN |\
> -				 BTRFS_HEADER_FLAG_RELOC |\
> -				 BTRFS_SUPER_FLAG_CHANGING_FSID |\
> -				 BTRFS_SUPER_FLAG_CHANGING_FSID_V2 |\
> -				 BTRFS_SUPER_FLAG_SEEDING |\
> -				 BTRFS_SUPER_FLAG_METADUMP |\
> -				 BTRFS_SUPER_FLAG_METADUMP_V2)
> -
>   static void __print_readable_flag(u64 flag, struct readable_flag_entry *array,
>   				  int array_size, u64 supported_flags)
>   {
> @@ -1995,26 +1990,34 @@ static void __print_readable_flag(u64 flag, struct readable_flag_entry *array,
>   
>   static void print_readable_compat_ro_flag(u64 flag)
>   {
> -	/*
> -	 * We know about the FREE_SPACE_TREE{,_VALID} bits, but we don't
> -	 * actually support them yet.
> -	 */
> +	u64 print_flags = 0;
> +
> +	for (int i = 0; i < compat_ro_flags_num; i++)
> +		print_flags |= compat_ro_flags_array[i].bit;
>   	return __print_readable_flag(flag, compat_ro_flags_array,
>   				     compat_ro_flags_num,
> -				     BTRFS_FEATURE_COMPAT_RO_SUPP);
> +				     print_flags);
>   }
>   
>   static void print_readable_incompat_flag(u64 flag)
>   {
> +	u64 print_flags = 0;
> +
> +	for (int i = 0; i < incompat_flags_num; i++)
> +		print_flags |= incompat_flags_array[i].bit;
>   	return __print_readable_flag(flag, incompat_flags_array,
>   				     incompat_flags_num,
> -				     BTRFS_FEATURE_INCOMPAT_SUPP);
> +				     print_flags);
>   }
>   
>   static void print_readable_super_flag(u64 flag)
>   {
> +	int print_flags = 0;

This line should be using u64, or it can not handle flags beyond u32.

Fixed in github.

Thanks,
Qu
> +
> +	for (int i = 0; i < super_flags_num; i++)
> +		print_flags |= super_flags_array[i].bit;
>   	return __print_readable_flag(flag, super_flags_array,
> -				     super_flags_num, BTRFS_SUPER_FLAG_SUPP);
> +				     super_flags_num, print_flags);
>   }
>   
>   static void print_sys_chunk_array(struct btrfs_super_block *sb)