All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option
@ 2023-10-19 22:29 Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 1/9] btrfs-progs: cross-port fs_types from kernel Qu Wenruo
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:29 UTC (permalink / raw)
  To: linux-btrfs

[CHANGELOG]
v2:
- Crossport fs_types from kernel
  So that we can have infrastructure more in-line with kernel code.
  Unfortunately we still need a new fs_ftypes_to_umode() helper for
  progs.

- More cleanups for btrfs_add_link()

- Make btrfs_add_link() have the same ability of the kernel one
  This is to allow btrfs_add_link() to link a subvolume to a parent
  inode.
  Unfortunately the parameter list is still quite different due to the
  lack of btrfs_inode and dentry.

- Merge btrfs_mksubvol() into btrfs_add_link()
  Since btrfs_add_link() can now handle linking of a subvolume, we
  can remove the progs specific code.

- New common/inode.[ch] for btrfs_create_subvol() and
  btrfs_make_root_dir()
  Those two functions are progs specific and would be utilized by code
  out of check/convert. Thus moving them to common/ would be better


Issue #42 (good number by the way) is suggesting a very useful feature
for rootfs image creation.

Currently we only support "mkfs.btrfs --rootdir" to fill the fs tree
with target directory, but there has no btrfs specific features
involved.

If we can create certain paths as subvolumes, not pure directories, it
can be very useful to create the whole btrfs image just by "mkfs.btrfs"

This series is the first step torwards this idea.

Now we have a new experimental option "--subvol" for mkfs.btrfs, but
with the following limits:

- No co-operation with --rootdir
  This requires --rootdir to have extra handling for any existing
  inodes.
  (Currently --rootdir assumes the fs tree is completely empty)

- No multiple --subvol options supports
  This requires us to collect and sort all the paths and start creating
  subvolumes from the shortest path.
  Furthermore this requires us to create subvolume under another
  subvolume.

Each limit would need a new series of patches to address, but this
series would already provide a working but not-that-useful
implementation of "--subvol" option, along with a basic test case for
it.

Qu Wenruo (9):
  btrfs-progs: cross-port fs_types from kernel
  btrfs-progs: remove add_backref parameter from btrfs_add_link()
  btrfs-progs: remove filetype parameter of btrfs_add_link()
  btrfs-progs: merge btrfs_mksubvol() into btrfs_add_link()
  btrfs-progs: enhance btrfs_mkdir() function
  btrfs-progs: enhance btrfs_create_root() function
  btrfs-progs: use a unified btrfs_make_subvol() implementation
  btrfs-progs: mkfs: introduce experimental --subvol option
  btrfs-progs: mkfs-tests: introduce a test case to verify --subvol
    option

 Makefile                                   |   2 +
 check/main.c                               |  17 +-
 check/mode-common.c                        |  12 +-
 check/mode-common.h                        |  32 --
 check/mode-lowmem.c                        |  40 ++-
 common/inode.c                             |  90 ++++++
 common/inode.h                             |  16 +
 convert/main.c                             | 139 ++++++---
 include/kerncompat.h                       |   1 +
 kernel-shared/ctree.c                      | 106 +++----
 kernel-shared/ctree.h                      |  17 +-
 kernel-shared/fs_types.c                   |  62 ++++
 kernel-shared/fs_types.h                   |  87 ++++++
 kernel-shared/inode.c                      | 347 ++++++++++-----------
 mkfs/common.c                              |  39 ---
 mkfs/common.h                              |   2 -
 mkfs/main.c                                | 106 ++-----
 mkfs/rootdir.c                             | 157 ++++++++++
 mkfs/rootdir.h                             |   1 +
 tests/mkfs-tests/031-subvol-option/test.sh |  39 +++
 tune/convert-bgt.c                         |   3 +-
 tune/quota.c                               |   2 +-
 22 files changed, 829 insertions(+), 488 deletions(-)
 create mode 100644 common/inode.c
 create mode 100644 common/inode.h
 create mode 100644 kernel-shared/fs_types.c
 create mode 100644 kernel-shared/fs_types.h
 create mode 100755 tests/mkfs-tests/031-subvol-option/test.sh

--
2.42.0


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

* [PATCH v2 1/9] btrfs-progs: cross-port fs_types from kernel
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 2/9] btrfs-progs: remove add_backref parameter from btrfs_add_link() Qu Wenruo
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

In btrfs-progs we're using imode_to_type() and btrfs_type_to_imode() to
convert between the inode mode bits to filetype used in
DIR_ITEM/DIR_ENTRY.

However all BTRFS_FT_* are using the same bits of FT_*, and in kernel
we're utilizing this feature, thus to convert from imode to BTRFS_FT_*,
we just use btrfs_inode_type() which calls fs_umode_to_ftype().

To sync the code, this patch would:

- Cross-port fs_types.[ch] from kernel
  For the ecisting fs_umode_to_ftype() it's direct code copy.
  And those synced code would be in kernel-shared/, thus callers
  out of check/ directory can also utilize them.

- Introduce new fs_ftype_to_umode() helper
  Unlike kernel which can always grab inode->i_mode, here in progs we
  needs to convert ftype back to umode for DIR_ITEM/DIR_INDEX
  verification.

- Remove the btrfs-progs specific helpers
  This includes:
  * imode_ty_type()
    Replaced by fs_umode_to_ftype().

  * btrfs_type_to_imode()
    Replaced by fs_ftype_to_umode().

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Makefile                 |  1 +
 check/main.c             | 12 +++---
 check/mode-common.c      |  4 +-
 check/mode-common.h      | 32 ---------------
 check/mode-lowmem.c      | 22 +++++-----
 include/kerncompat.h     |  1 +
 kernel-shared/ctree.h    |  6 +++
 kernel-shared/fs_types.c | 62 ++++++++++++++++++++++++++++
 kernel-shared/fs_types.h | 87 ++++++++++++++++++++++++++++++++++++++++
 kernel-shared/inode.c    | 10 +++++
 10 files changed, 186 insertions(+), 51 deletions(-)
 create mode 100644 kernel-shared/fs_types.c
 create mode 100644 kernel-shared/fs_types.h

diff --git a/Makefile b/Makefile
index 725e5caa88bd..273a1f0e3b7c 100644
--- a/Makefile
+++ b/Makefile
@@ -183,6 +183,7 @@ objects = \
 	kernel-shared/extent_io.o	\
 	kernel-shared/file-item.o	\
 	kernel-shared/file.o	\
+	kernel-shared/fs_types.o	\
 	kernel-shared/free-space-cache.o	\
 	kernel-shared/free-space-tree.o	\
 	kernel-shared/inode-item.o	\
