btrfs-progs: use btrfs_link_subvolume() to replace btrfs_mksubvol()

The function btrfs_mksubvol() is very different between btrfs-progs and
kernel, the former version is really just linking a subvolume to another
directory inode, but the kernel version is really to make a completely
new subvolume.

Instead of same-named function, introduce btrfs_link_subvolume() and use
it to replace the old btrfs_mksubvol().

This is done by:

- Introduce btrfs_link_subvolume()
  Which does extra checks before doing any modification:
  * Make sure the target inode is a directory
  * Make sure no filename conflict

  Then do the linkage:
  * Add the dir_item/dir_index into the parent inode
  * Add the forward and backward root refs into tree root

- Introduce link_image_subvolume() helper
  Currently btrfs_mksubvol() has a dedicated convert filename retry
  behavior, which is unnecessary and should be done by the convert code.

  Now move the filename retry behavior into the helper.

- Remove btrfs_mksubvol()
  Since there is only one caller utilizing btrfs_mksubvol(), and it's
  now gone, we can remove the old btrfs_mksubvol().

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

check/main.c

@@ -2361,10 +2361,10 @@ static int repair_inode_backrefs(struct btrfs_root *root,
 			struct btrfs_trans_handle *trans;
 			struct btrfs_key location;
 
-			ret = check_dir_conflict(root, backref->name,
+			ret = btrfs_check_dir_conflict(root, backref->name,
-						 backref->namelen,
+						       backref->namelen,
-						 backref->dir,
+						       backref->dir,
-						 backref->index);
+						       backref->index);
 			if (ret) {
 				/*
 				 * let nlink fixing routine to handle it,

common/root-tree-utils.c

@@ -105,3 +105,110 @@ error:
 	btrfs_abort_transaction(trans, ret);
 	return ret;
 }
+
+/*
+ * Link subvoume @subvol as @name under directory inode @parent_dir of
+ * subvolume @parent_root.
+ */
+int btrfs_link_subvolume(struct btrfs_trans_handle *trans,
+			 struct btrfs_root *parent_root,
+			 u64 parent_dir, const char *name,
+			 int namelen, struct btrfs_root *subvol)
+{
+	struct btrfs_root *tree_root = trans->fs_info->tree_root;
+	struct btrfs_path path = { 0 };
+	struct btrfs_key key;
+	struct btrfs_inode_item *ii;
+	u64 index;
+	u64 isize;
+	u32 imode;
+	int ret;
+
+	UASSERT(namelen && namelen <= BTRFS_NAME_LEN);
+
+	/* Make sure @parent_dir is a directory. */
+	key.objectid = parent_dir;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	ret = btrfs_search_slot(NULL, parent_root, &key, &path, 0, 0);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0) {
+		btrfs_release_path(&path);
+		return ret;
+	}
+	ii = btrfs_item_ptr(path.nodes[0], path.slots[0], struct btrfs_inode_item);
+	imode = btrfs_inode_mode(path.nodes[0], ii);
+	btrfs_release_path(&path);
+
+	if (!S_ISDIR(imode)) {
+		ret = -EUCLEAN;
+		error("%s: inode %llu of subvolume %llu is not a directory",
+		      __func__, parent_dir, parent_root->root_key.objectid);
+		return ret;
+	}
+
+	ret = btrfs_find_free_dir_index(parent_root, parent_dir, &index);
+	if (ret < 0)
+		return ret;
+
+	/* Filename conflicts check. */
+	ret = btrfs_check_dir_conflict(parent_root, name, namelen, parent_dir,
+				       index);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Now everything is fine, add the link.
+	 * From now on, every error would lead to transaction abort.
+	 *
+	 * Add the dir_item/index first.
+	 */
+	ret = btrfs_insert_dir_item(trans, parent_root, name, namelen,
+				    parent_dir, &subvol->root_key,
+				    BTRFS_FT_DIR, index);
+	if (ret < 0)
+		goto abort;
+
+	/* Update inode size of the parent inode */
+	key.objectid = parent_dir;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	ret = btrfs_search_slot(trans, parent_root, &key, &path, 1, 1);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0) {
+		btrfs_release_path(&path);
+		goto abort;
+	}
+	ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_inode_item);
+	isize = btrfs_inode_size(path.nodes[0], ii);
+	isize += namelen * 2;
+	btrfs_set_inode_size(path.nodes[0], ii, isize);
+	btrfs_mark_buffer_dirty(path.nodes[0]);
+	btrfs_release_path(&path);
+
+	/* Add the root backref. */
+	ret = btrfs_add_root_ref(trans, tree_root, subvol->root_key.objectid,
+				 BTRFS_ROOT_BACKREF_KEY,
+				 parent_root->root_key.objectid, parent_dir,
+				 index, name, namelen);
+	if (ret < 0)
+		goto abort;
+
+	/* Then forward ref*/
+	ret = btrfs_add_root_ref(trans, tree_root,
+				 parent_root->root_key.objectid,
+				 BTRFS_ROOT_REF_KEY, subvol->root_key.objectid,
+				 parent_dir, index, name, namelen);
+	if (ret < 0)
+		goto abort;
+	/* For now, all root should have its refs == 1 already.
+	 * So no need to update the root refs. */
+	UASSERT(btrfs_root_refs(&subvol->root_item) == 1);
+	return 0;
+abort:
+	btrfs_abort_transaction(trans, ret);
+	return ret;
+}

