Re: [PATCH 1/2] fs: btrfs: implement opendir(), readdir() and closedir()

[Qu Wenruo <wqu@suse.com>]

在 2026/6/26 02:54, Alexey Charkov 写道:
> Add support for generic directory iteration with opendir(), readdir() and
> closedir() in the btrfs filesystem driver, following the ext4fs
> implementation for opendir()/closedir() and the btrfs_iter_dir() function
> for readdir().
> 
> Signed-off-by: Alexey Charkov <alchark@flipper.net>
> ---
>   fs/btrfs/btrfs.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   fs/btrfs/ctree.h    |  2 ++
>   fs/btrfs/dir-item.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   fs/fs.c             |  4 ++-
>   include/btrfs.h     |  5 +++
>   5 files changed, 193 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c
> index f3087f690fa4..c647c8dedf4e 100644
> --- a/fs/btrfs/btrfs.c
> +++ b/fs/btrfs/btrfs.c
> @@ -9,6 +9,7 @@
>   #include <malloc.h>
>   #include <u-boot/uuid.h>
>   #include <linux/time.h>
> +#include <fs.h>
>   #include "btrfs.h"
>   #include "crypto/hash.h"
>   #include "disk-io.h"
> @@ -159,6 +160,97 @@ int btrfs_ls(const char *path)
>   	return 0;
>   }
>   
> +struct btrfs_dir_stream {
> +	struct fs_dir_stream parent;
> +	struct fs_dirent dirent;
> +	char *dirname;
> +	u64 offset;

This doesn't look correct to me, for an opened dir, we should have at 
least the root id or pointer, and an inode number.

Not just an @dirname and re-do the path resolution again and again.

In fact, I do not even think we should save @dirname here.


> +};
> +
> +int btrfs_opendir(const char *dirname, struct fs_dir_stream **dirsp)
> +{
> +	struct btrfs_fs_info *fs_info = current_fs_info;
> +	struct btrfs_dir_stream *dirs;
> +	struct btrfs_root *root;
> +	u64 ino;
> +	u8 type;
> +	int ret;
> +
> +	*dirsp = NULL;
> +	ASSERT(fs_info);
> +
> +	ret = btrfs_lookup_path(fs_info->fs_root, BTRFS_FIRST_FREE_OBJECTID,
> +				dirname, &root, &ino, &type, 40);

You just discard the most important @root and @ino.

So what is the point here?
> +	if (ret < 0)
> +		return ret;
> +	if (type != BTRFS_FT_DIR)
> +		return -ENOTDIR;
> +
> +	dirs = calloc(1, sizeof(*dirs));
> +	if (!dirs)
> +		return -ENOMEM;
> +	dirs->dirname = strdup(dirname);
> +	if (!dirs->dirname) {
> +		free(dirs);
> +		return -ENOMEM;
> +	}
> +
> +	*dirsp = (struct fs_dir_stream *)dirs;
> +	return 0;
> +}
> +
> +int btrfs_readdir(struct fs_dir_stream *fs_dirs, struct fs_dirent **dentp)
> +{
> +	struct btrfs_dir_stream *dirs = (struct btrfs_dir_stream *)fs_dirs;
> +	struct btrfs_fs_info *fs_info = current_fs_info;
> +	struct fs_dirent *dent = &dirs->dirent;
> +	struct btrfs_root *root;
> +	u64 ino;
> +	u8 type;
> +	int ret;
> +
> +	*dentp = NULL;
> +	ASSERT(fs_info);
> +
> +	ret = btrfs_lookup_path(fs_info->fs_root, BTRFS_FIRST_FREE_OBJECTID,
> +				dirs->dirname, &root, &ino, &type, 40);

No, doing the same path resolution again and again is not sane.
> +	if (ret < 0)
> +		return ret;
> +	if (type != BTRFS_FT_DIR)
> +		return -ENOTDIR;
> +
> +	memset(dent, 0, sizeof(*dent));
> +	ret = btrfs_next_dir_entry(root, ino, &dirs->offset, dent->name,
> +				   sizeof(dent->name), &type);
> +	if (ret < 0)
> +		return ret;
> +	if (ret > 0)
> +		return -ENOENT;
> +
> +	switch (type) {
> +	case BTRFS_FT_DIR:
> +		dent->type = FS_DT_DIR;
> +		break;
> +	case BTRFS_FT_SYMLINK:
> +		dent->type = FS_DT_LNK;
> +		break;
> +	default:
> +		dent->type = FS_DT_REG;
> +		break;

So it looks like u-boot only supports the above 3 types, and unlike 
linux kernel the values doesn't match the btrfs internal ones.

In that case, I'd still prefer a proper convertor function.

[...]
> +
> +		*offset = key.offset + 1;
> +		type = btrfs_dir_type(path.nodes[0], di);
> +
> +		/* XATTRs share the key space but are not directory entries. */
> +		if (type == BTRFS_FT_XATTR) {

This doesn't looks correct again.

XATTR has their own keys, they should not show up among DIR_INDEX keys.

Thanks,
Qu