diff --git a/check/main.c b/check/main.c
index 6cf8016670f4..8dfb6ba2a8e8 100644
--- a/check/main.c
+++ b/check/main.c
@@ -835,7 +835,7 @@ static void maybe_free_inode_rec(struct cache_tree *inode_cache,
 	if (!rec->found_inode_item)
 		return;
 
-	filetype = imode_to_type(rec->imode);
+	filetype = btrfs_inode_type(rec->imode);
 	list_for_each_entry_safe(backref, tmp, &rec->backrefs, list) {
 		if (backref->found_dir_item && backref->found_dir_index) {
 			if (backref->filetype != filetype)
@@ -2142,7 +2142,7 @@ static int add_missing_dir_index(struct btrfs_root *root,
 	disk_key.offset = 0;
 
 	btrfs_set_dir_item_key(leaf, dir_item, &disk_key);
-	btrfs_set_dir_flags(leaf, dir_item, imode_to_type(rec->imode));
+	btrfs_set_dir_flags(leaf, dir_item, btrfs_inode_type(rec->imode));
 	btrfs_set_dir_data_len(leaf, dir_item, 0);
 	btrfs_set_dir_name_len(leaf, dir_item, backref->namelen);
 	name_ptr = (unsigned long)(dir_item + 1);
@@ -2329,7 +2329,7 @@ static int repair_inode_backrefs(struct btrfs_root *root,
 			ret = btrfs_insert_dir_item(trans, root, backref->name,
 						    backref->namelen,
 						    backref->dir, &location,
-						    imode_to_type(rec->imode),
+						    btrfs_inode_type(rec->imode),
 						    backref->index);
 			BUG_ON(ret);
 			btrfs_commit_transaction(trans, root);
@@ -2363,7 +2363,7 @@ static int find_file_type(struct inode_record *rec, u8 *type)
 
 	/* For inode item recovered case */
 	if (rec->found_inode_item) {
-		*type = imode_to_type(rec->imode);
+		*type = btrfs_inode_type(rec->imode);
 		return 0;
 	}
 
@@ -2622,7 +2622,7 @@ static int repair_inode_no_item(struct btrfs_trans_handle *trans,
 	}
 
 	ret = btrfs_new_inode(trans, root, rec->ino,
-			      mode | btrfs_type_to_imode(filetype));
+			      mode | fs_ftype_to_umode(filetype));
 	if (ret < 0)
 		goto out;
 
@@ -2634,7 +2634,7 @@ static int repair_inode_no_item(struct btrfs_trans_handle *trans,
 	 * We just fill the record and return
 	 */
 	rec->found_dir_item = 1;
-	rec->imode = mode | btrfs_type_to_imode(filetype);
+	rec->imode = mode | fs_ftype_to_umode(filetype);
 	rec->nlink = 0;
 	rec->errors &= ~I_ERR_NO_INODE_ITEM;
 	/* Ensure the inode_nlinks repair function will be called */
diff --git a/check/mode-common.c b/check/mode-common.c
index 7afd9ed96fd2..34e5267bfd8c 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -773,7 +773,7 @@ static int find_file_type_dir_index(struct btrfs_root *root, u64 ino, u64 dirid,
 	if (name_len != len || memcmp(namebuf, name, len))
 		goto out;
 	found = true;
-	*imode_ret = btrfs_type_to_imode(filetype);
+	*imode_ret = fs_ftype_to_umode(filetype);
 out:
 	btrfs_release_path(&path);
 	if (!found && !ret)
@@ -832,7 +832,7 @@ static int find_file_type_dir_item(struct btrfs_root *root, u64 ino, u64 dirid,
 				   (unsigned long)(di + 1), len);
 		if (name_len != len || memcmp(namebuf, name, len))
 			continue;
-		*imode_ret = btrfs_type_to_imode(filetype);
+		*imode_ret = fs_ftype_to_umode(filetype);
 		found = true;
 		goto out;
 	}
diff --git a/check/mode-common.h b/check/mode-common.h
index 894bbbb8141b..83296baf4a99 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -86,23 +86,6 @@ extern int check_data_csum;
 extern struct btrfs_fs_info *gfs_info;
 extern struct cache_tree *roots_info_cache;
 
-static inline u8 imode_to_type(u32 imode)
-{
-#define S_SHIFT 12
-	static unsigned char btrfs_type_by_mode[S_IFMT >> S_SHIFT] = {
-		[S_IFREG >> S_SHIFT]	= BTRFS_FT_REG_FILE,
-		[S_IFDIR >> S_SHIFT]	= BTRFS_FT_DIR,
-		[S_IFCHR >> S_SHIFT]	= BTRFS_FT_CHRDEV,
-		[S_IFBLK >> S_SHIFT]	= BTRFS_FT_BLKDEV,
-		[S_IFIFO >> S_SHIFT]	= BTRFS_FT_FIFO,
-		[S_IFSOCK >> S_SHIFT]	= BTRFS_FT_SOCK,
-		[S_IFLNK >> S_SHIFT]	= BTRFS_FT_SYMLINK,
-	};
-
-	return btrfs_type_by_mode[(imode & S_IFMT) >> S_SHIFT];
-#undef S_SHIFT
-}
-
 static inline bool fs_root_objectid(u64 objectid)
 {
 	if (objectid == BTRFS_TREE_RELOC_OBJECTID ||
@@ -167,21 +150,6 @@ static inline bool is_valid_imode(u32 imode)
 
 int recow_extent_buffer(struct btrfs_root *root, struct extent_buffer *eb);
 
-static inline u32 btrfs_type_to_imode(u8 type)
-{
-	static u32 imode_by_btrfs_type[] = {
-		[BTRFS_FT_REG_FILE]	= S_IFREG,
-		[BTRFS_FT_DIR]		= S_IFDIR,
-		[BTRFS_FT_CHRDEV]	= S_IFCHR,
-		[BTRFS_FT_BLKDEV]	= S_IFBLK,
-		[BTRFS_FT_FIFO]		= S_IFIFO,
-		[BTRFS_FT_SOCK]		= S_IFSOCK,
-		[BTRFS_FT_SYMLINK]	= S_IFLNK,
-	};
-
-	return imode_by_btrfs_type[(type)];
-}
-
 int get_extent_item_generation(u64 bytenr, u64 *gen_ret);
 
 /*
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 3b2807cc5de9..dd05fa47384f 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1200,19 +1200,19 @@ next:
 	key.type = BTRFS_DIR_INDEX_KEY;
 	key.offset = index;
 	tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
-			    imode_to_type(mode));
+				btrfs_inode_type(mode));
 
 	/* Find related dir_item */
 	key.objectid = ref_key->offset;
 	key.type = BTRFS_DIR_ITEM_KEY;
 	key.offset = btrfs_name_hash(namebuf, len);
 	tmp_err |= find_dir_item(root, &key, &location, namebuf, len,
-			    imode_to_type(mode));
+				 btrfs_inode_type(mode));
 end:
 	if (tmp_err && opt_check_repair) {
 		ret = repair_ternary_lowmem(root, ref_key->offset,
 					    ref_key->objectid, index, namebuf,
-					    name_len, imode_to_type(mode),
+					    name_len, btrfs_inode_type(mode),
 					    tmp_err);
 		if (!ret) {
 			need_research = 1;
@@ -1220,7 +1220,7 @@ end:
 		}
 	}
 	print_inode_ref_err(root, ref_key, index, namebuf, name_len,
-			    imode_to_type(mode), tmp_err);
+			    btrfs_inode_type(mode), tmp_err);
 	err |= tmp_err;
 	len = sizeof(*ref) + name_len;
 	ref = (struct btrfs_inode_ref *)((char *)ref + len);
@@ -1782,7 +1782,7 @@ begin:
 		ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
 				    struct btrfs_inode_item);
 		mode = btrfs_inode_mode(path->nodes[0], ii);
-		if (imode_to_type(mode) != filetype) {
+		if (btrfs_inode_type(mode) != filetype) {
 			tmp_err |= INODE_ITEM_MISMATCH;
 			goto next;
 		}
@@ -1813,7 +1813,7 @@ next:
 		if (tmp_err && opt_check_repair) {
 			ret = repair_dir_item(root, di_key,
 					      location.objectid, index,
-					      imode_to_type(mode), namebuf,
+					      btrfs_inode_type(mode), namebuf,
 					      name_len, tmp_err);
 			if (ret != tmp_err) {
 				need_research = 1;
@@ -2635,7 +2635,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 	nbytes = btrfs_inode_nbytes(node, ii);
 	mode = btrfs_inode_mode(node, ii);
 	flags = btrfs_inode_flags(node, ii);
-	dir = imode_to_type(mode) == BTRFS_FT_DIR;
+	dir = btrfs_inode_type(mode) == BTRFS_FT_DIR;
 	nlink = btrfs_inode_nlink(node, ii);
 	generation = btrfs_inode_generation(node, ii);
 	transid = btrfs_inode_transid(node, ii);
@@ -2729,7 +2729,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 			if (!dir) {
 				warning("root %llu INODE[%llu] mode %u shouldn't have DIR_INDEX[%llu %llu]",
 					root->objectid,	inode_id,
-					imode_to_type(mode), key.objectid,
+					btrfs_inode_type(mode), key.objectid,
 					key.offset);
 			}
 			if (is_orphan && key.type == BTRFS_DIR_INDEX_KEY)
@@ -2772,7 +2772,7 @@ out:
 
 		if ((nlink != 1 || refs != 1) && opt_check_repair) {
 			ret = repair_inode_nlinks_lowmem(root, path, inode_id,
-				namebuf, name_len, refs, imode_to_type(mode),
+				namebuf, name_len, refs, btrfs_inode_type(mode),
 				&nlink);
 		}
 
@@ -2808,7 +2808,7 @@ out:
 			if (opt_check_repair)
 				ret = repair_inode_nlinks_lowmem(root, path,
 					 inode_id, namebuf, name_len, refs,
-					 imode_to_type(mode), &nlink);
+					 btrfs_inode_type(mode), &nlink);
 			if (!opt_check_repair || ret) {
 				err |= LINK_COUNT_ERROR;
 				error(
@@ -5192,7 +5192,7 @@ static int check_fs_first_inode(struct btrfs_root *root)
 		ii = btrfs_item_ptr(path.nodes[0], path.slots[0],
 				    struct btrfs_inode_item);
 		mode = btrfs_inode_mode(path.nodes[0], ii);
-		if (imode_to_type(mode) != BTRFS_FT_DIR)
+		if (btrfs_inode_type(mode) != BTRFS_FT_DIR)
 			err |= INODE_ITEM_MISMATCH;
 	}
 
diff --git a/include/kerncompat.h b/include/kerncompat.h
index 18b474f6aa41..a7b075a4ff80 100644
--- a/include/kerncompat.h
+++ b/include/kerncompat.h
@@ -179,6 +179,7 @@ typedef long long s64;
 typedef int s32;
 #endif
 
+typedef unsigned short umode_t;
 typedef u64 sector_t;
 
 struct vma_shared { int prio_tree_node; };
diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index bcf11d870061..2df8166970be 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -31,6 +31,7 @@
 #include "kernel-shared/accessors.h"
 #include "kernel-shared/extent-io-tree.h"
 #include "kernel-shared/locking.h"
+#include "kernel-shared/fs_types.h"
 #include "crypto/crc32c.h"
 #include "common/extent-cache.h"
 
@@ -1213,6 +1214,11 @@ static inline int is_fstree(u64 rootid)
 void btrfs_uuid_to_key(const u8 *uuid, struct btrfs_key *key);
 
 /* inode.c */
+static inline u8 btrfs_inode_type(u32 inode_mode)
+{
+	return fs_umode_to_ftype(inode_mode);
+}
+
 int check_dir_conflict(struct btrfs_root *root, char *name, int namelen,
 		u64 dir, u64 index);
 int btrfs_new_inode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
diff --git a/kernel-shared/fs_types.c b/kernel-shared/fs_types.c
new file mode 100644
index 000000000000..193d7ee80d93
--- /dev/null
+++ b/kernel-shared/fs_types.c
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Cross-ported from kernel fs/fs_types.c, with the following changes:
+ *
+ * - Add new ftype to imode converter.
+ *   This is btrfs-progs specific, as in kernel we always have a inode
+ *   to grab it's mode.
+ *   But in progs we need do convert from umode to ftype for verification.
+ */
+
+#include "kernel-shared/fs_types.h"
+
+/*
+ * dirent file type to fs on-disk file type conversion
+ * Values not initialized explicitly are FT_UNKNOWN (0).
+ */
+static const unsigned char fs_ftype_by_dtype[DT_MAX] = {
+	[DT_REG]	= FT_REG_FILE,
+	[DT_DIR]	= FT_DIR,
+	[DT_LNK]	= FT_SYMLINK,
+	[DT_CHR]	= FT_CHRDEV,
+	[DT_BLK]	= FT_BLKDEV,
+	[DT_FIFO]	= FT_FIFO,
+	[DT_SOCK]	= FT_SOCK,
+};
+
+/**
+ * fs_umode_to_ftype() - file mode to on-disk file type.
+ * @mode: The file mode to convert.
+ *
+ * This function converts the file mode value to the on-disk file type (FT_*).
+ *
+ * Context: Any context.
+ * Return:
+ * * FT_UNKNOWN		- Unknown type
+ * * FT_REG_FILE	- Regular file
+ * * FT_DIR		- Directory
+ * * FT_CHRDEV		- Character device
+ * * FT_BLKDEV		- Block device
+ * * FT_FIFO		- FIFO
+ * * FT_SOCK		- Local-domain socket
+ * * FT_SYMLINK		- Symbolic link
+ */
+unsigned char fs_umode_to_ftype(umode_t mode)
+{
+	return fs_ftype_by_dtype[S_DT(mode)];
+}
+
+umode_t fs_ftype_to_umode(unsigned char ftype)
+{
+	static u32 imode_by_ftype[] = {
+		[FT_REG_FILE]     = S_IFREG,
+		[FT_DIR]          = S_IFDIR,
+		[FT_CHRDEV]       = S_IFCHR,
+		[FT_BLKDEV]       = S_IFBLK,
+		[FT_FIFO]         = S_IFIFO,
+		[FT_SOCK]         = S_IFSOCK,
+		[FT_SYMLINK]      = S_IFLNK,
+       };
+       return imode_by_ftype[(ftype)];
+}
diff --git a/kernel-shared/fs_types.h b/kernel-shared/fs_types.h
new file mode 100644
index 000000000000..bdac8318413b
--- /dev/null
+++ b/kernel-shared/fs_types.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_FS_TYPES_H
+#define _LINUX_FS_TYPES_H
+
+#include <dirent.h>
+#include <sys/stat.h>
+#include "kerncompat.h"
+
+/*
+ * This is cross-ported from kernel include/linux/fs_types.h, changes are:
+ *
+ * - New ftype to umode convertor
+ * - Use glibc's dirent.h to replace the DT_* declarations
+ */
+
+/*
+ * This is a header for the common implementation of dirent
+ * to fs on-disk file type conversion.  Although the fs on-disk
+ * bits are specific to every file system, in practice, many
+ * file systems use the exact same on-disk format to describe
+ * the lower 3 file type bits that represent the 7 POSIX file
+ * types.
+ *
+ * It is important to note that the definitions in this
+ * header MUST NOT change. This would break both the
+ * userspace ABI and the on-disk format of filesystems
+ * using this code.
+ *
+ * All those file systems can use this generic code for the
+ * conversions.
+ */
+
+/*
+ * struct dirent file types
+ * exposed to user via getdents(2), readdir(3)
+ *
+ * These match bits 12..15 of stat.st_mode
+ * (ie "(i_mode >> 12) & 15").
+ */
+#define S_DT_SHIFT	12
+#define S_DT(mode)	(((mode) & S_IFMT) >> S_DT_SHIFT)
+#define S_DT_MASK	(S_IFMT >> S_DT_SHIFT)
+
+/* these are defined by POSIX and also present in glibc's dirent.h */
+/*
+#define DT_UNKNOWN	0
+#define DT_FIFO		1
+#define DT_CHR		2
+#define DT_DIR		4
+#define DT_BLK		6
+#define DT_REG		8
+#define DT_LNK		10
+#define DT_SOCK		12
+#define DT_WHT		14
+*/
+
+#define DT_MAX		(S_DT_MASK + 1) /* 16 */
+
+/*
+ * fs on-disk file types.
+ * Only the low 3 bits are used for the POSIX file types.
+ * Other bits are reserved for fs private use.
+ * These definitions are shared and used by multiple filesystems,
+ * and MUST NOT change under any circumstances.
+ *
+ * Note that no fs currently stores the whiteout type on-disk,
+ * so whiteout dirents are exposed to user as DT_CHR.
+ */
+#define FT_UNKNOWN	0
+#define FT_REG_FILE	1
+#define FT_DIR		2
+#define FT_CHRDEV	3
+#define FT_BLKDEV	4
+#define FT_FIFO		5
+#define FT_SOCK		6
+#define FT_SYMLINK	7
+
+#define FT_MAX		8
+
+/*
+ * declarations for helper functions, accompanying implementation
+ * is in fs/fs_types.c
+ */
+extern unsigned char fs_umode_to_ftype(umode_t mode);
+extern umode_t fs_ftype_to_umode(unsigned char ftype);
+
+#endif
diff --git a/kernel-shared/inode.c b/kernel-shared/inode.c
index 3d420787c8f9..1893e48001af 100644
--- a/kernel-shared/inode.c
+++ b/kernel-shared/inode.c
@@ -33,9 +33,19 @@
 #include "kernel-shared/ctree.h"
 #include "kernel-shared/transaction.h"
 #include "kernel-shared/disk-io.h"
+#include "kernel-shared/fs_types.h"
 #include "common/messages.h"
 #include "common/internal.h"
 
+static_assert(BTRFS_FT_UNKNOWN == FT_UNKNOWN);
+static_assert(BTRFS_FT_REG_FILE == FT_REG_FILE);
+static_assert(BTRFS_FT_DIR == FT_DIR);
+static_assert(BTRFS_FT_CHRDEV == FT_CHRDEV);
+static_assert(BTRFS_FT_BLKDEV == FT_BLKDEV);
+static_assert(BTRFS_FT_FIFO == FT_FIFO);
+static_assert(BTRFS_FT_SOCK == FT_SOCK);
+static_assert(BTRFS_FT_SYMLINK == FT_SYMLINK);
+
 /*
  * Find a free inode index for later btrfs_add_link().
  * Currently just search from the largest dir_index and +1.
-- 
2.42.0


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

* [PATCH v2 2/9] btrfs-progs: remove add_backref parameter from btrfs_add_link()
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 1/9] btrfs-progs: cross-port fs_types from kernel Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 3/9] btrfs-progs: remove filetype parameter of btrfs_add_link() Qu Wenruo
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

The function btrfs_add_link() has a parameter @add_backref, to indicate
if the operation should add an INODE_REF for the child inode.

However all call sites are passing 1 for @add_backref, and in fact if
intentionally passing 0 for @add_backref for most cases, it would only
lead to missing INODE_REF and cause inconsistency.

And for call sites that want to ignore existing INODE_REF, they would
have already pass 1 for @ignore_existed.

So we can safely remmove the @add_backref parameter.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          |  2 +-
 check/mode-common.c   |  4 ++--
 check/mode-lowmem.c   |  2 +-
 convert/main.c        |  2 +-
 kernel-shared/ctree.h |  2 +-
 kernel-shared/inode.c | 52 +++++++++++++++++++++----------------------
 6 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/check/main.c b/check/main.c
index 8dfb6ba2a8e8..b53ce9327fb4 100644
--- a/check/main.c
+++ b/check/main.c
@@ -2456,7 +2456,7 @@ static int reset_nlink(struct btrfs_trans_handle *trans,
 	list_for_each_entry(backref, &rec->backrefs, list) {
 		ret = btrfs_add_link(trans, root, rec->ino, backref->dir,
 				     backref->name, backref->namelen,
-				     backref->filetype, &backref->index, 1, 0);
+				     backref->filetype, &backref->index, 0);
 		if (ret < 0)
 			goto out;
 	}
diff --git a/check/mode-common.c b/check/mode-common.c
index 34e5267bfd8c..647b65cb3f03 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -464,7 +464,7 @@ int link_inode_to_lostfound(struct btrfs_trans_handle *trans,
 		goto out;
 	}
 	ret = btrfs_add_link(trans, root, ino, lost_found_ino,
-			     namebuf, name_len, filetype, NULL, 1, 0);
+			     namebuf, name_len, filetype, NULL, 0);
 	/*
 	 * Add ".INO" suffix several times to handle case where
 	 * "FILENAME.INO" is already taken by another file.
@@ -481,7 +481,7 @@ int link_inode_to_lostfound(struct btrfs_trans_handle *trans,
 			 ".%llu", ino);
 		name_len += count_digits(ino) + 1;
 		ret = btrfs_add_link(trans, root, ino, lost_found_ino, namebuf,
-				     name_len, filetype, NULL, 1, 0);
+				     name_len, filetype, NULL, 0);
 	}
 	if (ret < 0) {
 		errno = -ret;
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index dd05fa47384f..66a9c84415a8 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1042,7 +1042,7 @@ static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
 		if (ret)
 			goto out;
 		ret = btrfs_add_link(trans, root, ino, dir_ino, name, name_len,
-			       filetype, &index, 1, 1);
+			       filetype, &index, 1);
 		goto out;
 	}
 out:
diff --git a/convert/main.c b/convert/main.c
index c9e50c036f92..16c517422259 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -841,7 +841,7 @@ static int create_image(struct btrfs_root *root,
 		goto out;
 	}
 	ret = btrfs_add_link(trans, root, ino, BTRFS_FIRST_FREE_OBJECTID, name,
-			     strlen(name), BTRFS_FT_REG_FILE, NULL, 1, 0);
+			     strlen(name), BTRFS_FT_REG_FILE, NULL, 0);
 	if (ret < 0) {
 		errno = -ret;
 		error("failed to link ino %llu to '/%s' in root %llu: %m",
diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index 2df8166970be..03fd25368181 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -1227,7 +1227,7 @@ int btrfs_change_inode_flags(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root, u64 ino, u64 flags);
 int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		   u64 ino, u64 parent_ino, char *name, int namelen,
-		   u8 type, u64 *index, int add_backref, int ignore_existed);
+		   u8 type, u64 *index, int ignore_existed);
 int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		 u64 ino, u64 parent_ino, u64 index, const char *name,
 		 int namelen, int add_orphan);
diff --git a/kernel-shared/inode.c b/kernel-shared/inode.c
index 1893e48001af..ba8c8ad02a78 100644
--- a/kernel-shared/inode.c
+++ b/kernel-shared/inode.c
@@ -178,7 +178,7 @@ out:
  */
 int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		   u64 ino, u64 parent_ino, char *name, int namelen,
-		   u8 type, u64 *index, int add_backref, int ignore_existed)
+		   u8 type, u64 *index, int ignore_existed)
 {
 	struct btrfs_path *path;
 	struct btrfs_key key;
@@ -205,33 +205,31 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		goto out;
 
 	/* Add inode ref */
-	if (add_backref) {
-		ret = btrfs_insert_inode_ref(trans, root, name, namelen,
-					     ino, parent_ino, ret_index);
-		if (ret < 0 && !(ignore_existed && ret == -EEXIST))
-			goto out;
+	ret = btrfs_insert_inode_ref(trans, root, name, namelen,
+				     ino, parent_ino, ret_index);
+	if (ret < 0 && !(ignore_existed && ret == -EEXIST))
+		goto out;
 
-		/* do not update nlinks if existed */
-		if (!ret) {
-			/* Update nlinks for the inode */
-			key.objectid = ino;
-			key.type = BTRFS_INODE_ITEM_KEY;
-			key.offset = 0;
-			ret = btrfs_search_slot(trans, root, &key, path, 1, 1);
-			if (ret) {
-				if (ret > 0)
-					ret = -ENOENT;
-				goto out;
-			}
-			inode_item = btrfs_item_ptr(path->nodes[0],
-				    path->slots[0], struct btrfs_inode_item);
-			nlink = btrfs_inode_nlink(path->nodes[0], inode_item);
-			nlink++;
-			btrfs_set_inode_nlink(path->nodes[0], inode_item,
-					      nlink);
-			btrfs_mark_buffer_dirty(path->nodes[0]);
-			btrfs_release_path(path);
+	/* do not update nlinks if existed */
+	if (!ret) {
+		/* Update nlinks for the inode */
+		key.objectid = ino;
+		key.type = BTRFS_INODE_ITEM_KEY;
+		key.offset = 0;
+		ret = btrfs_search_slot(trans, root, &key, path, 1, 1);
+		if (ret) {
+			if (ret > 0)
+				ret = -ENOENT;
+			goto out;
 		}
+		inode_item = btrfs_item_ptr(path->nodes[0],
+			    path->slots[0], struct btrfs_inode_item);
+		nlink = btrfs_inode_nlink(path->nodes[0], inode_item);
+		nlink++;
+		btrfs_set_inode_nlink(path->nodes[0], inode_item,
+				      nlink);
+		btrfs_mark_buffer_dirty(path->nodes[0]);
+		btrfs_release_path(path);
 	}
 
 	/* Add dir_item and dir_index */
@@ -582,7 +580,7 @@ int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	if (ret)
 		goto out;
 	ret = btrfs_add_link(trans, root, ret_ino, parent_ino, name, namelen,
-			     BTRFS_FT_DIR, NULL, 1, 0);
+			     BTRFS_FT_DIR, NULL, 0);
 	if (ret)
 		goto out;
 out:
-- 
2.42.0


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

* [PATCH v2 3/9] btrfs-progs: remove filetype parameter of btrfs_add_link()
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 1/9] btrfs-progs: cross-port fs_types from kernel Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 2/9] btrfs-progs: remove add_backref parameter from btrfs_add_link() Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 4/9] btrfs-progs: merge btrfs_mksubvol() into btrfs_add_link() Qu Wenruo
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

The function btrfs_add_link() utilized @filetype parameter to insert the
DIR_ITEM/DIR_INDEX.

However for all call sites of btrfs_add_link() we already have the
proper inode number of the child, and can grab the imode of that inode,
then convert it to filetype.

Thus no need to grab the @filetype parameter.

This is still true for repair code, where if we hit a inode without its
INODE_ITEM, the repair is always done by inserting a new INODE_ITEM
first, then do btrfs_add_link(), thus we should still be able to grab
the imode.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          |  2 +-
 check/mode-common.c   |  4 ++--
 check/mode-lowmem.c   | 18 ++++++++----------
 convert/main.c        |  2 +-
 kernel-shared/ctree.h |  2 +-
 kernel-shared/inode.c | 23 ++++++++++++++++++++---
 6 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/check/main.c b/check/main.c
index b53ce9327fb4..4beeb0adae76 100644
--- a/check/main.c
+++ b/check/main.c
@@ -2456,7 +2456,7 @@ static int reset_nlink(struct btrfs_trans_handle *trans,
 	list_for_each_entry(backref, &rec->backrefs, list) {
 		ret = btrfs_add_link(trans, root, rec->ino, backref->dir,
 				     backref->name, backref->namelen,
-				     backref->filetype, &backref->index, 0);
+				     &backref->index, 0);
 		if (ret < 0)
 			goto out;
 	}
diff --git a/check/mode-common.c b/check/mode-common.c
index 647b65cb3f03..1f42743c390a 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -464,7 +464,7 @@ int link_inode_to_lostfound(struct btrfs_trans_handle *trans,
 		goto out;
 	}
 	ret = btrfs_add_link(trans, root, ino, lost_found_ino,
-			     namebuf, name_len, filetype, NULL, 0);
+			     namebuf, name_len, NULL, 0);
 	/*
 	 * Add ".INO" suffix several times to handle case where
 	 * "FILENAME.INO" is already taken by another file.
@@ -481,7 +481,7 @@ int link_inode_to_lostfound(struct btrfs_trans_handle *trans,
 			 ".%llu", ino);
 		name_len += count_digits(ino) + 1;
 		ret = btrfs_add_link(trans, root, ino, lost_found_ino, namebuf,
-				     name_len, filetype, NULL, 0);
+				     name_len, NULL, 0);
 	}
 	if (ret < 0) {
 		errno = -ret;
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 66a9c84415a8..275f6d16ebc7 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1007,8 +1007,7 @@ out:
  * returns not 0 means on error;
  */
 static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
-			  u64 index, char *name, int name_len, u8 filetype,
-			  int err)
+			  u64 index, char *name, int name_len, int err)
 {
 	struct btrfs_trans_handle *trans;
 	int stage = 0;
@@ -1042,19 +1041,19 @@ static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
 		if (ret)
 			goto out;
 		ret = btrfs_add_link(trans, root, ino, dir_ino, name, name_len,
-			       filetype, &index, 1);
+				     &index, 1);
 		goto out;
 	}
 out:
 	btrfs_commit_transaction(trans, root);
 
 	if (ret)
