* [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option
@ 2023-10-16 4:38 Qu Wenruo
2023-10-16 4:38 ` [PATCH 1/6] btrfs-progs: enhance btrfs_mkdir() function Qu Wenruo
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Qu Wenruo @ 2023-10-16 4:38 UTC (permalink / raw)
To: linux-btrfs
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 (6):
btrfs-progs: enhance btrfs_mkdir() function
btrfs-progs: enhance and rename btrfs_mksubvol() 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
convert/main.c | 60 ++------
kernel-shared/ctree.c | 106 ++++++--------
kernel-shared/ctree.h | 12 +-
kernel-shared/inode.c | 129 ++++++++++++-----
kernel-shared/root-tree.c | 86 +++++++++++
mkfs/common.c | 39 -----
mkfs/common.h | 2 -
mkfs/main.c | 103 ++++----------
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 +-
13 files changed, 473 insertions(+), 266 deletions(-)
create mode 100755 tests/mkfs-tests/031-subvol-option/test.sh
--
2.42.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/6] btrfs-progs: enhance btrfs_mkdir() function
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
@ 2023-10-16 4:38 ` Qu Wenruo
2023-10-16 4:38 ` [PATCH 2/6] btrfs-progs: enhance and rename btrfs_mksubvol() function Qu Wenruo
` (5 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2023-10-16 4:38 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 3d420787c8f9..50bb460acc79 100644
--- a/kernel-shared/inode.c
+++ b/kernel-shared/inode.c
@@ -526,18 +526,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);
@@ -551,23 +574,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;
@@ -576,7 +595,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] 19+ messages in thread
* [PATCH 2/6] btrfs-progs: enhance and rename btrfs_mksubvol() function
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
2023-10-16 4:38 ` [PATCH 1/6] btrfs-progs: enhance btrfs_mkdir() function Qu Wenruo
@ 2023-10-16 4:38 ` Qu Wenruo
2023-10-16 4:38 ` [PATCH 3/6] btrfs-progs: enhance btrfs_create_root() function Qu Wenruo
` (4 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2023-10-16 4:38 UTC (permalink / raw)
To: linux-btrfs
The function btrfs_mksubvol() is currently only utilized by
btrfs-convert, to create a subvolume to contain the image file.
With the incoming support for "mkfs.btrfs --subvolume" option, we need
the following features:
- Create the subvolume under a specified directory
The old can only create the subvolume under the rootdir.
- Add some basic sanity checks
Making sure the parent inode exists and is a directory inode.
And also the function name is confusing, the work of btrfs_mksubvol() is
really linking a subvolume under a directory, not really create the full
subvolume.
This patch would add those needed features, and rename the function to
btrfs_link_subvol().
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
convert/main.c | 5 +--
kernel-shared/ctree.h | 5 +--
kernel-shared/inode.c | 76 +++++++++++++++++++++++++++++++------------
3 files changed, 62 insertions(+), 24 deletions(-)
diff --git a/convert/main.c b/convert/main.c
index c9e50c036f92..73740fe26d55 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1311,8 +1311,9 @@ 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);
+ image_root = btrfs_link_subvol(root, btrfs_root_dirid(&root->root_item),
+ subvol_name, CONV_IMAGE_SUBVOL_OBJECTID,
+ true);
if (!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 bcf11d870061..a10a30e36b94 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -1230,8 +1230,9 @@ int btrfs_add_orphan_item(struct btrfs_trans_handle *trans,
u64 ino);
int btrfs_mkdir(struct btrfs_trans_handle *trans, struct btrfs_root *root,
char *name, int namelen, u64 parent_ino, u64 *ino, int mode);
-struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root, const char *base,
- u64 root_objectid, bool convert);
+struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root, u64 parent_ino,
+ const char *name, u64 root_objectid,
+ bool retry_suffix_names);
int btrfs_find_free_objectid(struct btrfs_trans_handle *trans,
struct btrfs_root *fs_root,
u64 dirid, u64 *objectid);
diff --git a/kernel-shared/inode.c b/kernel-shared/inode.c
index 50bb460acc79..ebb28b7666cc 100644
--- a/kernel-shared/inode.c
+++ b/kernel-shared/inode.c
@@ -601,11 +601,19 @@ out:
return ret;
}
-struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
- const char *base, u64 root_objectid,
- bool convert)
+/*
+ * Link a subvolume with @root_objectid to @parent_ino of @root, with the name
+ * @base.
+ *
+ * If @retry_suffix_names is true, and if there is already a dir item with the
+ * same name of @base, then it would retry with an increasing number as suffix,
+ * until a conflict-free name is found, or have tried too many times.
+ */
+struct btrfs_root *btrfs_link_subvol(struct btrfs_root *root, u64 parent_ino,
+ const char *name, u64 root_objectid,
+ bool retry_suffix_names)
{
- struct btrfs_trans_handle *trans;
+ struct btrfs_trans_handle *trans = NULL;
struct btrfs_fs_info *fs_info = root->fs_info;
struct btrfs_root *tree_root = fs_info->tree_root;
struct btrfs_root *new_root = NULL;
@@ -613,32 +621,58 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
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)
+ len = strlen(name);
+ if (len == 0 || len > BTRFS_NAME_LEN) {
+ error("invalid name length %u", len);
return NULL;
+ }
- key.objectid = dirid;
+ /* Make sure @parent_ino of @root 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 locate directory inode %llu of root %lld: %m",
+ parent_ino, root->root_key.objectid);
+ goto fail;
+ }
+ inode_item = btrfs_item_ptr(path.nodes[0], path.slots[0],
+ struct btrfs_inode_item);
+ if (!S_ISDIR(btrfs_inode_mode(path.nodes[0], inode_item))) {
+ ret = -EUCLEAN;
+ error("inode %llu of root %lld is not a directory",
+ parent_ino, root->root_key.objectid);
+ goto fail;
+ }
+ btrfs_release_path(&path);
+
+ /* Locate one free dir index number. */
+ key.objectid = parent_ino;
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);
+ parent_ino, 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)
+ if (key.objectid == parent_ino && key.type == BTRFS_DIR_INDEX_KEY)
index = key.offset + 1;
}
btrfs_release_path(&path);
@@ -651,14 +685,14 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
goto fail;
}
- key.objectid = dirid;
+ key.objectid = parent_ino;
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);
+ parent_ino, ret);
goto fail;
}
leaf = path.nodes[0];
@@ -669,21 +703,21 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
key.offset = (u64)-1;
key.type = BTRFS_ROOT_ITEM_KEY;
- memcpy(buf, base, len);
- if (convert) {
+ memcpy(buf, name, len);
+ if (retry_suffix_names) {
for (i = 0; i < 1024; i++) {
ret = btrfs_insert_dir_item(trans, root, buf, len,
- dirid, &key, BTRFS_FT_DIR, index);
+ parent_ino, &key, BTRFS_FT_DIR, index);
if (ret != -EEXIST)
break;
- len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", base, i);
+ len = snprintf(buf, ARRAY_SIZE(buf), "%s%d", name, i);
if (len < 1 || len > BTRFS_NAME_LEN) {
ret = -EINVAL;
break;
}
}
} else {
- ret = btrfs_insert_dir_item(trans, root, buf, len, dirid, &key,
+ ret = btrfs_insert_dir_item(trans, root, buf, len, parent_ino, &key,
BTRFS_FT_DIR, index);
}
if (ret)
@@ -698,7 +732,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
ret = btrfs_add_root_ref(trans, tree_root, root_objectid,
BTRFS_ROOT_BACKREF_KEY,
root->root_key.objectid,
- dirid, index, buf, len);
+ parent_ino, index, buf, len);
if (ret) {
error("unable to add root backref for %llu: %d",
root->root_key.objectid, ret);
@@ -708,7 +742,7 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
/* 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);
+ parent_ino, index, buf, len);
if (ret) {
error("unable to add root ref for %llu: %d",
root->root_key.objectid, ret);
@@ -728,6 +762,8 @@ struct btrfs_root *btrfs_mksubvol(struct btrfs_root *root,
new_root = NULL;
}
fail:
+ if (trans && ret < 0)
+ btrfs_abort_transaction(trans, ret);
return new_root;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/6] btrfs-progs: enhance btrfs_create_root() function
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
2023-10-16 4:38 ` [PATCH 1/6] btrfs-progs: enhance btrfs_mkdir() function Qu Wenruo
2023-10-16 4:38 ` [PATCH 2/6] btrfs-progs: enhance and rename btrfs_mksubvol() function Qu Wenruo
@ 2023-10-16 4:38 ` Qu Wenruo
2023-10-16 4:38 ` [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation Qu Wenruo
` (3 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2023-10-16 4:38 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 a10a30e36b94..1dda40e96a60 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -988,8 +988,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 7d0ffac309e8..9584386da4ca 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1006,7 +1006,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] 19+ messages in thread
* [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
` (2 preceding siblings ...)
2023-10-16 4:38 ` [PATCH 3/6] btrfs-progs: enhance btrfs_create_root() function Qu Wenruo
@ 2023-10-16 4:38 ` Qu Wenruo
2023-10-17 13:49 ` Josef Bacik
2023-10-16 4:38 ` [PATCH 5/6] btrfs-progs: mkfs: introduce experimental --subvol option Qu Wenruo
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2023-10-16 4:38 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 kernel-shared/root-tree.[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>
---
convert/main.c | 55 +++++--------------------
kernel-shared/ctree.h | 4 ++
kernel-shared/root-tree.c | 86 +++++++++++++++++++++++++++++++++++++++
mkfs/common.c | 39 ------------------
mkfs/common.h | 2 -
mkfs/main.c | 78 +++--------------------------------
6 files changed, 107 insertions(+), 157 deletions(-)
diff --git a/convert/main.c b/convert/main.c
index 73740fe26d55..453e2c003c20 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -915,44 +915,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.
@@ -1012,6 +974,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;
@@ -1057,15 +1020,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/kernel-shared/ctree.h b/kernel-shared/ctree.h
index 1dda40e96a60..ea459464063d 100644
--- a/kernel-shared/ctree.h
+++ b/kernel-shared/ctree.h
@@ -1134,6 +1134,10 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
*item);
int btrfs_find_last_root(struct btrfs_root *root, u64 objectid, struct
btrfs_root_item *item, struct btrfs_key *key);
+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);
/* dir-item.c */
int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
*root, const char *name, int name_len, u64 dir,
diff --git a/kernel-shared/root-tree.c b/kernel-shared/root-tree.c
index 33f9e4697b18..1fe7d535fdc7 100644
--- a/kernel-shared/root-tree.c
+++ b/kernel-shared/root-tree.c
@@ -19,12 +19,15 @@
#include "kerncompat.h"
#include <errno.h>
#include <string.h>
+#include <time.h>
#include "kernel-lib/bitops.h"
#include "kernel-shared/accessors.h"
#include "kernel-shared/extent_io.h"
#include "kernel-shared/uapi/btrfs_tree.h"
#include "kernel-shared/ctree.h"
#include "kernel-shared/disk-io.h"
+#include "kernel-shared/transaction.h"
+#include "common/messages.h"
int btrfs_find_last_root(struct btrfs_root *root, u64 objectid,
struct btrfs_root_item *item, struct btrfs_key *key)
@@ -224,3 +227,86 @@ int btrfs_add_root_ref(struct btrfs_trans_handle *trans,
btrfs_free_path(path);
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;
+}
+
+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/mkfs/common.c b/mkfs/common.c
index d400413c7d41..34798f79026b 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -752,45 +752,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 06ddc926390f..659529b90f6e 100644
--- a/mkfs/common.h
+++ b/mkfs/common.h
@@ -98,8 +98,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 9584386da4ca..42aa68b7ecf4 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -705,75 +705,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)
{
@@ -1138,6 +1069,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 };
@@ -1887,9 +1819,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] 19+ messages in thread
* [PATCH 5/6] btrfs-progs: mkfs: introduce experimental --subvol option
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
` (3 preceding siblings ...)
2023-10-16 4:38 ` [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation Qu Wenruo
@ 2023-10-16 4:38 ` Qu Wenruo
2023-10-17 13:54 ` Josef Bacik
2023-10-16 4:38 ` [PATCH 6/6] btrfs-progs: mkfs-tests: introduce a test case to verify " Qu Wenruo
2023-10-19 18:19 ` [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental " Goffredo Baroncelli
6 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2023-10-16 4:38 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 | 23 ++++++++
mkfs/rootdir.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++
mkfs/rootdir.h | 1 +
3 files changed, 181 insertions(+)
diff --git a/mkfs/main.c b/mkfs/main.c
index 42aa68b7ecf4..6bf30b758572 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -434,6 +434,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"),
@@ -1107,6 +1110,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();
@@ -1119,6 +1123,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
GETOPT_VAL_SHRINK = GETOPT_VAL_FIRST,
GETOPT_VAL_CHECKSUM,
GETOPT_VAL_GLOBAL_ROOTS,
+ GETOPT_VAL_SUBVOL,
};
static const struct option long_options[] = {
{ "byte-count", required_argument, NULL, 'b' },
@@ -1145,6 +1150,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}
@@ -1274,6 +1280,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);
@@ -1310,6 +1320,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;
@@ -1846,6 +1861,14 @@ raid_groups:
goto out;
}
+ /* Create the subvolumes. */
+ 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 f6328c9df2ec..9c06f0a81593 100644
--- a/mkfs/rootdir.c
+++ b/mkfs/rootdir.c
@@ -36,6 +36,7 @@
#include "kernel-shared/disk-io.h"
#include "kernel-shared/transaction.h"
#include "kernel-shared/file-item.h"
+#include "kernel-shared/messages.h"
#include "common/internal.h"
#include "common/messages.h"
#include "common/path-utils.h"
@@ -975,3 +976,159 @@ 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_commit_transaction(trans, fs_info->tree_root);
+ if (ret < 0) {
+ errno = -ret;
+ error("failed to commit transaction: %m");
+ goto out;
+ }
+ subvol = btrfs_link_subvol(fs_info->fs_root, parent_ino, filename,
+ subvol->root_key.objectid, false);
+ if (IS_ERR(subvol)) {
+ ret = PTR_ERR(subvol);
+ errno = -ret;
+ error("failed to link subvol %s: %m", filename);
+ goto out;
+ }
+ ret = 0;
+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] 19+ messages in thread
* [PATCH 6/6] btrfs-progs: mkfs-tests: introduce a test case to verify --subvol option
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
` (4 preceding siblings ...)
2023-10-16 4:38 ` [PATCH 5/6] btrfs-progs: mkfs: introduce experimental --subvol option Qu Wenruo
@ 2023-10-16 4:38 ` Qu Wenruo
2023-10-19 18:19 ` [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental " Goffredo Baroncelli
6 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2023-10-16 4:38 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] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-16 4:38 ` [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation Qu Wenruo
@ 2023-10-17 13:49 ` Josef Bacik
2023-10-17 20:14 ` Qu Wenruo
0 siblings, 1 reply; 19+ messages in thread
From: Josef Bacik @ 2023-10-17 13:49 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, Oct 16, 2023 at 03:08:50PM +1030, Qu Wenruo wrote:
> 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 kernel-shared/root-tree.[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>
> ---
> convert/main.c | 55 +++++--------------------
> kernel-shared/ctree.h | 4 ++
> kernel-shared/root-tree.c | 86 +++++++++++++++++++++++++++++++++++++++
> mkfs/common.c | 39 ------------------
> mkfs/common.h | 2 -
> mkfs/main.c | 78 +++--------------------------------
> 6 files changed, 107 insertions(+), 157 deletions(-)
>
> diff --git a/convert/main.c b/convert/main.c
> index 73740fe26d55..453e2c003c20 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -915,44 +915,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.
> @@ -1012,6 +974,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;
> @@ -1057,15 +1020,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/kernel-shared/ctree.h b/kernel-shared/ctree.h
> index 1dda40e96a60..ea459464063d 100644
> --- a/kernel-shared/ctree.h
> +++ b/kernel-shared/ctree.h
> @@ -1134,6 +1134,10 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
> *item);
> int btrfs_find_last_root(struct btrfs_root *root, u64 objectid, struct
> btrfs_root_item *item, struct btrfs_key *key);
> +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);
> /* dir-item.c */
> int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
> *root, const char *name, int name_len, u64 dir,
> diff --git a/kernel-shared/root-tree.c b/kernel-shared/root-tree.c
> index 33f9e4697b18..1fe7d535fdc7 100644
> --- a/kernel-shared/root-tree.c
> +++ b/kernel-shared/root-tree.c
We're moving towards a world where kernel-shared will be an exact-ish copy of
the kernel code. Please put helpers like this in common/, I did this for
several of the extent tree related helpers we need for fsck, this is a good fit
for that. Thanks,
Josef
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/6] btrfs-progs: mkfs: introduce experimental --subvol option
2023-10-16 4:38 ` [PATCH 5/6] btrfs-progs: mkfs: introduce experimental --subvol option Qu Wenruo
@ 2023-10-17 13:54 ` Josef Bacik
2023-10-17 20:13 ` Qu Wenruo
0 siblings, 1 reply; 19+ messages in thread
From: Josef Bacik @ 2023-10-17 13:54 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, Oct 16, 2023 at 03:08:51PM +1030, Qu Wenruo wrote:
> 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 | 23 ++++++++
> mkfs/rootdir.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++
> mkfs/rootdir.h | 1 +
> 3 files changed, 181 insertions(+)
>
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 42aa68b7ecf4..6bf30b758572 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -434,6 +434,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
I assume you're doing EXPERIMENTAL because you want to un-gate it once you
remove all the restrictions? Thanks,
Josef
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/6] btrfs-progs: mkfs: introduce experimental --subvol option
2023-10-17 13:54 ` Josef Bacik
@ 2023-10-17 20:13 ` Qu Wenruo
0 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2023-10-17 20:13 UTC (permalink / raw)
To: Josef Bacik, Qu Wenruo; +Cc: linux-btrfs
On 2023/10/18 00:24, Josef Bacik wrote:
> On Mon, Oct 16, 2023 at 03:08:51PM +1030, Qu Wenruo wrote:
>> 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 | 23 ++++++++
>> mkfs/rootdir.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++
>> mkfs/rootdir.h | 1 +
>> 3 files changed, 181 insertions(+)
>>
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index 42aa68b7ecf4..6bf30b758572 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -434,6 +434,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
>
> I assume you're doing EXPERIMENTAL because you want to un-gate it once you
> remove all the restrictions? Thanks,
Exactly.
Thanks,
Qu
>
> Josef
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-17 13:49 ` Josef Bacik
@ 2023-10-17 20:14 ` Qu Wenruo
2023-10-17 23:11 ` David Sterba
0 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2023-10-17 20:14 UTC (permalink / raw)
To: Josef Bacik, Qu Wenruo; +Cc: linux-btrfs
On 2023/10/18 00:19, Josef Bacik wrote:
> On Mon, Oct 16, 2023 at 03:08:50PM +1030, Qu Wenruo wrote:
>> 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 kernel-shared/root-tree.[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>
>> ---
>> convert/main.c | 55 +++++--------------------
>> kernel-shared/ctree.h | 4 ++
>> kernel-shared/root-tree.c | 86 +++++++++++++++++++++++++++++++++++++++
>> mkfs/common.c | 39 ------------------
>> mkfs/common.h | 2 -
>> mkfs/main.c | 78 +++--------------------------------
>> 6 files changed, 107 insertions(+), 157 deletions(-)
>>
>> diff --git a/convert/main.c b/convert/main.c
>> index 73740fe26d55..453e2c003c20 100644
>> --- a/convert/main.c
>> +++ b/convert/main.c
>> @@ -915,44 +915,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.
>> @@ -1012,6 +974,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;
>> @@ -1057,15 +1020,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/kernel-shared/ctree.h b/kernel-shared/ctree.h
>> index 1dda40e96a60..ea459464063d 100644
>> --- a/kernel-shared/ctree.h
>> +++ b/kernel-shared/ctree.h
>> @@ -1134,6 +1134,10 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
>> *item);
>> int btrfs_find_last_root(struct btrfs_root *root, u64 objectid, struct
>> btrfs_root_item *item, struct btrfs_key *key);
>> +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);
>> /* dir-item.c */
>> int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
>> *root, const char *name, int name_len, u64 dir,
>> diff --git a/kernel-shared/root-tree.c b/kernel-shared/root-tree.c
>> index 33f9e4697b18..1fe7d535fdc7 100644
>> --- a/kernel-shared/root-tree.c
>> +++ b/kernel-shared/root-tree.c
>
> We're moving towards a world where kernel-shared will be an exact-ish copy of
> the kernel code. Please put helpers like this in common/, I did this for
> several of the extent tree related helpers we need for fsck, this is a good fit
> for that. Thanks,
Sure, and this also reminds me to copy whatever we can from kernel.
I'll update this with possible more copy from kernel.
Thanks,
Qu
>
> Josef
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-17 20:14 ` Qu Wenruo
@ 2023-10-17 23:11 ` David Sterba
2023-10-17 23:50 ` Qu Wenruo
0 siblings, 1 reply; 19+ messages in thread
From: David Sterba @ 2023-10-17 23:11 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Josef Bacik, Qu Wenruo, linux-btrfs
On Wed, Oct 18, 2023 at 06:44:22AM +1030, Qu Wenruo wrote:
> On 2023/10/18 00:19, Josef Bacik wrote:
> > On Mon, Oct 16, 2023 at 03:08:50PM +1030, Qu Wenruo wrote:
> >> --- a/kernel-shared/ctree.h
> >> +++ b/kernel-shared/ctree.h
> >> @@ -1134,6 +1134,10 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
> >> *item);
> >> int btrfs_find_last_root(struct btrfs_root *root, u64 objectid, struct
> >> btrfs_root_item *item, struct btrfs_key *key);
> >> +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);
> >> /* dir-item.c */
> >> int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
> >> *root, const char *name, int name_len, u64 dir,
> >> diff --git a/kernel-shared/root-tree.c b/kernel-shared/root-tree.c
> >> index 33f9e4697b18..1fe7d535fdc7 100644
> >> --- a/kernel-shared/root-tree.c
> >> +++ b/kernel-shared/root-tree.c
> >
> > We're moving towards a world where kernel-shared will be an exact-ish copy of
> > the kernel code. Please put helpers like this in common/, I did this for
> > several of the extent tree related helpers we need for fsck, this is a good fit
> > for that. Thanks,
>
> Sure, and this also reminds me to copy whatever we can from kernel.
I do syncs from kernel before a release but all the low hanging fruit is
probably gone so it needs targeted updates.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-17 23:11 ` David Sterba
@ 2023-10-17 23:50 ` Qu Wenruo
2023-10-24 17:38 ` David Sterba
0 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2023-10-17 23:50 UTC (permalink / raw)
To: dsterba, Qu Wenruo; +Cc: Josef Bacik, linux-btrfs
On 2023/10/18 09:41, David Sterba wrote:
> On Wed, Oct 18, 2023 at 06:44:22AM +1030, Qu Wenruo wrote:
>> On 2023/10/18 00:19, Josef Bacik wrote:
>>> On Mon, Oct 16, 2023 at 03:08:50PM +1030, Qu Wenruo wrote:
>>>> --- a/kernel-shared/ctree.h
>>>> +++ b/kernel-shared/ctree.h
>>>> @@ -1134,6 +1134,10 @@ int btrfs_update_root(struct btrfs_trans_handle *trans, struct btrfs_root
>>>> *item);
>>>> int btrfs_find_last_root(struct btrfs_root *root, u64 objectid, struct
>>>> btrfs_root_item *item, struct btrfs_key *key);
>>>> +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);
>>>> /* dir-item.c */
>>>> int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, struct btrfs_root
>>>> *root, const char *name, int name_len, u64 dir,
>>>> diff --git a/kernel-shared/root-tree.c b/kernel-shared/root-tree.c
>>>> index 33f9e4697b18..1fe7d535fdc7 100644
>>>> --- a/kernel-shared/root-tree.c
>>>> +++ b/kernel-shared/root-tree.c
>>>
>>> We're moving towards a world where kernel-shared will be an exact-ish copy of
>>> the kernel code. Please put helpers like this in common/, I did this for
>>> several of the extent tree related helpers we need for fsck, this is a good fit
>>> for that. Thanks,
>>
>> Sure, and this also reminds me to copy whatever we can from kernel.
>
> I do syncs from kernel before a release but all the low hanging fruit is
> probably gone so it needs targeted updates.
For the immediate target it's btrfs_inode and involved VFS structures
for inodes/dir entries.
In progs we don't have a structure to locate a unique inode (need both
rootid and ino number), nor to do any path resolution.
This makes it almost impossible to proper sync the code.
But introduce btrfs_inode to btrfs-progs would also be a little
overkilled, as we don't have that many users.
(Only the new --rootdir with --subvol combination).
But anyway, I'll keep the mindset of code syncing for the incoming
feature, and hopefully to find some better ideas to sync them.
Thanks,
Qu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
` (5 preceding siblings ...)
2023-10-16 4:38 ` [PATCH 6/6] btrfs-progs: mkfs-tests: introduce a test case to verify " Qu Wenruo
@ 2023-10-19 18:19 ` Goffredo Baroncelli
6 siblings, 0 replies; 19+ messages in thread
From: Goffredo Baroncelli @ 2023-10-19 18:19 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
Hi Qu,
On 16/10/2023 06.38, Qu Wenruo wrote:
> 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:
I have a suggestion and an enhancement request:
- why not use --mk-subvol? to me it seems more clear that a subvolume will be created
- add another option like --mk-default-subvolume that:
- create a subvolume
- make it the default.
The use case is the following: in btrfs it is easy make a snapshot, and move/rename/delete
subvolumes and snapshot. The only exception is the subvolid=5 (or /). You cannot delete it.
So if you want to do a snapshot of / and then rollback to this snapshot, it is complicated
because you cannot remove the original subvolume.
Starting from a subvolume different from subvolid=5 makes this easier.
>
> - 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 (6):
> btrfs-progs: enhance btrfs_mkdir() function
> btrfs-progs: enhance and rename btrfs_mksubvol() 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
>
> convert/main.c | 60 ++------
> kernel-shared/ctree.c | 106 ++++++--------
> kernel-shared/ctree.h | 12 +-
> kernel-shared/inode.c | 129 ++++++++++++-----
> kernel-shared/root-tree.c | 86 +++++++++++
> mkfs/common.c | 39 -----
> mkfs/common.h | 2 -
> mkfs/main.c | 103 ++++----------
> 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 +-
> 13 files changed, 473 insertions(+), 266 deletions(-)
> create mode 100755 tests/mkfs-tests/031-subvol-option/test.sh
>
> --
> 2.42.0
>
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-17 23:50 ` Qu Wenruo
@ 2023-10-24 17:38 ` David Sterba
2023-10-24 20:44 ` Qu Wenruo
0 siblings, 1 reply; 19+ messages in thread
From: David Sterba @ 2023-10-24 17:38 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, Josef Bacik, linux-btrfs
On Wed, Oct 18, 2023 at 10:20:57AM +1030, Qu Wenruo wrote:
> >>> We're moving towards a world where kernel-shared will be an exact-ish copy of
> >>> the kernel code. Please put helpers like this in common/, I did this for
> >>> several of the extent tree related helpers we need for fsck, this is a good fit
> >>> for that. Thanks,
> >>
> >> Sure, and this also reminds me to copy whatever we can from kernel.
> >
> > I do syncs from kernel before a release but all the low hanging fruit is
> > probably gone so it needs targeted updates.
>
> For the immediate target it's btrfs_inode and involved VFS structures
> for inodes/dir entries.
>
> In progs we don't have a structure to locate a unique inode (need both
> rootid and ino number), nor to do any path resolution.
>
> This makes it almost impossible to proper sync the code.
>
> But introduce btrfs_inode to btrfs-progs would also be a little
> overkilled, as we don't have that many users.
> (Only the new --rootdir with --subvol combination).
I have an idea for using this functionality, but you may not like it -
we could implement FUSE. The missing code is exactly about inodes, path
resolution and subvolumes. You have the other project, with a different
license, although there's a lot shared code. You can keep it so u-boot
can do the sync and keep the read-only support. I'd like to have full
read-write support with subvolumes and devices (if there's ioctl pass
through), but it's not urgent. Having the basic inode/path support would
be good for mkfs even in a smaller scope.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-24 17:38 ` David Sterba
@ 2023-10-24 20:44 ` Qu Wenruo
2023-10-25 16:18 ` David Sterba
0 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2023-10-24 20:44 UTC (permalink / raw)
To: dsterba, Qu Wenruo; +Cc: Josef Bacik, linux-btrfs
On 2023/10/25 04:08, David Sterba wrote:
> On Wed, Oct 18, 2023 at 10:20:57AM +1030, Qu Wenruo wrote:
>>>>> We're moving towards a world where kernel-shared will be an exact-ish copy of
>>>>> the kernel code. Please put helpers like this in common/, I did this for
>>>>> several of the extent tree related helpers we need for fsck, this is a good fit
>>>>> for that. Thanks,
>>>>
>>>> Sure, and this also reminds me to copy whatever we can from kernel.
>>>
>>> I do syncs from kernel before a release but all the low hanging fruit is
>>> probably gone so it needs targeted updates.
>>
>> For the immediate target it's btrfs_inode and involved VFS structures
>> for inodes/dir entries.
>>
>> In progs we don't have a structure to locate a unique inode (need both
>> rootid and ino number), nor to do any path resolution.
>>
>> This makes it almost impossible to proper sync the code.
>>
>> But introduce btrfs_inode to btrfs-progs would also be a little
>> overkilled, as we don't have that many users.
>> (Only the new --rootdir with --subvol combination).
>
> I have an idea for using this functionality, but you may not like it -
> we could implement FUSE.
In fact I really like it.
> The missing code is exactly about inodes, path
> resolution and subvolumes. You have the other project, with a different
> license, although there's a lot shared code. You can keep it so u-boot
> can do the sync and keep the read-only support. I'd like to have full
> read-write support with subvolumes and devices (if there's ioctl pass
> through), but it's not urgent. Having the basic inode/path support would
> be good for mkfs even in a smaller scope.
The existing blockage would be fsck.
If we want FUSE, inode is super handy, but for fsck doing super low
level fixes, it can be a burden instead.
As it needs to repair INODE_REF/DIR_INDEX/DIR_ITEMs, sometimes even
missing INODE_ITEMs, not sure how hard it would be to maintain both
btrfs_inode and low-level code.
There are one big limiting factor in FUSE, we can not control the device
number, unlike kernel.
This means even we implemented the subvolume code (like my btrfs-fuse
project), there is no way to detect subvolume boundary.
Then comes with some other super personal concerns:
- Can we go Rust instead of C?
- Can we have a less restrict license to maximize the possibility of
code share?
Well, I should ask this question to GRUB....
But a more hand-free license like MIT may really help for bootloaders.
Thanks,
Qu
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-24 20:44 ` Qu Wenruo
@ 2023-10-25 16:18 ` David Sterba
2023-10-25 22:41 ` Qu Wenruo
2023-10-25 22:57 ` Neal Gompa
0 siblings, 2 replies; 19+ messages in thread
From: David Sterba @ 2023-10-25 16:18 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, Josef Bacik, linux-btrfs
On Wed, Oct 25, 2023 at 07:14:58AM +1030, Qu Wenruo wrote:
>
>
> On 2023/10/25 04:08, David Sterba wrote:
> > On Wed, Oct 18, 2023 at 10:20:57AM +1030, Qu Wenruo wrote:
> >>>>> We're moving towards a world where kernel-shared will be an exact-ish copy of
> >>>>> the kernel code. Please put helpers like this in common/, I did this for
> >>>>> several of the extent tree related helpers we need for fsck, this is a good fit
> >>>>> for that. Thanks,
> >>>>
> >>>> Sure, and this also reminds me to copy whatever we can from kernel.
> >>>
> >>> I do syncs from kernel before a release but all the low hanging fruit is
> >>> probably gone so it needs targeted updates.
> >>
> >> For the immediate target it's btrfs_inode and involved VFS structures
> >> for inodes/dir entries.
> >>
> >> In progs we don't have a structure to locate a unique inode (need both
> >> rootid and ino number), nor to do any path resolution.
> >>
> >> This makes it almost impossible to proper sync the code.
> >>
> >> But introduce btrfs_inode to btrfs-progs would also be a little
> >> overkilled, as we don't have that many users.
> >> (Only the new --rootdir with --subvol combination).
> >
> > I have an idea for using this functionality, but you may not like it -
> > we could implement FUSE.
>
> In fact I really like it.
>
> > The missing code is exactly about inodes, path
> > resolution and subvolumes. You have the other project, with a different
> > license, although there's a lot shared code. You can keep it so u-boot
> > can do the sync and keep the read-only support. I'd like to have full
> > read-write support with subvolumes and devices (if there's ioctl pass
> > through), but it's not urgent. Having the basic inode/path support would
> > be good for mkfs even in a smaller scope.
>
> The existing blockage would be fsck.
> If we want FUSE, inode is super handy, but for fsck doing super low
> level fixes, it can be a burden instead.
> As it needs to repair INODE_REF/DIR_INDEX/DIR_ITEMs, sometimes even
> missing INODE_ITEMs, not sure how hard it would be to maintain both
> btrfs_inode and low-level code.
I'd have to look what exactly are the problems but yes check is special
in many ways. It could be possible to have an "enhanced" inode used in
check and regular inode everywhere else.
> There are one big limiting factor in FUSE, we can not control the device
> number, unlike kernel.
> This means even we implemented the subvolume code (like my btrfs-fuse
> project), there is no way to detect subvolume boundary.
This could be a problem. We can set the inode number to 256 but
comparing two random files if they're in the same subvolume would
require traversing the whole path. This would not work with 'find -xdev'
and similar.
> Then comes with some other super personal concerns:
>
> - Can we go Rust instead of C?
I know rust on the very beginner level, and I don't think we have enough
rust knowledge in the developers group. The language syntax or features
are still evolving, we'd lose the build support on any older distros or
distros that don't keep up with the versions. The C-rust
interoperability is good but it can become a burden. I'm peeking to the
kernel rust support from time to time and I can't comprehend what it's
doing.
> - Can we have a less restrict license to maximize the possibility of
> code share?
The way it's now it's next to impossible. Sharing GPL code works among
GPL projects, anything else must be written from scratch. I don't know
how much you did that in the btrfs-fuse project
> Well, I should ask this question to GRUB....
> But a more hand-free license like MIT may really help for bootloaders.
You can keep your btrfs-fuse to be the code base with loose license for
bootloaders, but you can't copy any code.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-25 16:18 ` David Sterba
@ 2023-10-25 22:41 ` Qu Wenruo
2023-10-25 22:57 ` Neal Gompa
1 sibling, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2023-10-25 22:41 UTC (permalink / raw)
To: dsterba; +Cc: Qu Wenruo, Josef Bacik, linux-btrfs
On 2023/10/26 02:48, David Sterba wrote:
> On Wed, Oct 25, 2023 at 07:14:58AM +1030, Qu Wenruo wrote:
>>
>>
>> On 2023/10/25 04:08, David Sterba wrote:
>>> On Wed, Oct 18, 2023 at 10:20:57AM +1030, Qu Wenruo wrote:
>>>>>>> We're moving towards a world where kernel-shared will be an exact-ish copy of
>>>>>>> the kernel code. Please put helpers like this in common/, I did this for
>>>>>>> several of the extent tree related helpers we need for fsck, this is a good fit
>>>>>>> for that. Thanks,
>>>>>>
>>>>>> Sure, and this also reminds me to copy whatever we can from kernel.
>>>>>
>>>>> I do syncs from kernel before a release but all the low hanging fruit is
>>>>> probably gone so it needs targeted updates.
>>>>
>>>> For the immediate target it's btrfs_inode and involved VFS structures
>>>> for inodes/dir entries.
>>>>
>>>> In progs we don't have a structure to locate a unique inode (need both
>>>> rootid and ino number), nor to do any path resolution.
>>>>
>>>> This makes it almost impossible to proper sync the code.
>>>>
>>>> But introduce btrfs_inode to btrfs-progs would also be a little
>>>> overkilled, as we don't have that many users.
>>>> (Only the new --rootdir with --subvol combination).
>>>
>>> I have an idea for using this functionality, but you may not like it -
>>> we could implement FUSE.
>>
>> In fact I really like it.
>>
>>> The missing code is exactly about inodes, path
>>> resolution and subvolumes. You have the other project, with a different
>>> license, although there's a lot shared code. You can keep it so u-boot
>>> can do the sync and keep the read-only support. I'd like to have full
>>> read-write support with subvolumes and devices (if there's ioctl pass
>>> through), but it's not urgent. Having the basic inode/path support would
>>> be good for mkfs even in a smaller scope.
>>
>> The existing blockage would be fsck.
>> If we want FUSE, inode is super handy, but for fsck doing super low
>> level fixes, it can be a burden instead.
>> As it needs to repair INODE_REF/DIR_INDEX/DIR_ITEMs, sometimes even
>> missing INODE_ITEMs, not sure how hard it would be to maintain both
>> btrfs_inode and low-level code.
>
> I'd have to look what exactly are the problems but yes check is special
> in many ways. It could be possible to have an "enhanced" inode used in
> check and regular inode everywhere else.
>
>> There are one big limiting factor in FUSE, we can not control the device
>> number, unlike kernel.
>> This means even we implemented the subvolume code (like my btrfs-fuse
>> project), there is no way to detect subvolume boundary.
>
> This could be a problem. We can set the inode number to 256 but
> comparing two random files if they're in the same subvolume would
> require traversing the whole path. This would not work with 'find -xdev'
> and similar.
>
>> Then comes with some other super personal concerns:
>>
>> - Can we go Rust instead of C?
>
> I know rust on the very beginner level, and I don't think we have enough
> rust knowledge in the developers group. The language syntax or features
> are still evolving, we'd lose the build support on any older distros or
> distros that don't keep up with the versions. The C-rust
> interoperability is good but it can become a burden. I'm peeking to the
> kernel rust support from time to time and I can't comprehend what it's
> doing.
>
>> - Can we have a less restrict license to maximize the possibility of
>> code share?
>
> The way it's now it's next to impossible. Sharing GPL code works among
> GPL projects, anything else must be written from scratch. I don't know
> how much you did that in the btrfs-fuse project
It's manageable for a read-only project to start from scratch, at least
for the implementation part.
(It takes me around 2 weeks for the main part of the code)
However I intentionally keep a lot of function names (especially for
accessors.[ch]) the same, while only re-implement the code.
But I guess it's never possible for btrfs-progs due to the amount of
contributors.
That's also why I'd try to explore rust, as in that case we need to
re-implement everything anyway.
Thanks,
Qu
>
>> Well, I should ask this question to GRUB....
>> But a more hand-free license like MIT may really help for bootloaders.
>
> You can keep your btrfs-fuse to be the code base with loose license for
> bootloaders, but you can't copy any code.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation
2023-10-25 16:18 ` David Sterba
2023-10-25 22:41 ` Qu Wenruo
@ 2023-10-25 22:57 ` Neal Gompa
1 sibling, 0 replies; 19+ messages in thread
From: Neal Gompa @ 2023-10-25 22:57 UTC (permalink / raw)
To: dsterba; +Cc: Qu Wenruo, Qu Wenruo, Josef Bacik, linux-btrfs
On Wed, Oct 25, 2023 at 12:26 PM David Sterba <dsterba@suse.cz> wrote:
>
> On Wed, Oct 25, 2023 at 07:14:58AM +1030, Qu Wenruo wrote:
> >
> >
> > On 2023/10/25 04:08, David Sterba wrote:
> > > On Wed, Oct 18, 2023 at 10:20:57AM +1030, Qu Wenruo wrote:
> > >>>>> We're moving towards a world where kernel-shared will be an exact-ish copy of
> > >>>>> the kernel code. Please put helpers like this in common/, I did this for
> > >>>>> several of the extent tree related helpers we need for fsck, this is a good fit
> > >>>>> for that. Thanks,
> > >>>>
> > >>>> Sure, and this also reminds me to copy whatever we can from kernel.
> > >>>
> > >>> I do syncs from kernel before a release but all the low hanging fruit is
> > >>> probably gone so it needs targeted updates.
> > >>
> > >> For the immediate target it's btrfs_inode and involved VFS structures
> > >> for inodes/dir entries.
> > >>
> > >> In progs we don't have a structure to locate a unique inode (need both
> > >> rootid and ino number), nor to do any path resolution.
> > >>
> > >> This makes it almost impossible to proper sync the code.
> > >>
> > >> But introduce btrfs_inode to btrfs-progs would also be a little
> > >> overkilled, as we don't have that many users.
> > >> (Only the new --rootdir with --subvol combination).
> > >
> > > I have an idea for using this functionality, but you may not like it -
> > > we could implement FUSE.
> >
> > In fact I really like it.
> >
> > > The missing code is exactly about inodes, path
> > > resolution and subvolumes. You have the other project, with a different
> > > license, although there's a lot shared code. You can keep it so u-boot
> > > can do the sync and keep the read-only support. I'd like to have full
> > > read-write support with subvolumes and devices (if there's ioctl pass
> > > through), but it's not urgent. Having the basic inode/path support would
> > > be good for mkfs even in a smaller scope.
> >
> > The existing blockage would be fsck.
> > If we want FUSE, inode is super handy, but for fsck doing super low
> > level fixes, it can be a burden instead.
> > As it needs to repair INODE_REF/DIR_INDEX/DIR_ITEMs, sometimes even
> > missing INODE_ITEMs, not sure how hard it would be to maintain both
> > btrfs_inode and low-level code.
>
> I'd have to look what exactly are the problems but yes check is special
> in many ways. It could be possible to have an "enhanced" inode used in
> check and regular inode everywhere else.
>
> > There are one big limiting factor in FUSE, we can not control the device
> > number, unlike kernel.
> > This means even we implemented the subvolume code (like my btrfs-fuse
> > project), there is no way to detect subvolume boundary.
>
> This could be a problem. We can set the inode number to 256 but
> comparing two random files if they're in the same subvolume would
> require traversing the whole path. This would not work with 'find -xdev'
> and similar.
>
Couldn't you use CUSE to create a btrfs-fuse device that could give
you that information?
> > Then comes with some other super personal concerns:
> >
> > - Can we go Rust instead of C?
>
> I know rust on the very beginner level, and I don't think we have enough
> rust knowledge in the developers group. The language syntax or features
> are still evolving, we'd lose the build support on any older distros or
> distros that don't keep up with the versions. The C-rust
> interoperability is good but it can become a burden. I'm peeking to the
> kernel rust support from time to time and I can't comprehend what it's
> doing.
>
> > - Can we have a less restrict license to maximize the possibility of
> > code share?
>
> The way it's now it's next to impossible. Sharing GPL code works among
> GPL projects, anything else must be written from scratch. I don't know
> how much you did that in the btrfs-fuse project
>
> > Well, I should ask this question to GRUB....
> > But a more hand-free license like MIT may really help for bootloaders.
>
> You can keep your btrfs-fuse to be the code base with loose license for
> bootloaders, but you can't copy any code.
There's also an independent implementation that's LGPLv3 (admittedly
targeted at Windows, but it could still be useful):
https://github.com/maharmstone/btrfs
But even a full-fledged GPLv2 implementation would be fine if we could
wrap it in FUSE+CUSE and as an EFI module.
Most of the freakout is with the GNU v3 licenses (though I disagree
with those who don't like them), and our stack is basically GNU v2.
--
真実はいつも一つ!/ Always, there's only one truth!
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-10-25 22:57 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-16 4:38 [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental --subvol option Qu Wenruo
2023-10-16 4:38 ` [PATCH 1/6] btrfs-progs: enhance btrfs_mkdir() function Qu Wenruo
2023-10-16 4:38 ` [PATCH 2/6] btrfs-progs: enhance and rename btrfs_mksubvol() function Qu Wenruo
2023-10-16 4:38 ` [PATCH 3/6] btrfs-progs: enhance btrfs_create_root() function Qu Wenruo
2023-10-16 4:38 ` [PATCH 4/6] btrfs-progs: use a unified btrfs_make_subvol() implementation Qu Wenruo
2023-10-17 13:49 ` Josef Bacik
2023-10-17 20:14 ` Qu Wenruo
2023-10-17 23:11 ` David Sterba
2023-10-17 23:50 ` Qu Wenruo
2023-10-24 17:38 ` David Sterba
2023-10-24 20:44 ` Qu Wenruo
2023-10-25 16:18 ` David Sterba
2023-10-25 22:41 ` Qu Wenruo
2023-10-25 22:57 ` Neal Gompa
2023-10-16 4:38 ` [PATCH 5/6] btrfs-progs: mkfs: introduce experimental --subvol option Qu Wenruo
2023-10-17 13:54 ` Josef Bacik
2023-10-17 20:13 ` Qu Wenruo
2023-10-16 4:38 ` [PATCH 6/6] btrfs-progs: mkfs-tests: introduce a test case to verify " Qu Wenruo
2023-10-19 18:19 ` [PATCH 0/6] btrfs-progs: mkfs: introduce an experimental " Goffredo Baroncelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).