btrfs-progs: mkfs: add subvolume support to mkfs

PATCH Superseded Yingyi Luo 5 participants 17 messages Aug 25, 2017 – Sep 29, 2017 Btrfs file system

lore

Share & tools

lore

Yingyi Luo

author

2017-08-25 23:25 UTC #0 lore raw

cover btrfs-progs: mkfs: add subvolume support to mkfs

From: yingyil <[email protected]>

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(-)

Signature · 1 line

2.14.1.342.g6490525c54-goog

Yingyi Luo

author

2017-08-25 23:25 UTC #1 lore raw

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

From: yingyil <[email protected]>

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

Signed-off-by yingyil <[email protected]>

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

convert/main.c +0 ~~-161~~

index 0deccd9..93609ed 100644
@@ -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.

mkfs/main.c +68 -5

121 lines not on this page · raw message

utils.c +161 -0

164 lines not on this page · raw message

utils.h +5 -0

index 091f8fa..777476f 100644
@@ -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

Signature · 1 line

2.14.1.342.g6490525c54-goog

Qu Wenruo

2017-08-27 23:39 UTC #2 lore raw

Quoted 8 lines · Yingyi Luo

From: yingyil <[email protected]>
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

Quoted 498 lines

Signed-off-by yingyil <[email protected]>

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

convert/main.c +0 ~~-161~~

index 0deccd9..93609ed 100644
@@ -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.

mkfs/main.c +68 -5

index 2b109a5..107cfc2 100644
@@ -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,

utils.c +161 -0

index bb04913..c9bbbed 100644
@@ -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;
+}

utils.h +5 -0

index 091f8fa..777476f 100644
@@ -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

Goffredo Baroncelli

2017-08-28 17:29 UTC #3 lore raw

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

Quoted 21 lines · Qu Wenruo

Quoted 8 lines · Yingyi Luo
From: yingyil <[email protected]>
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
Quoted 3 lines
Signed-off-by yingyil <[email protected]>
---

[...]

Quoted 15 lines

mkfs/main.c +2 -0

@@ -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)
+{

Quoted 8 lines

mkfs/main.c static char *parse_label(const char *input) · line 426

Quoted 8 lines

mkfs/main.c +8 -0

+    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 ?

Quoted 2 lines

Quoted 2 lines
mkfs/main.c +2 -0
+}
+

[...]

Quoted 11 lines

mkfs/main.c +4 -0

@@ -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;

Quoted 8 lines

mkfs/main.c int main(int argc, char **argv) · line 1563

Quoted 8 lines

mkfs/main.c +5 -0

@@ -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)

Quoted 6 lines

mkfs/main.c +0 -0

       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;
           }

[...]

Quoted 14 lines

ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,

utils.c +5 -0

index bb04913..c9bbbed 100644
@@ -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;

[....]

Quoted 5 lines

Quoted 5 lines
+
+    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);

Quoted 8 lines

Quoted 8 lines
+        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...

Quoted 7 lines

Quoted 7 lines
+    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

Signature · 2 lines

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

Anand Jain

2017-08-29 01:56 UTC #4 lore raw

In reply to Yingyi Luo

Quoted 7 lines

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

Yingyi Luo

author

2017-08-30 17:27 UTC #5 lore raw

v2 cover add subvolume support to mkfs

In reply to Yingyi Luo

From: yingyil <[email protected]>

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(-)

Signature · 1 line

2.14.1.342.g6490525c54-goog

Yingyi Luo

author

2017-08-30 17:28 UTC #6 lore raw

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

From: yingyil <[email protected]>

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

Signed-off-by yingyil <[email protected]>

---
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(-)

convert/main.c +1 ~~-124~~

index 03da9e49..8f7fec25 100644
@@ -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;

utils.c +123 -0

126 lines not on this page · raw message

utils.h +3 -0

index 091f8fab..886590fd 100644
@@ -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

Signature · 1 line

2.14.1.342.g6490525c54-goog

David Sterba

2017-09-12 17:27 UTC #7 lore raw

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

Quoted 3 lines · Yingyi Luo

From: yingyil <[email protected]>
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.

Yingyi Luo

author

2017-09-15 17:54 UTC #8 lore raw

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

In reply to Yingyi Luo

From: yingyil <[email protected]>

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

Signed-off-by yingyil <[email protected]>

---
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(-)

convert/main.c +2 ~~-124~~

index 7ec6202d..0babf0e8 100644
@@ -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;

ctree.h +2 -0

index 81c193f1..22313576 100644
@@ -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,

inode.c +124 -0

133 lines not on this page · raw message

Signature · 1 line

2.14.1.690.gbb1197296e-goog

Yingyi Luo

author

2017-08-30 17:28 UTC #9 lore raw

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

In reply to Yingyi Luo

From: yingyil <[email protected]>

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 <[email protected]>

---
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(-)

convert/main.c +1 -1

index 8f7fec25..19a60782 100644
@@ -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;

utils.c +15 ~~-10~~

index 32382328..20ea9e74 100644
@@ -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;

utils.h +1 -1

index 886590fd..28aa8373 100644
@@ -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

Signature · 1 line

2.14.1.342.g6490525c54-goog

David Sterba

2017-09-12 17:29 UTC #10 lore raw

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

Quoted 78 lines · Yingyi Luo

From: yingyil <[email protected]>

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 <[email protected]>

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

convert/main.c +1 -1

index 8f7fec25..19a60782 100644
@@ -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;

utils.c +15 ~~-10~~

index 32382328..20ea9e74 100644
@@ -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;

utils.h +0 -0

index 886590fd..28aa8373 100644

utils.h unsigned int rand_range(unsigned int upper); · line 174

Quoted 5 lines

utils.h +1 -1

@@ -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.

Yingyi Luo

author

2017-09-15 18:18 UTC #11 lore raw

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

In reply to Yingyi Luo

From: yingyil <[email protected]>

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 <[email protected]>

---
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(-)

convert/main.c +1 -1

index 0babf0e8..882daf7c 100644
@@ -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;

ctree.h +3 -1

index 22313576..ae25eed5 100644
@@ -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,

inode.c +16 ~~-10~~

index 5dd816bf..ea812ce8 100644
@@ -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;

Signature · 1 line

2.14.1.690.gbb1197296e-goog

Yingyi Luo

author

2017-08-30 17:28 UTC #12 lore raw

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

In reply to Yingyi Luo

From: yingyil <[email protected]>

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 <[email protected]>

---
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(-)

mkfs/main.c +3 -4

index afd68bc5..869c11fa 100644
@@ -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;

Signature · 1 line

2.14.1.342.g6490525c54-goog

David Sterba

2017-09-12 17:30 UTC #13 lore raw

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

Quoted 7 lines · Yingyi Luo

From: yingyil <[email protected]>
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 <[email protected]>

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

Yingyi Luo

author

2017-08-30 17:28 UTC #14 lore raw

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

In reply to Yingyi Luo

From: yingyil <[email protected]>

Signed-off-by yingyil <[email protected]>

---
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(-)

mkfs/main.c +70 -5

index 869c11fa..21abc4a4 100644
@@ -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);

Signature · 1 line

2.14.1.342.g6490525c54-goog

David Sterba

2017-09-12 17:49 UTC #15 lore raw

Quoted 5 lines · Yingyi Luo

From: yingyil <[email protected]>
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.

David Sterba

2017-09-29 13:48 UTC #16 lore raw

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

In reply to Yingyi Luo

Quoted 25 lines · Yingyi Luo

From: yingyil <[email protected]>
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.

Quoted 8 lines

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.