-		error("fail to repair inode %llu name %s filetype %u",
-		      ino, name, filetype);
+		error("fail to repair inode %llu name %s",
+		      ino, name);
 	else
-		printf("%s ref/dir_item of inode %llu name %s filetype %u\n",
+		printf("%s ref/dir_item of inode %llu name %s\n",
 		       stage == 2 ? "Delete" : "Add",
-		       ino, name, filetype);
+		       ino, name);
 
 	return ret;
 }
@@ -1212,8 +1211,7 @@ end:
 	if (tmp_err && opt_check_repair) {
 		ret = repair_ternary_lowmem(root, ref_key->offset,
 					    ref_key->objectid, index, namebuf,
-					    name_len, btrfs_inode_type(mode),
-					    tmp_err);
+					    name_len, tmp_err);
 		if (!ret) {
 			need_research = 1;
 			goto begin;
@@ -1619,7 +1617,7 @@ static int repair_dir_item(struct btrfs_root *root, struct btrfs_key *di_key,
 
 	if (err & ~(INODE_ITEM_MISMATCH | INODE_ITEM_MISSING)) {
 		ret = repair_ternary_lowmem(root, dirid, ino, index, namebuf,
-					    name_len, filetype, err);
+					    name_len, err);
 		if (!ret) {
 			err &= ~(DIR_INDEX_MISMATCH | DIR_INDEX_MISSING);
 			err &= ~(DIR_ITEM_MISMATCH | DIR_ITEM_MISSING);
diff --git a/convert/main.c b/convert/main.c
index 16c517422259..c1047bacbe01 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -841,7 +841,7 @@ static int create_image(struct btrfs_root *root,
 		goto out;
 	}
 	ret = btrfs_add_link(trans, root, ino, BTRFS_FIRST_FREE_OBJECTID, name,
-			     strlen(name), BTRFS_FT_REG_FILE, NULL, 0);
+			     strlen(name), NULL, 0);
 	if (ret < 0) {
 		errno = -ret;
 		error("failed to link ino %llu to '/%s' in root %llu: %m",
diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index 03fd25368181..501feaa08a0a 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -1227,7 +1227,7 @@ int btrfs_change_inode_flags(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root, u64 ino, u64 flags);
 int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		   u64 ino, u64 parent_ino, char *name, int namelen,
-		   u8 type, u64 *index, int ignore_existed);
+		   u64 *index, int ignore_existed);
 int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		 u64 ino, u64 parent_ino, u64 index, const char *name,
 		 int namelen, int add_orphan);
diff --git a/kernel-shared/inode.c b/kernel-shared/inode.c
index ba8c8ad02a78..43ab685a45e1 100644
--- a/kernel-shared/inode.c
+++ b/kernel-shared/inode.c
@@ -178,11 +178,12 @@ out:
  */
 int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		   u64 ino, u64 parent_ino, char *name, int namelen,
-		   u8 type, u64 *index, int ignore_existed)
+		   u64 *index, int ignore_existed)
 {
 	struct btrfs_path *path;
 	struct btrfs_key key;
 	struct btrfs_inode_item *inode_item;
+	u32 imode;
 	u32 nlink;
 	u64 inode_size;
 	u64 ret_index = 0;
@@ -192,6 +193,22 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	if (!path)
 		return -ENOMEM;
 
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret > 0) {
+		ret = -ENOENT;
+		/* fallthrough */
+	}
+	if (ret < 0)
+		goto out;
+
+	inode_item = btrfs_item_ptr(path->nodes[0], path->slots[0],
+				    struct btrfs_inode_item);
+	imode = btrfs_inode_mode(path->nodes[0], inode_item);
+	btrfs_release_path(path);
+
 	if (index && *index) {
 		ret_index = *index;
 	} else {
@@ -237,7 +254,7 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	key.type = BTRFS_INODE_ITEM_KEY;
 	key.offset = 0;
 	ret = btrfs_insert_dir_item(trans, root, name, namelen, parent_ino,
-				    &key, type, ret_index);
+				    &key, fs_umode_to_ftype(imode), ret_index);
 	if (ret < 0)
 		goto out;
 
