Re: [PATCH] AOSP: erofs-utils: fix sub directory prefix path in canned fs_config

[Gao Xiang <hsiangkao@redhat.com>]

Hi Yue,

On Tue, Dec 22, 2020 at 10:04:30AM +0800, Yue Hu wrote:
> From: Yue Hu <huyue2@yulong.com>
> 
> We observe that creating image failed to find [%s] in canned fs_config
> using --fs-config-file option under Android 10.
> 
> Notice that canned fs_config has a prefix to sub directory if mount_point
> presents. However, erofs_fspath() does not contain the prefix.
> 

Thanks for your patch. Let me play with it a bit this weekend (I'm not
quite familiar with canned fs_config, it would be of great help to add
some hints/steps for me to confirm this issue.... since some other vendors
already use it without report (maybe they don't use canned fs_config.)

> Moreover, we should not add the mount point to fspath on root inode for
> fs_config() branch.

Is there some descriptive words or reference for this? To be honest,
I'm quite unsure about this kind of Android-specific things... :(

Thanks,
Gao Xiang

> 
> Signed-off-by: Yue Hu <huyue2@yulong.com>
> ---
>  include/erofs/config.h |  4 ++++
>  lib/inode.c            | 29 +++++++++++++++++++----------
>  mkfs/main.c            | 14 ++++++++++----
>  3 files changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/include/erofs/config.h b/include/erofs/config.h
> index 02ddf59..1277eda 100644
> --- a/include/erofs/config.h
> +++ b/include/erofs/config.h
> @@ -58,6 +58,10 @@ struct erofs_configure {
>  	char *mount_point;
>  	char *target_out_path;
>  	char *fs_config_file;
> +	void (*fs_config_func)(const char *path, int dir,
> +			       const char *target_out_path,
> +			       unsigned *uid, unsigned *gid,
> +			       unsigned *mode, uint64_t *capabilities);
>  #endif
>  };
>  
> diff --git a/lib/inode.c b/lib/inode.c
> index eb2e0f2..d0805cd 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -684,20 +684,29 @@ int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
>  	char *fspath;
>  
>  	inode->capabilities = 0;
> -	if (cfg.fs_config_file)
> -		canned_fs_config(erofs_fspath(path),
> -				 S_ISDIR(st->st_mode),
> -				 cfg.target_out_path,
> -				 &uid, &gid, &mode, &inode->capabilities);
> -	else if (cfg.mount_point) {
> +
> +	if (erofs_fspath(path)[0] == '\0')
> +		goto e_fspath;
> +
> +	if (cfg.mount_point) {
>  		if (asprintf(&fspath, "%s/%s", cfg.mount_point,
>  			     erofs_fspath(path)) <= 0)
>  			return -ENOMEM;
> -
> -		fs_config(fspath, S_ISDIR(st->st_mode),
> -			  cfg.target_out_path,
> -			  &uid, &gid, &mode, &inode->capabilities);
> +		if (cfg.fs_config_func)
> +			cfg.fs_config_func(fspath,
> +					   S_ISDIR(st->st_mode),
> +					   cfg.target_out_path,
> +					   &uid, &gid, &mode,
> +					   &inode->capabilities);
>  		free(fspath);
> +	} else {
> +e_fspath:
> +		if (cfg.fs_config_func)
> +			cfg.fs_config_func(erofs_fspath(path),
> +					   S_ISDIR(st->st_mode),
> +					   cfg.target_out_path,
> +					   &uid, &gid, &mode,
> +					   &inode->capabilities);
>  	}
>  	st->st_uid = uid;
>  	st->st_gid = gid;
> diff --git a/mkfs/main.c b/mkfs/main.c
> index c63b274..684767c 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -474,10 +474,16 @@ int main(int argc, char **argv)
>  	}
>  
>  #ifdef WITH_ANDROID
> -	if (cfg.fs_config_file &&
> -	    load_canned_fs_config(cfg.fs_config_file) < 0) {
> -		erofs_err("failed to load fs config %s", cfg.fs_config_file);
> -		return 1;
> +	cfg.fs_config_func = NULL;
> +	if (cfg.fs_config_file) {
> +		if (load_canned_fs_config(cfg.fs_config_file) < 0) {
> +			erofs_err("failed to load fs config %s",
> +					cfg.fs_config_file);
> +			return 1;
> +		}
> +		cfg.fs_config_func = canned_fs_config;
> +	} else if (cfg.mount_point) {
> +		cfg.fs_config_func = fs_config;
>  	}
>  #endif
>  
> -- 
> 1.9.1
>