On Wed, Jul 01, 2020 at 11:25:27AM +0800, Qu Wenruo wrote:
> >> +struct btrfs_root *btrfs_get_new_fs_root(struct btrfs_fs_info *fs_info,
> >> + u64 objectid, dev_t anon_dev)
> >> +{
> >> + return __get_fs_root(fs_info, objectid, anon_dev, true);
> >> +}
> >
> > This does not look like a good API, we should keep btrfs_get_fs_root and
> > add the anon_bdev initialization to the callers, there are only a few.
> >
>
> A few = over 25?
>
> I have switched to keep btrfs_get_fs_root(), but you won't like the summary:
>
> Old:
> fs/btrfs/disk-io.h | 2 ++
> fs/btrfs/ioctl.c | 21 ++++++++++++++++-
> fs/btrfs/transaction.c | 3 ++-
> fs/btrfs/transaction.h | 2 ++
> 5 files changed, 70 insertions(+), 9 deletions(-)
>
> New:
> fs/btrfs/backref.c | 4 ++--
> fs/btrfs/disk-io.c | 42 ++++++++++++++++++++++++++++++++++--------
> fs/btrfs/disk-io.h | 3 ++-
> fs/btrfs/export.c | 2 +-
> fs/btrfs/file.c | 2 +-
> fs/btrfs/inode.c | 2 +-
> fs/btrfs/ioctl.c | 31 +++++++++++++++++++++++++------
> fs/btrfs/relocation.c | 11 ++++++-----
> fs/btrfs/root-tree.c | 2 +-
> fs/btrfs/scrub.c | 2 +-
> fs/btrfs/send.c | 4 ++--
> fs/btrfs/super.c | 2 +-
> fs/btrfs/transaction.c | 3 ++-
> fs/btrfs/transaction.h | 2 ++
> fs/btrfs/tree-log.c | 2 +-
> fs/btrfs/uuid-tree.c | 2 +-
> 16 files changed, 83 insertions(+), 33 deletions(-)
>
> Do we really go that direction?
You're right, I don't like the summary and I don't like the code either.
Adding the anon_dev argument to btrfs_get_fs_root is wrong and I have
never suggested that. What I meant is to put the actual id allocation
to the callers where the subvolume is created, ie only 2 places.