@@ -580,7 +597,7 @@ int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	if (ret)
 		goto out;
 	ret = btrfs_add_link(trans, root, ret_ino, parent_ino, name, namelen,
-			     BTRFS_FT_DIR, NULL, 0);
+			     NULL, 0);
 	if (ret)
 		goto out;
 out:
-- 
2.42.0


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

* [PATCH v2 4/9] btrfs-progs: merge btrfs_mksubvol() into btrfs_add_link()
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
                   ` (2 preceding siblings ...)
  2023-10-19 22:30 ` [PATCH v2 3/9] btrfs-progs: remove filetype parameter of btrfs_add_link() Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 5/9] btrfs-progs: enhance btrfs_mkdir() function Qu Wenruo
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

The function btrfs_mksubvol() is only utilized by convert, and its
functionality is not to create a subvolume, but linking one to fs_tree.

In kernel code, btrfs_add_link() can handle the following cases:

- Linking one inode to a directory inside the same subvolume
- Linking one root to a directory inside another subvolume

Although the parameters are different in btrfs-progs, there is not much
reason not to do the same in btrfs-progs.

So this patch would:

- Make btrfs_add_link() able to handle linking subvolume
  * Add a @parent_root parameter
  * Rename existing @ino/@root to @child_ino/@child_root
  * Add extra check to make sure if linking a subvolume, the @child_ino
    is the rootdir
  * If linking a subvolume, insert root and backref for @child_root
  * If linking a subvolume, use the root_key of @child_root

- Use btrfs_add_link() to implement link_image_subvol() for convert
  Since btrfs_add_link() would return -EEXIST before inserting any
  dir items, it's very safe to retry for a new subvolume name.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          |   2 +-
 check/mode-common.c   |   6 +-
 check/mode-lowmem.c   |   4 +-
 convert/main.c        |  83 ++++++++++++++-
 kernel-shared/ctree.h |   6 +-
 kernel-shared/inode.c | 241 +++++++++++++++---------------------------
 6 files changed, 172 insertions(+), 170 deletions(-)

diff --git a/check/main.c b/check/main.c
index 4beeb0adae76..4a88cc8f8708 100644
--- a/check/main.c
+++ b/check/main.c
@@ -2454,7 +2454,7 @@ static int reset_nlink(struct btrfs_trans_handle *trans,
 	 * add_link() will handle the nlink inc, so new nlink must be correct
 	 */
 	list_for_each_entry(backref, &rec->backrefs, list) {
-		ret = btrfs_add_link(trans, root, rec->ino, backref->dir,
+		ret = btrfs_add_link(trans, root, rec->ino, root, backref->dir,
 				     backref->name, backref->namelen,
 				     &backref->index, 0);
 		if (ret < 0)
diff --git a/check/mode-common.c b/check/mode-common.c
index 1f42743c390a..0dca0a789a4d 100644
--- a/check/mode-common.c
+++ b/check/mode-common.c
@@ -463,7 +463,7 @@ int link_inode_to_lostfound(struct btrfs_trans_handle *trans,
 		error("failed to create '%s' dir: %m", dir_name);
 		goto out;
 	}
-	ret = btrfs_add_link(trans, root, ino, lost_found_ino,
+	ret = btrfs_add_link(trans, root, ino, root, lost_found_ino,
 			     namebuf, name_len, NULL, 0);
 	/*
 	 * Add ".INO" suffix several times to handle case where
@@ -480,8 +480,8 @@ int link_inode_to_lostfound(struct btrfs_trans_handle *trans,
 		snprintf(namebuf + name_len, BTRFS_NAME_LEN - name_len,
 			 ".%llu", ino);
 		name_len += count_digits(ino) + 1;
-		ret = btrfs_add_link(trans, root, ino, lost_found_ino, namebuf,
-				     name_len, NULL, 0);
+		ret = btrfs_add_link(trans, root, ino, root, lost_found_ino,
+				     namebuf, name_len, NULL, 0);
 	}
 	if (ret < 0) {
 		errno = -ret;
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 275f6d16ebc7..b52316754753 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1040,8 +1040,8 @@ static int repair_ternary_lowmem(struct btrfs_root *root, u64 dir_ino, u64 ino,
 				name_len, 0);
 		if (ret)
 			goto out;
-		ret = btrfs_add_link(trans, root, ino, dir_ino, name, name_len,
-				     &index, 1);
+		ret = btrfs_add_link(trans, root, ino, root, dir_ino, name,
+				     name_len, &index, 1);
 		goto out;
 	}
 out:
diff --git a/convert/main.c b/convert/main.c
index c1047bacbe01..bcd310228fcd 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -104,6 +104,7 @@
 #include "kernel-shared/transaction.h"
 #include "kernel-shared/free-space-tree.h"
 #include "kernel-shared/file-item.h"
+#include "kernel-shared/ctree.h"
 #include "crypto/hash.h"
 #include "common/defs.h"
 #include "common/extent-cache.h"
@@ -840,8 +841,8 @@ static int create_image(struct btrfs_root *root,
 			ino, root->root_key.objectid);
 		goto out;
 	}
-	ret = btrfs_add_link(trans, root, ino, BTRFS_FIRST_FREE_OBJECTID, name,
-			     strlen(name), NULL, 0);
+	ret = btrfs_add_link(trans, root, ino, root, BTRFS_FIRST_FREE_OBJECTID,
+			     name, strlen(name), NULL, 0);
 	if (ret < 0) {
 		errno = -ret;
 		error("failed to link ino %llu to '/%s' in root %llu: %m",
@@ -1145,6 +1146,79 @@ static int convert_open_fs(const char *devname,
 	return -1;
 }
 
+static struct btrfs_root *link_image_subvol(struct btrfs_fs_info *fs_info,
+					    const char *base)
+{
+	struct btrfs_root *fs_root = fs_info->fs_root;
+	struct btrfs_root *image_root;
+	struct btrfs_trans_handle *trans;
+	struct btrfs_key key;
+	char buf[BTRFS_NAME_LEN + 1]; /* for snprintf null */
+	int len;
+	int ret;
+
+	strcpy(buf, base);
+	len = strlen(base);
+	if (len == 0 || len > BTRFS_NAME_LEN) {
+		ret = -EINVAL;
+		error("invalid image subvolume name: %s", base);
+		image_root = ERR_PTR(ret);
+		goto out;
+	}
+
+	/*
+	 * 1 root ref, 1 root backref,
+	 * 1 dir_index, 1 dir_item.
+	 */
+	trans = btrfs_start_transaction(fs_root, 4);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		errno = -ret;
+		error_msg(ERROR_MSG_START_TRANS, "%m");
+		image_root = ERR_PTR(ret);
+		goto out;
+	}
+	key.objectid = CONV_IMAGE_SUBVOL_OBJECTID;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = (u64)-1;
+	image_root = btrfs_read_fs_root(fs_info, &key);
+	if (IS_ERR(image_root)) {
+		ret = PTR_ERR(image_root);
+		errno = -ret;
+		error("failed to read convert image subvolume: %m");
+		image_root = ERR_PTR(ret);
+		goto out;
+	}
+	for (int i = 0; i < 1024; i++) {
+		ret = btrfs_add_link(trans,
+			image_root, btrfs_root_dirid(&image_root->root_item),
+			fs_root, btrfs_root_dirid(&fs_root->root_item),
+			buf, len, NULL, 0);
+		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 < 0) {
+		errno = -ret;
+		error("failed to link image subvolume: %m");
+		image_root = ERR_PTR(ret);
+		goto out;
+	}
+	ret = btrfs_commit_transaction(trans, fs_root);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to commit transaction: %m");
+		image_root = ERR_PTR(ret);
+		goto out;
+	}
+out:
+	return image_root;
+}
+
 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,
@@ -1311,9 +1385,8 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize,
 		task_deinit(ctx.info);
 	}
 
-	image_root = btrfs_mksubvol(root, subvol_name,
-				    CONV_IMAGE_SUBVOL_OBJECTID, true);
-	if (!image_root) {
+	image_root = link_image_subvol(root->fs_info, subvol_name);
+	if (IS_ERR(image_root)) {
 		error("unable to link subvolume %s", subvol_name);
 		goto fail;
 	}
diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index 501feaa08a0a..ff43fefca802 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -1225,8 +1225,10 @@ 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,
 			     struct btrfs_root *root, u64 ino, u64 flags);
-int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
-		   u64 ino, u64 parent_ino, char *name, int namelen,
+int btrfs_add_link(struct btrfs_trans_handle *trans,
+		   struct btrfs_root *child_root, u64 child_ino,
+		   struct btrfs_root *parent_root, u64 parent_ino,
+		   char *name, int namelen,
 		   u64 *index, int ignore_existed);
 int btrfs_unlink(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		 u64 ino, u64 parent_ino, u64 index, const char *name,
diff --git a/kernel-shared/inode.c b/kernel-shared/inode.c
index 43ab685a45e1..38ca882141e2 100644
--- a/kernel-shared/inode.c
+++ b/kernel-shared/inode.c
@@ -170,33 +170,44 @@ out:
 }
 
 /*
- * Add dir_item/index for 'parent_ino' if add_backref is true, also insert a
- * backref from the ino to parent dir and update the nlink(Kernel version does
- * not do this thing)
+ * Link child inode (@child_ino of @child_root) under the directory of parent
+ * inode (@parent_ino of @parent_root, must be a directory inode), using
+ * @name.
  *
- * Currently only supports adding link from an inode to another inode.
+ * If @child_root and @parent_root are different, @child_ino must be the rootdir
+ * of @child_root.
+ *
+ * If @index is not NULL, and points to a non-zero value, this function would use
+ * *index as the DIR_INDEX, caller must ensure there is no conflicts.
+ * If @ignore_existed is true, any conflicting DIR_ITEM/DIR_INDEX would be ignored.
  */
