[PATCH 1/2] btrfs-progs: add set_uuid param to btrfs_make_subvolume

[PATCH 1/2] btrfs-progs: add set_uuid param to btrfs_make_subvolume

[Mark Harmstone]

Adds a set_uuid parameter to btrfs_make_subvolume, which generates a
uuid for the new root and adds it to the uuid tree.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 common/root-tree-utils.c | 72 +++++++++++++++++++++++++++++++++++++++-
 common/root-tree-utils.h |  5 ++-
 convert/main.c           |  5 +--
 mkfs/main.c              | 59 ++------------------------------
 4 files changed, 80 insertions(+), 61 deletions(-)

diff --git a/common/root-tree-utils.c b/common/root-tree-utils.c
index 6a57c51a..f9343304 100644
--- a/common/root-tree-utils.c
+++ b/common/root-tree-utils.c
@@ -15,6 +15,7 @@
  */
 
 #include <time.h>
+#include <uuid/uuid.h>
 #include "common/root-tree-utils.h"
 #include "common/messages.h"
 #include "kernel-shared/disk-io.h"
@@ -58,13 +59,70 @@ error:
 	return ret;
 }
 
+int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
+			u64 subvol_id_cpu)
+{
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_root *uuid_root = fs_info->uuid_root;
+	int ret;
+	struct btrfs_path *path = NULL;
+	struct btrfs_key key;
+	struct extent_buffer *eb;
+	int slot;
+	unsigned long offset;
+	__le64 subvol_id_le;
+
+	btrfs_uuid_to_key(uuid, type, &key);
+
+	path = btrfs_alloc_path();
+	if (!path) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = btrfs_insert_empty_item(trans, uuid_root, path, &key, sizeof(subvol_id_le));
+	if (ret < 0 && ret != -EEXIST) {
+		warning(
+		"inserting uuid item failed (0x%016llx, 0x%016llx) type %u: %d",
+			key.objectid, key.offset, type, ret);
+		goto out;
+	}
+
+	if (ret >= 0) {
+		/* Add an item for the type for the first time. */
+		eb = path->nodes[0];
+		slot = path->slots[0];
+		offset = btrfs_item_ptr_offset(eb, slot);
+	} else {
+		/*
+		 * ret == -EEXIST case, an item with that type already exists.
+		 * Extend the item and store the new subvol_id at the end.
+		 */
+		btrfs_extend_item(path, sizeof(subvol_id_le));
+		eb = path->nodes[0];
+		slot = path->slots[0];
+		offset = btrfs_item_ptr_offset(eb, slot);
+		offset += btrfs_item_size(eb, slot) - sizeof(subvol_id_le);
+	}
+
+	ret = 0;
+	subvol_id_le = cpu_to_le64(subvol_id_cpu);
+	write_extent_buffer(eb, &subvol_id_le, offset, sizeof(subvol_id_le));
+	btrfs_mark_buffer_dirty(eb);
+
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 /*
  * Create a subvolume and initialize its content with the top inode.
  *
  * The created tree root would have its root_ref as 1.
  * Thus for subvolumes caller needs to properly add ROOT_BACKREF items.
  */
-int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid)
+int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid,
+			 bool set_uuid)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *root;
@@ -96,6 +154,18 @@ int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid)
 	ret = btrfs_make_root_dir(trans, root, BTRFS_FIRST_FREE_OBJECTID);
 	if (ret < 0)
 		goto error;
+
+	if (set_uuid) {
+		uuid_generate(root->root_item.uuid);
+
+		ret = btrfs_uuid_tree_add(trans, root->root_item.uuid,
+					  BTRFS_UUID_KEY_SUBVOL, objectid);
+		if (ret < 0) {
+			error("failed to add uuid entry");
+			goto error;
+		}
+	}
+
 	ret = btrfs_update_root(trans, fs_info->tree_root, &root->root_key,
 				&root->root_item);
 	if (ret < 0)
diff --git a/common/root-tree-utils.h b/common/root-tree-utils.h
index 0c4ece24..78731dd5 100644
--- a/common/root-tree-utils.h
+++ b/common/root-tree-utils.h
@@ -21,10 +21,13 @@
 
 int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
 			struct btrfs_root *root, u64 objectid);
-int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid);
+int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid,
+			 bool set_uuid);
 int btrfs_link_subvolume(struct btrfs_trans_handle *trans,
 			 struct btrfs_root *parent_root,
 			 u64 parent_dir, const char *name,
 			 int namelen, struct btrfs_root *subvol);