common/root-tree-utils.h

@@ -22,5 +22,9 @@
 int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
 			struct btrfs_root *root, u64 objectid);
 int btrfs_make_subvolume(struct btrfs_trans_handle *trans, u64 objectid);
+int btrfs_link_subvolume(struct btrfs_trans_handle *trans,
+			 struct btrfs_root *parent_root,
+			 u64 parent_dir, const char *name,
+			 int namelen, struct btrfs_root *subvol);
 
 #endif

convert/main.c

@@ -1110,6 +1110,62 @@ static int convert_open_fs(const char *devname,
 	return -1;
 }
 
+static int link_image_subvolume(struct btrfs_root *image_root, char *name)
+{
+	struct btrfs_fs_info *fs_info = image_root->fs_info;
+	struct btrfs_root *fs_root = fs_info->fs_root;
+	struct btrfs_trans_handle *trans;
+	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf, null terminated. */
+	int ret;
+
+	/*
+	 * 1 root backref 1 root forward ref, 1 dir item 1 dir index,
+	 * and finally one root item.
+	 */
+	trans = btrfs_start_transaction(fs_root, 5);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		errno = -ret;
+		error("failed to start transaction to link subvolume: %m");
+		return PTR_ERR(trans);
+	}
+	ret = btrfs_link_subvolume(trans, fs_root, btrfs_root_dirid(&fs_root->root_item),
+			name, strlen(name), image_root);
+	/* No filename conflicts, all done. */
+	if (!ret)
+		goto commit;
+	/* Other unexpected errors. */
+	if (ret != -EEXIST) {
+		btrfs_abort_transaction(trans, ret);
+		return ret;
+	}
+
+	/* Filename conflicts detected, try different names. */
+	for (int i = 0; i < 1024; i++) {
+		int len;
+
+		len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", name, i);
+		if (len == 0 || len > BTRFS_NAME_LEN) {
+			error("failed to find an alternative name for image_root");
+			ret = -EINVAL;
+			goto abort;
+		}
+		ret = btrfs_link_subvolume(trans, fs_root,
+					   btrfs_root_dirid(&fs_root->root_item),
+					   buf, len, image_root);
+		if (!ret)
+			break;
+		if (ret != -EEXIST)
+			goto abort;
+	}
+commit:
+	ret = btrfs_commit_transaction(trans, fs_info->tree_root);
+	return ret;
+abort:
+	btrfs_abort_transaction(trans, ret);
+	return ret;
+}
+
 static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		const char *fslabel, int progress,
 		struct btrfs_mkfs_features *features, u16 csum_type,
