public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works
@ 2025-05-24  2:08 Qu Wenruo
  2025-05-24  2:08 ` [PATCH 1/9] btrfs-progs: convert: add feature dependency checks for bgt Qu Wenruo
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

There is a long bug that, "btrfs-convert -O bgt" doesn't create a fs
with bgt feature at all.

In fact "mkfs.btrfs -O bgt" is no different than "mkfs.btrfs -O ^bgt".

The root cause is explained and fixed in the second last patch.

The first 7 patches are mostly preparation and cleanup for properly
intorduce block group tree at temprory fs creation time.

Qu Wenruo (9):
  btrfs-progs: convert: add feature dependency checks for bgt
  btrfs-progs: convert: replace the bytenr check with a UASSERT()
  btrfs-progs: convert: simplify insert_temp_root_item()
  btrfs-progs: convert: simplify insert_temp_dev_item() and
    insert_temp_chunk_item()
  btrfs-progs: convert: simplify insert_temp_dev_extent()
  btrfs-progs: convert: simplify insert_temp_extent_item() and
    insert_temp_block_group()
  btrfs-progs: convert: merge setup_temp_fs_tree() and
    setup_temp_csum_tree()
  btrfs-progs: convert: implement the block group tree support properly
  btrfs-progs: convert-tests: add a test case for bgt feature

 convert/common.c                              | 337 +++++++++---------
 convert/main.c                                |   6 +
 .../028-block-group-tree/test.sh              |  20 ++
 3 files changed, 201 insertions(+), 162 deletions(-)
 create mode 100755 tests/convert-tests/028-block-group-tree/test.sh

--
2.49.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] btrfs-progs: convert: add feature dependency checks for bgt
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 2/9] btrfs-progs: convert: replace the bytenr check with a UASSERT() Qu Wenruo
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

Block group tree requires no_holes and free-space-tree features, add
such check just like mkfs.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/convert/main.c b/convert/main.c
index 0dc75c9eb1c6..264ee1dc89e0 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1212,6 +1212,12 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 
 	if (btrfs_check_nodesize(nodesize, blocksize, features))
 		goto fail;
+	if (features->compat_ro_flags & BTRFS_FEATURE_COMPAT_RO_BLOCK_GROUP_TREE &&
+	    (!(features->incompat_flags & BTRFS_FEATURE_INCOMPAT_NO_HOLES) ||
+	     !(features->compat_ro_flags & BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE))) {
+		error("block group tree requires no-holes and free-space-tree features");
+		goto fail;
+	}
 	fd = open(devname, O_RDWR);
 	if (fd < 0) {
 		error("unable to open %s: %m", devname);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/9] btrfs-progs: convert: replace the bytenr check with a UASSERT()
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
  2025-05-24  2:08 ` [PATCH 1/9] btrfs-progs: convert: add feature dependency checks for bgt Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 3/9] btrfs-progs: convert: simplify insert_temp_root_item() Qu Wenruo
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

The bytenr sequence of all roots are controlled by our code, so if
something went wrong with the sequence, it's a bug.

A UASSERT() is more suitable for this case.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/common.c | 34 +++++-----------------------------
 1 file changed, 5 insertions(+), 29 deletions(-)

diff --git a/convert/common.c b/convert/common.c
index 2cbe45180ce9..0b567da2312e 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -260,19 +260,8 @@ static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
 	 * Provided bytenr must in ascending order, or tree root will have a
 	 * bad key order.
 	 */
-	if (!(root_bytenr < extent_bytenr && extent_bytenr < dev_bytenr &&
-	      dev_bytenr < fs_bytenr && fs_bytenr < csum_bytenr)) {
-		error("bad tree bytenr order: "
-				"root < extent %llu < %llu, "
-				"extent < dev %llu < %llu, "
-				"dev < fs %llu < %llu, "
-				"fs < csum %llu < %llu",
-				root_bytenr, extent_bytenr,
-				extent_bytenr, dev_bytenr,
-				dev_bytenr, fs_bytenr,
-				fs_bytenr, csum_bytenr);
-		return -EINVAL;
-	}
+	UASSERT(root_bytenr < extent_bytenr && extent_bytenr < dev_bytenr &&
+	        dev_bytenr < fs_bytenr && fs_bytenr < csum_bytenr);
 	buf = malloc(sizeof(*buf) + cfg->nodesize);
 	if (!buf)
 		return -ENOMEM;
@@ -703,22 +692,9 @@ static int setup_temp_extent_tree(int fd, struct btrfs_mkfs_config *cfg,
 	 * We must ensure provided bytenr are in ascending order,
 	 * or extent tree key order will be broken.
 	 */
-	if (!(chunk_bytenr < root_bytenr && root_bytenr < extent_bytenr &&
-	      extent_bytenr < dev_bytenr && dev_bytenr < fs_bytenr &&
-	      fs_bytenr < csum_bytenr)) {
-		error("bad tree bytenr order: "
-				"chunk < root %llu < %llu, "
-				"root < extent %llu < %llu, "
-				"extent < dev %llu < %llu, "
-				"dev < fs %llu < %llu, "
-				"fs < csum %llu < %llu",
-				chunk_bytenr, root_bytenr,
-				root_bytenr, extent_bytenr,
-				extent_bytenr, dev_bytenr,
-				dev_bytenr, fs_bytenr,
-				fs_bytenr, csum_bytenr);
-		return -EINVAL;
-	}
+	UASSERT(chunk_bytenr < root_bytenr && root_bytenr < extent_bytenr &&
+		extent_bytenr < dev_bytenr && dev_bytenr < fs_bytenr &&
+		fs_bytenr < csum_bytenr);
 	buf = malloc(sizeof(*buf) + cfg->nodesize);
 	if (!buf)
 		return -ENOMEM;
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/9] btrfs-progs: convert: simplify insert_temp_root_item()
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
  2025-05-24  2:08 ` [PATCH 1/9] btrfs-progs: convert: add feature dependency checks for bgt Qu Wenruo
  2025-05-24  2:08 ` [PATCH 2/9] btrfs-progs: convert: replace the bytenr check with a UASSERT() Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 4/9] btrfs-progs: convert: simplify insert_temp_dev_item() and insert_temp_chunk_item() Qu Wenruo
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

The function requires parameters @slot and @itemoff to record where the
next item should land.

But this is overkilled, as after inserting an item, the temporary extent
buffer will have its header nritems and the item pointer updated.

We can use that header nritems and item pointer to get where the next
item should land.