+int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
+			u64 subvol_id_cpu);
 
 #endif
diff --git a/convert/main.c b/convert/main.c
index 078ef64e..c863ce91 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1022,13 +1022,14 @@ static int init_btrfs(struct btrfs_mkfs_config *cfg, struct btrfs_root *root,
 			     BTRFS_FIRST_FREE_OBJECTID);
 
 	/* subvol for fs image file */
-	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID);
+	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID, false);
 	if (ret < 0) {
 		error("failed to create subvolume image root: %d", ret);
 		goto err;
 	}
 	/* subvol for data relocation tree */
-	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID);
+	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID,
+				   false);
 	if (ret < 0) {
 		error("failed to create DATA_RELOC root: %d", ret);
 		goto err;
diff --git a/mkfs/main.c b/mkfs/main.c
index cf5cae45..0bdb414a 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -735,62 +735,6 @@ static void update_chunk_allocation(struct btrfs_fs_info *fs_info,
 	}
 }
 
-static int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid,
-			       u8 type, u64 subvol_id_cpu)
-{
-	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *uuid_root = fs_info->uuid_root;
-	int ret;
-	struct btrfs_path *path = NULL;
-	struct btrfs_key key;
-	struct extent_buffer *eb;
-	int slot;
-	unsigned long offset;
-	__le64 subvol_id_le;
-
-	btrfs_uuid_to_key(uuid, type, &key);
-
-	path = btrfs_alloc_path();
-	if (!path) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	ret = btrfs_insert_empty_item(trans, uuid_root, path, &key, sizeof(subvol_id_le));
-	if (ret < 0 && ret != -EEXIST) {
-		warning(
-		"inserting uuid item failed (0x%016llx, 0x%016llx) type %u: %d",
-			key.objectid, key.offset, type, ret);
-		goto out;
-	}
-
-	if (ret >= 0) {
-		/* Add an item for the type for the first time. */
-		eb = path->nodes[0];
-		slot = path->slots[0];
-		offset = btrfs_item_ptr_offset(eb, slot);
-	} else {
-		/*
-		 * ret == -EEXIST case, an item with that type already exists.
-		 * Extend the item and store the new subvol_id at the end.
-		 */
-		btrfs_extend_item(path, sizeof(subvol_id_le));
-		eb = path->nodes[0];
-		slot = path->slots[0];
-		offset = btrfs_item_ptr_offset(eb, slot);
-		offset += btrfs_item_size(eb, slot) - sizeof(subvol_id_le);
-	}
-
-	ret = 0;
-	subvol_id_le = cpu_to_le64(subvol_id_cpu);
-	write_extent_buffer(eb, &subvol_id_le, offset, sizeof(subvol_id_le));
-	btrfs_mark_buffer_dirty(eb);
-
-out:
-	btrfs_free_path(path);
-	return ret;
-}
-
 static int create_uuid_tree(struct btrfs_trans_handle *trans)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
@@ -1871,7 +1815,8 @@ raid_groups:
 		goto out;
 	}
 
-	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID);
+	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID,
+				   false);
 	if (ret) {
 		error("unable to create data reloc tree: %d", ret);
 		goto out;
-- 
2.44.2

[PATCH 2/2] btrfs-progs: set subvol uuids when converting

[Mark Harmstone]

Currently when using btrfs-convert, neither the main subvolume nor the
image subvolume get uuids assigned, nor is the uuid tree created.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
---
 common/root-tree-utils.c | 29 +++++++++++++++++++++++++++++
 common/root-tree-utils.h |  1 +
 convert/common.c         | 14 +++++++++-----
 convert/main.c           |  8 +++++++-
 mkfs/main.c              | 29 -----------------------------
 5 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/common/root-tree-utils.c b/common/root-tree-utils.c
index f9343304..9495178c 100644
--- a/common/root-tree-utils.c
+++ b/common/root-tree-utils.c
@@ -59,6 +59,35 @@ error:
 	return ret;
 }
 
+int create_uuid_tree(struct btrfs_trans_handle *trans)
+{
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_root *root;
+	struct btrfs_key key = {
+		.objectid = BTRFS_UUID_TREE_OBJECTID,
+		.type = BTRFS_ROOT_ITEM_KEY,
+	};
+	int ret = 0;
+
+	UASSERT(fs_info->uuid_root == NULL);
+	root = btrfs_create_tree(trans, &key);
+	if (IS_ERR(root)) {
+		ret = PTR_ERR(root);
+		goto out;
+	}
+
+	add_root_to_dirty_list(root);
+	fs_info->uuid_root = root;
+	ret = btrfs_uuid_tree_add(trans, fs_info->fs_root->root_item.uuid,
+				  BTRFS_UUID_KEY_SUBVOL,
+				  fs_info->fs_root->root_key.objectid);
+	if (ret < 0)
+		btrfs_abort_transaction(trans, ret);
+
+out:
+	return ret;
+}
+
 int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
 			u64 subvol_id_cpu)
 {
diff --git a/common/root-tree-utils.h b/common/root-tree-utils.h
index 78731dd5..aec1849b 100644
--- a/common/root-tree-utils.h
+++ b/common/root-tree-utils.h
@@ -29,5 +29,6 @@ int btrfs_link_subvolume(struct btrfs_trans_handle *trans,
 			 int namelen, struct btrfs_root *subvol);
 int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
 			u64 subvol_id_cpu);
+int create_uuid_tree(struct btrfs_trans_handle *trans);
 
 #endif
diff --git a/convert/common.c b/convert/common.c
index b093fdb5..667f38a4 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -190,7 +190,7 @@ static int setup_temp_extent_buffer(struct extent_buffer *buf,
 static void insert_temp_root_item(struct extent_buffer *buf,
 				  struct btrfs_mkfs_config *cfg,
 				  int *slot, u32 *itemoff, u64 objectid,
-				  u64 bytenr)
+				  u64 bytenr, bool set_uuid)
 {
 	struct btrfs_root_item root_item;
 	struct btrfs_inode_item *inode_item;
@@ -210,6 +210,9 @@ static void insert_temp_root_item(struct extent_buffer *buf,
 	btrfs_set_root_generation(&root_item, 1);
 	btrfs_set_root_bytenr(&root_item, bytenr);
 
+	if (set_uuid)
+		uuid_generate(root_item.uuid);
+
 	memset(&disk_key, 0, sizeof(disk_key));
 	btrfs_set_disk_key_type(&disk_key, BTRFS_ROOT_ITEM_KEY);
 	btrfs_set_disk_key_objectid(&disk_key, objectid);
@@ -281,13 +284,14 @@ static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
 		goto out;
 
 	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_EXTENT_TREE_OBJECTID, extent_bytenr);
+			      BTRFS_EXTENT_TREE_OBJECTID, extent_bytenr,
+			      false);
 	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_DEV_TREE_OBJECTID, dev_bytenr);
+			      BTRFS_DEV_TREE_OBJECTID, dev_bytenr, false);
 	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_FS_TREE_OBJECTID, fs_bytenr);
+			      BTRFS_FS_TREE_OBJECTID, fs_bytenr, true);
 	insert_temp_root_item(buf, cfg, &slot, &itemoff,
-			      BTRFS_CSUM_TREE_OBJECTID, csum_bytenr);
+			      BTRFS_CSUM_TREE_OBJECTID, csum_bytenr, false);
 
 	ret = write_temp_extent_buffer(fd, buf, root_bytenr, cfg);
 out:
diff --git a/convert/main.c b/convert/main.c
index c863ce91..8cfd9407 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1021,8 +1021,14 @@ static int init_btrfs(struct btrfs_mkfs_config *cfg, struct btrfs_root *root,
 	btrfs_set_root_dirid(&fs_info->fs_root->root_item,
 			     BTRFS_FIRST_FREE_OBJECTID);
 
+	ret = create_uuid_tree(trans);
+	if (ret) {
+		error("failed to create uuid tree: %d", ret);
+		goto err;
+	}
+
 	/* subvol for fs image file */
-	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID, false);
+	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID, true);
 	if (ret < 0) {
 		error("failed to create subvolume image root: %d", ret);
 		goto err;
diff --git a/mkfs/main.c b/mkfs/main.c
index 0bdb414a..a69aa24b 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -735,35 +735,6 @@ static void update_chunk_allocation(struct btrfs_fs_info *fs_info,
 	}
 }
 
-static int create_uuid_tree(struct btrfs_trans_handle *trans)
-{
-	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *root;
-	struct btrfs_key key = {
-		.objectid = BTRFS_UUID_TREE_OBJECTID,
-		.type = BTRFS_ROOT_ITEM_KEY,
-	};
-	int ret = 0;
-
-	UASSERT(fs_info->uuid_root == NULL);
-	root = btrfs_create_tree(trans, &key);
-	if (IS_ERR(root)) {
-		ret = PTR_ERR(root);
-		goto out;
-	}
-
-	add_root_to_dirty_list(root);
-	fs_info->uuid_root = root;
-	ret = btrfs_uuid_tree_add(trans, fs_info->fs_root->root_item.uuid,
-				  BTRFS_UUID_KEY_SUBVOL,
-				  fs_info->fs_root->root_key.objectid);
-	if (ret < 0)
-		btrfs_abort_transaction(trans, ret);
-
-out:
-	return ret;
-}
-
 static int create_global_root(struct btrfs_trans_handle *trans, u64 objectid,
 			      int root_id)
 {
-- 
2.44.2

Re: [PATCH 2/2] btrfs-progs: set subvol uuids when converting

[Josef Bacik]

On Fri, Jul 19, 2024 at 04:03:24PM +0100, Mark Harmstone wrote:
> Currently when using btrfs-convert, neither the main subvolume nor the
> image subvolume get uuids assigned, nor is the uuid tree created.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
>  common/root-tree-utils.c | 29 +++++++++++++++++++++++++++++
>  common/root-tree-utils.h |  1 +
>  convert/common.c         | 14 +++++++++-----
>  convert/main.c           |  8 +++++++-
>  mkfs/main.c              | 29 -----------------------------
>  5 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/common/root-tree-utils.c b/common/root-tree-utils.c
> index f9343304..9495178c 100644
> --- a/common/root-tree-utils.c
> +++ b/common/root-tree-utils.c
> @@ -59,6 +59,35 @@ error:
>  	return ret;
>  }
>  
> +int create_uuid_tree(struct btrfs_trans_handle *trans)

If we're exporting a new thing then it needs to be renamed to
btrfs_create_uuid_tree.

Also a convert test for this would be good, to validate we're getting the UUID
tree on convert.  Thanks,

Josef

Re: [PATCH 1/2] btrfs-progs: add set_uuid param to btrfs_make_subvolume

[Josef Bacik]

On Fri, Jul 19, 2024 at 04:03:23PM +0100, Mark Harmstone wrote:
> Adds a set_uuid parameter to btrfs_make_subvolume, which generates a
> uuid for the new root and adds it to the uuid tree.
> 
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH 1/2] btrfs-progs: add set_uuid param to btrfs_make_subvolume

[Qu Wenruo]

在 2024/7/20 00:33, Mark Harmstone 写道:
> Adds a set_uuid parameter to btrfs_make_subvolume, which generates a
> uuid for the new root and adds it to the uuid tree.
>
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
> ---
>   common/root-tree-utils.c | 72 +++++++++++++++++++++++++++++++++++++++-
>   common/root-tree-utils.h |  5 ++-
>   convert/main.c           |  5 +--
>   mkfs/main.c              | 59 ++------------------------------
>   4 files changed, 80 insertions(+), 61 deletions(-)
>
> diff --git a/common/root-tree-utils.c b/common/root-tree-utils.c
> index 6a57c51a..f9343304 100644
> --- a/common/root-tree-utils.c
> +++ b/common/root-tree-utils.c
> @@ -15,6 +15,7 @@
>    */
>
>   #include <time.h>
> +#include <uuid/uuid.h>
>   #include "common/root-tree-utils.h"
>   #include "common/messages.h"
>   #include "kernel-shared/disk-io.h"
> @@ -58,13 +59,70 @@ error:
>   	return ret;
>   }
>
> +int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
> +			u64 subvol_id_cpu)
> +{
> +	struct btrfs_fs_info *fs_info = trans->fs_info;
> +	struct btrfs_root *uuid_root = fs_info->uuid_root;
> +	int ret;
> +	struct btrfs_path *path = NULL;
> +	struct btrfs_key key;
> +	struct extent_buffer *eb;
> +	int slot;
> +	unsigned long offset;
> +	__le64 subvol_id_le;
> +
> +	btrfs_uuid_to_key(uuid, type, &key);
> +
> +	path = btrfs_alloc_path();
> +	if (!path) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ret = btrfs_insert_empty_item(trans, uuid_root, path, &key, sizeof(subvol_id_le));
> +	if (ret < 0 && ret != -EEXIST) {
> +		warning(
> +		"inserting uuid item failed (0x%016llx, 0x%016llx) type %u: %d",
> +			key.objectid, key.offset, type, ret);
> +		goto out;
> +	}
> +
> +	if (ret >= 0) {
> +		/* Add an item for the type for the first time. */
> +		eb = path->nodes[0];
> +		slot = path->slots[0];
> +		offset = btrfs_item_ptr_offset(eb, slot);
> +	} else {
> +		/*
> +		 * ret == -EEXIST case, an item with that type already exists.
> +		 * Extend the item and store the new subvol_id at the end.
> +		 */
> +		btrfs_extend_item(path, sizeof(subvol_id_le));
> +		eb = path->nodes[0];
> +		slot = path->slots[0];
> +		offset = btrfs_item_ptr_offset(eb, slot);
> +		offset += btrfs_item_size(eb, slot) - sizeof(subvol_id_le);
> +	}
> +
> +	ret = 0;
> +	subvol_id_le = cpu_to_le64(subvol_id_cpu);
> +	write_extent_buffer(eb, &subvol_id_le, offset, sizeof(subvol_id_le));
> +	btrfs_mark_buffer_dirty(eb);
> +
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
>   /*
>    * Create a subvolume and initialize its content with the top inode.
>    *
>    * The created tree root would have its root_ref as 1.
>    * Thus for subvolumes caller needs to properly add ROOT_BACKREF items.
>    */
> -int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid)
> +int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid,
> +			 bool set_uuid)
>   {
>   	struct btrfs_fs_info *fs_info = trans->fs_info;
>   	struct btrfs_root *root;
> @@ -96,6 +154,18 @@ int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid)
>   	ret = btrfs_make_root_dir(trans, root, BTRFS_FIRST_FREE_OBJECTID);
>   	if (ret < 0)
>   		goto error;
> +
> +	if (set_uuid) {
> +		uuid_generate(root->root_item.uuid);
> +
> +		ret = btrfs_uuid_tree_add(trans, root->root_item.uuid,
> +					  BTRFS_UUID_KEY_SUBVOL, objectid);
> +		if (ret < 0) {
> +			error("failed to add uuid entry");
> +			goto error;
> +		}
> +	}
> +
>   	ret = btrfs_update_root(trans, fs_info->tree_root, &root->root_key,
>   				&root->root_item);
>   	if (ret < 0)
> diff --git a/common/root-tree-utils.h b/common/root-tree-utils.h
> index 0c4ece24..78731dd5 100644
> --- a/common/root-tree-utils.h
> +++ b/common/root-tree-utils.h
> @@ -21,10 +21,13 @@
>
>   int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
>   			struct btrfs_root *root, u64 objectid);
> -int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid);
> +int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid,
> +			 bool set_uuid);
>   int btrfs_link_subvolume(struct btrfs_trans_handle *trans,
>   			 struct btrfs_root *parent_root,
>   			 u64 parent_dir, const char *name,
>   			 int namelen, struct btrfs_root *subvol);
> +int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
> +			u64 subvol_id_cpu);
>
>   #endif
> diff --git a/convert/main.c b/convert/main.c
> index 078ef64e..c863ce91 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -1022,13 +1022,14 @@ static int init_btrfs(struct btrfs_mkfs_config *cfg, struct btrfs_root *root,
>   			     BTRFS_FIRST_FREE_OBJECTID);
>
>   	/* subvol for fs image file */
> -	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID);
> +	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID, false);
>   	if (ret < 0) {
>   		error("failed to create subvolume image root: %d", ret);
>   		goto err;
>   	}
>   	/* subvol for data relocation tree */
> -	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID);
> +	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID,
> +				   false);
>   	if (ret < 0) {
>   		error("failed to create DATA_RELOC root: %d", ret);
>   		goto err;
> diff --git a/mkfs/main.c b/mkfs/main.c
> index cf5cae45..0bdb414a 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -735,62 +735,6 @@ static void update_chunk_allocation(struct btrfs_fs_info *fs_info,
>   	}
>   }
>
> -static int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid,
> -			       u8 type, u64 subvol_id_cpu)
> -{
> -	struct btrfs_fs_info *fs_info = trans->fs_info;
> -	struct btrfs_root *uuid_root = fs_info->uuid_root;
> -	int ret;
> -	struct btrfs_path *path = NULL;
> -	struct btrfs_key key;
> -	struct extent_buffer *eb;
> -	int slot;
> -	unsigned long offset;
> -	__le64 subvol_id_le;
> -
> -	btrfs_uuid_to_key(uuid, type, &key);
> -
> -	path = btrfs_alloc_path();
> -	if (!path) {
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> -
> -	ret = btrfs_insert_empty_item(trans, uuid_root, path, &key, sizeof(subvol_id_le));
> -	if (ret < 0 && ret != -EEXIST) {
> -		warning(
> -		"inserting uuid item failed (0x%016llx, 0x%016llx) type %u: %d",
> -			key.objectid, key.offset, type, ret);
> -		goto out;
> -	}
> -
> -	if (ret >= 0) {
> -		/* Add an item for the type for the first time. */
> -		eb = path->nodes[0];
> -		slot = path->slots[0];
> -		offset = btrfs_item_ptr_offset(eb, slot);
> -	} else {
> -		/*
> -		 * ret == -EEXIST case, an item with that type already exists.
> -		 * Extend the item and store the new subvol_id at the end.
> -		 */
> -		btrfs_extend_item(path, sizeof(subvol_id_le));
> -		eb = path->nodes[0];
> -		slot = path->slots[0];
> -		offset = btrfs_item_ptr_offset(eb, slot);
> -		offset += btrfs_item_size(eb, slot) - sizeof(subvol_id_le);
> -	}
> -
> -	ret = 0;
> -	subvol_id_le = cpu_to_le64(subvol_id_cpu);
> -	write_extent_buffer(eb, &subvol_id_le, offset, sizeof(subvol_id_le));
> -	btrfs_mark_buffer_dirty(eb);
> -
> -out:
> -	btrfs_free_path(path);
> -	return ret;
> -}
> -
>   static int create_uuid_tree(struct btrfs_trans_handle *trans)
>   {
>   	struct btrfs_fs_info *fs_info = trans->fs_info;
> @@ -1871,7 +1815,8 @@ raid_groups:
>   		goto out;
>   	}
>
> -	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID);
> +	ret = btrfs_make_subvolume(trans, BTRFS_DATA_RELOC_TREE_OBJECTID,
> +				   false);
>   	if (ret) {
>   		error("unable to create data reloc tree: %d", ret);
>   		goto out;

Re: [PATCH 2/2] btrfs-progs: set subvol uuids when converting

[Qu Wenruo]

在 2024/7/20 00:33, Mark Harmstone 写道:
> Currently when using btrfs-convert, neither the main subvolume nor the
> image subvolume get uuids assigned, nor is the uuid tree created.
>
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> ---
>   common/root-tree-utils.c | 29 +++++++++++++++++++++++++++++
>   common/root-tree-utils.h |  1 +
>   convert/common.c         | 14 +++++++++-----
>   convert/main.c           |  8 +++++++-
>   mkfs/main.c              | 29 -----------------------------
>   5 files changed, 46 insertions(+), 35 deletions(-)
>
> diff --git a/common/root-tree-utils.c b/common/root-tree-utils.c
> index f9343304..9495178c 100644
> --- a/common/root-tree-utils.c
> +++ b/common/root-tree-utils.c
> @@ -59,6 +59,35 @@ error:
>   	return ret;
>   }
>
> +int create_uuid_tree(struct btrfs_trans_handle *trans)
> +{
> +	struct btrfs_fs_info *fs_info = trans->fs_info;
> +	struct btrfs_root *root;
> +	struct btrfs_key key = {
> +		.objectid = BTRFS_UUID_TREE_OBJECTID,
> +		.type = BTRFS_ROOT_ITEM_KEY,
> +	};
> +	int ret = 0;
> +
> +	UASSERT(fs_info->uuid_root == NULL);
> +	root = btrfs_create_tree(trans, &key);
> +	if (IS_ERR(root)) {
> +		ret = PTR_ERR(root);
> +		goto out;
> +	}
> +
> +	add_root_to_dirty_list(root);
> +	fs_info->uuid_root = root;
> +	ret = btrfs_uuid_tree_add(trans, fs_info->fs_root->root_item.uuid,
> +				  BTRFS_UUID_KEY_SUBVOL,
> +				  fs_info->fs_root->root_key.objectid);
> +	if (ret < 0)
> +		btrfs_abort_transaction(trans, ret);
> +
> +out:
> +	return ret;
> +}
> +
>   int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
>   			u64 subvol_id_cpu)
>   {
> diff --git a/common/root-tree-utils.h b/common/root-tree-utils.h
> index 78731dd5..aec1849b 100644
> --- a/common/root-tree-utils.h
> +++ b/common/root-tree-utils.h
> @@ -29,5 +29,6 @@ int btrfs_link_subvolume(struct btrfs_trans_handle *trans,
>   			 int namelen, struct btrfs_root *subvol);
>   int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
>   			u64 subvol_id_cpu);
> +int create_uuid_tree(struct btrfs_trans_handle *trans);
>
>   #endif
> diff --git a/convert/common.c b/convert/common.c
> index b093fdb5..667f38a4 100644
> --- a/convert/common.c
> +++ b/convert/common.c
> @@ -190,7 +190,7 @@ static int setup_temp_extent_buffer(struct extent_buffer *buf,
>   static void insert_temp_root_item(struct extent_buffer *buf,
>   				  struct btrfs_mkfs_config *cfg,
>   				  int *slot, u32 *itemoff, u64 objectid,
> -				  u64 bytenr)
> +				  u64 bytenr, bool set_uuid)