@@ -1276,10 +1332,10 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		task_deinit(ctx.info);
 	}
 
-	image_root = btrfs_mksubvol(root, subvol_name,
+	ret = link_image_subvolume(image_root, subvol_name);
-				    CONV_IMAGE_SUBVOL_OBJECTID, true);
+	if (ret < 0) {
-	if (!image_root) {
+		errno = -ret;
-		error("unable to link subvolume %s", subvol_name);
+		error("unable to link subvolume %s: %m", subvol_name);
 		goto fail;
 	}
 

kernel-shared/ctree.h

@@ -1212,8 +1212,10 @@ static inline int is_fstree(u64 rootid)
 void btrfs_uuid_to_key(const u8 *uuid, u8 type, struct btrfs_key *key);
 
 /* inode.c */
-int check_dir_conflict(struct btrfs_root *root, char *name, int namelen,
+int btrfs_find_free_dir_index(struct btrfs_root *root, u64 dir_ino,
-		u64 dir, u64 index);
+			      u64 *ret_ino);
+int btrfs_check_dir_conflict(struct btrfs_root *root, const char *name,
+			     int namelen, u64 dir, u64 index);
 int btrfs_new_inode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		u64 ino, u32 mode);
 int btrfs_change_inode_flags(struct btrfs_trans_handle *trans,
@@ -1229,8 +1231,6 @@ 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, bool convert);
 int btrfs_find_free_objectid(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *fs_root,
 			     u64 dirid, u64 *objectid);

kernel-shared/inode.c

@@ -40,8 +40,8 @@
  * Find a free inode index for later btrfs_add_link().
  * Currently just search from the largest dir_index and +1.
  */
-static int btrfs_find_free_dir_index(struct btrfs_root *root, u64 dir_ino,
+int btrfs_find_free_dir_index(struct btrfs_root *root, u64 dir_ino,
-				     u64 *ret_ino)
+			      u64 *ret_ino)
 {
 	struct btrfs_path *path;
 	struct btrfs_key key;
@@ -93,8 +93,8 @@ out:
 }
 
 /* Check the dir_item/index conflicts before insert */
-int check_dir_conflict(struct btrfs_root *root, char *name, int namelen,
+int btrfs_check_dir_conflict(struct btrfs_root *root, const char *name,
-		       u64 dir, u64 index)
+			     int namelen, u64 dir, u64 index)
 {
 	struct btrfs_path *path;
 	struct btrfs_key key;
@@ -190,7 +190,7 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 			goto out;
 	}
 
-	ret = check_dir_conflict(root, name, namelen, parent_ino, ret_index);
+	ret = btrfs_check_dir_conflict(root, name, namelen, parent_ino, ret_index);
 	if (ret < 0 && !(ignore_existed && ret == -EEXIST))
 		goto out;
 
@@ -582,136 +582,6 @@ out:
 	return ret;
 }
 
-struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
-				  const char *base, u64 root_objectid,
-				  bool convert)
-{
-	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 = { 0 };
-	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;
-
-	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)) {
-		ret = PTR_ERR(trans);
-		errno = -ret;
-		error_msg(ERROR_MSG_START_TRANS, "%m");
-		goto fail;
-	}
-
-	key.objectid = dirid;
-	key.type =  BTRFS_INODE_ITEM_KEY;
-	key.offset = 0;
-
-	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.type = BTRFS_ROOT_ITEM_KEY;
-	key.offset = (u64)-1;
-
-	memcpy(buf, base, len);
-	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;
-
-	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) {
-		errno = -ret;
-		error_msg(ERROR_MSG_COMMIT_TRANS, "%m");
-		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:
-	return new_root;
-}
-
 /*
  * Walk the tree of allocated inodes and find a hole.
  */