| Message ID | 7b951f3a0619880f35f2490e2e251eb35e2f2292.1697430866.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: mkfs: introduce an experimental --subvol option | expand |
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
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
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.
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
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.
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
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.
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.
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!
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; }
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(-)