This removes the external counter to record @slot and @itemoff.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/common.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/convert/common.c b/convert/common.c
index 0b567da2312e..df68b696f8f5 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -189,17 +189,29 @@ static int setup_temp_extent_buffer(struct extent_buffer *buf,
 	return 0;
 }
 
+static u32 get_item_offset(const struct extent_buffer *eb,
+			   const struct btrfs_mkfs_config *cfg)
+{
+	u32 slot = btrfs_header_nritems(eb);
+
+	if (slot)
+		return btrfs_item_offset(eb, slot - 1);
+	else
+		return cfg->leaf_data_size;
+}
+
 static void insert_temp_root_item(struct extent_buffer *buf,
 				  struct btrfs_mkfs_config *cfg,
-				  int *slot, u32 *itemoff, u64 objectid,
-				  u64 bytenr)
+				  u64 objectid, u64 bytenr)
 {
 	struct btrfs_root_item root_item;
 	struct btrfs_inode_item *inode_item;
 	struct btrfs_disk_key disk_key;
+	u32 slot = btrfs_header_nritems(buf);
+	u32 itemoff = get_item_offset(buf, cfg);
 
-	btrfs_set_header_nritems(buf, *slot + 1);
-	(*itemoff) -= sizeof(root_item);
+	btrfs_set_header_nritems(buf, slot + 1);
+	itemoff -= sizeof(root_item);
 	memset(&root_item, 0, sizeof(root_item));
 	inode_item = &root_item.inode;
 	btrfs_set_stack_inode_generation(inode_item, 1);
@@ -217,13 +229,12 @@ static void insert_temp_root_item(struct extent_buffer *buf,
 	btrfs_set_disk_key_objectid(&disk_key, objectid);
 	btrfs_set_disk_key_offset(&disk_key, 0);
 
-	btrfs_set_item_key(buf, &disk_key, *slot);
-	btrfs_set_item_offset(buf, *slot, *itemoff);
-	btrfs_set_item_size(buf, *slot, sizeof(root_item));
+	btrfs_set_item_key(buf, &disk_key, slot);
+	btrfs_set_item_offset(buf, slot, itemoff);
+	btrfs_set_item_size(buf, slot, sizeof(root_item));
 	write_extent_buffer(buf, &root_item,
-			    btrfs_item_ptr_offset(buf, *slot),
+			    btrfs_item_ptr_offset(buf, slot),
 			    sizeof(root_item));
-	(*slot)++;
 }
 
 /*
@@ -252,8 +263,6 @@ static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
 				u64 dev_bytenr, u64 fs_bytenr, u64 csum_bytenr)
 {
 	struct extent_buffer *buf = NULL;
-	u32 itemoff = cfg->leaf_data_size;
-	int slot = 0;
 	int ret;
 
 	/*
@@ -271,14 +280,10 @@ static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
 	if (ret < 0)
 		goto out;
 
-	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_EXTENT_TREE_OBJECTID, extent_bytenr);
-	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_DEV_TREE_OBJECTID, dev_bytenr);
-	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_FS_TREE_OBJECTID, fs_bytenr);
-	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_CSUM_TREE_OBJECTID, csum_bytenr);
+	insert_temp_root_item(buf, cfg, BTRFS_EXTENT_TREE_OBJECTID, extent_bytenr);
+	insert_temp_root_item(buf, cfg, BTRFS_DEV_TREE_OBJECTID, dev_bytenr);
+	insert_temp_root_item(buf, cfg, BTRFS_FS_TREE_OBJECTID, fs_bytenr);
+	insert_temp_root_item(buf, cfg, BTRFS_CSUM_TREE_OBJECTID, csum_bytenr);
 
 	ret = write_temp_extent_buffer(fd, buf, root_bytenr, cfg);
 out:
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/9] btrfs-progs: convert: simplify insert_temp_dev_item() and insert_temp_chunk_item()
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
                   ` (2 preceding siblings ...)
  2025-05-24  2:08 ` [PATCH 3/9] btrfs-progs: convert: simplify insert_temp_root_item() Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 5/9] btrfs-progs: convert: simplify insert_temp_dev_extent() Qu Wenruo
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

These functions require parameters @slot and @itemoff to record where the
next item should land.

But this is overkilled, as after inserting an item, the temporary extent
buffer will have its header nritems and the item pointer updated.

We can use that header nritems and item pointer to get where the next
item should land.

This removes the external counter to record @slot and @itemoff.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/common.c | 46 +++++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/convert/common.c b/convert/common.c
index df68b696f8f5..b21ad4c8fa7b 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -292,14 +292,15 @@ out:
 }
 
 static int insert_temp_dev_item(int fd, struct extent_buffer *buf,
-				struct btrfs_mkfs_config *cfg,
-				int *slot, u32 *itemoff)
+				struct btrfs_mkfs_config *cfg)
 {
 	struct btrfs_disk_key disk_key;
 	struct btrfs_dev_item *dev_item;
 	unsigned char dev_uuid[BTRFS_UUID_SIZE];
 	unsigned char fsid[BTRFS_FSID_SIZE];
 	struct btrfs_super_block super;
+	u32 slot = btrfs_header_nritems(buf);
+	u32 itemoff = get_item_offset(buf, cfg);
 	int ret;
 
 	ret = pread(fd, &super, BTRFS_SUPER_INFO_SIZE, cfg->super_bytenr);
@@ -308,17 +309,17 @@ static int insert_temp_dev_item(int fd, struct extent_buffer *buf,
 		goto out;
 	}
 
-	btrfs_set_header_nritems(buf, *slot + 1);
-	(*itemoff) -= sizeof(*dev_item);
+	btrfs_set_header_nritems(buf, slot + 1);
+	itemoff -= sizeof(*dev_item);
 	/* setup device item 1, 0 is for replace case */
 	btrfs_set_disk_key_type(&disk_key, BTRFS_DEV_ITEM_KEY);
 	btrfs_set_disk_key_objectid(&disk_key, BTRFS_DEV_ITEMS_OBJECTID);
 	btrfs_set_disk_key_offset(&disk_key, 1);
-	btrfs_set_item_key(buf, &disk_key, *slot);
-	btrfs_set_item_offset(buf, *slot, *itemoff);
-	btrfs_set_item_size(buf, *slot, sizeof(*dev_item));
+	btrfs_set_item_key(buf, &disk_key, slot);
+	btrfs_set_item_offset(buf, slot, itemoff);
+	btrfs_set_item_size(buf, slot, sizeof(*dev_item));
 
-	dev_item = btrfs_item_ptr(buf, *slot, struct btrfs_dev_item);
+	dev_item = btrfs_item_ptr(buf, slot, struct btrfs_dev_item);
 	/* Generate device uuid */
 	uuid_generate(dev_uuid);
 	write_extent_buffer(buf, dev_uuid,
@@ -346,19 +347,19 @@ static int insert_temp_dev_item(int fd, struct extent_buffer *buf,
 	read_extent_buffer(buf, &super.dev_item, (unsigned long)dev_item,
 			   sizeof(*dev_item));
 	ret = write_temp_super(fd, &super, cfg->super_bytenr);
-	(*slot)++;
 out:
 	return ret;
 }
 
 static int insert_temp_chunk_item(int fd, struct extent_buffer *buf,
 				  struct btrfs_mkfs_config *cfg,
-				  int *slot, u32 *itemoff, u64 start, u64 len,
-				  u64 type)
+				  u64 start, u64 len, u64 type)
 {
 	struct btrfs_chunk *chunk;
 	struct btrfs_disk_key disk_key;
 	struct btrfs_super_block sb;
+	u32 slot = btrfs_header_nritems(buf);
+	u32 itemoff = get_item_offset(buf, cfg);
 	int ret = 0;
 
 	ret = pread(fd, &sb, BTRFS_SUPER_INFO_SIZE, cfg->super_bytenr);
@@ -367,16 +368,16 @@ static int insert_temp_chunk_item(int fd, struct extent_buffer *buf,
 		return ret;
 	}
 
-	btrfs_set_header_nritems(buf, *slot + 1);
-	(*itemoff) -= btrfs_chunk_item_size(1);
+	btrfs_set_header_nritems(buf, slot + 1);
+	itemoff -= btrfs_chunk_item_size(1);
 	btrfs_set_disk_key_type(&disk_key, BTRFS_CHUNK_ITEM_KEY);
 	btrfs_set_disk_key_objectid(&disk_key, BTRFS_FIRST_CHUNK_TREE_OBJECTID);
 	btrfs_set_disk_key_offset(&disk_key, start);
-	btrfs_set_item_key(buf, &disk_key, *slot);
-	btrfs_set_item_offset(buf, *slot, *itemoff);
-	btrfs_set_item_size(buf, *slot, btrfs_chunk_item_size(1));
+	btrfs_set_item_key(buf, &disk_key, slot);
+	btrfs_set_item_offset(buf, slot, itemoff);
+	btrfs_set_item_size(buf, slot, btrfs_chunk_item_size(1));
 
-	chunk = btrfs_item_ptr(buf, *slot, struct btrfs_chunk);
+	chunk = btrfs_item_ptr(buf, slot, struct btrfs_chunk);
 	btrfs_set_chunk_length(buf, chunk, len);
 	btrfs_set_chunk_owner(buf, chunk, BTRFS_EXTENT_TREE_OBJECTID);
 	btrfs_set_chunk_stripe_len(buf, chunk, BTRFS_STRIPE_LEN);
@@ -392,7 +393,6 @@ static int insert_temp_chunk_item(int fd, struct extent_buffer *buf,
 	write_extent_buffer(buf, sb.dev_item.uuid,
 			    (unsigned long)btrfs_stripe_dev_uuid_nr(chunk, 0),
 			    BTRFS_UUID_SIZE);
-	(*slot)++;
 
 	/*
 	 * If it's system chunk, also copy it to super block.
@@ -422,8 +422,6 @@ static int setup_temp_chunk_tree(int fd, struct btrfs_mkfs_config *cfg,
 				 u64 chunk_bytenr)
 {
 	struct extent_buffer *buf = NULL;
-	u32 itemoff = cfg->leaf_data_size;
-	int slot = 0;
 	int ret;
 
 	/* Must ensure SYS chunk starts before META chunk */
@@ -440,17 +438,15 @@ static int setup_temp_chunk_tree(int fd, struct btrfs_mkfs_config *cfg,
 	if (ret < 0)
 		goto out;
 
-	ret = insert_temp_dev_item(fd, buf, cfg, &slot, &itemoff);
+	ret = insert_temp_dev_item(fd, buf, cfg);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_chunk_item(fd, buf, cfg, &slot, &itemoff,
-				     sys_chunk_start,
+	ret = insert_temp_chunk_item(fd, buf, cfg, sys_chunk_start,
 				     BTRFS_MKFS_SYSTEM_GROUP_SIZE,
 				     BTRFS_BLOCK_GROUP_SYSTEM);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_chunk_item(fd, buf, cfg, &slot, &itemoff,
-				     meta_chunk_start,
+	ret = insert_temp_chunk_item(fd, buf, cfg, meta_chunk_start,
 				     BTRFS_CONVERT_META_GROUP_SIZE,
 				     BTRFS_BLOCK_GROUP_METADATA);
 	if (ret < 0)
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/9] btrfs-progs: convert: simplify insert_temp_dev_extent()
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
                   ` (3 preceding siblings ...)
  2025-05-24  2:08 ` [PATCH 4/9] btrfs-progs: convert: simplify insert_temp_dev_item() and insert_temp_chunk_item() Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 6/9] btrfs-progs: convert: simplify insert_temp_extent_item() and insert_temp_block_group() Qu Wenruo
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

This function requires parameters @slot and @itemoff to record where the
next item should land.

But this is overkilled, as after inserting an item, the temporary extent
buffer will have its header nritems and the item pointer updated.

We can use that header nritems and item pointer to get where the next
item should land.

This removes the external counter to record @slot and @itemoff.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/common.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/convert/common.c b/convert/common.c
index b21ad4c8fa7b..a9d3b395b37b 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -459,28 +459,29 @@ out:
 }
 
 static void insert_temp_dev_extent(struct extent_buffer *buf,
-				   int *slot, u32 *itemoff, u64 start, u64 len)
+				   struct btrfs_mkfs_config *cfg, u64 start, u64 len)
 {
 	struct btrfs_dev_extent *dev_extent;
 	struct btrfs_disk_key disk_key;
+	u32 slot = btrfs_header_nritems(buf);
+	u32 itemoff = get_item_offset(buf, cfg);
 
-	btrfs_set_header_nritems(buf, *slot + 1);
-	(*itemoff) -= sizeof(*dev_extent);
+	btrfs_set_header_nritems(buf, slot + 1);
+	itemoff -= sizeof(*dev_extent);
 	btrfs_set_disk_key_type(&disk_key, BTRFS_DEV_EXTENT_KEY);
 	btrfs_set_disk_key_objectid(&disk_key, 1);
 	btrfs_set_disk_key_offset(&disk_key, start);
-	btrfs_set_item_key(buf, &disk_key, *slot);
-	btrfs_set_item_offset(buf, *slot, *itemoff);
-	btrfs_set_item_size(buf, *slot, sizeof(*dev_extent));
+	btrfs_set_item_key(buf, &disk_key, slot);
+	btrfs_set_item_offset(buf, slot, itemoff);
+	btrfs_set_item_size(buf, slot, sizeof(*dev_extent));
 
-	dev_extent = btrfs_item_ptr(buf, *slot, struct btrfs_dev_extent);
+	dev_extent = btrfs_item_ptr(buf, slot, struct btrfs_dev_extent);
 	btrfs_set_dev_extent_chunk_objectid(buf, dev_extent,
 					    BTRFS_FIRST_CHUNK_TREE_OBJECTID);
 	btrfs_set_dev_extent_length(buf, dev_extent, len);
 	btrfs_set_dev_extent_chunk_offset(buf, dev_extent, start);
 	btrfs_set_dev_extent_chunk_tree(buf, dev_extent,
 					BTRFS_CHUNK_TREE_OBJECTID);
-	(*slot)++;
 }
 
 static int setup_temp_dev_tree(int fd, struct btrfs_mkfs_config *cfg,
@@ -488,8 +489,6 @@ static int setup_temp_dev_tree(int fd, struct btrfs_mkfs_config *cfg,
 			       u64 dev_bytenr)
 {
 	struct extent_buffer *buf = NULL;
-	u32 itemoff = cfg->leaf_data_size;
-	int slot = 0;
 	int ret;
 
 	/* Must ensure SYS chunk starts before META chunk */
@@ -505,9 +504,9 @@ static int setup_temp_dev_tree(int fd, struct btrfs_mkfs_config *cfg,
 				       BTRFS_DEV_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	insert_temp_dev_extent(buf, &slot, &itemoff, sys_chunk_start,
+	insert_temp_dev_extent(buf, cfg, sys_chunk_start,
 			       BTRFS_MKFS_SYSTEM_GROUP_SIZE);
-	insert_temp_dev_extent(buf, &slot, &itemoff, meta_chunk_start,
+	insert_temp_dev_extent(buf, cfg, meta_chunk_start,
 			       BTRFS_CONVERT_META_GROUP_SIZE);
 	ret = write_temp_extent_buffer(fd, buf, dev_bytenr, cfg);
 out:
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/9] btrfs-progs: convert: simplify insert_temp_extent_item() and insert_temp_block_group()
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
                   ` (4 preceding siblings ...)
  2025-05-24  2:08 ` [PATCH 5/9] btrfs-progs: convert: simplify insert_temp_dev_extent() Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 7/9] btrfs-progs: convert: merge setup_temp_fs_tree() and setup_temp_csum_tree() Qu Wenruo
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

These functions require parameters @slot and @itemoff to record where the
next item should land.

But this is overkilled, as after inserting an item, the temporary extent
buffer will have its header nritems and the item pointer updated.

We can use that header nritems and item pointer to get where the next
item should land.

This removes the external counter to record @slot and @itemoff.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/common.c | 72 +++++++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/convert/common.c b/convert/common.c
index a9d3b395b37b..d062a1306c9e 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -567,8 +567,7 @@ out:
  */
 static int insert_temp_extent_item(int fd, struct extent_buffer *buf,
 				   struct btrfs_mkfs_config *cfg,
-				   int *slot, u32 *itemoff, u64 bytenr,
-				   u64 ref_root)
+				   u64 bytenr, u64 ref_root)
 {
 	struct extent_buffer *tmp;
 	struct btrfs_extent_item *ei;
@@ -576,6 +575,8 @@ static int insert_temp_extent_item(int fd, struct extent_buffer *buf,
 	struct btrfs_disk_key disk_key;
 	struct btrfs_disk_key tree_info_key;
 	struct btrfs_tree_block_info *info;
+	u32 slot = btrfs_header_nritems(buf);
+	u32 itemoff = get_item_offset(buf, cfg);
 	int itemsize;
 	int skinny_metadata = cfg->features.incompat_flags &
 			      BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA;
@@ -587,8 +588,8 @@ static int insert_temp_extent_item(int fd, struct extent_buffer *buf,
 		itemsize = sizeof(*ei) + sizeof(*iref) +
 			   sizeof(struct btrfs_tree_block_info);
 
-	btrfs_set_header_nritems(buf, *slot + 1);
-	*(itemoff) -= itemsize;
+	btrfs_set_header_nritems(buf, slot + 1);
+	itemoff -= itemsize;
 
 	if (skinny_metadata) {
 		btrfs_set_disk_key_type(&disk_key, BTRFS_METADATA_ITEM_KEY);
@@ -599,11 +600,11 @@ static int insert_temp_extent_item(int fd, struct extent_buffer *buf,
 	}
 	btrfs_set_disk_key_objectid(&disk_key, bytenr);
 
-	btrfs_set_item_key(buf, &disk_key, *slot);
-	btrfs_set_item_offset(buf, *slot, *itemoff);
-	btrfs_set_item_size(buf, *slot, itemsize);
+	btrfs_set_item_key(buf, &disk_key, slot);
+	btrfs_set_item_offset(buf, slot, itemoff);
+	btrfs_set_item_size(buf, slot, itemsize);
 
-	ei = btrfs_item_ptr(buf, *slot, struct btrfs_extent_item);
+	ei = btrfs_item_ptr(buf, slot, struct btrfs_extent_item);
 	btrfs_set_extent_refs(buf, ei, 1);
 	btrfs_set_extent_generation(buf, ei, 1);
 	btrfs_set_extent_flags(buf, ei, BTRFS_EXTENT_FLAG_TREE_BLOCK);
@@ -618,7 +619,6 @@ static int insert_temp_extent_item(int fd, struct extent_buffer *buf,
 					 BTRFS_TREE_BLOCK_REF_KEY);
 	btrfs_set_extent_inline_ref_offset(buf, iref, ref_root);
 
-	(*slot)++;
 	if (skinny_metadata)
 		return 0;
 
@@ -654,28 +654,28 @@ out:
 
 static void insert_temp_block_group(struct extent_buffer *buf,
 				   struct btrfs_mkfs_config *cfg,
-				   int *slot, u32 *itemoff,
 				   u64 bytenr, u64 len, u64 used, u64 flag)
 {
 	struct btrfs_block_group_item bgi;
 	struct btrfs_disk_key disk_key;
+	u32 slot = btrfs_header_nritems(buf);
+	u32 itemoff = get_item_offset(buf, cfg);
 
-	btrfs_set_header_nritems(buf, *slot + 1);
-	(*itemoff) -= sizeof(bgi);
+	btrfs_set_header_nritems(buf, slot + 1);
+	itemoff -= sizeof(bgi);
 	btrfs_set_disk_key_type(&disk_key, BTRFS_BLOCK_GROUP_ITEM_KEY);
 	btrfs_set_disk_key_objectid(&disk_key, bytenr);
 	btrfs_set_disk_key_offset(&disk_key, len);
-	btrfs_set_item_key(buf, &disk_key, *slot);
-	btrfs_set_item_offset(buf, *slot, *itemoff);
-	btrfs_set_item_size(buf, *slot, sizeof(bgi));
+	btrfs_set_item_key(buf, &disk_key, slot);
+	btrfs_set_item_offset(buf, slot, itemoff);
+	btrfs_set_item_size(buf, slot, sizeof(bgi));
 
 	btrfs_set_stack_block_group_flags(&bgi, flag);
 	btrfs_set_stack_block_group_used(&bgi, used);
 	btrfs_set_stack_block_group_chunk_objectid(&bgi,
 			BTRFS_FIRST_CHUNK_TREE_OBJECTID);
-	write_extent_buffer(buf, &bgi, btrfs_item_ptr_offset(buf, *slot),
+	write_extent_buffer(buf, &bgi, btrfs_item_ptr_offset(buf, slot),
 			    sizeof(bgi));
-	(*slot)++;
 }
 
 static int setup_temp_extent_tree(int fd, struct btrfs_mkfs_config *cfg,
@@ -684,8 +684,6 @@ static int setup_temp_extent_tree(int fd, struct btrfs_mkfs_config *cfg,
 				  u64 fs_bytenr, u64 csum_bytenr)
 {
 	struct extent_buffer *buf = NULL;
-	u32 itemoff = cfg->leaf_data_size;
-	int slot = 0;
 	int ret;
 
 	/*
@@ -704,39 +702,39 @@ static int setup_temp_extent_tree(int fd, struct btrfs_mkfs_config *cfg,
 	if (ret < 0)
 		goto out;
 
-	ret = insert_temp_extent_item(fd, buf, cfg, &slot, &itemoff,
-			chunk_bytenr, BTRFS_CHUNK_TREE_OBJECTID);
+	ret = insert_temp_extent_item(fd, buf, cfg,  chunk_bytenr,
+				      BTRFS_CHUNK_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
 
-	insert_temp_block_group(buf, cfg, &slot, &itemoff, chunk_bytenr,
-			BTRFS_MKFS_SYSTEM_GROUP_SIZE, cfg->nodesize,
-			BTRFS_BLOCK_GROUP_SYSTEM);
+	insert_temp_block_group(buf, cfg, chunk_bytenr,
+				BTRFS_MKFS_SYSTEM_GROUP_SIZE, cfg->nodesize,
+				BTRFS_BLOCK_GROUP_SYSTEM);
 
-	ret = insert_temp_extent_item(fd, buf, cfg, &slot, &itemoff,
-			root_bytenr, BTRFS_ROOT_TREE_OBJECTID);
+	ret = insert_temp_extent_item(fd, buf, cfg, root_bytenr,
+				      BTRFS_ROOT_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
 
 	/* 5 tree block used, root, extent, dev, fs and csum*/
-	insert_temp_block_group(buf, cfg, &slot, &itemoff, root_bytenr,
-			BTRFS_CONVERT_META_GROUP_SIZE, cfg->nodesize * 5,
-			BTRFS_BLOCK_GROUP_METADATA);
+	insert_temp_block_group(buf, cfg, root_bytenr,
+				BTRFS_CONVERT_META_GROUP_SIZE, cfg->nodesize * 5,
+				BTRFS_BLOCK_GROUP_METADATA);
 
-	ret = insert_temp_extent_item(fd, buf, cfg, &slot, &itemoff,
-			extent_bytenr, BTRFS_EXTENT_TREE_OBJECTID);
+	ret = insert_temp_extent_item(fd, buf, cfg, extent_bytenr,
+				      BTRFS_EXTENT_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_extent_item(fd, buf, cfg, &slot, &itemoff,
-			dev_bytenr, BTRFS_DEV_TREE_OBJECTID);
+	ret = insert_temp_extent_item(fd, buf, cfg, dev_bytenr,
+				      BTRFS_DEV_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_extent_item(fd, buf, cfg, &slot, &itemoff,
-			fs_bytenr, BTRFS_FS_TREE_OBJECTID);
+	ret = insert_temp_extent_item(fd, buf, cfg, fs_bytenr,
+				      BTRFS_FS_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_extent_item(fd, buf, cfg, &slot, &itemoff,
-			csum_bytenr, BTRFS_CSUM_TREE_OBJECTID);
+	ret = insert_temp_extent_item(fd, buf, cfg, csum_bytenr,
+				      BTRFS_CSUM_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
 
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/9] btrfs-progs: convert: merge setup_temp_fs_tree() and setup_temp_csum_tree()
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
                   ` (5 preceding siblings ...)
  2025-05-24  2:08 ` [PATCH 6/9] btrfs-progs: convert: simplify insert_temp_extent_item() and insert_temp_block_group() Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 8/9] btrfs-progs: convert: implement the block group tree support properly Qu Wenruo
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

Both fs and csum trees are empty at make_convert_btrfs(), no need to use
two different functions to do that.

Merge them into a common setup_temp_empty_tree() instead.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/common.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/convert/common.c b/convert/common.c
index d062a1306c9e..b00d69cedd68 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -514,8 +514,8 @@ out:
 	return ret;
 }
 
-static int setup_temp_fs_tree(int fd, struct btrfs_mkfs_config *cfg,
-			      u64 fs_bytenr)
+static int setup_temp_empty_tree(int fd, struct btrfs_mkfs_config *cfg,
+				 u64 root_bytenr, u64 owner)
 {
 	struct extent_buffer *buf = NULL;
 	int ret;
@@ -523,36 +523,13 @@ static int setup_temp_fs_tree(int fd, struct btrfs_mkfs_config *cfg,
 	buf = malloc(sizeof(*buf) + cfg->nodesize);
 	if (!buf)
 		return -ENOMEM;
-	ret = setup_temp_extent_buffer(buf, cfg, fs_bytenr,
-				       BTRFS_FS_TREE_OBJECTID);
+	ret = setup_temp_extent_buffer(buf, cfg, root_bytenr, owner);
 	if (ret < 0)
 		goto out;
 	/*
 	 * Temporary fs tree is completely empty.
 	 */
-	ret = write_temp_extent_buffer(fd, buf, fs_bytenr, cfg);
-out:
-	free(buf);
-	return ret;
-}
-
-static int setup_temp_csum_tree(int fd, struct btrfs_mkfs_config *cfg,
-				u64 csum_bytenr)
-{
-	struct extent_buffer *buf = NULL;
-	int ret;
-
-	buf = malloc(sizeof(*buf) + cfg->nodesize);
-	if (!buf)
-		return -ENOMEM;
-	ret = setup_temp_extent_buffer(buf, cfg, csum_bytenr,
-				       BTRFS_CSUM_TREE_OBJECTID);
-	if (ret < 0)
-		goto out;
-	/*
-	 * Temporary csum tree is completely empty.
-	 */
-	ret = write_temp_extent_buffer(fd, buf, csum_bytenr, cfg);
+	ret = write_temp_extent_buffer(fd, buf, root_bytenr, cfg);
 out:
 	free(buf);
 	return ret;
@@ -867,10 +844,10 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg,
 				  dev_bytenr);
 	if (ret < 0)
 		goto out;
-	ret = setup_temp_fs_tree(fd, cfg, fs_bytenr);
+	ret = setup_temp_empty_tree(fd, cfg, fs_bytenr, BTRFS_FS_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	ret = setup_temp_csum_tree(fd, cfg, csum_bytenr);
+	ret = setup_temp_empty_tree(fd, cfg, csum_bytenr, BTRFS_CSUM_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
 	/*
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/9] btrfs-progs: convert: implement the block group tree support properly
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
                   ` (6 preceding siblings ...)
  2025-05-24  2:08 ` [PATCH 7/9] btrfs-progs: convert: merge setup_temp_fs_tree() and setup_temp_csum_tree() Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-24  2:08 ` [PATCH 9/9] btrfs-progs: convert-tests: add a test case for bgt feature Qu Wenruo
  2025-05-30 20:07 ` [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works David Sterba
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

Previously there are some problems related to btrfs-convert bgt support,
that it doesn't work at all, caused by the following reasons:

- We never update the super block with extra compat ro flags
  Even if we set "-O bgt" flags, it will not set the compat ro flags,
  and everything just go non-bgt routine.

  Meanwhile other compat ro flags are for free-space-tree, and
  free-space-tree is rebuilt after the full convert is done.
  Thus this bug won't cause any problem for fst features, but only
  affecting bgt so far.

- No extra handling to create block group tree

Fix above problems by:

- Set the proper compat RO flag for the temporary super block
  We should only set the compat RO flags except the two FST related
  bits.
  As FST is handled after conversion, we should not set the flag at that
  timing.

- Add block group tree root item and its backrefs
  So the initial temporary fs will have a proper block group tree.

  The only tricky part is for the extent tree population, where we have
  to put all block group items into the block group tree other than the
  extent tree.

With these two points addressed, now block group tree can be properly
enabled for btrfs-convert.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 convert/common.c | 108 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 85 insertions(+), 23 deletions(-)

diff --git a/convert/common.c b/convert/common.c
index b00d69cedd68..c2210f2686b3 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -150,6 +150,13 @@ static int setup_temp_super(int fd, struct btrfs_mkfs_config *cfg,
 	btrfs_set_super_chunk_root(&super, chunk_bytenr);
 	btrfs_set_super_cache_generation(&super, -1);
 	btrfs_set_super_incompat_flags(&super, cfg->features.incompat_flags);
+	/*
+	 * Do not set fst related flags yet, it will be handled after
+	 * the fs is converted.
+	 */
+	btrfs_set_super_compat_ro_flags(&super, cfg->features.compat_ro_flags &
+			~(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |
+			  BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID));
 	if (cfg->label)
 		strncpy_null(super.label, cfg->label, BTRFS_LABEL_SIZE);
 
@@ -200,6 +207,12 @@ static u32 get_item_offset(const struct extent_buffer *eb,
 		return cfg->leaf_data_size;
 }
 
+static bool btrfs_is_bgt(const struct btrfs_mkfs_config *cfg)
+{
+	return cfg->features.compat_ro_flags &
+	       BTRFS_FEATURE_COMPAT_RO_BLOCK_GROUP_TREE;
+}
+
 static void insert_temp_root_item(struct extent_buffer *buf,
 				  struct btrfs_mkfs_config *cfg,
 				  u64 objectid, u64 bytenr)
@@ -260,7 +273,8 @@ static inline int write_temp_extent_buffer(int fd, struct extent_buffer *buf,
 
 static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
 				u64 root_bytenr, u64 extent_bytenr,
-				u64 dev_bytenr, u64 fs_bytenr, u64 csum_bytenr)
+				u64 dev_bytenr, u64 fs_bytenr, u64 csum_bytenr,
+				u64 bgt_bytenr)
 {
 	struct extent_buffer *buf = NULL;
 	int ret;
@@ -270,7 +284,8 @@ static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
 	 * bad key order.
 	 */
 	UASSERT(root_bytenr < extent_bytenr && extent_bytenr < dev_bytenr &&
-	        dev_bytenr < fs_bytenr && fs_bytenr < csum_bytenr);
+	        dev_bytenr < fs_bytenr && fs_bytenr < csum_bytenr &&
+		csum_bytenr < bgt_bytenr);
 	buf = malloc(sizeof(*buf) + cfg->nodesize);
 	if (!buf)
 		return -ENOMEM;
@@ -284,6 +299,9 @@ static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
 	insert_temp_root_item(buf, cfg, BTRFS_DEV_TREE_OBJECTID, dev_bytenr);
 	insert_temp_root_item(buf, cfg, BTRFS_FS_TREE_OBJECTID, fs_bytenr);
 	insert_temp_root_item(buf, cfg, BTRFS_CSUM_TREE_OBJECTID, csum_bytenr);
+	if (btrfs_is_bgt(cfg))
+		insert_temp_root_item(buf, cfg, BTRFS_BLOCK_GROUP_TREE_OBJECTID,
+				      bgt_bytenr);
 
 	ret = write_temp_extent_buffer(fd, buf, root_bytenr, cfg);
 out:
@@ -658,9 +676,12 @@ static void insert_temp_block_group(struct extent_buffer *buf,
 static int setup_temp_extent_tree(int fd, struct btrfs_mkfs_config *cfg,
 				  u64 chunk_bytenr, u64 root_bytenr,
 				  u64 extent_bytenr, u64 dev_bytenr,
-				  u64 fs_bytenr, u64 csum_bytenr)
+				  u64 fs_bytenr, u64 csum_bytenr,
+				  u64 bgt_bytenr)
 {
-	struct extent_buffer *buf = NULL;
+	struct extent_buffer *extent_buf = NULL;
+	struct extent_buffer *bg_buf = NULL;
+	const bool is_bgt = btrfs_is_bgt(cfg);
 	int ret;
 
 	/*
@@ -669,55 +690,85 @@ static int setup_temp_extent_tree(int fd, struct btrfs_mkfs_config *cfg,
 	 */
 	UASSERT(chunk_bytenr < root_bytenr && root_bytenr < extent_bytenr &&
 		extent_bytenr < dev_bytenr && dev_bytenr < fs_bytenr &&
-		fs_bytenr < csum_bytenr);
-	buf = malloc(sizeof(*buf) + cfg->nodesize);
-	if (!buf)
-		return -ENOMEM;
+		fs_bytenr < csum_bytenr && csum_bytenr < bgt_bytenr);
+	extent_buf = malloc(sizeof(*extent_buf) + cfg->nodesize);
+	if (!extent_buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
-	ret = setup_temp_extent_buffer(buf, cfg, extent_bytenr,
+	ret = setup_temp_extent_buffer(extent_buf, cfg, extent_bytenr,
 				       BTRFS_EXTENT_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
 
-	ret = insert_temp_extent_item(fd, buf, cfg,  chunk_bytenr,
+	if (is_bgt) {
+		bg_buf = malloc(sizeof(*bg_buf) + cfg->nodesize);
+		if (!bg_buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		ret = setup_temp_extent_buffer(bg_buf, cfg, bgt_bytenr,
+					       BTRFS_BLOCK_GROUP_TREE_OBJECTID);
+		if (ret < 0)
+			goto out;
+	}
+
+	ret = insert_temp_extent_item(fd, extent_buf, cfg,  chunk_bytenr,
 				      BTRFS_CHUNK_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
 
-	insert_temp_block_group(buf, cfg, chunk_bytenr,
+	insert_temp_block_group(is_bgt ? bg_buf : extent_buf, cfg, chunk_bytenr,
 				BTRFS_MKFS_SYSTEM_GROUP_SIZE, cfg->nodesize,
 				BTRFS_BLOCK_GROUP_SYSTEM);
 
-	ret = insert_temp_extent_item(fd, buf, cfg, root_bytenr,
+	ret = insert_temp_extent_item(fd, extent_buf, cfg, root_bytenr,
 				      BTRFS_ROOT_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
 
-	/* 5 tree block used, root, extent, dev, fs and csum*/
-	insert_temp_block_group(buf, cfg, root_bytenr,
-				BTRFS_CONVERT_META_GROUP_SIZE, cfg->nodesize * 5,
+	/*
+	 * 5 tree block used, root, extent, dev, fs and csum.
+	 * Plus bg tree if specified.
+	 */
+	insert_temp_block_group(is_bgt ? bg_buf : extent_buf, cfg, root_bytenr,
+				BTRFS_CONVERT_META_GROUP_SIZE,
+				is_bgt ? cfg->nodesize * 6 : cfg->nodesize * 5,
 				BTRFS_BLOCK_GROUP_METADATA);
 
-	ret = insert_temp_extent_item(fd, buf, cfg, extent_bytenr,
+	ret = insert_temp_extent_item(fd, extent_buf, cfg, extent_bytenr,
 				      BTRFS_EXTENT_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_extent_item(fd, buf, cfg, dev_bytenr,
+	ret = insert_temp_extent_item(fd, extent_buf, cfg, dev_bytenr,
 				      BTRFS_DEV_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_extent_item(fd, buf, cfg, fs_bytenr,
+	ret = insert_temp_extent_item(fd, extent_buf, cfg, fs_bytenr,
 				      BTRFS_FS_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
-	ret = insert_temp_extent_item(fd, buf, cfg, csum_bytenr,
+	ret = insert_temp_extent_item(fd, extent_buf, cfg, csum_bytenr,
 				      BTRFS_CSUM_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
+	if (btrfs_is_bgt(cfg)) {
+		ret = insert_temp_extent_item(fd, extent_buf, cfg, bgt_bytenr,
+				      BTRFS_BLOCK_GROUP_TREE_OBJECTID);
+		if (ret < 0)
+			goto out;
+	}
+
+	ret = write_temp_extent_buffer(fd, extent_buf, extent_bytenr, cfg);
+	if (ret < 0)
+		goto out;
+	if (is_bgt)
+		ret = write_temp_extent_buffer(fd, bg_buf, bgt_bytenr, cfg);
 
-	ret = write_temp_extent_buffer(fd, buf, extent_bytenr, cfg);
 out:
-	free(buf);
+	free(extent_buf);
+	free(bg_buf);
 	return ret;
 }
 
@@ -751,6 +802,7 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg,
 {
 	struct cache_tree *free_space = &cctx->free_space;
 	struct cache_tree *used_space = &cctx->used_space;
+	const bool is_bgt = btrfs_is_bgt(cfg);
 	u64 sys_chunk_start;
 	u64 meta_chunk_start;
 	/* chunk tree bytenr, in system chunk */
@@ -761,6 +813,7 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg,
 	u64 dev_bytenr;
 	u64 fs_bytenr;
 	u64 csum_bytenr;
+	u64 bgt_bytenr = (u64)-1;
 	int ret;
 
 	/* Source filesystem must be opened, checked and analyzed in advance */
@@ -814,6 +867,7 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg,
 	 *  | +nodesize * 2	| device root	|
 	 *  | +nodesize * 3	| fs tree	|
 	 *  | +nodesize * 4	| csum tree	|
+	 *  | +nodesize * 5	| bg tree	| (Optional)
 	 *  -------------------------------------
 	 * Inside the allocated system chunk, the layout will be:
 	 *  | offset		| contents	|
@@ -827,13 +881,15 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg,
 	dev_bytenr = meta_chunk_start + cfg->nodesize * 2;
 	fs_bytenr = meta_chunk_start + cfg->nodesize * 3;
 	csum_bytenr = meta_chunk_start + cfg->nodesize * 4;
+	if (is_bgt)
+		bgt_bytenr = meta_chunk_start + cfg->nodesize * 5;
 
 	ret = setup_temp_super(fd, cfg, root_bytenr, chunk_bytenr);
 	if (ret < 0)
 		goto out;
 
 	ret = setup_temp_root_tree(fd, cfg, root_bytenr, extent_bytenr,
-				   dev_bytenr, fs_bytenr, csum_bytenr);
+				   dev_bytenr, fs_bytenr, csum_bytenr, bgt_bytenr);
 	if (ret < 0)
 		goto out;
 	ret = setup_temp_chunk_tree(fd, cfg, sys_chunk_start, meta_chunk_start,
@@ -850,13 +906,19 @@ int make_convert_btrfs(int fd, struct btrfs_mkfs_config *cfg,
 	ret = setup_temp_empty_tree(fd, cfg, csum_bytenr, BTRFS_CSUM_TREE_OBJECTID);
 	if (ret < 0)
 		goto out;
+	if (is_bgt) {
+		ret = setup_temp_empty_tree(fd, cfg, bgt_bytenr,
+					    BTRFS_BLOCK_GROUP_TREE_OBJECTID);
+		if (ret < 0)
+			goto out;
+	}
 	/*
 	 * Setup extent tree last, since it may need to read tree block key
 	 * for non-skinny metadata case.
 	 */
 	ret = setup_temp_extent_tree(fd, cfg, chunk_bytenr, root_bytenr,
 				     extent_bytenr, dev_bytenr, fs_bytenr,
-				     csum_bytenr);
+				     csum_bytenr, bgt_bytenr);
 out:
 	return ret;
 }
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 9/9] btrfs-progs: convert-tests: add a test case for bgt feature
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
                   ` (7 preceding siblings ...)
  2025-05-24  2:08 ` [PATCH 8/9] btrfs-progs: convert: implement the block group tree support properly Qu Wenruo
@ 2025-05-24  2:08 ` Qu Wenruo
  2025-05-30 20:07 ` [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works David Sterba
  9 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2025-05-24  2:08 UTC (permalink / raw)
  To: linux-btrfs

Previously "btrfs-convert -O bgt" will not cause any error, but the
results fs has no block-group-tree feature at all, making it no
different than "btrfs-convert -O ^bgt".

This is a big bug that never caught by our existing convert runs.
001-ext2-basic and 003-ext4-basic all tested bgt feature, but doesn't
really check if the resulted fs really have bgt flags set.

To patch the hole, add a new test case, which will do the regular bgt
convert, but at the end also do a super block dump and verify the
BLOCK_GROUP_TREE flag is properly set.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../028-block-group-tree/test.sh              | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100755 tests/convert-tests/028-block-group-tree/test.sh

diff --git a/tests/convert-tests/028-block-group-tree/test.sh b/tests/convert-tests/028-block-group-tree/test.sh
new file mode 100755
index 000000000000..7b30ac7de7ae
--- /dev/null
+++ b/tests/convert-tests/028-block-group-tree/test.sh
@@ -0,0 +1,20 @@
+#!/bin/bash
+# Make sure btrfs-convert can create a fs with bgt feature.
+
+source "$TEST_TOP/common" || exit
+source "$TEST_TOP/common.convert" || exit
+
+setup_root_helper
+prepare_test_dev
+
+check_global_prereq mkfs.ext4
+check_prereq btrfs-convert
+check_prereq btrfs
+
+convert_test_prep_fs ext4 mke2fs -t ext4 -b 4096
+run_check_umount_test_dev
+convert_test_do_convert bgt 16384
+
+# Manually check the super block to make sure it has BGT flag.
+run_check_stdout "$TOP/btrfs" inspect-internal dump-super "$TEST_DEV" |\
+	grep -q "BLOCK_GROUP_TREE" || _fail "No block-group-tree feature enabled"
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works
  2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
                   ` (8 preceding siblings ...)
  2025-05-24  2:08 ` [PATCH 9/9] btrfs-progs: convert-tests: add a test case for bgt feature Qu Wenruo
@ 2025-05-30 20:07 ` David Sterba
  9 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2025-05-30 20:07 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Sat, May 24, 2025 at 11:38:05AM +0930, Qu Wenruo wrote:
> There is a long bug that, "btrfs-convert -O bgt" doesn't create a fs
> with bgt feature at all.
> 
> In fact "mkfs.btrfs -O bgt" is no different than "mkfs.btrfs -O ^bgt".
> 
> The root cause is explained and fixed in the second last patch.
> 
> The first 7 patches are mostly preparation and cleanup for properly
> intorduce block group tree at temprory fs creation time.

Thanks for fixing it, this escaped everybody. The cleanups done along
the way are good.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-05-30 20:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-24  2:08 [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works Qu Wenruo
2025-05-24  2:08 ` [PATCH 1/9] btrfs-progs: convert: add feature dependency checks for bgt Qu Wenruo
2025-05-24  2:08 ` [PATCH 2/9] btrfs-progs: convert: replace the bytenr check with a UASSERT() Qu Wenruo
2025-05-24  2:08 ` [PATCH 3/9] btrfs-progs: convert: simplify insert_temp_root_item() Qu Wenruo
2025-05-24  2:08 ` [PATCH 4/9] btrfs-progs: convert: simplify insert_temp_dev_item() and insert_temp_chunk_item() Qu Wenruo
2025-05-24  2:08 ` [PATCH 5/9] btrfs-progs: convert: simplify insert_temp_dev_extent() Qu Wenruo
2025-05-24  2:08 ` [PATCH 6/9] btrfs-progs: convert: simplify insert_temp_extent_item() and insert_temp_block_group() Qu Wenruo
2025-05-24  2:08 ` [PATCH 7/9] btrfs-progs: convert: merge setup_temp_fs_tree() and setup_temp_csum_tree() Qu Wenruo
2025-05-24  2:08 ` [PATCH 8/9] btrfs-progs: convert: implement the block group tree support properly Qu Wenruo
2025-05-24  2:08 ` [PATCH 9/9] btrfs-progs: convert-tests: add a test case for bgt feature Qu Wenruo
2025-05-30 20:07 ` [PATCH 0/9] btrfs-progs: convert: fix a long bug that bgt feature never works David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox