[PATCH 0/1] btrfs-progs: mkfs: add subvolume support to mkfs

[PATCH 0/1] btrfs-progs: mkfs: add subvolume support to mkfs

[Yingyi Luo]

From: yingyil <yingyil@google.com>

Add -S/--subvol [NAME] option to configure. It enables users to create a
subvolume under the toplevel volume and populate the created subvolume
with files from the rootdir specified by -r/--rootdir option.

Two functions link_subvol() and create_subvol() are moved from
convert/main.c to utils.c to enable code reuse.

yingyil (1):
  btrfs-progs: mkfs: add subvolume support to mkfs

 convert/main.c | 161 ---------------------------------------------------------
 mkfs/main.c    |  73 ++++++++++++++++++++++++--
 utils.c        | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h        |   5 ++
 4 files changed, 234 insertions(+), 166 deletions(-)

-- 
2.14.1.342.g6490525c54-goog

[PATCH 1/1] btrfs-progs: mkfs: add subvolume support to mkfs

[Yingyi Luo]

From: yingyil <yingyil@google.com>

Add -S/--subvol [NAME] option to configure. It enables users to create a
subvolume under the toplevel volume and populate the created subvolume
with files from the rootdir specified by -r/--rootdir option.

Two functions link_subvol() and create_subvol() are moved from
convert/main.c to utils.c to enable code reuse.

Signed-off-by: yingyil <yingyil@google.com>
---
 convert/main.c | 161 ---------------------------------------------------------
 mkfs/main.c    |  73 ++++++++++++++++++++++++--
 utils.c        | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h        |   5 ++
 4 files changed, 234 insertions(+), 166 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 0deccd9..93609ed 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -832,167 +832,6 @@ out:
 	return ret;
 }
 
-static struct btrfs_root* link_subvol(struct btrfs_root *root,
-		const char *base, u64 root_objectid)
-{
-	struct btrfs_trans_handle *trans;
-	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct btrfs_root *tree_root = fs_info->tree_root;
-	struct btrfs_root *new_root = NULL;
-	struct btrfs_path path;
-	struct btrfs_inode_item *inode_item;
-	struct extent_buffer *leaf;
-	struct btrfs_key key;
-	u64 dirid = btrfs_root_dirid(&root->root_item);
-	u64 index = 2;
-	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
-	int len;
-	int i;
-	int ret;
-
-	len = strlen(base);
-	if (len == 0 || len > BTRFS_NAME_LEN)
-		return NULL;
-
-	btrfs_init_path(&path);
-	key.objectid = dirid;
-	key.type = BTRFS_DIR_INDEX_KEY;
-	key.offset = (u64)-1;
-
-	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
-	if (ret <= 0) {
-		error("search for DIR_INDEX dirid %llu failed: %d",
-				(unsigned long long)dirid, ret);
-		goto fail;
-	}
-
-	if (path.slots[0] > 0) {
-		path.slots[0]--;
-		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
-		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
-			index = key.offset + 1;
-	}
-	btrfs_release_path(&path);
-
-	trans = btrfs_start_transaction(root, 1);
-	if (!trans) {
-		error("unable to start transaction");
-		goto fail;
-	}
-
-	key.objectid = dirid;
-	key.offset = 0;
-	key.type =  BTRFS_INODE_ITEM_KEY;
-
-	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
-	if (ret) {
-		error("search for INODE_ITEM %llu failed: %d",
-				(unsigned long long)dirid, ret);
-		goto fail;
-	}
-	leaf = path.nodes[0];
-	inode_item = btrfs_item_ptr(leaf, path.slots[0],
-				    struct btrfs_inode_item);
-
-	key.objectid = root_objectid;
-	key.offset = (u64)-1;
-	key.type = BTRFS_ROOT_ITEM_KEY;
-
-	memcpy(buf, base, len);
-	for (i = 0; i < 1024; i++) {
-		ret = btrfs_insert_dir_item(trans, root, buf, len,
-					    dirid, &key, BTRFS_FT_DIR, index);
-		if (ret != -EEXIST)
-			break;
-		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
-		if (len < 1 || len > BTRFS_NAME_LEN) {
-			ret = -EINVAL;
-			break;
-		}
-	}
-	if (ret)
-		goto fail;
-
-	btrfs_set_inode_size(leaf, inode_item, len * 2 +
-			     btrfs_inode_size(leaf, inode_item));
-	btrfs_mark_buffer_dirty(leaf);
-	btrfs_release_path(&path);
-
-	/* add the backref first */
-	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
-				 BTRFS_ROOT_BACKREF_KEY,
-				 root->root_key.objectid,
-				 dirid, index, buf, len);
-	if (ret) {
-		error("unable to add root backref for %llu: %d",
-				root->root_key.objectid, ret);
-		goto fail;
-	}
-
-	/* now add the forward ref */
-	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
-				 BTRFS_ROOT_REF_KEY, root_objectid,
-				 dirid, index, buf, len);
-	if (ret) {
-		error("unable to add root ref for %llu: %d",
-				root->root_key.objectid, ret);
-		goto fail;
-	}
-
-	ret = btrfs_commit_transaction(trans, root);
-	if (ret) {
-		error("transaction commit failed: %d", ret);
-		goto fail;
-	}
-
-	new_root = btrfs_read_fs_root(fs_info, &key);
-	if (IS_ERR(new_root)) {
-		error("unable to fs read root: %lu", PTR_ERR(new_root));
-		new_root = NULL;
-	}
-fail:
-	btrfs_init_path(&path);
-	return new_root;
-}
-
-static int create_subvol(struct btrfs_trans_handle *trans,
-			 struct btrfs_root *root, u64 root_objectid)
-{
-	struct extent_buffer *tmp;
-	struct btrfs_root *new_root;
-	struct btrfs_key key;
-	struct btrfs_root_item root_item;
-	int ret;
-
-	ret = btrfs_copy_root(trans, root, root->node, &tmp,
-			      root_objectid);
-	if (ret)
-		return ret;
-
-	memcpy(&root_item, &root->root_item, sizeof(root_item));
-	btrfs_set_root_bytenr(&root_item, tmp->start);
-	btrfs_set_root_level(&root_item, btrfs_header_level(tmp));
-	btrfs_set_root_generation(&root_item, trans->transid);
-	free_extent_buffer(tmp);
-
-	key.objectid = root_objectid;
-	key.type = BTRFS_ROOT_ITEM_KEY;
-	key.offset = trans->transid;
-	ret = btrfs_insert_root(trans, root->fs_info->tree_root,
-				&key, &root_item);
-
-	key.offset = (u64)-1;
-	new_root = btrfs_read_fs_root(root->fs_info, &key);
-	if (!new_root || IS_ERR(new_root)) {
-		error("unable to fs read root: %lu", PTR_ERR(new_root));
-		return PTR_ERR(new_root);
-	}
-
-	ret = btrfs_make_root_dir(trans, new_root, BTRFS_FIRST_FREE_OBJECTID);
-
-	return ret;
-}
-
 /*
  * New make_btrfs() has handle system and meta chunks quite well.
  * So only need to add remaining data chunks.
diff --git a/mkfs/main.c b/mkfs/main.c
index 2b109a5..107cfc2 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -365,6 +365,7 @@ static void print_usage(int ret)
 	printf("  creation:\n");
 	printf("\t-b|--byte-count SIZE    set filesystem size to SIZE (on the first device)\n");
 	printf("\t-r|--rootdir DIR        copy files from DIR to the image root directory\n");
+	printf("\t-S|--subvol NAME        create a sunvolume with NAME and copy files from ROOTDIR to the subvolume\n");
 	printf("\t-K|--nodiscard          do not perform whole device TRIM\n");
 	printf("\t-f|--force              force overwrite of existing filesystem\n");
 	printf("  general:\n");
@@ -413,6 +414,18 @@ static char *parse_label(const char *input)
 	return strdup(input);
 }
 
+static char *parse_subvol_name(const char *input)
+{
+	int len = strlen(input);
+
+	if (len >= BTRFS_SUBVOL_NAME_MAX) {
+		error("subvolume name %s is too long (max %d)",
+			input, BTRFS_SUBVOL_NAME_MAX - 1);
+		exit(1);
+	}
+	return strdup(input);
+}
+
 static int add_directory_items(struct btrfs_trans_handle *trans,
 			       struct btrfs_root *root, u64 objectid,
 			       ino_t parent_inum, const char *name,
@@ -1420,6 +1433,8 @@ int main(int argc, char **argv)
 	int discard = 1;
 	int ssd = 0;
 	int force_overwrite = 0;
+	char *subvol_name = NULL;
+	int subvol_name_set = 0;
 	char *source_dir = NULL;
 	int source_dir_set = 0;
 	u64 num_of_meta_chunks = 0;
@@ -1446,6 +1461,7 @@ int main(int argc, char **argv)
 			{ "sectorsize", required_argument, NULL, 's' },
 			{ "data", required_argument, NULL, 'd' },
 			{ "version", no_argument, NULL, 'V' },
+			{ "subvol", required_argument, NULL, 'S'},
 			{ "rootdir", required_argument, NULL, 'r' },
 			{ "nodiscard", no_argument, NULL, 'K' },
 			{ "features", required_argument, NULL, 'O' },
@@ -1455,7 +1471,7 @@ int main(int argc, char **argv)
 			{ NULL, 0, NULL, 0}
 		};
 
-		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:r:U:VMKq",
+		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:S:r:U:VMKq",
 				long_options, NULL);
 		if (c < 0)
 			break;
@@ -1517,6 +1533,10 @@ int main(int argc, char **argv)
 						PACKAGE_STRING);
 				exit(0);
 				break;
+			case 'S':
+				subvol_name = parse_subvol_name(optarg);
+				subvol_name_set = 1;
+				break;
 			case 'r':
 				source_dir = optarg;
 				source_dir_set = 1;
@@ -1537,6 +1557,11 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (subvol_name_set && !source_dir_set) {
+		error("root directory needs to be set");
+		exit(1);
+	}
+
 	if (verbose) {
 		printf("%s\n", PACKAGE_STRING);
 		printf("See %s for more information.\n\n", PACKAGE_URL);
@@ -1876,10 +1901,48 @@ raid_groups:
 			goto out;
 		}
 
-		ret = make_image(source_dir, root, fd);
-		if (ret) {
-			error("error wihle filling filesystem: %d", ret);
-			goto out;
+		if (subvol_name_set) {
+			u64 dirid, objectid;
+			struct btrfs_root *file_root;
+
+			dirid = btrfs_root_dirid(&fs_info->tree_root->root_item);
+			ret = btrfs_find_free_objectid(NULL, fs_info->tree_root, dirid, &objectid);
+			if (ret) {
+				error("unable to find a free objectid: %d", ret);
+				goto out;
+			}
+			trans = btrfs_start_transaction(root, 1);
+			if (!trans) {
+				error("unable to start transaction");
+				ret = -EINVAL;
+				goto out;
+			}
+			ret = create_subvol(trans, root, objectid);
+			if (ret < 0) {
+				error("failed to create subvolume: %d", ret);
+				goto out;
+			}
+			ret = btrfs_commit_transaction(trans, root);
+			if (ret) {
+				error("unable to commit transaction: %d", ret);
+				goto out;
+			}
+			file_root = link_subvol(root, subvol_name, objectid);
+			if (!file_root) {
+				error("unable to link the subvolume %s", subvol_name);
+				goto out;
+			}
+			ret = make_image(source_dir, file_root, fd);
+			if (ret) {
+				error("error while filling filesystem: %d", ret);
+				goto out;
+			}
+		} else {
+			ret = make_image(source_dir, root, fd);
+			if (ret) {
+				error("error while filling filesystem: %d", ret);
+				goto out;
+			}
 		}
 	}
 	ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
diff --git a/utils.c b/utils.c
index bb04913..c9bbbed 100644
--- a/utils.c
+++ b/utils.c
@@ -2574,3 +2574,164 @@ u8 rand_u8(void)
 void btrfs_config_init(void)
 {
 }
+
+struct btrfs_root *link_subvol(struct btrfs_root *root,
+		const char *base, u64 root_objectid)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_root *new_root = NULL;
+	struct btrfs_path path;
+	struct btrfs_inode_item *inode_item;
+	struct extent_buffer *leaf;
+	struct btrfs_key key;
+	u64 dirid = btrfs_root_dirid(&root->root_item);
+	u64 index = 2;
+	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
+	int len;
+	int i;
+	int ret;
+
+	len = strlen(base);
+	if (len == 0 || len > BTRFS_NAME_LEN)
+		return NULL;
+
+	btrfs_init_path(&path);
+	key.objectid = dirid;
+	key.type = BTRFS_DIR_INDEX_KEY;
+	key.offset = (u64)-1;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret <= 0) {
+		error("search for DIR_INDEX dirid %llu failed: %d",
+				(unsigned long long)dirid, ret);
+		goto fail;
+	}
+
+	if (path.slots[0] > 0) {
+		path.slots[0]--;
+		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
+			index = key.offset + 1;
+	}
+	btrfs_release_path(&path);
+
+	trans = btrfs_start_transaction(root, 1);
+	if (!trans) {
+		error("unable to start transaction");
+		goto fail;
+        }
+
+	key.objectid = dirid;
+	key.offset = 0;
+	key.type =  BTRFS_INODE_ITEM_KEY;
+
+	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
+	if (ret) {
+		error("search for INODE_ITEM %llu failed: %d",
+				(unsigned long long)dirid, ret);
+		goto fail;
+	}
+	leaf = path.nodes[0];
+	inode_item = btrfs_item_ptr(leaf, path.slots[0],
+				    struct btrfs_inode_item);
+
+	key.objectid = root_objectid;
+	key.offset = (u64)-1;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+
+	memcpy(buf, base, len);
+	for (i = 0; i < 1024; i++) {
+		ret = btrfs_insert_dir_item(trans, root, buf, len,
+					    dirid, &key, BTRFS_FT_DIR, index);
+		if (ret != -EEXIST)
+			break;
+		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
+		if (len < 1 || len > BTRFS_NAME_LEN) {
+			ret = -EINVAL;
+			break;
+		}
+	}
+	if (ret)
+		goto fail;
+
+	btrfs_set_inode_size(leaf, inode_item, len * 2 +
+			     btrfs_inode_size(leaf, inode_item));
+	btrfs_mark_buffer_dirty(leaf);
+	btrfs_release_path(&path);
+
+	/* add the backref first */
+	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
+				 BTRFS_ROOT_BACKREF_KEY,
+				 root->root_key.objectid,
+				 dirid, index, buf, len);
+	if (ret) {
+		error("unable to add root backref for %llu: %d",
+				root->root_key.objectid, ret);
+		goto fail;
+	}
+
+        /* now add the forward ref */
+	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
+				 BTRFS_ROOT_REF_KEY, root_objectid,
+				 dirid, index, buf, len);
+	if (ret) {
+		error("unable to add root ref for %llu: %d",
+				root->root_key.objectid, ret);
+		goto fail;
+	}
+
+	ret = btrfs_commit_transaction(trans, root);
+	if (ret) {
+		error("transaction commit failed: %d", ret);
+		goto fail;
+	}
+
+	new_root = btrfs_read_fs_root(fs_info, &key);
+	if (IS_ERR(new_root)) {
+		error("unable to fs read root: %lu", PTR_ERR(new_root));
+		new_root = NULL;
+	}
+fail:
+	btrfs_init_path(&path);
+	return new_root;
+}
+
+int create_subvol(struct btrfs_trans_handle *trans,
+		  struct btrfs_root *root, u64 root_objectid)
+{
+	struct extent_buffer *tmp;
+	struct btrfs_root *new_root;
+	struct btrfs_key key;
+	struct btrfs_root_item root_item;
+	int ret;
+
+	ret = btrfs_copy_root(trans, root, root->node, &tmp,
+			      root_objectid);
+	if (ret)
+		return ret;
+
+	memcpy(&root_item, &root->root_item, sizeof(root_item));
+	btrfs_set_root_bytenr(&root_item, tmp->start);
+	btrfs_set_root_level(&root_item, btrfs_header_level(tmp));
+	btrfs_set_root_generation(&root_item, trans->transid);
+	free_extent_buffer(tmp);
+
+	key.objectid = root_objectid;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = trans->transid;
+	ret = btrfs_insert_root(trans, root->fs_info->tree_root,
+				&key, &root_item);
+
+	key.offset = (u64)-1;
+	new_root = btrfs_read_fs_root(root->fs_info, &key);
+	if (!new_root || IS_ERR(new_root)) {
+		error("unable to fs read root: %lu", PTR_ERR(new_root));
+		return PTR_ERR(new_root);
+	}
+
+	ret = btrfs_make_root_dir(trans, new_root, BTRFS_FIRST_FREE_OBJECTID);
+
+	return ret;
+}
diff --git a/utils.h b/utils.h
index 091f8fa..777476f 100644
--- a/utils.h
+++ b/utils.h
@@ -170,4 +170,9 @@ u64 rand_u64(void);
 unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 
+struct btrfs_root *link_subvol(struct btrfs_root *root,
+			       const char *base, u64 root_objectid);
+int create_subvol(struct btrfs_trans_handle *trans,
+		  struct btrfs_root *root, u64 root_objectid);
+
 #endif
-- 
2.14.1.342.g6490525c54-goog

Re: [PATCH 1/1] btrfs-progs: mkfs: add subvolume support to mkfs

[Qu Wenruo]

On 2017年08月26日 07:21, Yingyi Luo wrote:
> From: yingyil <yingyil@google.com>
> 
> Add -S/--subvol [NAME] option to configure. It enables users to create a
> subvolume under the toplevel volume and populate the created subvolume
> with files from the rootdir specified by -r/--rootdir option.
> 
> Two functions link_subvol() and create_subvol() are moved from
> convert/main.c to utils.c to enable code reuse.

What about split the patch as the code move of link/create_subvol() 
makes review a little difficult.

BTW, if exporting link/create_subvol(), what about adding "btrfs_" prefix?

Thanks,
Qu
> 
> Signed-off-by: yingyil <yingyil@google.com>
> ---
>   convert/main.c | 161 ---------------------------------------------------------
>   mkfs/main.c    |  73 ++++++++++++++++++++++++--
>   utils.c        | 161 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   utils.h        |   5 ++
>   4 files changed, 234 insertions(+), 166 deletions(-)
> 
> diff --git a/convert/main.c b/convert/main.c
> index 0deccd9..93609ed 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -832,167 +832,6 @@ out:
>   	return ret;
>   }
>   
> -static struct btrfs_root* link_subvol(struct btrfs_root *root,
> -		const char *base, u64 root_objectid)
> -{
> -	struct btrfs_trans_handle *trans;
> -	struct btrfs_fs_info *fs_info = root->fs_info;
> -	struct btrfs_root *tree_root = fs_info->tree_root;
> -	struct btrfs_root *new_root = NULL;
> -	struct btrfs_path path;
> -	struct btrfs_inode_item *inode_item;
> -	struct extent_buffer *leaf;
> -	struct btrfs_key key;
> -	u64 dirid = btrfs_root_dirid(&root->root_item);
> -	u64 index = 2;
> -	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
> -	int len;
> -	int i;
> -	int ret;
> -
> -	len = strlen(base);
> -	if (len == 0 || len > BTRFS_NAME_LEN)
> -		return NULL;
> -
> -	btrfs_init_path(&path);
> -	key.objectid = dirid;
> -	key.type = BTRFS_DIR_INDEX_KEY;
> -	key.offset = (u64)-1;
> -
> -	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> -	if (ret <= 0) {
> -		error("search for DIR_INDEX dirid %llu failed: %d",
> -				(unsigned long long)dirid, ret);
> -		goto fail;
> -	}
> -
> -	if (path.slots[0] > 0) {
> -		path.slots[0]--;
> -		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
> -		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
> -			index = key.offset + 1;
> -	}
> -	btrfs_release_path(&path);
> -
> -	trans = btrfs_start_transaction(root, 1);
> -	if (!trans) {
> -		error("unable to start transaction");
> -		goto fail;
> -	}
> -
> -	key.objectid = dirid;
> -	key.offset = 0;
> -	key.type =  BTRFS_INODE_ITEM_KEY;
> -
> -	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
> -	if (ret) {
> -		error("search for INODE_ITEM %llu failed: %d",
> -				(unsigned long long)dirid, ret);
> -		goto fail;
> -	}
> -	leaf = path.nodes[0];
> -	inode_item = btrfs_item_ptr(leaf, path.slots[0],
> -				    struct btrfs_inode_item);
> -
> -	key.objectid = root_objectid;
> -	key.offset = (u64)-1;
> -	key.type = BTRFS_ROOT_ITEM_KEY;
> -
> -	memcpy(buf, base, len);
> -	for (i = 0; i < 1024; i++) {
> -		ret = btrfs_insert_dir_item(trans, root, buf, len,
> -					    dirid, &key, BTRFS_FT_DIR, index);
> -		if (ret != -EEXIST)
> -			break;
> -		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
> -		if (len < 1 || len > BTRFS_NAME_LEN) {
> -			ret = -EINVAL;
> -			break;
> -		}
> -	}
> -	if (ret)
> -		goto fail;
> -
> -	btrfs_set_inode_size(leaf, inode_item, len * 2 +
> -			     btrfs_inode_size(leaf, inode_item));
> -	btrfs_mark_buffer_dirty(leaf);
> -	btrfs_release_path(&path);
> -
> -	/* add the backref first */
> -	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
> -				 BTRFS_ROOT_BACKREF_KEY,
> -				 root->root_key.objectid,
> -				 dirid, index, buf, len);
> -	if (ret) {
> -		error("unable to add root backref for %llu: %d",
> -				root->root_key.objectid, ret);
> -		goto fail;
> -	}
> -
> -	/* now add the forward ref */
> -	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
> -				 BTRFS_ROOT_REF_KEY, root_objectid,
> -				 dirid, index, buf, len);
> -	if (ret) {
> -		error("unable to add root ref for %llu: %d",
> -				root->root_key.objectid, ret);
> -		goto fail;
> -	}
> -
> -	ret = btrfs_commit_transaction(trans, root);
> -	if (ret) {
> -		error("transaction commit failed: %d", ret);
> -		goto fail;
> -	}
> -
> -	new_root = btrfs_read_fs_root(fs_info, &key);
> -	if (IS_ERR(new_root)) {
> -		error("unable to fs read root: %lu", PTR_ERR(new_root));
> -		new_root = NULL;
> -	}
> -fail:
> -	btrfs_init_path(&path);
> -	return new_root;
> -}
> -
> -static int create_subvol(struct btrfs_trans_handle *trans,
> -			 struct btrfs_root *root, u64 root_objectid)
> -{
> -	struct extent_buffer *tmp;
> -	struct btrfs_root *new_root;
> -	struct btrfs_key key;
> -	struct btrfs_root_item root_item;
> -	int ret;
> -
> -	ret = btrfs_copy_root(trans, root, root->node, &tmp,
> -			      root_objectid);
> -	if (ret)
> -		return ret;
> -
> -	memcpy(&root_item, &root->root_item, sizeof(root_item));
> -	btrfs_set_root_bytenr(&root_item, tmp->start);
> -	btrfs_set_root_level(&root_item, btrfs_header_level(tmp));
> -	btrfs_set_root_generation(&root_item, trans->transid);
> -	free_extent_buffer(tmp);
> -
> -	key.objectid = root_objectid;
> -	key.type = BTRFS_ROOT_ITEM_KEY;
> -	key.offset = trans->transid;
> -	ret = btrfs_insert_root(trans, root->fs_info->tree_root,
> -				&key, &root_item);
> -
> -	key.offset = (u64)-1;
> -	new_root = btrfs_read_fs_root(root->fs_info, &key);
> -	if (!new_root || IS_ERR(new_root)) {
> -		error("unable to fs read root: %lu", PTR_ERR(new_root));
> -		return PTR_ERR(new_root);
> -	}
> -
> -	ret = btrfs_make_root_dir(trans, new_root, BTRFS_FIRST_FREE_OBJECTID);
> -
> -	return ret;
> -}
> -
>   /*
>    * New make_btrfs() has handle system and meta chunks quite well.
>    * So only need to add remaining data chunks.
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 2b109a5..107cfc2 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -365,6 +365,7 @@ static void print_usage(int ret)
>   	printf("  creation:\n");
>   	printf("\t-b|--byte-count SIZE    set filesystem size to SIZE (on the first device)\n");
>   	printf("\t-r|--rootdir DIR        copy files from DIR to the image root directory\n");
> +	printf("\t-S|--subvol NAME        create a sunvolume with NAME and copy files from ROOTDIR to the subvolume\n");
>   	printf("\t-K|--nodiscard          do not perform whole device TRIM\n");
>   	printf("\t-f|--force              force overwrite of existing filesystem\n");
>   	printf("  general:\n");
> @@ -413,6 +414,18 @@ static char *parse_label(const char *input)
>   	return strdup(input);
>   }
>   
> +static char *parse_subvol_name(const char *input)
> +{
> +	int len = strlen(input);
> +
> +	if (len >= BTRFS_SUBVOL_NAME_MAX) {
> +		error("subvolume name %s is too long (max %d)",
> +			input, BTRFS_SUBVOL_NAME_MAX - 1);
> +		exit(1);
> +	}
> +	return strdup(input);
> +}
> +
>   static int add_directory_items(struct btrfs_trans_handle *trans,
>   			       struct btrfs_root *root, u64 objectid,
>   			       ino_t parent_inum, const char *name,
> @@ -1420,6 +1433,8 @@ int main(int argc, char **argv)
>   	int discard = 1;
>   	int ssd = 0;
>   	int force_overwrite = 0;
> +	char *subvol_name = NULL;
> +	int subvol_name_set = 0;
>   	char *source_dir = NULL;
>   	int source_dir_set = 0;
>   	u64 num_of_meta_chunks = 0;
> @@ -1446,6 +1461,7 @@ int main(int argc, char **argv)
>   			{ "sectorsize", required_argument, NULL, 's' },
>   			{ "data", required_argument, NULL, 'd' },
>   			{ "version", no_argument, NULL, 'V' },
> +			{ "subvol", required_argument, NULL, 'S'},
>   			{ "rootdir", required_argument, NULL, 'r' },
>   			{ "nodiscard", no_argument, NULL, 'K' },
>   			{ "features", required_argument, NULL, 'O' },
> @@ -1455,7 +1471,7 @@ int main(int argc, char **argv)
>   			{ NULL, 0, NULL, 0}
>   		};
>   
> -		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:r:U:VMKq",
> +		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:S:r:U:VMKq",
>   				long_options, NULL);
>   		if (c < 0)
>   			break;
> @@ -1517,6 +1533,10 @@ int main(int argc, char **argv)
>   						PACKAGE_STRING);
>   				exit(0);
>   				break;
> +			case 'S':
> +				subvol_name = parse_subvol_name(optarg);
> +				subvol_name_set = 1;
> +				break;
>   			case 'r':
>   				source_dir = optarg;
>   				source_dir_set = 1;
> @@ -1537,6 +1557,11 @@ int main(int argc, char **argv)
>   		}
>   	}
>   
> +	if (subvol_name_set && !source_dir_set) {
> +		error("root directory needs to be set");
> +		exit(1);
> +	}
> +
>   	if (verbose) {
>   		printf("%s\n", PACKAGE_STRING);
>   		printf("See %s for more information.\n\n", PACKAGE_URL);
> @@ -1876,10 +1901,48 @@ raid_groups:
>   			goto out;
>   		}
>   
> -		ret = make_image(source_dir, root, fd);
> -		if (ret) {
> -			error("error wihle filling filesystem: %d", ret);
> -			goto out;
> +		if (subvol_name_set) {
> +			u64 dirid, objectid;
> +			struct btrfs_root *file_root;
> +
> +			dirid = btrfs_root_dirid(&fs_info->tree_root->root_item);
> +			ret = btrfs_find_free_objectid(NULL, fs_info->tree_root, dirid, &objectid);
> +			if (ret) {
> +				error("unable to find a free objectid: %d", ret);
> +				goto out;
> +			}
> +			trans = btrfs_start_transaction(root, 1);
> +			if (!trans) {
> +				error("unable to start transaction");
> +				ret = -EINVAL;
> +				goto out;
> +			}
> +			ret = create_subvol(trans, root, objectid);
> +			if (ret < 0) {
> +				error("failed to create subvolume: %d", ret);
> +				goto out;
> +			}
> +			ret = btrfs_commit_transaction(trans, root);
> +			if (ret) {
> +				error("unable to commit transaction: %d", ret);
> +				goto out;
> +			}
> +			file_root = link_subvol(root, subvol_name, objectid);
> +			if (!file_root) {
> +				error("unable to link the subvolume %s", subvol_name);
> +				goto out;
> +			}
> +			ret = make_image(source_dir, file_root, fd);
> +			if (ret) {
> +				error("error while filling filesystem: %d", ret);
> +				goto out;
> +			}
> +		} else {
> +			ret = make_image(source_dir, root, fd);
> +			if (ret) {
> +				error("error while filling filesystem: %d", ret);
> +				goto out;
> +			}
>   		}
>   	}
>   	ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
> diff --git a/utils.c b/utils.c
> index bb04913..c9bbbed 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -2574,3 +2574,164 @@ u8 rand_u8(void)
>   void btrfs_config_init(void)
>   {
>   }
> +
> +struct btrfs_root *link_subvol(struct btrfs_root *root,
> +		const char *base, u64 root_objectid)
> +{
> +	struct btrfs_trans_handle *trans;
> +	struct btrfs_fs_info *fs_info = root->fs_info;
> +	struct btrfs_root *tree_root = fs_info->tree_root;
> +	struct btrfs_root *new_root = NULL;
> +	struct btrfs_path path;
> +	struct btrfs_inode_item *inode_item;
> +	struct extent_buffer *leaf;
> +	struct btrfs_key key;
> +	u64 dirid = btrfs_root_dirid(&root->root_item);
> +	u64 index = 2;
> +	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
> +	int len;
> +	int i;
> +	int ret;
> +
> +	len = strlen(base);
> +	if (len == 0 || len > BTRFS_NAME_LEN)
> +		return NULL;
> +
> +	btrfs_init_path(&path);
> +	key.objectid = dirid;
> +	key.type = BTRFS_DIR_INDEX_KEY;
> +	key.offset = (u64)-1;
> +
> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
> +	if (ret <= 0) {
> +		error("search for DIR_INDEX dirid %llu failed: %d",
> +				(unsigned long long)dirid, ret);
> +		goto fail;
> +	}
> +
> +	if (path.slots[0] > 0) {
> +		path.slots[0]--;
> +		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
> +		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
> +			index = key.offset + 1;
> +	}
> +	btrfs_release_path(&path);
> +
> +	trans = btrfs_start_transaction(root, 1);
> +	if (!trans) {
> +		error("unable to start transaction");
> +		goto fail;
> +        }
> +
> +	key.objectid = dirid;
> +	key.offset = 0;
> +	key.type =  BTRFS_INODE_ITEM_KEY;
> +
> +	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
> +	if (ret) {
> +		error("search for INODE_ITEM %llu failed: %d",
> +				(unsigned long long)dirid, ret);
> +		goto fail;
> +	}
> +	leaf = path.nodes[0];
> +	inode_item = btrfs_item_ptr(leaf, path.slots[0],
> +				    struct btrfs_inode_item);
> +
> +	key.objectid = root_objectid;
> +	key.offset = (u64)-1;
> +	key.type = BTRFS_ROOT_ITEM_KEY;
> +
> +	memcpy(buf, base, len);
> +	for (i = 0; i < 1024; i++) {
> +		ret = btrfs_insert_dir_item(trans, root, buf, len,
> +					    dirid, &key, BTRFS_FT_DIR, index);
> +		if (ret != -EEXIST)
> +			break;
> +		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
> +		if (len < 1 || len > BTRFS_NAME_LEN) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +	}
> +	if (ret)
> +		goto fail;
> +
> +	btrfs_set_inode_size(leaf, inode_item, len * 2 +
> +			     btrfs_inode_size(leaf, inode_item));
> +	btrfs_mark_buffer_dirty(leaf);
> +	btrfs_release_path(&path);
> +
> +	/* add the backref first */
> +	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
> +				 BTRFS_ROOT_BACKREF_KEY,
> +				 root->root_key.objectid,
> +				 dirid, index, buf, len);
> +	if (ret) {
> +		error("unable to add root backref for %llu: %d",
> +				root->root_key.objectid, ret);
> +		goto fail;
> +	}
> +
> +        /* now add the forward ref */
> +	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
> +				 BTRFS_ROOT_REF_KEY, root_objectid,
> +				 dirid, index, buf, len);
> +	if (ret) {
> +		error("unable to add root ref for %llu: %d",
> +				root->root_key.objectid, ret);
> +		goto fail;
> +	}
> +
> +	ret = btrfs_commit_transaction(trans, root);
> +	if (ret) {
> +		error("transaction commit failed: %d", ret);
> +		goto fail;
> +	}
> +
> +	new_root = btrfs_read_fs_root(fs_info, &key);
> +	if (IS_ERR(new_root)) {
> +		error("unable to fs read root: %lu", PTR_ERR(new_root));
> +		new_root = NULL;
> +	}
> +fail:
> +	btrfs_init_path(&path);
> +	return new_root;
> +}
> +
> +int create_subvol(struct btrfs_trans_handle *trans,
> +		  struct btrfs_root *root, u64 root_objectid)
> +{
> +	struct extent_buffer *tmp;
> +	struct btrfs_root *new_root;
> +	struct btrfs_key key;
> +	struct btrfs_root_item root_item;
> +	int ret;
> +
> +	ret = btrfs_copy_root(trans, root, root->node, &tmp,
> +			      root_objectid);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(&root_item, &root->root_item, sizeof(root_item));
> +	btrfs_set_root_bytenr(&root_item, tmp->start);
> +	btrfs_set_root_level(&root_item, btrfs_header_level(tmp));
> +	btrfs_set_root_generation(&root_item, trans->transid);
> +	free_extent_buffer(tmp);
> +
> +	key.objectid = root_objectid;
> +	key.type = BTRFS_ROOT_ITEM_KEY;
> +	key.offset = trans->transid;
> +	ret = btrfs_insert_root(trans, root->fs_info->tree_root,
> +				&key, &root_item);
> +
> +	key.offset = (u64)-1;
> +	new_root = btrfs_read_fs_root(root->fs_info, &key);
> +	if (!new_root || IS_ERR(new_root)) {
> +		error("unable to fs read root: %lu", PTR_ERR(new_root));
> +		return PTR_ERR(new_root);
> +	}
> +
> +	ret = btrfs_make_root_dir(trans, new_root, BTRFS_FIRST_FREE_OBJECTID);
> +
> +	return ret;
> +}
> diff --git a/utils.h b/utils.h
> index 091f8fa..777476f 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -170,4 +170,9 @@ u64 rand_u64(void);
>   unsigned int rand_range(unsigned int upper);
>   void init_rand_seed(u64 seed);
>   
> +struct btrfs_root *link_subvol(struct btrfs_root *root,
> +			       const char *base, u64 root_objectid);
> +int create_subvol(struct btrfs_trans_handle *trans,
> +		  struct btrfs_root *root, u64 root_objectid);
> +
>   #endif
> 

Re: [PATCH 1/1] btrfs-progs: mkfs: add subvolume support to mkfs

[Goffredo Baroncelli]

Hi All,


unfortunately, your patch crashes on my PC

$ truncate -s 100G /tmp/disk.img
$ sudo losetup -f /tmp/disk.img

$ # good case
$ sudo ./mkfs.btrfs -f -r /tmp/empty/  /dev/loop0
btrfs-progs v4.12.1-1-gf80d059c
See http://btrfs.wiki.kernel.org for more information.

Making image is completed.
Label:              (null)
UUID:               7cb4927c-d24a-41b3-8151-277ad9064008
Node size:          16384
Sector size:        4096
Filesystem size:    28.00MiB
Block group profiles:
  Data:             single           10.75MiB
  System:           DUP               4.00MiB
SSD detected:       no
Incompat features:  extref, skinny-metadata
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1    28.00MiB  /dev/loop0

$ # bad case
$ sudo ./mkfs.btrfs -f -S prova -r /tmp/empty/  /dev/loop0
btrfs-progs v4.12.1-1-gf80d059c
See http://btrfs.wiki.kernel.org for more information.

ERROR: failed to create subvolume: -17
transaction.h:42: btrfs_start_transaction: BUG_ON `fs_info->running_transaction` triggered, value 884442943152
./mkfs.btrfs(+0x15674)[0xcdeb52c674]
./mkfs.btrfs(close_ctree_fs_info+0x313)[0xcdeb52e80f]
./mkfs.btrfs(main+0x1028)[0xcdeb52381e]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7f85a5d1f2e1]
./mkfs.btrfs(_start+0x2a)[0xcdeb520e9a]
Aborted


Below some further comments

On 08/28/2017 01:39 AM, Qu Wenruo wrote:
> 
> 
> On 2017年08月26日 07:21, Yingyi Luo wrote:
>> From: yingyil <yingyil@google.com>
>>
>> Add -S/--subvol [NAME] option to configure. It enables users to create a
>> subvolume under the toplevel volume and populate the created subvolume
>> with files from the rootdir specified by -r/--rootdir option.
>>
>> Two functions link_subvol() and create_subvol() are moved from
>> convert/main.c to utils.c to enable code reuse.
> 
> What about split the patch as the code move of link/create_subvol() makes review a little difficult.
> 
> BTW, if exporting link/create_subvol(), what about adding "btrfs_" prefix?
> 
> Thanks,
> Qu
>>
>> Signed-off-by: yingyil <yingyil@google.com>
>> ---
[...]
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -365,6 +365,7 @@ static void print_usage(int ret)
>>       printf("  creation:\n");
>>       printf("\t-b|--byte-count SIZE    set filesystem size to SIZE (on the first device)\n");
>>       printf("\t-r|--rootdir DIR        copy files from DIR to the image root directory\n");
>> +    printf("\t-S|--subvol NAME        create a sunvolume with NAME and copy files from ROOTDIR to the subvolume\n");
>>       printf("\t-K|--nodiscard          do not perform whole device TRIM\n");
>>       printf("\t-f|--force              force overwrite of existing filesystem\n");
>>       printf("  general:\n");
>> @@ -413,6 +414,18 @@ static char *parse_label(const char *input)
>>       return strdup(input);
>>   }
>>   +static char *parse_subvol_name(const char *input)
>> +{
>> +    int len = strlen(input);
>> +
>> +    if (len >= BTRFS_SUBVOL_NAME_MAX) {
>> +        error("subvolume name %s is too long (max %d)",
>> +            input, BTRFS_SUBVOL_NAME_MAX - 1);
>> +        exit(1);
>> +    }
>> +    return strdup(input);

why use strdup ?

>> +}
>> +
[...]

>> @@ -1517,6 +1533,10 @@ int main(int argc, char **argv)
>>                           PACKAGE_STRING);
>>                   exit(0);
>>                   break;
>> +            case 'S':
>> +                subvol_name = parse_subvol_name(optarg);
>> +                subvol_name_set = 1;
>> +                break;
>>               case 'r':
>>                   source_dir = optarg;
>>                   source_dir_set = 1;
>> @@ -1537,6 +1557,11 @@ int main(int argc, char **argv)
>>           }
>>       }
>> +    if (subvol_name_set && !source_dir_set) {
>> +        error("root directory needs to be set");
>> +        exit(1);
>> +    }
>> +

To me it seems reasonable to create an empty subvolume (below more comments)

>>       if (verbose) {
>>           printf("%s\n", PACKAGE_STRING);
>>           printf("See %s for more information.\n\n", PACKAGE_URL);
>> @@ -1876,10 +1901,48 @@ raid_groups:
>>               goto out;
>>           }
[...]

>>       ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
>> diff --git a/utils.c b/utils.c
>> index bb04913..c9bbbed 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -2574,3 +2574,164 @@ u8 rand_u8(void)
>>   void btrfs_config_init(void)
>>   {
>>   }
>> +
>> +struct btrfs_root *link_subvol(struct btrfs_root *root,
>> +        const char *base, u64 root_objectid)
>> +{
>> +    struct btrfs_trans_handle *trans;
[....]

>> +
>> +    memcpy(buf, base, len);
>> +    for (i = 0; i < 1024; i++) {
>> +        ret = btrfs_insert_dir_item(trans, root, buf, len,
>> +                        dirid, &key, BTRFS_FT_DIR, index);
>> +        if (ret != -EEXIST)
>> +            break;
>> +        len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
>> +        if (len < 1 || len > BTRFS_NAME_LEN) {
>> +            ret = -EINVAL;
>> +            break;
>> +        }
>> +    }

Does make sense this ? If the subvolume creation fails, what is the reason to try to create another subvolume with the same name followed by a number ? I suppose that there is some corner case for the convert code...

>> +    if (ret)
>> +        goto fail;
>> +
>> +    btrfs_set_inode_size(leaf, inode_item, len * 2 +
>> +                 btrfs_inode_size(leaf, inode_item));
>> +    btrfs_mark_buffer_dirty(leaf);
>> +    btrfs_release_path(&path);
[...]


If possible I would like to ask another feature: an option to mark default the created subvolume. Use case: usually the root filesystem is stored inside the subvolid=5; this is a complication if you want to swap it with a snapshot. I think that it would be useful if the mkfs.btrfs creates a "default" subvolume inside the root subvolume (id=5), and set it as default. So a subsequent snapshot+swap will be more easy. 

Ideally it would be useful if this could be the default behavior. But I suppose that someone (i.e. the distribution installer) could encounter some regressions.


BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 0/1] btrfs-progs: mkfs: add subvolume support to mkfs

[Anand Jain]

> Add -S/--subvol [NAME] option to configure. It enables users to create a
> subvolume under the toplevel volume 

 > and populate the created subvolume
 > with files from the rootdir specified by -r/--rootdir option.

  This brings two enhancements, those might be good ideas, but stating a 
specific use case will add the required clarity.

Thanks, Anand

[PATCH 0/4 v2] add subvolume support to mkfs

[Yingyi Luo]

From: yingyil <yingyil@google.com>

Hi all,

Thanks for all the comments you gave to me. I really appriciate it. I've
updated the patch, split it into small ones so that it may be easier
for you to see the changes and review.

One use case of this feature is that it allows easier operations on subvolumes,
such as migrating subvolumes across images. With the current btrfs tool, we have
to do this manually by creating an empty subvolume then moving the files over.
This feature which creates a subvolume for a given directory when making btrfs
images simplifies the process and makes the logic cleaner.

Goffredo: Thanks again for you comments and pointing out the bug. It happened
to be that I skipped the error code checking when I tested the functionality.
The error came from a duplicate call of btrfs_make_root_dir() from
make_root_dir() in mkfs/main.c and create_subvol() in convert/main.c.

strdup() is used to get a returnable copy of the literal string.
FYI, parse_label() used strdup() to parse the input string. I think using
strdup() would make it consistent with the current code.

Extending the feature would be a good idea. I'll try to work on it later.

Best regards,
Yingyi

yingyil (4):
  btrfs-progs: convert: move link_subvol out of main
  btrfs-progs: add a parameter to btrfs_link_subvol
  btrfs-progs: mkfs: refactor create_data_reloc_tree
  btrfs-progs: mkfs: add subvolume support to mkfs

 convert/main.c | 125 +------------------------------------------------------
 mkfs/main.c    |  82 ++++++++++++++++++++++++++++++++----
 utils.c        | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h        |   3 ++
 4 files changed, 205 insertions(+), 133 deletions(-)

-- 
2.14.1.342.g6490525c54-goog

[PATCH 1/4 v2] btrfs-progs: convert: move link_subvol out of main

[Yingyi Luo]

From: yingyil <yingyil@google.com>

link_subvol() is moved to utils.c and renamed as btrfs_link_subvol().
The change cascades down to the callchain.

Signed-off-by: yingyil <yingyil@google.com>
---
v2: split the original patch to make code movement into a separate patch.

 convert/main.c | 125 +--------------------------------------------------------
 utils.c        | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utils.h        |   3 ++
 3 files changed, 127 insertions(+), 124 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 03da9e49..8f7fec25 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -834,129 +834,6 @@ out:
 	return ret;
 }
 
-static struct btrfs_root* link_subvol(struct btrfs_root *root,
-		const char *base, u64 root_objectid)
-{
-	struct btrfs_trans_handle *trans;
-	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct btrfs_root *tree_root = fs_info->tree_root;
-	struct btrfs_root *new_root = NULL;
-	struct btrfs_path path;
-	struct btrfs_inode_item *inode_item;
-	struct extent_buffer *leaf;
-	struct btrfs_key key;
-	u64 dirid = btrfs_root_dirid(&root->root_item);
-	u64 index = 2;
-	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
-	int len;
-	int i;
-	int ret;
-
-	len = strlen(base);
-	if (len == 0 || len > BTRFS_NAME_LEN)
-		return NULL;
-
-	btrfs_init_path(&path);
-	key.objectid = dirid;
-	key.type = BTRFS_DIR_INDEX_KEY;
-	key.offset = (u64)-1;
-
-	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
-	if (ret <= 0) {
-		error("search for DIR_INDEX dirid %llu failed: %d",
-				(unsigned long long)dirid, ret);
-		goto fail;
-	}
-
-	if (path.slots[0] > 0) {
-		path.slots[0]--;
-		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
-		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
-			index = key.offset + 1;
-	}
-	btrfs_release_path(&path);
-
-	trans = btrfs_start_transaction(root, 1);
-	if (!trans) {
-		error("unable to start transaction");
-		goto fail;
-	}
-
-	key.objectid = dirid;
-	key.offset = 0;
-	key.type =  BTRFS_INODE_ITEM_KEY;
-
-	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
-	if (ret) {
-		error("search for INODE_ITEM %llu failed: %d",
-				(unsigned long long)dirid, ret);
-		goto fail;
-	}
-	leaf = path.nodes[0];
-	inode_item = btrfs_item_ptr(leaf, path.slots[0],
-				    struct btrfs_inode_item);
-
-	key.objectid = root_objectid;
-	key.offset = (u64)-1;
-	key.type = BTRFS_ROOT_ITEM_KEY;
-
-	memcpy(buf, base, len);
-	for (i = 0; i < 1024; i++) {
-		ret = btrfs_insert_dir_item(trans, root, buf, len,
-					    dirid, &key, BTRFS_FT_DIR, index);
-		if (ret != -EEXIST)
-			break;
-		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
-		if (len < 1 || len > BTRFS_NAME_LEN) {
-			ret = -EINVAL;
-			break;
-		}
-	}
-	if (ret)
-		goto fail;
-
-	btrfs_set_inode_size(leaf, inode_item, len * 2 +
-			     btrfs_inode_size(leaf, inode_item));
-	btrfs_mark_buffer_dirty(leaf);
-	btrfs_release_path(&path);
-
-	/* add the backref first */
-	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
-				 BTRFS_ROOT_BACKREF_KEY,
-				 root->root_key.objectid,
-				 dirid, index, buf, len);
-	if (ret) {
-		error("unable to add root backref for %llu: %d",
-				root->root_key.objectid, ret);
-		goto fail;
-	}
-
-	/* now add the forward ref */
-	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
-				 BTRFS_ROOT_REF_KEY, root_objectid,
-				 dirid, index, buf, len);
-	if (ret) {
-		error("unable to add root ref for %llu: %d",
-				root->root_key.objectid, ret);
-		goto fail;
-	}
-
-	ret = btrfs_commit_transaction(trans, root);
-	if (ret) {
-		error("transaction commit failed: %d", ret);
-		goto fail;
-	}
-
-	new_root = btrfs_read_fs_root(fs_info, &key);
-	if (IS_ERR(new_root)) {
-		error("unable to fs read root: %lu", PTR_ERR(new_root));
-		new_root = NULL;
-	}
-fail:
-	btrfs_init_path(&path);
-	return new_root;
-}
-
 static int create_subvol(struct btrfs_trans_handle *trans,
 			 struct btrfs_root *root, u64 root_objectid)
 {
@@ -1315,7 +1192,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		task_deinit(ctx.info);
 	}
 
-	image_root = link_subvol(root, subvol_name, CONV_IMAGE_SUBVOL_OBJECTID);
+	image_root = btrfs_link_subvol(root, subvol_name, CONV_IMAGE_SUBVOL_OBJECTID);
 	if (!image_root) {
 		error("unable to link subvolume %s", subvol_name);
 		goto fail;
diff --git a/utils.c b/utils.c
index bb049133..32382328 100644
--- a/utils.c
+++ b/utils.c
@@ -2574,3 +2574,126 @@ u8 rand_u8(void)
 void btrfs_config_init(void)
 {
 }
+
+struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root,
+		const char *base, u64 root_objectid)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_root *new_root = NULL;
+	struct btrfs_path path;
+	struct btrfs_inode_item *inode_item;
+	struct extent_buffer *leaf;
+	struct btrfs_key key;
+	u64 dirid = btrfs_root_dirid(&root->root_item);
+	u64 index = 2;
+	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
+	int len;
+	int i;
+	int ret;
+
+	len = strlen(base);
+	if (len == 0 || len > BTRFS_NAME_LEN)
+		return NULL;
+
+	btrfs_init_path(&path);
+	key.objectid = dirid;
+	key.type = BTRFS_DIR_INDEX_KEY;
+	key.offset = (u64)-1;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret <= 0) {
+		error("search for DIR_INDEX dirid %llu failed: %d",
+				(unsigned long long)dirid, ret);
+		goto fail;
+	}
+
+	if (path.slots[0] > 0) {
+		path.slots[0]--;
+		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
+			index = key.offset + 1;
+	}
+	btrfs_release_path(&path);
+
+	trans = btrfs_start_transaction(root, 1);
+	if (!trans) {
+		error("unable to start transaction");
+		goto fail;
+	}
+
+	key.objectid = dirid;
+	key.offset = 0;
+	key.type =  BTRFS_INODE_ITEM_KEY;
+
+	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
+	if (ret) {
+		error("search for INODE_ITEM %llu failed: %d",
+				(unsigned long long)dirid, ret);
+		goto fail;
+	}
+	leaf = path.nodes[0];
+	inode_item = btrfs_item_ptr(leaf, path.slots[0],
+				    struct btrfs_inode_item);
+
+	key.objectid = root_objectid;
+	key.offset = (u64)-1;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+
+	memcpy(buf, base, len);
+	for (i = 0; i < 1024; i++) {
+		ret = btrfs_insert_dir_item(trans, root, buf, len, dirid,
+					    &key, BTRFS_FT_DIR, index);
+		if (ret != -EEXIST)
+			break;
+		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
+		if (len < 1 || len > BTRFS_NAME_LEN) {
+			ret = -EINVAL;
+			break;
+		}
+	}
+	if (ret)
+		goto fail;
+
+	btrfs_set_inode_size(leaf, inode_item, len * 2 +
+			     btrfs_inode_size(leaf, inode_item));
+	btrfs_mark_buffer_dirty(leaf);
+	btrfs_release_path(&path);
+
+	/* add the backref first */
+	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
+				 BTRFS_ROOT_BACKREF_KEY,
+				 root->root_key.objectid,
+				 dirid, index, buf, len);
+	if (ret) {
+		error("unable to add root backref for %llu: %d",
+				root->root_key.objectid, ret);
+		goto fail;
+	}
+
+	/* now add the forward ref */
+	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
+				 BTRFS_ROOT_REF_KEY, root_objectid,
+				 dirid, index, buf, len);
+	if (ret) {
+		error("unable to add root ref for %llu: %d",
+				root->root_key.objectid, ret);
+		goto fail;
+	}
+
+	ret = btrfs_commit_transaction(trans, root);
+	if (ret) {
+		error("transaction commit failed: %d", ret);
+		goto fail;
+	}
+
+	new_root = btrfs_read_fs_root(fs_info, &key);
+	if (IS_ERR(new_root)) {
+		error("unable to fs read root: %lu", PTR_ERR(new_root));
+		new_root = NULL;
+	}
+fail:
+	btrfs_init_path(&path);
+	return new_root;
+}
diff --git a/utils.h b/utils.h
index 091f8fab..886590fd 100644
--- a/utils.h
+++ b/utils.h
@@ -170,4 +170,7 @@ u64 rand_u64(void);
 unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 
+struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root, const char *base,
+				     u64 root_objectid);
+
 #endif
-- 
2.14.1.342.g6490525c54-goog

Re: [PATCH 1/4 v2] btrfs-progs: convert: move link_subvol out of main

[David Sterba]

On Wed, Aug 30, 2017 at 10:24:47AM -0700, Yingyi Luo wrote:
> From: yingyil <yingyil@google.com>
> 
> link_subvol() is moved to utils.c and renamed as btrfs_link_subvol().

The file utils.c is a catch-all file, but the subvolume function belongs
to inode features, similar as in the kernel sources (although there it's
in ioctl.c . Currently link_subvol is misleading name, as it's really
"create_subvol". Please rename it to btrfs_mksubvol and move it to
inode.c.

[PATCH 1/4 v3] btrfs-progs: convert: move link_subvol out of main

[Yingyi Luo]

From: yingyil <yingyil@google.com>

link_subvol() is moved to inode.c and renamed as btrfs_mksubvol().
The change cascades down to the callchain.

Signed-off-by: yingyil <yingyil@google.com>
---
v3: moved link_subvol to inode.c and put its header in ctree.h. The name is
changed to btrfs_mksubvol. Thanks for your comments.

 convert/main.c | 126 +--------------------------------------------------------
 ctree.h        |   2 +
 inode.c        | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+), 124 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 7ec6202d..0babf0e8 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -838,129 +838,6 @@ out:
 	return ret;
 }
 
-static struct btrfs_root* link_subvol(struct btrfs_root *root,
-		const char *base, u64 root_objectid)
-{
-	struct btrfs_trans_handle *trans;
-	struct btrfs_fs_info *fs_info = root->fs_info;
-	struct btrfs_root *tree_root = fs_info->tree_root;
-	struct btrfs_root *new_root = NULL;
-	struct btrfs_path path;
-	struct btrfs_inode_item *inode_item;
-	struct extent_buffer *leaf;
-	struct btrfs_key key;
-	u64 dirid = btrfs_root_dirid(&root->root_item);
-	u64 index = 2;
-	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
-	int len;
-	int i;
-	int ret;
-
-	len = strlen(base);
-	if (len == 0 || len > BTRFS_NAME_LEN)
-		return NULL;
-
-	btrfs_init_path(&path);
-	key.objectid = dirid;
-	key.type = BTRFS_DIR_INDEX_KEY;
-	key.offset = (u64)-1;
-
-	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
-	if (ret <= 0) {
-		error("search for DIR_INDEX dirid %llu failed: %d",
-				(unsigned long long)dirid, ret);
-		goto fail;
-	}
-
-	if (path.slots[0] > 0) {
-		path.slots[0]--;
-		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
-		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
-			index = key.offset + 1;
-	}
-	btrfs_release_path(&path);
-
-	trans = btrfs_start_transaction(root, 1);
-	if (IS_ERR(trans)) {
-		error("unable to start transaction");
-		goto fail;
-	}
-
-	key.objectid = dirid;
-	key.offset = 0;
-	key.type =  BTRFS_INODE_ITEM_KEY;
-
-	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
-	if (ret) {
-		error("search for INODE_ITEM %llu failed: %d",
-				(unsigned long long)dirid, ret);
-		goto fail;
-	}
-	leaf = path.nodes[0];
-	inode_item = btrfs_item_ptr(leaf, path.slots[0],
-				    struct btrfs_inode_item);
-
-	key.objectid = root_objectid;
-	key.offset = (u64)-1;
-	key.type = BTRFS_ROOT_ITEM_KEY;
-
-	memcpy(buf, base, len);
-	for (i = 0; i < 1024; i++) {
-		ret = btrfs_insert_dir_item(trans, root, buf, len,
-					    dirid, &key, BTRFS_FT_DIR, index);
-		if (ret != -EEXIST)
-			break;
-		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
-		if (len < 1 || len > BTRFS_NAME_LEN) {
-			ret = -EINVAL;
-			break;
-		}
-	}
-	if (ret)
-		goto fail;
-
-	btrfs_set_inode_size(leaf, inode_item, len * 2 +
-			     btrfs_inode_size(leaf, inode_item));
-	btrfs_mark_buffer_dirty(leaf);
-	btrfs_release_path(&path);
-
-	/* add the backref first */
-	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
-				 BTRFS_ROOT_BACKREF_KEY,
-				 root->root_key.objectid,
-				 dirid, index, buf, len);
-	if (ret) {
-		error("unable to add root backref for %llu: %d",
-				root->root_key.objectid, ret);
-		goto fail;
-	}
-
-	/* now add the forward ref */
-	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
-				 BTRFS_ROOT_REF_KEY, root_objectid,
-				 dirid, index, buf, len);
-	if (ret) {
-		error("unable to add root ref for %llu: %d",
-				root->root_key.objectid, ret);
-		goto fail;
-	}
-
-	ret = btrfs_commit_transaction(trans, root);
-	if (ret) {
-		error("transaction commit failed: %d", ret);
-		goto fail;
-	}
-
-	new_root = btrfs_read_fs_root(fs_info, &key);
-	if (IS_ERR(new_root)) {
-		error("unable to fs read root: %lu", PTR_ERR(new_root));
-		new_root = NULL;
-	}
-fail:
-	btrfs_init_path(&path);
-	return new_root;
-}
-
 static int create_subvol(struct btrfs_trans_handle *trans,
 			 struct btrfs_root *root, u64 root_objectid)
 {
@@ -1319,7 +1196,8 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		task_deinit(ctx.info);
 	}
 
-	image_root = link_subvol(root, subvol_name, CONV_IMAGE_SUBVOL_OBJECTID);
+	image_root = btrfs_mksubvol(root, subvol_name,
+				    CONV_IMAGE_SUBVOL_OBJECTID);
 	if (!image_root) {
 		error("unable to link subvolume %s", subvol_name);
 		goto fail;
diff --git a/ctree.h b/ctree.h
index 81c193f1..22313576 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2750,6 +2750,8 @@ int btrfs_add_orphan_item(struct btrfs_trans_handle *trans,
 			  u64 ino);
 int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		char *name, int namelen, u64 parent_ino, u64 *ino, int mode);
+struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root, const char *base,
+				  u64 root_objectid);
 
 /* file.c */
 int btrfs_get_extent(struct btrfs_trans_handle *trans,
diff --git a/inode.c b/inode.c
index 6b8bf40f..5dd816bf 100644
--- a/inode.c
+++ b/inode.c
@@ -28,6 +28,7 @@
 #include "transaction.h"
 #include "disk-io.h"
 #include "time.h"
+#include "messages.h"
 
 /*
  * Find a free inode index for later btrfs_add_link().
@@ -570,3 +571,126 @@ out:
 		*ino = ret_ino;
 	return ret;
 }
+
+struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
+				  const char *base, u64 root_objectid)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_root *new_root = NULL;
+	struct btrfs_path path;
+	struct btrfs_inode_item *inode_item;
+	struct extent_buffer *leaf;
+	struct btrfs_key key;
+	u64 dirid = btrfs_root_dirid(&root->root_item);
+	u64 index = 2;
+	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
+	int len;
+	int i;
+	int ret;
+
+	len = strlen(base);
+	if (len == 0 || len > BTRFS_NAME_LEN)
+		return NULL;
+
+	btrfs_init_path(&path);
+	key.objectid = dirid;
+	key.type = BTRFS_DIR_INDEX_KEY;
+	key.offset = (u64)-1;
+
+	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
+	if (ret <= 0) {
+		error("search for DIR_INDEX dirid %llu failed: %d",
+				(unsigned long long)dirid, ret);
+		goto fail;
+	}
+
+	if (path.slots[0] > 0) {
+		path.slots[0]--;
+		btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+		if (key.objectid == dirid && key.type == BTRFS_DIR_INDEX_KEY)
+			index = key.offset + 1;
+	}
+	btrfs_release_path(&path);
+
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		error("unable to start transaction");
+		goto fail;
+	}
+
+	key.objectid = dirid;
+	key.offset = 0;
+	key.type =  BTRFS_INODE_ITEM_KEY;
+
+	ret = btrfs_lookup_inode(trans, root, &path, &key, 1);
+	if (ret) {
+		error("search for INODE_ITEM %llu failed: %d",
+				(unsigned long long)dirid, ret);
+		goto fail;
+	}
+	leaf = path.nodes[0];
+	inode_item = btrfs_item_ptr(leaf, path.slots[0],
+				    struct btrfs_inode_item);
+
+	key.objectid = root_objectid;
+	key.offset = (u64)-1;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+
+	memcpy(buf, base, len);
+	for (i = 0; i < 1024; i++) {
+		ret = btrfs_insert_dir_item(trans, root, buf, len,
+					    dirid, &key, BTRFS_FT_DIR, index);
+		if (ret != -EEXIST)
+			break;
+		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
+		if (len < 1 || len > BTRFS_NAME_LEN) {
+			ret = -EINVAL;
+			break;
+		}
+	}
+	if (ret)
+		goto fail;
+
+	btrfs_set_inode_size(leaf, inode_item, len * 2 +
+			     btrfs_inode_size(leaf, inode_item));
+	btrfs_mark_buffer_dirty(leaf);
+	btrfs_release_path(&path);
+
+	/* add the backref first */
+	ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
+				 BTRFS_ROOT_BACKREF_KEY,
+				 root->root_key.objectid,
+				 dirid, index, buf, len);
+	if (ret) {
+		error("unable to add root backref for %llu: %d",
+				root->root_key.objectid, ret);
+		goto fail;
+	}
+
+	/* now add the forward ref */
+	ret = btrfs_add_root_ref(trans, tree_root, root->root_key.objectid,
+				 BTRFS_ROOT_REF_KEY, root_objectid,
+				 dirid, index, buf, len);
+	if (ret) {
+		error("unable to add root ref for %llu: %d",
+				root->root_key.objectid, ret);
+		goto fail;
+	}
+
+	ret = btrfs_commit_transaction(trans, root);
+	if (ret) {
+		error("transaction commit failed: %d", ret);
+		goto fail;
+	}
+
+	new_root = btrfs_read_fs_root(fs_info, &key);
+	if (IS_ERR(new_root)) {
+		error("unable to fs read root: %lu", PTR_ERR(new_root));
+		new_root = NULL;
+	}
+fail:
+	btrfs_init_path(&path);
+	return new_root;
+}
-- 
2.14.1.690.gbb1197296e-goog

[PATCH 2/4 v2] btrfs-progs: add a parameter to btrfs_link_subvol

[Yingyi Luo]

From: yingyil <yingyil@google.com>

A convert parameter is added as a flag to indicate if btrfs_link_subvol()
is used for btrfs-convert. The change cascades down to the callchain.

Signed-off-by: yingyil <yingyil@google.com>
---
v2: Added a flag for btrfs_link_subvol() function so that it can be used in a
more general way. For example, if mkfs calls this function, it will return
directly with an error code if the subvolume creation fails.

 convert/main.c |  2 +-
 utils.c        | 25 +++++++++++++++----------
 utils.h        |  2 +-
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 8f7fec25..19a60782 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1192,7 +1192,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		task_deinit(ctx.info);
 	}
 
-	image_root = btrfs_link_subvol(root, subvol_name, CONV_IMAGE_SUBVOL_OBJECTID);
+	image_root = btrfs_link_subvol(root, subvol_name, CONV_IMAGE_SUBVOL_OBJECTID, 1);
 	if (!image_root) {
 		error("unable to link subvolume %s", subvol_name);
 		goto fail;
diff --git a/utils.c b/utils.c
index 32382328..20ea9e74 100644
--- a/utils.c
+++ b/utils.c
@@ -2576,7 +2576,7 @@ void btrfs_config_init(void)
 }
 
 struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root,
-		const char *base, u64 root_objectid)
+		const char *base, u64 root_objectid, int convert)
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -2642,16 +2642,21 @@ struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root,
 	key.type = BTRFS_ROOT_ITEM_KEY;
 
 	memcpy(buf, base, len);
-	for (i = 0; i < 1024; i++) {
-		ret = btrfs_insert_dir_item(trans, root, buf, len, dirid,
-					    &key, BTRFS_FT_DIR, index);
-		if (ret != -EEXIST)
-			break;
-		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
-		if (len < 1 || len > BTRFS_NAME_LEN) {
-			ret = -EINVAL;
-			break;
+	if (convert) {
+		for (i = 0; i < 1024; i++) {
+			ret = btrfs_insert_dir_item(trans, root, buf, len, dirid,
+						    &key, BTRFS_FT_DIR, index);
+			if (ret != -EEXIST)
+				break;
+			len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
+			if (len < 1 || len > BTRFS_NAME_LEN) {
+				ret = -EINVAL;
+				break;
+			}
 		}
+	} else {
+		ret = btrfs_insert_dir_item(trans, root, buf, len, dirid, &key,
+					    BTRFS_FT_DIR, index);
 	}
 	if (ret)
 		goto fail;
diff --git a/utils.h b/utils.h
index 886590fd..28aa8373 100644
--- a/utils.h
+++ b/utils.h
@@ -171,6 +171,6 @@ unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 
 struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root, const char *base,
-				     u64 root_objectid);
+				     u64 root_objectid, int convert);
 
 #endif
-- 
2.14.1.342.g6490525c54-goog

Re: [PATCH 2/4 v2] btrfs-progs: add a parameter to btrfs_link_subvol

[David Sterba]

On Wed, Aug 30, 2017 at 10:24:48AM -0700, Yingyi Luo wrote:
> From: yingyil <yingyil@google.com>
> 
> A convert parameter is added as a flag to indicate if btrfs_link_subvol()
> is used for btrfs-convert. The change cascades down to the callchain.
> 
> Signed-off-by: yingyil <yingyil@google.com>
> ---
> v2: Added a flag for btrfs_link_subvol() function so that it can be used in a
> more general way. For example, if mkfs calls this function, it will return
> directly with an error code if the subvolume creation fails.
> 
>  convert/main.c |  2 +-
>  utils.c        | 25 +++++++++++++++----------
>  utils.h        |  2 +-
>  3 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/convert/main.c b/convert/main.c
> index 8f7fec25..19a60782 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -1192,7 +1192,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
>  		task_deinit(ctx.info);
>  	}
>  
> -	image_root = btrfs_link_subvol(root, subvol_name, CONV_IMAGE_SUBVOL_OBJECTID);
> +	image_root = btrfs_link_subvol(root, subvol_name, CONV_IMAGE_SUBVOL_OBJECTID, 1);
>  	if (!image_root) {
>  		error("unable to link subvolume %s", subvol_name);
>  		goto fail;
> diff --git a/utils.c b/utils.c
> index 32382328..20ea9e74 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -2576,7 +2576,7 @@ void btrfs_config_init(void)
>  }
>  
>  struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root,
> -		const char *base, u64 root_objectid)
> +		const char *base, u64 root_objectid, int convert)
>  {
>  	struct btrfs_trans_handle *trans;
>  	struct btrfs_fs_info *fs_info = root->fs_info;
> @@ -2642,16 +2642,21 @@ struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root,
>  	key.type = BTRFS_ROOT_ITEM_KEY;
>  
>  	memcpy(buf, base, len);
> -	for (i = 0; i < 1024; i++) {
> -		ret = btrfs_insert_dir_item(trans, root, buf, len, dirid,
> -					    &key, BTRFS_FT_DIR, index);
> -		if (ret != -EEXIST)
> -			break;
> -		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
> -		if (len < 1 || len > BTRFS_NAME_LEN) {
> -			ret = -EINVAL;
> -			break;
> +	if (convert) {
> +		for (i = 0; i < 1024; i++) {
> +			ret = btrfs_insert_dir_item(trans, root, buf, len, dirid,
> +						    &key, BTRFS_FT_DIR, index);
> +			if (ret != -EEXIST)
> +				break;
> +			len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
> +			if (len < 1 || len > BTRFS_NAME_LEN) {
> +				ret = -EINVAL;
> +				break;
> +			}
>  		}
> +	} else {
> +		ret = btrfs_insert_dir_item(trans, root, buf, len, dirid, &key,
> +					    BTRFS_FT_DIR, index);
>  	}
>  	if (ret)
>  		goto fail;
> diff --git a/utils.h b/utils.h
> index 886590fd..28aa8373 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -171,6 +171,6 @@ unsigned int rand_range(unsigned int upper);
>  void init_rand_seed(u64 seed);
>  
>  struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root, const char *base,
> -				     u64 root_objectid);
> +				     u64 root_objectid, int convert);

Please make it 'bool', otherwise ok.

[PATCH 2/4 v3] btrfs-progs: add a parameter to btrfs_mksubvol

[Yingyi Luo]

From: yingyil <yingyil@google.com>

A convert parameter is added as a flag to indicate if btrfs_mksubvol()
is used for btrfs-convert. The change cascades down to the callchain.

Signed-off-by: yingyil <yingyil@google.com>
---
v3: changed the convert flag from int type to bool type.

 convert/main.c |  2 +-
 ctree.h        |  4 +++-
 inode.c        | 26 ++++++++++++++++----------
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/convert/main.c b/convert/main.c
index 0babf0e8..882daf7c 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1197,7 +1197,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 	}
 
 	image_root = btrfs_mksubvol(root, subvol_name,
-				    CONV_IMAGE_SUBVOL_OBJECTID);
+				    CONV_IMAGE_SUBVOL_OBJECTID, true);
 	if (!image_root) {
 		error("unable to link subvolume %s", subvol_name);
 		goto fail;
diff --git a/ctree.h b/ctree.h
index 22313576..ae25eed5 100644
--- a/ctree.h
+++ b/ctree.h
@@ -19,6 +19,8 @@
 #ifndef __BTRFS_CTREE_H__
 #define __BTRFS_CTREE_H__
 
+#include <stdbool.h>
+
 #if BTRFS_FLAT_INCLUDES
 #include "list.h"
 #include "kerncompat.h"
@@ -2751,7 +2753,7 @@ int btrfs_add_orphan_item(struct btrfs_trans_handle *trans,
 int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		char *name, int namelen, u64 parent_ino, u64 *ino, int mode);
 struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root, const char *base,
-				  u64 root_objectid);
+				  u64 root_objectid, bool convert);
 
 /* file.c */
 int btrfs_get_extent(struct btrfs_trans_handle *trans,
diff --git a/inode.c b/inode.c
index 5dd816bf..ea812ce8 100644
--- a/inode.c
+++ b/inode.c
@@ -573,7 +573,8 @@ out:
 }
 
 struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
-				  const char *base, u64 root_objectid)
+				  const char *base, u64 root_objectid,
+				  bool convert)
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -639,16 +640,21 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
 	key.type = BTRFS_ROOT_ITEM_KEY;
 
 	memcpy(buf, base, len);
-	for (i = 0; i < 1024; i++) {
-		ret = btrfs_insert_dir_item(trans, root, buf, len,
-					    dirid, &key, BTRFS_FT_DIR, index);
-		if (ret != -EEXIST)
-			break;
-		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
-		if (len < 1 || len > BTRFS_NAME_LEN) {
-			ret = -EINVAL;
-			break;
+	if (convert) {
+		for (i = 0; i < 1024; i++) {
+			ret = btrfs_insert_dir_item(trans, root, buf, len,
+					dirid, &key, BTRFS_FT_DIR, index);
+			if (ret != -EEXIST)
+				break;
+			len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
+			if (len < 1 || len > BTRFS_NAME_LEN) {
+				ret = -EINVAL;
+				break;
+			}
 		}
+	} else {
+		ret = btrfs_insert_dir_item(trans, root, buf, len, dirid, &key,
+					    BTRFS_FT_DIR, index);
 	}
 	if (ret)
 		goto fail;
-- 
2.14.1.690.gbb1197296e-goog

[PATCH 3/4 v2] btrfs-progs: mkfs: refactor create_data_reloc_tree

[Yingyi Luo]

From: yingyil <yingyil@google.com>

Add an objectid parameter to make the function a general one for
inserting root items and rename it to create_tree. The change cascades
down to the callchain.

Signed-off-by: yingyil <yingyil@google.com>
---
v2: utilize create_tree() function for creating a subvolume, instead of
create_subvol() in convert/main.c.

 mkfs/main.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index afd68bc5..869c11fa 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -320,13 +320,12 @@ static int create_raid_groups(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-static int create_data_reloc_tree(struct btrfs_trans_handle *trans,
-				  struct btrfs_root *root)
+static int create_tree(struct btrfs_trans_handle *trans,
+			struct btrfs_root *root, u64 objectid)
 {
 	struct btrfs_key location;
 	struct btrfs_root_item root_item;
 	struct extent_buffer *tmp;
-	u64 objectid = BTRFS_DATA_RELOC_TREE_OBJECTID;
 	int ret;
 
 	ret = btrfs_copy_root(trans, root, root->node, &tmp, objectid);
@@ -1846,7 +1845,7 @@ raid_groups:
 		}
 	}
 
-	ret = create_data_reloc_tree(trans, root);
+	ret = create_tree(trans, root, BTRFS_DATA_RELOC_TREE_OBJECTID);
 	if (ret) {
 		error("unable to create data reloc tree: %d", ret);
 		goto out;
-- 
2.14.1.342.g6490525c54-goog

Re: [PATCH 3/4 v2] btrfs-progs: mkfs: refactor create_data_reloc_tree

[David Sterba]

On Wed, Aug 30, 2017 at 10:24:49AM -0700, Yingyi Luo wrote:
> From: yingyil <yingyil@google.com>
> 
> Add an objectid parameter to make the function a general one for
> inserting root items and rename it to create_tree. The change cascades
> down to the callchain.
> 
> Signed-off-by: yingyil <yingyil@google.com>

The patch is independent of the others, applied. Thanks.

[PATCH 4/4 v2] btrfs-progs: mkfs: add subvolume support to mkfs

[Yingyi Luo]

From: yingyil <yingyil@google.com>

Add -S/--subvol [NAME] option to configure. It enables users to create a
subvolume under the toplevel volume and populate the created subvolume
with files from the rootdir specified by -r/--rootdir option.

Signed-off-by: yingyil <yingyil@google.com>
---
v2: Fixed the bug for subvolume creation failure: calling create_tree(), instead
of create_subvol() in convert/main.c since btrfs_make_root_dir() in
create_subvol() is not needed to be called in mkfs.

 mkfs/main.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 5 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index 869c11fa..21abc4a4 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -363,6 +363,7 @@ static void print_usage(int ret)
 	printf("\t-U|--uuid UUID          specify the filesystem UUID (must be unique)\n");
 	printf("  creation:\n");
 	printf("\t-b|--byte-count SIZE    set filesystem size to SIZE (on the first device)\n");
+	printf("\t-S|--subvol NAME        create a sunvolume with NAME and copy files from ROOTDIR to the subvolume\n");
 	printf("\t-r|--rootdir DIR        copy files from DIR to the image root directory\n");
 	printf("\t-K|--nodiscard          do not perform whole device TRIM\n");
 	printf("\t-f|--force              force overwrite of existing filesystem\n");
@@ -412,6 +413,18 @@ static char *parse_label(const char *input)
 	return strdup(input);
 }
 
+static char *parse_subvol_name(const char *input)
+{
+	int len = strlen(input);
+
+	if (len >= BTRFS_SUBVOL_NAME_MAX) {
+		error("subvolume name %s is too long (max %d)",
+			input, BTRFS_SUBVOL_NAME_MAX - 1);
+		exit(1);
+	}
+	return strdup(input);
+}
+
 static int add_directory_items(struct btrfs_trans_handle *trans,
 			       struct btrfs_root *root, u64 objectid,
 			       ino_t parent_inum, const char *name,
@@ -1418,6 +1431,8 @@ int main(int argc, char **argv)
 	int discard = 1;
 	int ssd = 0;
 	int force_overwrite = 0;
+	char *subvol_name = NULL;
+	int subvol_name_set = 0;
 	char *source_dir = NULL;
 	int source_dir_set = 0;
 	u64 num_of_meta_chunks = 0;
@@ -1444,6 +1459,7 @@ int main(int argc, char **argv)
 			{ "sectorsize", required_argument, NULL, 's' },
 			{ "data", required_argument, NULL, 'd' },
 			{ "version", no_argument, NULL, 'V' },
+			{ "subvol", required_argument, NULL, 'S'},
 			{ "rootdir", required_argument, NULL, 'r' },
 			{ "nodiscard", no_argument, NULL, 'K' },
 			{ "features", required_argument, NULL, 'O' },
@@ -1453,7 +1469,7 @@ int main(int argc, char **argv)
 			{ NULL, 0, NULL, 0}
 		};
 
-		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:r:U:VMKq",
+		c = getopt_long(argc, argv, "A:b:fl:n:s:m:d:L:O:S:r:U:VMKq",
 				long_options, NULL);
 		if (c < 0)
 			break;
@@ -1514,6 +1530,10 @@ int main(int argc, char **argv)
 				printf("mkfs.btrfs, part of %s\n",
 						PACKAGE_STRING);
 				goto success;
+			case 'S':
+				subvol_name = parse_subvol_name(optarg);
+				subvol_name_set = 1;
+				break;
 			case 'r':
 				source_dir = optarg;
 				source_dir_set = 1;
@@ -1534,6 +1554,11 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if (subvol_name_set && !source_dir_set) {
+		error("root directory needs to be set");
+		goto error;
+	}
+
 	if (verbose) {
 		printf("%s\n", PACKAGE_STRING);
 		printf("See %s for more information.\n\n", PACKAGE_URL);
@@ -1872,10 +1897,48 @@ raid_groups:
 			goto out;
 		}
 
-		ret = make_image(source_dir, root);
-		if (ret) {
-			error("error wihle filling filesystem: %d", ret);
-			goto out;
+		if (subvol_name_set) {
+			u64 dirid, objectid;
+			struct btrfs_root *file_root;
+
+			dirid = btrfs_root_dirid(&fs_info->tree_root->root_item);
+			ret = btrfs_find_free_objectid(NULL, fs_info->tree_root, dirid, &objectid);
+			if (ret) {
+				error("unable to find a free objectid: %d", ret);
+				goto out;
+			}
+			trans = btrfs_start_transaction(root, 1);
+			if (!trans) {
+				error("unable to start transaction");
+				ret = -EINVAL;
+				goto out;
+			}
+			ret = create_tree(trans, root, objectid);
+			if (ret < 0) {
+				error("failed to create subvolume: %d", ret);
+				goto out;
+			}
+			ret = btrfs_commit_transaction(trans, root);
+			if (ret) {
+				error("unable to commit transaction: %d", ret);
+				goto out;
+			}
+			file_root = btrfs_link_subvol(root, subvol_name, objectid, 0);
+			if (!file_root) {
+				error("unable to link the subvolume %s", subvol_name);
+				goto out;
+			}
+			ret = make_image(source_dir, file_root);
+			if (ret) {
+				error("error while filling filesystem: %d", ret);
+				goto out;
+			}
+		} else {
+			ret = make_image(source_dir, root);
+			if (ret) {
+				error("error while filling filesystem: %d", ret);
+				goto out;
+			}
 		}
 	}
 	ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
@@ -1938,6 +2001,7 @@ out:
 
 	btrfs_close_all_devices();
 	free(label);
+	free(subvol_name);
 
 	return !!ret;
 error:
@@ -1945,6 +2009,7 @@ error:
 		close(fd);
 
 	free(label);
+	free(subvol_name);
 	exit(1);
 success:
 	exit(0);
-- 
2.14.1.342.g6490525c54-goog

Re: [PATCH 4/4 v2] btrfs-progs: mkfs: add subvolume support to mkfs

[David Sterba]

On Wed, Aug 30, 2017 at 10:24:50AM -0700, Yingyi Luo wrote:
> From: yingyil <yingyil@google.com>
> 
> Add -S/--subvol [NAME] option to configure. It enables users to create a
> subvolume under the toplevel volume and populate the created subvolume
> with files from the rootdir specified by -r/--rootdir option.

Sounds good, I think we can achieve more than just subvol + copy of a
directory, with a extension of the command line ui. This should allow to
create multiple subvolumes, nested (or not) with contents of the
directory copied over any pre-created contents from --rootdir.

First, the option --subvol could be specified repeatedly. Each path
component that would not exist will become a directory, the last one
will be subvolume.

With --rootdir among the --subvol options, all existing paths would be
reused (as long files and directories/subvolumes don't clash).

Example, immitating one of used root filesystem layout:

expected result
/
/@ (subvolume
/@/boot (subvolume)
/@/home (subvolume)
/@/var/log (subvolume)
/@/tmp (subvolume)

and over that, same as if some tarball is unpacked, creating /bin,
/etc, /usr, /sbin etc.

would be generated by by something like:

$ mkfs.btrfs -S /@ -S /@/boot -S /@/home -S /@/var/log -S /@/tmp --rootdir rootfiles/ /path/to/device

Reusing --rootdir might be a bad idea, so we'd have add a new option
that would specify the target path and the source directory.

Your patchset is a good start and I'm not asking you to implement the
above, but to let you know that the end result might be more general
compared to the usecase that brought you here.

Re: [PATCH 0/4 v2] add subvolume support to mkfs

[David Sterba]

On Wed, Aug 30, 2017 at 10:24:46AM -0700, Yingyi Luo wrote:
> From: yingyil <yingyil@google.com>
> 
> Hi all,
> 
> Thanks for all the comments you gave to me. I really appriciate it. I've
> updated the patch, split it into small ones so that it may be easier
> for you to see the changes and review.
> 
> One use case of this feature is that it allows easier operations on subvolumes,
> such as migrating subvolumes across images. With the current btrfs tool, we have
> to do this manually by creating an empty subvolume then moving the files over.
> This feature which creates a subvolume for a given directory when making btrfs
> images simplifies the process and makes the logic cleaner.
> 
> Goffredo: Thanks again for you comments and pointing out the bug. It happened
> to be that I skipped the error code checking when I tested the functionality.
> The error came from a duplicate call of btrfs_make_root_dir() from
> make_root_dir() in mkfs/main.c and create_subvol() in convert/main.c.
> 
> strdup() is used to get a returnable copy of the literal string.
> FYI, parse_label() used strdup() to parse the input string. I think using
> strdup() would make it consistent with the current code.
> 
> Extending the feature would be a good idea. I'll try to work on it later.
> 
> Best regards,
> Yingyi
> 
> yingyil (4):
>   btrfs-progs: convert: move link_subvol out of main
>   btrfs-progs: add a parameter to btrfs_link_subvol
>   btrfs-progs: mkfs: refactor create_data_reloc_tree
>   btrfs-progs: mkfs: add subvolume support to mkfs

The updated patches have been added to devel. Before this will be
released, the propsed semantics of subvol/rootdir will have to be
implemented.

At the moment, overlaying files from rootdir over any previously created
subvolume does not work, there are probably missing checks for existing
dirs so this ends up with a duplicate entry.