-int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
-		   u64 ino, u64 parent_ino, char *name, int namelen,
+int btrfs_add_link(struct btrfs_trans_handle *trans,
+		   struct btrfs_root *child_root, u64 child_ino,
+		   struct btrfs_root *parent_root, u64 parent_ino,
+		   char *name, int namelen,
 		   u64 *index, int ignore_existed)
 {
 	struct btrfs_path *path;
 	struct btrfs_key key;
 	struct btrfs_inode_item *inode_item;
+	bool link_subvol = false;
 	u32 imode;
 	u32 nlink;
 	u64 inode_size;
 	u64 ret_index = 0;
 	int ret = 0;
 
+	if (btrfs_comp_cpu_keys(&child_root->root_key, &parent_root->root_key))
+		link_subvol = true;
+
 	path = btrfs_alloc_path();
 	if (!path)
 		return -ENOMEM;
 
-	key.objectid = ino;
+	key.objectid = child_ino;
 	key.type = BTRFS_INODE_ITEM_KEY;
 	key.offset = 0;
-	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	ret = btrfs_search_slot(NULL, child_root, &key, path, 0, 0);
 	if (ret > 0) {
 		ret = -ENOENT;
 		/* fallthrough */
@@ -209,31 +220,72 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	imode = btrfs_inode_mode(path->nodes[0], inode_item);
 	btrfs_release_path(path);
 
+	/*
+	 * If linking a subvolume, child_ino must be the rootdir of that
+	 * subvolume.
+	 */
+	if (link_subvol && child_ino != btrfs_root_dirid(&child_root->root_item)) {
+		error("child ino is not rootdir, has %llu expect %llu",
+		      child_ino, btrfs_root_dirid(&child_root->root_item));
+		ret = -EUCLEAN;
+		goto out;
+	}
+
 	if (index && *index) {
 		ret_index = *index;
 	} else {
-		ret = btrfs_find_free_dir_index(root, parent_ino, &ret_index);
+		ret = btrfs_find_free_dir_index(parent_root, parent_ino,
+						&ret_index);
 		if (ret < 0)
 			goto out;
 	}
 
-	ret = check_dir_conflict(root, name, namelen, parent_ino, ret_index);
+	ret = check_dir_conflict(parent_root, name, namelen, parent_ino,
+				 ret_index);
 	if (ret < 0 && !(ignore_existed && ret == -EEXIST))
 		goto out;
 
-	/* Add inode ref */
-	ret = btrfs_insert_inode_ref(trans, root, name, namelen,
-				     ino, parent_ino, ret_index);
-	if (ret < 0 && !(ignore_existed && ret == -EEXIST))
-		goto out;
+	if (link_subvol) {
+		struct btrfs_root *tree_root = trans->fs_info->tree_root;
+
+		/* Add root backref. */
+		ret = btrfs_add_root_ref(trans, tree_root,
+				child_root->root_key.objectid,
+				BTRFS_ROOT_BACKREF_KEY,
+				parent_root->root_key.objectid,
+				parent_ino, ret_index, name, namelen);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to add backref for child root %lld: %m",
+			      child_root->root_key.objectid);
+			goto out;
+		}
+
+		ret = btrfs_add_root_ref(trans, tree_root,
+				parent_root->root_key.objectid,
+				BTRFS_ROOT_REF_KEY, child_root->root_key.objectid,
+				parent_ino, ret_index, name, namelen);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to add root ref for child root %lld: %m",
+			      child_root->root_key.objectid);
+			goto out;
+		}
+	} else {
+		/* Add inode ref */
+		ret = btrfs_insert_inode_ref(trans, child_root, name, namelen,
+					     child_ino, parent_ino, ret_index);
+		if (ret < 0 && !(ignore_existed && ret == -EEXIST))
+			goto out;
+	}
 
 	/* do not update nlinks if existed */
-	if (!ret) {
-		/* Update nlinks for the inode */
-		key.objectid = ino;
+	if (!ret && !link_subvol) {
+		/* Update nlinks for the child inode. */
+		key.objectid = child_ino;
 		key.type = BTRFS_INODE_ITEM_KEY;
 		key.offset = 0;
-		ret = btrfs_search_slot(trans, root, &key, path, 1, 1);
+		ret = btrfs_search_slot(trans, child_root, &key, path, 1, 1);
 		if (ret) {
 			if (ret > 0)
 				ret = -ENOENT;
@@ -250,11 +302,16 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	}
 
 	/* Add dir_item and dir_index */
-	key.objectid = ino;
-	key.type = BTRFS_INODE_ITEM_KEY;
-	key.offset = 0;
-	ret = btrfs_insert_dir_item(trans, root, name, namelen, parent_ino,
-				    &key, fs_umode_to_ftype(imode), ret_index);
+	if (link_subvol) {
+		memcpy(&key, &child_root->root_key, sizeof(struct btrfs_key));
+	} else {
+		key.objectid = child_ino;
+		key.type = BTRFS_INODE_ITEM_KEY;
+		key.offset = 0;
+	}
+	ret = btrfs_insert_dir_item(trans, parent_root, name, namelen,
+				    parent_ino, &key,
+				    fs_umode_to_ftype(imode), ret_index);
 	if (ret < 0)
 		goto out;
 
@@ -262,7 +319,7 @@ int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	key.objectid = parent_ino;
 	key.type = BTRFS_INODE_ITEM_KEY;
 	key.offset = 0;
-	ret = btrfs_search_slot(trans, root, &key, path, 1, 1);
+	ret = btrfs_search_slot(trans, parent_root, &key, path, 1, 1);
 	if (ret)
 		goto out;
 	inode_item = btrfs_item_ptr(path->nodes[0], path->slots[0],
@@ -596,8 +653,8 @@ int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	ret = btrfs_new_inode(trans, root, ret_ino, mode | S_IFDIR);
 	if (ret)
 		goto out;
-	ret = btrfs_add_link(trans, root, ret_ino, parent_ino, name, namelen,
-			     NULL, 0);
+	ret = btrfs_add_link(trans, root, ret_ino, root, parent_ino, name,
+			     namelen, NULL, 0);
 	if (ret)
 		goto out;
 out:
@@ -607,136 +664,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.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);
-	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.
  */
-- 
2.42.0


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

* [PATCH v2 5/9] btrfs-progs: enhance btrfs_mkdir() function
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
                   ` (3 preceding siblings ...)
  2023-10-19 22:30 ` [PATCH v2 4/9] btrfs-progs: merge btrfs_mksubvol() into btrfs_add_link() Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 6/9] btrfs-progs: enhance btrfs_create_root() function Qu Wenruo
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

The function btrfs_mkdir() is currently only utilized by btrfs check, to
create the lost+found directory.

However we're going to add extra callers for this function, to create
directories (and subvolumes) for the incoming "mkfs.btrfs --subvolume"
option.

Thus here we want extra checks for the @parent_ino:

- Make sure the parent inode exists
- Make sure the parent inode is indeed a directory

And since we're here, also convert the @path to a on-stack one to
prevent memory leakage.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/inode.c | 53 +++++++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/kernel-shared/inode.c b/kernel-shared/inode.c
index 38ca882141e2..c64485353b1d 100644
--- a/kernel-shared/inode.c
+++ b/kernel-shared/inode.c
@@ -608,18 +608,41 @@ 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_dir_item *dir_item;
-	struct btrfs_path *path;
+	struct btrfs_path path = { 0 };
+	struct btrfs_key key;
+	struct btrfs_inode_item *iitem;
 	u64 ret_ino = 0;
 	int ret = 0;
 
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
-
 	if (ino && *ino)
 		ret_ino = *ino;
 
-	dir_item = btrfs_lookup_dir_item(NULL, root, path, parent_ino,
+	/* Make sure the parent inode exists and is a directory. */
+	key.objectid = parent_ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	ret = btrfs_lookup_inode(NULL, root, &path, &key, 0);
+	if (ret > 0) {
+		ret = -ENOENT;
+		/* Fallthrough */
+	}
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to lookup inode %llu in root %lld: %m",
+		      parent_ino, root->root_key.objectid);
+		goto out;
+	}
+	iitem = btrfs_item_ptr(path.nodes[0], path.slots[0], struct btrfs_inode_item);
+	if (!S_ISDIR(btrfs_inode_mode(path.nodes[0], iitem))) {
+		ret = -EUCLEAN;
+		errno = -ret;
+		error("inode %llu in root %lld is not a directory", parent_ino,
+		      root->root_key.objectid);
+		goto out;
+	}
+	btrfs_release_path(&path);
+
+	dir_item = btrfs_lookup_dir_item(NULL, root, &path, parent_ino,
 					 name, namelen, 0);
 	if (IS_ERR(dir_item)) {
 		ret = PTR_ERR(dir_item);
@@ -633,23 +656,19 @@ int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		 * Already have conflicting name, check if it is a dir.
 		 * Either way, no need to continue.
 		 */
-		btrfs_dir_item_key_to_cpu(path->nodes[0], dir_item, &found_key);
+		btrfs_dir_item_key_to_cpu(path.nodes[0], dir_item, &found_key);
 		ret_ino = found_key.objectid;
-		if (btrfs_dir_ftype(path->nodes[0], dir_item) != BTRFS_FT_DIR)
+		if (btrfs_dir_ftype(path.nodes[0], dir_item) != BTRFS_FT_DIR)
 			ret = -EEXIST;
 		goto out;
 	}
 
-	if (!ret_ino)
-		/*
-		 * This is *UNSAFE* if some leaf is corrupted,
-		 * only used as a fallback method. Caller should either
-		 * ensure the fs is OK or pass ino with unused inode number.
-		 */
+	if (!ret_ino) {
 		ret = btrfs_find_free_objectid(NULL, root, parent_ino,
 					       &ret_ino);
-	if (ret)
-		goto out;
+		if (ret)
+			goto out;
+	}
 	ret = btrfs_new_inode(trans, root, ret_ino, mode | S_IFDIR);
 	if (ret)
 		goto out;
@@ -658,7 +677,7 @@ int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	if (ret)
 		goto out;
 out:
-	btrfs_free_path(path);
+	btrfs_release_path(&path);
 	if (ret == 0 && ino)
 		*ino = ret_ino;
 	return ret;
-- 
2.42.0


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

* [PATCH v2 6/9] btrfs-progs: enhance btrfs_create_root() function
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
                   ` (4 preceding siblings ...)
  2023-10-19 22:30 ` [PATCH v2 5/9] btrfs-progs: enhance btrfs_mkdir() function Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 7/9] btrfs-progs: use a unified btrfs_make_subvol() implementation Qu Wenruo
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

This patch does the following changes:

- Remove the @fs_info parameter
  It can be fetched from @trans.

- Update a comment
  Now the function can update fs_info pointers for not only quota tree,
  but also block group tree.

- Error out immediately if the @objectid is invalid

- Use btrfs_create_tree() to handle the root item/node creation

- Do not update fs_info root pointers before the root fully created
  This is to prevent an use-after-free if we failed to insert the root
  item.

- Add subvolume and data reloc tree into the fs roots cache
  This is to prevent eb leak on root->node.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/ctree.c | 106 ++++++++++++++++++------------------------
 kernel-shared/ctree.h |   3 +-
 mkfs/main.c           |   2 +-
 tune/convert-bgt.c    |   3 +-
 tune/quota.c          |   2 +-
 5 files changed, 48 insertions(+), 68 deletions(-)

diff --git a/kernel-shared/ctree.c b/kernel-shared/ctree.c
index 61cf125cc136..38c41135d438 100644
--- a/kernel-shared/ctree.c
+++ b/kernel-shared/ctree.c
@@ -26,8 +26,10 @@
 #include "kernel-shared/print-tree.h"
 #include "kernel-shared/tree-checker.h"
 #include "kernel-shared/volumes.h"
+#include "kernel-shared/messages.h"
 #include "common/internal.h"
 #include "common/messages.h"
+#include "common/rbtree-utils.h"
 
 static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root
 		      *root, struct btrfs_path *path, int level);
@@ -331,57 +333,17 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
  *
  * NOTE: Doesn't support tree with non-zero offset, like data reloc tree.
  */
-int btrfs_create_root(struct btrfs_trans_handle *trans,
-		      struct btrfs_fs_info *fs_info, u64 objectid)
+int btrfs_create_root(struct btrfs_trans_handle *trans, u64 objectid)
 {
-	struct extent_buffer *node;
-	struct btrfs_root *new_root;
-	struct btrfs_disk_key disk_key;
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct extent_buffer *node = NULL;
+	struct btrfs_root *new_root = NULL;
 	struct btrfs_key location;
-	struct btrfs_root_item root_item = { 0 };
-	int ret;
-
-	new_root = malloc(sizeof(*new_root));
-	if (!new_root)
-		return -ENOMEM;
-
-	btrfs_setup_root(new_root, fs_info, objectid);
-	if (!is_fstree(objectid))
-		set_bit(BTRFS_ROOT_TRACK_DIRTY, &new_root->state);
-	add_root_to_dirty_list(new_root);
-
-	new_root->objectid = objectid;
-	new_root->root_key.objectid = objectid;
-	new_root->root_key.type = BTRFS_ROOT_ITEM_KEY;
-	new_root->root_key.offset = 0;
-
-	node = btrfs_alloc_tree_block(trans, new_root, fs_info->nodesize,
-				      objectid, &disk_key, 0, 0, 0,
-				      BTRFS_NESTING_NORMAL);
-	if (IS_ERR(node)) {
-		ret = PTR_ERR(node);
-		error("failed to create root node for tree %llu: %d (%m)",
-		      objectid, ret);
-		return ret;
-	}
-	new_root->node = node;
-
-	memset_extent_buffer(node, 0, 0, sizeof(struct btrfs_header));
-	btrfs_set_header_bytenr(node, node->start);
-	btrfs_set_header_generation(node, trans->transid);
-	btrfs_set_header_backref_rev(node, BTRFS_MIXED_BACKREF_REV);
-	btrfs_set_header_owner(node, objectid);
-	write_extent_buffer_fsid(node, fs_info->fs_devices->metadata_uuid);
-	write_extent_buffer_chunk_tree_uuid(node, fs_info->chunk_tree_uuid);
-	btrfs_set_header_nritems(node, 0);
-	btrfs_set_header_level(node, 0);
-	ret = btrfs_inc_ref(trans, new_root, node, 0);
-	if (ret < 0)
-		goto free;
+	int ret = 0;
 
 	/*
 	 * Special tree roots may need to modify pointers in @fs_info
-	 * Only quota is supported yet.
+	 * Only quota and block group trees are supported yet.
 	 */
 	switch (objectid) {
 	case BTRFS_QUOTA_TREE_OBJECTID:
@@ -390,8 +352,6 @@ int btrfs_create_root(struct btrfs_trans_handle *trans,
 			ret = -EEXIST;
 			goto free;
 		}
-		fs_info->quota_root = new_root;
-		fs_info->quota_enabled = 1;
 		break;
 	case BTRFS_BLOCK_GROUP_TREE_OBJECTID:
 		if (fs_info->block_group_root) {
@@ -399,9 +359,7 @@ int btrfs_create_root(struct btrfs_trans_handle *trans,
 			ret = -EEXIST;
 			goto free;
 		}
-		fs_info->block_group_root = new_root;
 		break;
-
 	/*
 	 * Essential trees can't be created by this function, yet.
 	 * As we expect such skeleton exists, or a lot of functions like
@@ -414,27 +372,51 @@ int btrfs_create_root(struct btrfs_trans_handle *trans,
 		ret = -EEXIST;
 		goto free;
 	default:
-		/* Subvolume trees don't need special handling */
-		if (is_fstree(objectid))
+		/*
+		 * Subvolume trees don't need special handling.
+		 * In progs we also treat DATA_RELOC tree just as a subvolume.
+		 */
+		if (is_fstree(objectid) || objectid == BTRFS_DATA_RELOC_TREE_OBJECTID)
 			break;
 		/* Other special trees are not supported yet */
 		ret = -ENOTTY;
 		goto free;
 	}
-	btrfs_mark_buffer_dirty(node);
-	btrfs_set_root_bytenr(&root_item, btrfs_header_bytenr(node));
-	btrfs_set_root_level(&root_item, 0);
-	btrfs_set_root_generation(&root_item, trans->transid);
-	btrfs_set_root_dirid(&root_item, 0);
-	btrfs_set_root_refs(&root_item, 1);
-	btrfs_set_root_used(&root_item, fs_info->nodesize);
+
 	location.objectid = objectid;
 	location.type = BTRFS_ROOT_ITEM_KEY;
 	location.offset = 0;
+	new_root = btrfs_create_tree(trans, fs_info, &location);
+	if (IS_ERR(new_root)) {
+		ret = PTR_ERR(new_root);
+		errno = -ret;
+		error("failed to create new tree for rootid %lld: %m", objectid);
+		return ret;
+	}
 
-	ret = btrfs_insert_root(trans, fs_info->tree_root, &location, &root_item);
-	if (ret < 0)
-		goto free;
+	add_root_to_dirty_list(new_root);
+
+	switch (objectid) {
+	case BTRFS_QUOTA_TREE_OBJECTID:
+		fs_info->quota_root = new_root;
+		fs_info->quota_enabled = 1;
+		break;
+	case BTRFS_BLOCK_GROUP_TREE_OBJECTID:
+		fs_info->block_group_root = new_root;
+		break;
+	default:
+		/*
+		 * We need to add subvolume trees to its rb tree, or we will
+		 * leak root->node.
+		 */
+		ASSERT(is_fstree(objectid) ||
+		       objectid == BTRFS_DATA_RELOC_TREE_OBJECTID);
+		ret = rb_insert(&fs_info->fs_root_tree, &new_root->rb_node,
+				btrfs_fs_roots_compare_roots);
+		if (ret < 0)
+			goto free;
+		set_bit(BTRFS_ROOT_SHAREABLE, &new_root->state);
+	}
 	return ret;
 
 free:
diff --git a/kernel-shared/ctree.h b/kernel-shared/ctree.h
index ff43fefca802..34e14ea21420 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -989,8 +989,7 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
 		      struct btrfs_root *root,
 		      struct extent_buffer *buf,
 		      struct extent_buffer **cow_ret, u64 new_root_objectid);
-int btrfs_create_root(struct btrfs_trans_handle *trans,
-		      struct btrfs_fs_info *fs_info, u64 objectid);
+int btrfs_create_root(struct btrfs_trans_handle *trans, u64 objectid);
 void btrfs_extend_item(struct btrfs_path *path, u32 data_size);
 void btrfs_truncate_item(struct btrfs_path *path, u32 new_size, int from_end);
 int btrfs_split_item(struct btrfs_trans_handle *trans,
diff --git a/mkfs/main.c b/mkfs/main.c
index 3039fbb8a266..cf8922999b31 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1007,7 +1007,7 @@ static int setup_quota_root(struct btrfs_fs_info *fs_info)
 		error_msg(ERROR_MSG_START_TRANS, "%m");
 		return ret;
 	}
-	ret = btrfs_create_root(trans, fs_info, BTRFS_QUOTA_TREE_OBJECTID);
+	ret = btrfs_create_root(trans, BTRFS_QUOTA_TREE_OBJECTID);
 	if (ret < 0) {
 		error("failed to create quota root: %d (%m)", ret);
 		goto fail;
diff --git a/tune/convert-bgt.c b/tune/convert-bgt.c
index dd3a8c750604..a027e50b79a0 100644
--- a/tune/convert-bgt.c
+++ b/tune/convert-bgt.c
@@ -54,8 +54,7 @@ int convert_to_bg_tree(struct btrfs_fs_info *fs_info)
 	if (btrfs_super_flags(sb) & BTRFS_SUPER_FLAG_CHANGING_BG_TREE)
 		goto iterate_bgs;
 
-	ret = btrfs_create_root(trans, fs_info,
-				BTRFS_BLOCK_GROUP_TREE_OBJECTID);
+	ret = btrfs_create_root(trans, BTRFS_BLOCK_GROUP_TREE_OBJECTID);
 	if (ret < 0) {
 		error("failed to create block group root: %d", ret);
 		goto error;
diff --git a/tune/quota.c b/tune/quota.c
index a14f453078d5..5896a61e61d6 100644
--- a/tune/quota.c
+++ b/tune/quota.c
@@ -107,7 +107,7 @@ int enable_quota(struct btrfs_fs_info *fs_info, bool simple)
 		return ret;
 	}
 
-	ret = btrfs_create_root(trans, fs_info, BTRFS_QUOTA_TREE_OBJECTID);
+	ret = btrfs_create_root(trans, BTRFS_QUOTA_TREE_OBJECTID);
 	if (ret < 0) {
 		error("failed to create quota root: %d (%m)", ret);
 		goto fail;
-- 
2.42.0


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

* [PATCH v2 7/9] btrfs-progs: use a unified btrfs_make_subvol() implementation
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
                   ` (5 preceding siblings ...)
  2023-10-19 22:30 ` [PATCH v2 6/9] btrfs-progs: enhance btrfs_create_root() function Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 8/9] btrfs-progs: mkfs: introduce experimental --subvol option Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 9/9] btrfs-progs: mkfs-tests: introduce a test case to verify " Qu Wenruo
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

To create a subvolume there are several different helpers:

- create_subvol() from convert/main.c
  This relies one using an empty fs_root tree to copy its root item.
  But otherwise has a pretty well wrapper btrfs_ake_root_dir() helper to
  handle the inode item/ref insertion.

- create_data_reloc_tree() from mkfs/main.c
  This is already pretty usable for generic subvolume creation, the only
  bad code is the open-coded rootdir setup.

So here this patch introduce a better version with all the benefit from
the above implementations:

- Move btrfs_make_root_dir() into common/inode.[ch]

- Implement a unified btrfs_make_subvol() to replace above two implementations
  It would go with btrfs_create_root(), and btrfs_make_root_dir() to
  remove duplicated code, and return a btrfs_root pointer for caller
  to utilize.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 Makefile       |  1 +
 check/main.c   |  1 +
 common/inode.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++
 common/inode.h | 16 +++++++++
 convert/main.c | 56 +++++++------------------------
 mkfs/common.c  | 39 ----------------------
 mkfs/common.h  |  2 --
 mkfs/main.c    | 79 ++++----------------------------------------
 8 files changed, 127 insertions(+), 157 deletions(-)
 create mode 100644 common/inode.c
 create mode 100644 common/inode.h

diff --git a/Makefile b/Makefile
index 273a1f0e3b7c..e3534be600fd 100644
--- a/Makefile
+++ b/Makefile
@@ -209,6 +209,7 @@ objects = \
 	common/fsfeatures.o	\
 	common/help.o	\
 	common/inject-error.o	\
+	common/inode.o		\
 	common/messages.o	\
 	common/open-utils.o	\
 	common/parse-utils.o	\
diff --git a/check/main.c b/check/main.c
index 4a88cc8f8708..8c884a9332f8 100644
--- a/check/main.c
+++ b/check/main.c
@@ -48,6 +48,7 @@
 #include "kernel-shared/file-item.h"
 #include "kernel-shared/tree-checker.h"
 #include "common/defs.h"
+#include "common/inode.h"
 #include "common/extent-cache.h"
 #include "common/internal.h"
 #include "common/messages.h"
diff --git a/common/inode.c b/common/inode.c
new file mode 100644
index 000000000000..712d50d6e1a9
--- /dev/null
+++ b/common/inode.c
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <time.h>
+#include "kernel-shared/transaction.h"
+#include "kernel-shared/disk-io.h"
+#include "common/inode.h"
+#include "common/messages.h"
+
+int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
+			struct btrfs_root *root, u64 objectid)
+{
+	int ret;
+	struct btrfs_inode_item inode_item;
+	time_t now = time(NULL);
+
+	memset(&inode_item, 0, sizeof(inode_item));
+	btrfs_set_stack_inode_generation(&inode_item, trans->transid);
+	btrfs_set_stack_inode_size(&inode_item, 0);
+	btrfs_set_stack_inode_nlink(&inode_item, 1);
+	btrfs_set_stack_inode_nbytes(&inode_item, root->fs_info->nodesize);
+	btrfs_set_stack_inode_mode(&inode_item, S_IFDIR | 0755);
+	btrfs_set_stack_timespec_sec(&inode_item.atime, now);
+	btrfs_set_stack_timespec_nsec(&inode_item.atime, 0);
+	btrfs_set_stack_timespec_sec(&inode_item.ctime, now);
+	btrfs_set_stack_timespec_nsec(&inode_item.ctime, 0);
+	btrfs_set_stack_timespec_sec(&inode_item.mtime, now);
+	btrfs_set_stack_timespec_nsec(&inode_item.mtime, 0);
+	btrfs_set_stack_timespec_sec(&inode_item.otime, now);
+	btrfs_set_stack_timespec_nsec(&inode_item.otime, 0);
+
+	if (root->fs_info->tree_root == root)
+		btrfs_set_super_root_dir(root->fs_info->super_copy, objectid);
+
+	ret = btrfs_insert_inode(trans, root, objectid, &inode_item);
+	if (ret)
+		goto error;
+
+	ret = btrfs_insert_inode_ref(trans, root, "..", 2, objectid, objectid, 0);
+	if (ret)
+		goto error;
+
+	btrfs_set_root_dirid(&root->root_item, objectid);
+	ret = 0;
+error:
+	return ret;
+}
+
+struct btrfs_root *btrfs_create_subvol(struct btrfs_trans_handle *trans,
+				       u64 objectid)
+{
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_root *root;
+	struct btrfs_key key;
+	int ret;
+
+	ret = btrfs_create_root(trans, objectid);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to create root %lld: %m", objectid);
+		return ERR_PTR(ret);
+	}
+	key.objectid = objectid;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = 0;
+
+	root = btrfs_read_fs_root(fs_info, &key);
+	if (IS_ERR(root)) {
+		ret = PTR_ERR(root);
+		errno = -ret;
+		error("failed to read created subvolume %lld: %m", objectid);
+		return ERR_PTR(ret);
+	}
+
+	ret = btrfs_make_root_dir(trans, root, BTRFS_FIRST_FREE_OBJECTID);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to update root dir for root %llu",
+		      root->root_key.objectid);
+		return ERR_PTR(ret);
+	}
+	ret = btrfs_update_root(trans, trans->fs_info->tree_root,
+				&root->root_key, &root->root_item);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to update root %llu",
+		      root->root_key.objectid);
+		return ERR_PTR(ret);
+	}
+	return root;
+}
diff --git a/common/inode.h b/common/inode.h
new file mode 100644
index 000000000000..912caf1f7904
--- /dev/null
+++ b/common/inode.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This is the btrfs-progs specific inode related helpers, which is different
+ * from kernel code.
+ */
+#ifndef __BTRFS_INODE_H__
+#define __BTRFS_INODE_H__
+
+#include "kernel-shared/ctree.h"
+
+int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
+			struct btrfs_root *root, u64 objectid);
+struct btrfs_root *btrfs_create_subvol(struct btrfs_trans_handle *trans,
+				       u64 objectid);
+#endif
diff --git a/convert/main.c b/convert/main.c
index bcd310228fcd..991e45556c04 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -107,6 +107,7 @@
 #include "kernel-shared/ctree.h"
 #include "crypto/hash.h"
 #include "common/defs.h"
+#include "common/inode.h"
 #include "common/extent-cache.h"
 #include "common/internal.h"
 #include "common/cpu-utils.h"
@@ -916,44 +917,6 @@ out:
 	return ret;
 }
 
-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.
@@ -1013,6 +976,7 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
 static int init_btrfs(struct btrfs_mkfs_config *cfg, struct btrfs_root *root,
 			 struct btrfs_convert_context *cctx, u32 convert_flags)
 {
+	struct btrfs_root *created_root;
 	struct btrfs_key location;
 	struct btrfs_trans_handle *trans;
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -1058,15 +1022,19 @@ static int init_btrfs(struct btrfs_mkfs_config *cfg, struct btrfs_root *root,
 			     BTRFS_FIRST_FREE_OBJECTID);
 
 	/* subvol for fs image file */
-	ret = create_subvol(trans, root, CONV_IMAGE_SUBVOL_OBJECTID);
-	if (ret < 0) {
-		error("failed to create subvolume image root: %d", ret);
+	created_root = btrfs_create_subvol(trans, CONV_IMAGE_SUBVOL_OBJECTID);
+	if (IS_ERR(created_root)) {
+		ret = PTR_ERR(created_root);
+		errno = -ret;
+		error("failed to create subvolume image root: %m");
 		goto err;
 	}
 	/* subvol for data relocation tree */
-	ret = create_subvol(trans, root, BTRFS_DATA_RELOC_TREE_OBJECTID);
-	if (ret < 0) {
-		error("failed to create DATA_RELOC root: %d", ret);
+	created_root = btrfs_create_subvol(trans, BTRFS_DATA_RELOC_TREE_OBJECTID);
+	if (IS_ERR(created_root)) {
+		ret = PTR_ERR(created_root);
+		errno = -ret;
+		error("failed to create DATA_RELOC root: %m");
 		goto err;
 	}
 
diff --git a/mkfs/common.c b/mkfs/common.c
index 5e56b33dda6d..de370a86fd97 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -757,45 +757,6 @@ out:
 	return ret;
 }
 
-int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
-			struct btrfs_root *root, u64 objectid)
-{
-	int ret;
-	struct btrfs_inode_item inode_item;
-	time_t now = time(NULL);
-
-	memset(&inode_item, 0, sizeof(inode_item));
-	btrfs_set_stack_inode_generation(&inode_item, trans->transid);
-	btrfs_set_stack_inode_size(&inode_item, 0);
-	btrfs_set_stack_inode_nlink(&inode_item, 1);
-	btrfs_set_stack_inode_nbytes(&inode_item, root->fs_info->nodesize);
-	btrfs_set_stack_inode_mode(&inode_item, S_IFDIR | 0755);
-	btrfs_set_stack_timespec_sec(&inode_item.atime, now);
-	btrfs_set_stack_timespec_nsec(&inode_item.atime, 0);
-	btrfs_set_stack_timespec_sec(&inode_item.ctime, now);
-	btrfs_set_stack_timespec_nsec(&inode_item.ctime, 0);
-	btrfs_set_stack_timespec_sec(&inode_item.mtime, now);
-	btrfs_set_stack_timespec_nsec(&inode_item.mtime, 0);
-	btrfs_set_stack_timespec_sec(&inode_item.otime, now);
-	btrfs_set_stack_timespec_nsec(&inode_item.otime, 0);
-
-	if (root->fs_info->tree_root == root)
-		btrfs_set_super_root_dir(root->fs_info->super_copy, objectid);
-
-	ret = btrfs_insert_inode(trans, root, objectid, &inode_item);
-	if (ret)
-		goto error;
-
-	ret = btrfs_insert_inode_ref(trans, root, "..", 2, objectid, objectid, 0);
-	if (ret)
-		goto error;
-
-	btrfs_set_root_dirid(&root->root_item, objectid);
-	ret = 0;
-error:
-	return ret;
-}
-
 /*
  * Btrfs minimum size calculation is complicated, it should include at least:
  * 1. system group size
diff --git a/mkfs/common.h b/mkfs/common.h
index d9183c997bb2..d1c0eec80e1e 100644
--- a/mkfs/common.h
+++ b/mkfs/common.h
@@ -103,8 +103,6 @@ struct btrfs_mkfs_config {
 };
 
 int make_btrfs(int fd, struct btrfs_mkfs_config *cfg);
-int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
-			struct btrfs_root *root, u64 objectid);
 u64 btrfs_min_dev_size(u32 nodesize, int mixed, u64 meta_profile,
 		       u64 data_profile);
 int test_minimum_size(const char *file, u64 min_dev_size);
diff --git a/mkfs/main.c b/mkfs/main.c
index cf8922999b31..e5201fbc9c31 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -44,6 +44,7 @@
 #include "crypto/hash.h"
 #include "common/defs.h"
 #include "common/internal.h"
+#include "common/inode.h"
 #include "common/messages.h"
 #include "common/cpu-utils.h"
 #include "common/utils.h"
@@ -706,75 +707,6 @@ static void update_chunk_allocation(struct btrfs_fs_info *fs_info,
 	}
 }
 
-static int create_data_reloc_tree(struct btrfs_trans_handle *trans)
-{
-	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_inode_item *inode;
-	struct btrfs_root *root;
-	struct btrfs_path path = { 0 };
-	struct btrfs_key key = {
-		.objectid = BTRFS_DATA_RELOC_TREE_OBJECTID,
-		.type = BTRFS_ROOT_ITEM_KEY,
-	};
-	u64 ino = BTRFS_FIRST_FREE_OBJECTID;
-	char *name = "..";
-	int ret;
-
-	root = btrfs_create_tree(trans, fs_info, &key);
-	if (IS_ERR(root)) {
-		ret = PTR_ERR(root);
-		goto out;
-	}
-	/* Update dirid as created tree has default dirid 0 */
-	btrfs_set_root_dirid(&root->root_item, ino);
-	ret = btrfs_update_root(trans, fs_info->tree_root, &root->root_key,
-				&root->root_item);
-	if (ret < 0)
-		goto out;
-
-	/* Cache this tree so it can be cleaned up at close_ctree() */
-	ret = rb_insert(&fs_info->fs_root_tree, &root->rb_node,
-			btrfs_fs_roots_compare_roots);
-	if (ret < 0)
-		goto out;
-
-	/* Insert INODE_ITEM */
-	ret = btrfs_new_inode(trans, root, ino, 0755 | S_IFDIR);
-	if (ret < 0)
-		goto out;
-
-	/* then INODE_REF */
-	ret = btrfs_insert_inode_ref(trans, root, name, strlen(name), ino, ino,
-				     0);
-	if (ret < 0)
-		goto out;
-
-	/* Update nlink of that inode item */
-	key.objectid = ino;
-	key.type = BTRFS_INODE_ITEM_KEY;
-	key.offset = 0;
-
-	ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
-	if (ret > 0) {
-		ret = -ENOENT;
-		btrfs_release_path(&path);
-		goto out;
-	}
-	if (ret < 0) {
-		btrfs_release_path(&path);
-		goto out;
-	}
-	inode = btrfs_item_ptr(path.nodes[0], path.slots[0],
-			       struct btrfs_inode_item);
-	btrfs_set_inode_nlink(path.nodes[0], inode, 1);
-	btrfs_mark_buffer_dirty(path.nodes[0]);
-	btrfs_release_path(&path);
-	return 0;
-out:
-	btrfs_abort_transaction(trans, ret);
-	return ret;
-}
-
 static int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid,
 			       u8 type, u64 subvol_id_cpu)
 {
@@ -1139,6 +1071,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 {
 	char *file;
 	struct btrfs_root *root;
+	struct btrfs_root *data_reloc_root;
 	struct btrfs_fs_info *fs_info;
 	struct btrfs_trans_handle *trans;
 	struct open_ctree_args oca = { 0 };
@@ -1915,9 +1848,11 @@ raid_groups:
 		goto out;
 	}
 
-	ret = create_data_reloc_tree(trans);
-	if (ret) {
-		error("unable to create data reloc tree: %d", ret);
+	data_reloc_root = btrfs_create_subvol(trans, BTRFS_DATA_RELOC_TREE_OBJECTID);
+	if (IS_ERR(data_reloc_root)) {
+		ret = PTR_ERR(data_reloc_root);
+		errno = -ret;
+		error("unable to create data reloc tree: %m");
 		goto out;
 	}
 
-- 
2.42.0


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

* [PATCH v2 8/9] btrfs-progs: mkfs: introduce experimental --subvol option
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
                   ` (6 preceding siblings ...)
  2023-10-19 22:30 ` [PATCH v2 7/9] btrfs-progs: use a unified btrfs_make_subvol() implementation Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  2023-10-19 22:30 ` [PATCH v2 9/9] btrfs-progs: mkfs-tests: introduce a test case to verify " Qu Wenruo
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

Although mkfs.btrfs supports --rootdir to fill the target filesystem, it
doesn't have the ability to create any subvolume.

This patch introduce a very basic version of --subvol for mkfs.btrfs,
the limits are:

- No co-operation with --rootdir
  This requires --rootdir to have extra handling for any existing
  inodes.
  (Currently --rootdir assumes the fs tree is completely empty)

- No multiple --subvol options supports
  This requires us to collect and sort all the paths and start creating
  subvolumes from the shortest path.
  Furthermore this requires us to create subvolume under another
  subvolume.

For now, this patch focus on the basic checks on the provided subvolume
path, to wipe out any invalid things like ".." or something like "//////".

We support something like "//dir1/dir2///subvol///" just like VFS path
(duplicated '/' would just be ignored).

Issue: #42
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 mkfs/main.c    |  25 ++++++++
 mkfs/rootdir.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++
 mkfs/rootdir.h |   1 +
 3 files changed, 183 insertions(+)

diff --git a/mkfs/main.c b/mkfs/main.c
index e5201fbc9c31..c0fdf6e94a3c 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -436,6 +436,9 @@ static const char * const mkfs_usage[] = {
 	"Creation:",
 	OPTLINE("-b|--byte-count SIZE", "set size of each device to SIZE (filesystem size is sum of all device sizes)"),
 	OPTLINE("-r|--rootdir DIR", "copy files from DIR to the image root directory"),
+#if EXPERIMENTAL
+	OPTLINE("--subvol SUBVOL_NAME", "create a subvolume and all its parent directory"),
+#endif
 	OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"),
 	OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"),
 	OPTLINE("-f|--force", "force overwrite of existing filesystem"),
@@ -1110,6 +1113,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 	char *label = NULL;
 	int nr_global_roots = sysconf(_SC_NPROCESSORS_ONLN);
 	char *source_dir = NULL;
+	char *subvol = NULL;
 
 	cpu_detect_flags();
 	hash_init_accel();
@@ -1123,6 +1127,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 			GETOPT_VAL_CHECKSUM,
 			GETOPT_VAL_GLOBAL_ROOTS,
 			GETOPT_VAL_DEVICE_UUID,
+			GETOPT_VAL_SUBVOL,
 		};
 		static const struct option long_options[] = {
 			{ "byte-count", required_argument, NULL, 'b' },
@@ -1151,6 +1156,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 			{ "shrink", no_argument, NULL, GETOPT_VAL_SHRINK },
 #if EXPERIMENTAL
 			{ "num-global-roots", required_argument, NULL, GETOPT_VAL_GLOBAL_ROOTS },
+			{ "subvol", required_argument, NULL, GETOPT_VAL_SUBVOL},
 #endif
 			{ "help", no_argument, NULL, GETOPT_VAL_HELP },
 			{ NULL, 0, NULL, 0}
@@ -1283,6 +1289,10 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 				btrfs_warn_experimental("Feature: num-global-roots is part of exten-tree-v2");
 				nr_global_roots = (int)arg_strtou64(optarg);
 				break;
+			case GETOPT_VAL_SUBVOL:
+				btrfs_warn_experimental("Option --subvol is still experimental");
+				subvol = optarg;
+				break;
 			case GETOPT_VAL_HELP:
 			default:
 				usage(&mkfs_cmd, c != GETOPT_VAL_HELP);
@@ -1319,6 +1329,11 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
 		goto error;
 	}
 
+	if (source_dir && subvol) {
+		error("--subvol is not yet supported with --rootdir");
+		goto error;
+	}
+
 	if (*fs_uuid) {
 		uuid_t dummy_uuid;
 
@@ -1875,6 +1890,16 @@ raid_groups:
 		goto out;
 	}
 
+	/* Create the subvolumes. */
+	if (subvol) {
+		ret = btrfs_make_subvolume_at(fs_info, subvol);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to create subvolume \"%s\": %m", subvol);
+			goto out;
+		}
+	}
+
 	if (source_dir) {
 		pr_verbose(LOG_DEFAULT, "Rootdir from:       %s\n", source_dir);
 		ret = btrfs_mkfs_fill_dir(source_dir, root);
diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
index 4ae9f435a7b7..8d32ca4be77d 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -36,6 +36,8 @@
 #include "kernel-shared/disk-io.h"
 #include "kernel-shared/transaction.h"
 #include "kernel-shared/file-item.h"
+#include "kernel-shared/messages.h"
+#include "common/inode.h"
 #include "common/internal.h"
 #include "common/messages.h"
 #include "common/path-utils.h"
@@ -1012,3 +1014,158 @@ int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
 	}
 	return ret;
 }
+
+/*
+ * Create a subvolume at the path specififed by @full_path.
+ *
+ * The @full_path always starts at fs_tree root.
+ * All the parent directories would be created.
+ */
+int btrfs_make_subvolume_at(struct btrfs_fs_info *fs_info, const char *full_path)
+{
+	struct btrfs_root *root = fs_info->fs_root;
+	struct btrfs_root *subvol;
+	struct btrfs_trans_handle *trans;
+	u64 parent_ino = btrfs_root_dirid(&root->root_item);
+	u64 subvolid;
+	char *dump = strdup(full_path);
+	char *orig = dump;
+	char *filename;
+	int nr_filenames = 0;
+	int cur_filename = 0;
+	int ret = 0;
+
+	if (!dump)
+		return -ENOMEM;
+
+	/*
+	 * Get the number of valid filenames, this is to determine
+	 * if we're at the last filename (and needs to create a subvolume
+	 * other than a direcotry).
+	 */
+	while ((filename = strsep(&dump, "/")) != NULL) {
+		if (strlen(filename) == 0)
+			continue;
+		if (!strcmp(filename, "."))
+			continue;
+		if (!strcmp(filename, "..")) {
+			error("can not use \"..\" for subvolume path");
+			ret = -EINVAL;
+			goto out;
+		}
+		if (strlen(filename) > NAME_MAX) {
+			error("direcotry name \"%s\" is too long, limit is %d",
+			      filename, NAME_MAX);
+			ret = -EINVAL;
+			goto out;
+		}
+		nr_filenames++;
+	}
+	free(orig);
+	orig = NULL;
+	dump = NULL;
+
+	/* Just some garbage full of '/'. */
+	if (nr_filenames == 0) {
+		error("'%s' contains no valid subvolume name", full_path);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	dump = strdup(full_path);
+	orig = dump;
+
+	while ((filename = strsep(&dump, "/")) != NULL) {
+		u64 child_ino = 0;
+
+		if (strlen(filename) == 0)
+			continue;
+		if (!strcmp(filename, "."))
+			continue;
+		ASSERT(strcmp(filename, ".."));
+
+		if (cur_filename == nr_filenames - 1)
+			break;
+		/*
+		 * Need to modify the following items:
+		 * - Parent inode item
+		 *   Increase the size
+		 *
+		 * - Parent 1x DIR_INDEX and 1x DIR_ITEM items
+		 *
+		 * - New child inode item
+		 * - New child inode ref
+		 */
+		trans = btrfs_start_transaction(root, 4);
+		if (IS_ERR(trans)) {
+			errno = -ret;
+			error("failed to start transaction: %m");
+			ret = PTR_ERR(trans);
+			goto out;
+		}
+		ret = btrfs_mkdir(trans, root, filename, strlen(filename),
+				  parent_ino, &child_ino, 0755);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to create direcotry %s in root %lld: %m",
+			      filename, root->root_key.objectid);
+			goto out;
+		}
+		ret = btrfs_commit_transaction(trans, root);
+		if (ret < 0) {
+			errno = -ret;
+			error("failed to commit trans for direcotry %s in root %lld: %m",
+			      filename, root->root_key.objectid);
+			goto out;
+		}
+		parent_ino = child_ino;
+		cur_filename++;
+	}
+	if (!filename) {
+		ret = -EUCLEAN;
+		error("No valid subvolume name found");
+		goto out;
+	}
+
+	/* Create the final subvolume. */
+	trans = btrfs_start_transaction(fs_info->tree_root, 4);
+	if (IS_ERR(trans)) {
+		errno = -ret;
+		error("failed to start transaction for subvolume creation %s: %m",
+		      filename);
+		goto out;
+	}
+	ret = btrfs_find_free_objectid(NULL, fs_info->tree_root,
+				       BTRFS_FIRST_FREE_OBJECTID, &subvolid);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to find a free objectid for subvolume %s: %m",
+		      filename);
+		goto out;
+	}
+	subvol = btrfs_create_subvol(trans, subvolid);
+	if (IS_ERR(subvol)) {
+		ret = PTR_ERR(subvol);
+		errno = -ret;
+		error("failed to create subvolume %s: %m",
+		      filename);
+		goto out;
+	}
+
+	ret = btrfs_add_link(trans, subvol, btrfs_root_dirid(&subvol->root_item),
+			     fs_info->fs_root, parent_ino,
+			     filename, strlen(filename), NULL, false);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to link subvol %s: %m", filename);
+		goto out;
+	}
+	ret = btrfs_commit_transaction(trans, fs_info->tree_root);
+	if (ret < 0) {
+		errno = -ret;
+		error("failed to commit transaction: %m");
+	}
+out:
+	free(orig);
+	return ret;
+}
diff --git a/mkfs/rootdir.h b/mkfs/rootdir.h
index 8d5f6896c3d9..1b21f30e80b5 100644
--- a/mkfs/rootdir.h
+++ b/mkfs/rootdir.h
@@ -41,5 +41,6 @@ u64 btrfs_mkfs_size_dir(const char *dir_name, u32 sectorsize, u64 min_dev_size,
 			u64 meta_profile, u64 data_profile);
 int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret,
 			 bool shrink_file_size);
+int btrfs_make_subvolume_at(struct btrfs_fs_info *fs_info, const char *full_path);
 
 #endif
-- 
2.42.0


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

* [PATCH v2 9/9] btrfs-progs: mkfs-tests: introduce a test case to verify --subvol option
  2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
                   ` (7 preceding siblings ...)
  2023-10-19 22:30 ` [PATCH v2 8/9] btrfs-progs: mkfs: introduce experimental --subvol option Qu Wenruo
@ 2023-10-19 22:30 ` Qu Wenruo
  8 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2023-10-19 22:30 UTC (permalink / raw)
  To: linux-btrfs

This test case would try the following combinations:

- Valid cases
  * subvolume directly under fs tree
  * subvolume with several parent directories
  * subvolume path with unnecessary too many duplicated '/'s

- Invalid cases
  * subvolume path without any filename, e.g. "/////"
  * subvolume path with ".."

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/mkfs-tests/031-subvol-option/test.sh | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100755 tests/mkfs-tests/031-subvol-option/test.sh

diff --git a/tests/mkfs-tests/031-subvol-option/test.sh b/tests/mkfs-tests/031-subvol-option/test.sh
new file mode 100755
index 000000000000..eedd257b746e
--- /dev/null
+++ b/tests/mkfs-tests/031-subvol-option/test.sh
@@ -0,0 +1,39 @@
+#!/bin/bash
+# Verify "mkfs.btrfs --subvol" option can handle valid and invalid subvolume
+# paths correctly
+
+source "$TEST_TOP/common" || exit
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev
+
+# Make sure we have enabled experimental features
+output=$(run_check_stdout "$TOP/mkfs.btrfs" --help 2>&1 | grep -- --subvol)
+
+if [ -z "$output" ]; then
+	_not_run "experimental features are not enabled"
+fi
+
+# Valid case, simple subvolume directly under fs root.
+run_check_mkfs_test_dev --subvol "subv"
+run_check "$TOP/btrfs" check $TEST_DEV
+
+# Valid case, subvolume with several parent directories.
+run_check_mkfs_test_dev --subvol "dir1/dir2/dir3/subv"
+run_check "$TOP/btrfs" check $TEST_DEV
+
+# Valid case, subvolume with several parent directories, and
+# unnecessarily too many '/'s
+run_check_mkfs_test_dev --subvol "////dir1/dir2//dir3/dir4/////subv////"
+run_check "$TOP/btrfs" check $TEST_DEV
+
+# Invalid case, subvolume path contains no filename
+run_mustfail "invalid subvolume without filename should fail" \
+	"$TOP/mkfs.btrfs" -f --subvol "//////" $TEST_DEV
+
+# Invalid case, subvolume path contains ".."
+run_mustfail "invalid subvolume without filename should fail" \
+	"$TOP/mkfs.btrfs" -f --subvol "../subvol" $TEST_DEV
-- 
2.42.0


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

end of thread, other threads:[~2023-10-19 22:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-19 22:29 [PATCH v2 0/9] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 1/9] btrfs-progs: cross-port fs_types from kernel Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 2/9] btrfs-progs: remove add_backref parameter from btrfs_add_link() Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 3/9] btrfs-progs: remove filetype parameter of btrfs_add_link() Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 4/9] btrfs-progs: merge btrfs_mksubvol() into btrfs_add_link() Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 5/9] btrfs-progs: enhance btrfs_mkdir() function Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 6/9] btrfs-progs: enhance btrfs_create_root() function Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 7/9] btrfs-progs: use a unified btrfs_make_subvol() implementation Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 8/9] btrfs-progs: mkfs: introduce experimental --subvol option Qu Wenruo
2023-10-19 22:30 ` [PATCH v2 9/9] btrfs-progs: mkfs-tests: introduce a test case to verify " Qu Wenruo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.