Considering this is really a temporary root item, I believe we can skip
it the UUID step for now.
(Only one of the 4 callers are passing true for it).


I'm wondering if a kernel like uuid tree generation would be more
concentrated and easier to implement.

E.g. after the fs is fully converted, generate the uuid tree then update
the UUID for the involved subvolumes.

Thanks,
Qu
>   {
>   	struct btrfs_root_item root_item;
>   	struct btrfs_inode_item *inode_item;
> @@ -210,6 +210,9 @@ static void insert_temp_root_item(struct extent_buffer *buf,
>   	btrfs_set_root_generation(&root_item, 1);
>   	btrfs_set_root_bytenr(&root_item, bytenr);
>
> +	if (set_uuid)
> +		uuid_generate(root_item.uuid);
> +
>   	memset(&disk_key, 0, sizeof(disk_key));
>   	btrfs_set_disk_key_type(&disk_key, BTRFS_ROOT_ITEM_KEY);
>   	btrfs_set_disk_key_objectid(&disk_key, objectid);
> @@ -281,13 +284,14 @@ static int setup_temp_root_tree(int fd, struct btrfs_mkfs_config *cfg,
>   		goto out;
>
>   	insert_temp_root_item(buf, cfg, &slot, &itemoff,
> -			      BTRFS_EXTENT_TREE_OBJECTID, extent_bytenr);
> +			      BTRFS_EXTENT_TREE_OBJECTID, extent_bytenr,
> +			      false);
>   	insert_temp_root_item(buf, cfg, &slot, &itemoff,
> -			      BTRFS_DEV_TREE_OBJECTID, dev_bytenr);
> +			      BTRFS_DEV_TREE_OBJECTID, dev_bytenr, false);
>   	insert_temp_root_item(buf, cfg, &slot, &itemoff,
> -			      BTRFS_FS_TREE_OBJECTID, fs_bytenr);
> +			      BTRFS_FS_TREE_OBJECTID, fs_bytenr, true);
>   	insert_temp_root_item(buf, cfg, &slot, &itemoff,
> -			      BTRFS_CSUM_TREE_OBJECTID, csum_bytenr);
> +			      BTRFS_CSUM_TREE_OBJECTID, csum_bytenr, false);
>
>   	ret = write_temp_extent_buffer(fd, buf, root_bytenr, cfg);
>   out:
> diff --git a/convert/main.c b/convert/main.c
> index c863ce91..8cfd9407 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -1021,8 +1021,14 @@ static int init_btrfs(struct btrfs_mkfs_config *cfg, struct btrfs_root *root,
>   	btrfs_set_root_dirid(&fs_info->fs_root->root_item,
>   			     BTRFS_FIRST_FREE_OBJECTID);
>
> +	ret = create_uuid_tree(trans);
> +	if (ret) {
> +		error("failed to create uuid tree: %d", ret);
> +		goto err;
> +	}
> +
>   	/* subvol for fs image file */
> -	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID, false);
> +	ret = btrfs_make_subvolume(trans, CONV_IMAGE_SUBVOL_OBJECTID, true);
>   	if (ret < 0) {
>   		error("failed to create subvolume image root: %d", ret);
>   		goto err;
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 0bdb414a..a69aa24b 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -735,35 +735,6 @@ static void update_chunk_allocation(struct btrfs_fs_info *fs_info,
>   	}
>   }
>
> -static int create_uuid_tree(struct btrfs_trans_handle *trans)
> -{
> -	struct btrfs_fs_info *fs_info = trans->fs_info;
> -	struct btrfs_root *root;
> -	struct btrfs_key key = {
> -		.objectid = BTRFS_UUID_TREE_OBJECTID,
> -		.type = BTRFS_ROOT_ITEM_KEY,
> -	};
> -	int ret = 0;
> -
> -	UASSERT(fs_info->uuid_root == NULL);
> -	root = btrfs_create_tree(trans, &key);
> -	if (IS_ERR(root)) {
> -		ret = PTR_ERR(root);
> -		goto out;
> -	}
> -
> -	add_root_to_dirty_list(root);
> -	fs_info->uuid_root = root;
> -	ret = btrfs_uuid_tree_add(trans, fs_info->fs_root->root_item.uuid,
> -				  BTRFS_UUID_KEY_SUBVOL,
> -				  fs_info->fs_root->root_key.objectid);
> -	if (ret < 0)
> -		btrfs_abort_transaction(trans, ret);
> -
> -out:
> -	return ret;
> -}
> -
>   static int create_global_root(struct btrfs_trans_handle *trans, u64 objectid,
>   			      int root_id)
>   {

Re: [PATCH 2/2] btrfs-progs: set subvol uuids when converting

[David Sterba]

On Sat, Jul 20, 2024 at 07:58:30AM +0930, Qu Wenruo wrote:
> 在 2024/7/20 00:33, Mark Harmstone 写道:
> > Currently when using btrfs-convert, neither the main subvolume nor the
> > image subvolume get uuids assigned, nor is the uuid tree created.
> >
> > Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> > ---
> >   common/root-tree-utils.c | 29 +++++++++++++++++++++++++++++
> >   common/root-tree-utils.h |  1 +
> >   convert/common.c         | 14 +++++++++-----
> >   convert/main.c           |  8 +++++++-
> >   mkfs/main.c              | 29 -----------------------------
> >   5 files changed, 46 insertions(+), 35 deletions(-)
> >
> > diff --git a/common/root-tree-utils.c b/common/root-tree-utils.c
> > index f9343304..9495178c 100644
> > --- a/common/root-tree-utils.c
> > +++ b/common/root-tree-utils.c
> > @@ -59,6 +59,35 @@ error:
> >   	return ret;
> >   }
> >
> > +int create_uuid_tree(struct btrfs_trans_handle *trans)
> > +{
> > +	struct btrfs_fs_info *fs_info = trans->fs_info;
> > +	struct btrfs_root *root;
> > +	struct btrfs_key key = {
> > +		.objectid = BTRFS_UUID_TREE_OBJECTID,
> > +		.type = BTRFS_ROOT_ITEM_KEY,
> > +	};
> > +	int ret = 0;
> > +
> > +	UASSERT(fs_info->uuid_root == NULL);
> > +	root = btrfs_create_tree(trans, &key);
> > +	if (IS_ERR(root)) {
> > +		ret = PTR_ERR(root);
> > +		goto out;
> > +	}
> > +
> > +	add_root_to_dirty_list(root);
> > +	fs_info->uuid_root = root;
> > +	ret = btrfs_uuid_tree_add(trans, fs_info->fs_root->root_item.uuid,
> > +				  BTRFS_UUID_KEY_SUBVOL,
> > +				  fs_info->fs_root->root_key.objectid);
> > +	if (ret < 0)
> > +		btrfs_abort_transaction(trans, ret);
> > +
> > +out:
> > +	return ret;
> > +}
> > +
> >   int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
> >   			u64 subvol_id_cpu)
> >   {
> > diff --git a/common/root-tree-utils.h b/common/root-tree-utils.h
> > index 78731dd5..aec1849b 100644
> > --- a/common/root-tree-utils.h
> > +++ b/common/root-tree-utils.h
> > @@ -29,5 +29,6 @@ int btrfs_link_subvolume(struct btrfs_trans_handle *trans,
> >   			 int namelen, struct btrfs_root *subvol);
> >   int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
> >   			u64 subvol_id_cpu);
> > +int create_uuid_tree(struct btrfs_trans_handle *trans);
> >
> >   #endif
> > diff --git a/convert/common.c b/convert/common.c
> > index b093fdb5..667f38a4 100644
> > --- a/convert/common.c
> > +++ b/convert/common.c
> > @@ -190,7 +190,7 @@ static int setup_temp_extent_buffer(struct extent_buffer *buf,
> >   static void insert_temp_root_item(struct extent_buffer *buf,
> >   				  struct btrfs_mkfs_config *cfg,
> >   				  int *slot, u32 *itemoff, u64 objectid,
> > -				  u64 bytenr)
> > +				  u64 bytenr, bool set_uuid)
> 
> Considering this is really a temporary root item, I believe we can skip
> it the UUID step for now.
> (Only one of the 4 callers are passing true for it).
> 
> 
> I'm wondering if a kernel like uuid tree generation would be more
> concentrated and easier to implement.
> 
> E.g. after the fs is fully converted, generate the uuid tree then update
> the UUID for the involved subvolumes.

Agreed, this sounds like a better option.