Re: [PATCH 3/3] btrfs-progs: introduce btrfs_rebuild_uuid_tree() for mkfs and btrfs-convert

[David Sterba <dsterba@suse.cz>]

On Sat, Jul 27, 2024 at 08:03:23AM +0930, Qu Wenruo wrote:
> 
> 
> 在 2024/7/27 01:05, David Sterba 写道:
> > On Fri, Jul 26, 2024 at 07:29:55PM +0930, Qu Wenruo wrote:
> >> Currently mkfs uses its own create_uuid_tree(), but that function is
> >> only handling FS_TREE.
> >>
> >> This means for btrfs-convert, we do not generate the uuid tree, nor
> >> add the UUID of the image subvolume.
> >>
> >> This can be a problem if we're going to support multiple subvolumes
> >> during mkfs time.
> >>
> >> To address this inconvenience, this patch introduces a new helper,
> >> btrfs_rebuild_uuid_tree(), which will:
> >>
> >> - Create a new uuid tree if there is not one
> >>
> >> - Empty the existing uuid tree
> >>
> >> - Iterate through all subvolumes
> >>    * If the subvolume has no valid UUID, regenerate one
> >>    * Add the uuid entry for the subvolume UUID
> >>    * If the subvolume has received UUID, also add it to UUID tree
> >>
> >> By this, this new helper can handle all the uuid tree generation needs for:
> >>
> >> - Current mkfs
> >>    Only one uuid entry for FS_TREE
> >>
> >> - Current btrfs-convert
> >>    Only FS_TREE and the image subvolume
> >>
> >> - Future multi-subvolume mkfs
> >>    As we do the scan for all subvolumes.
> >>
> >> - Future "btrfs rescue rebuild-uuid-tree"
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> ---
> >>   common/root-tree-utils.c | 265 +++++++++++++++++++++++++++++++++++++++
> >>   common/root-tree-utils.h |   1 +
> >>   convert/main.c           |   5 +
> >>   mkfs/main.c              |  37 +-----
> >>   4 files changed, 274 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/common/root-tree-utils.c b/common/root-tree-utils.c
> >> index 6a57c51a8a74..13f89dbd5293 100644
> >> --- a/common/root-tree-utils.c
> >> +++ b/common/root-tree-utils.c
> >> @@ -15,9 +15,11 @@
> >>    */
> >>
> >>   #include <time.h>
> >> +#include <uuid/uuid.h>
> >>   #include "common/root-tree-utils.h"
> >>   #include "common/messages.h"
> >>   #include "kernel-shared/disk-io.h"
> >> +#include "kernel-shared/uuid-tree.h"
> >>
> >>   int btrfs_make_root_dir(struct btrfs_trans_handle *trans,
> >>   			struct btrfs_root *root, u64 objectid)
> >> @@ -212,3 +214,266 @@ abort:
> >>   	btrfs_abort_transaction(trans, ret);
> >>   	return ret;
> >>   }
> >> +
> >> +static int empty_tree(struct btrfs_root *root)
> >
> > Please rename this, it's confusing as empty can be a noun and verb so
> > it's not clear if it's a meant as a predicate or action.
> >
> Any recommendation? clear_tree()?

I've posted soem suggestions as the pull request review comments.