* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 10:51 ` [PATCH v2] " Gao Xiang
@ 2020-12-28 11:14 ` Huang Jianan
2020-12-28 11:20 ` Yue Hu
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Huang Jianan @ 2020-12-28 11:14 UTC (permalink / raw)
To: Gao Xiang, linux-erofs; +Cc: zhangshiming, Yue Hu, Yue Hu, guoweichao
Hi Xiang,
This patch works well with or without --fs-config-file in build.
Tested-by: Huang Jianan <huangjianan@oppo.com>
Thanks,
Jianan
在 2020/12/28 18:51, Gao Xiang 写道:
> From: Gao Xiang <hsiangkao@aol.com>
>
> "failed to find [%s] in canned fs_config" was observed by using
> "--fs-config-file" option as reported by Yue Hu [1].
>
> The root cause was that the mountpoint prefix to subdirectories is
> also needed if "--mount-point" presents. However, such prefix cannot
> be added by just using erofs_fspath().
>
> One exception is that the root directory itself needs to be handled
> specially for canned fs_config. For such case, the prefix of the root
> directory has to be dropped instead.
>
> [1] https://lkml.kernel.org/r/20201222020430.12512-1-zbestahu@gmail.com
>
> Link: https://lore.kernel.org/r/20201226062736.29920-1-hsiangkao@aol.com
> Fixes: 8a9e8046f170 ("AOSP: erofs-utils: add fs_config support")
> Reported-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@aol.com>
> ---
> changes since v2:
> - fix IS_ROOT misuse reported by Jianan, very sorry about this since
> I know little about canned fs_config.
>
> (please kindly test again...)
>
> lib/inode.c | 39 +++++++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/lib/inode.c b/lib/inode.c
> index 0c4839d..e6159c9 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -696,32 +696,43 @@ int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
> /* filesystem_config does not preserve file type bits */
> mode_t stat_file_type_mask = st->st_mode & S_IFMT;
> unsigned int uid = 0, gid = 0, mode = 0;
> - char *fspath;
> + const char *fspath;
> + char *decorated = NULL;
>
> inode->capabilities = 0;
> + if (!cfg.fs_config_file && !cfg.mount_point)
> + return 0;
> +
> + if (!cfg.mount_point ||
> + /* have to drop the mountpoint for rootdir of canned fsconfig */
> + (cfg.fs_config_file && erofs_fspath(path)[0] == '\0')) {
> + fspath = erofs_fspath(path);
> + } else {
> + if (asprintf(&decorated, "%s/%s", cfg.mount_point,
> + erofs_fspath(path)) <= 0)
> + return -ENOMEM;
> + fspath = decorated;
> + }
> +
> if (cfg.fs_config_file)
> - canned_fs_config(erofs_fspath(path),
> + canned_fs_config(fspath,
> S_ISDIR(st->st_mode),
> cfg.target_out_path,
> &uid, &gid, &mode, &inode->capabilities);
> - else if (cfg.mount_point) {
> - if (asprintf(&fspath, "%s/%s", cfg.mount_point,
> - erofs_fspath(path)) <= 0)
> - return -ENOMEM;
> -
> + else
> fs_config(fspath, S_ISDIR(st->st_mode),
> cfg.target_out_path,
> &uid, &gid, &mode, &inode->capabilities);
> - free(fspath);
> - }
> - st->st_uid = uid;
> - st->st_gid = gid;
> - st->st_mode = mode | stat_file_type_mask;
>
> erofs_dbg("/%s -> mode = 0x%x, uid = 0x%x, gid = 0x%x, "
> "capabilities = 0x%" PRIx64 "\n",
> - erofs_fspath(path),
> - mode, uid, gid, inode->capabilities);
> + fspath, mode, uid, gid, inode->capabilities);
> +
> + if (decorated)
> + free(decorated);
> + st->st_uid = uid;
> + st->st_gid = gid;
> + st->st_mode = mode | stat_file_type_mask;
> return 0;
> }
> #else
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 10:51 ` [PATCH v2] " Gao Xiang
2020-12-28 11:14 ` Huang Jianan
@ 2020-12-28 11:20 ` Yue Hu
2020-12-28 11:32 ` Gao Xiang
2020-12-28 11:46 ` Yue Hu
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Yue Hu @ 2020-12-28 11:20 UTC (permalink / raw)
To: Gao Xiang; +Cc: Yue Hu, zhangwen, linux-erofs
On Mon, 28 Dec 2020 18:51:46 +0800
Gao Xiang <hsiangkao@redhat.com> wrote:
> From: Gao Xiang <hsiangkao@aol.com>
>
> "failed to find [%s] in canned fs_config" was observed by using
> "--fs-config-file" option as reported by Yue Hu [1].
>
> The root cause was that the mountpoint prefix to subdirectories is
> also needed if "--mount-point" presents. However, such prefix cannot
> be added by just using erofs_fspath().
>
> One exception is that the root directory itself needs to be handled
> specially for canned fs_config. For such case, the prefix of the root
> directory has to be dropped instead.
>
> [1]
> https://lkml.kernel.org/r/20201222020430.12512-1-zbestahu@gmail.com
>
> Link:
> https://lore.kernel.org/r/20201226062736.29920-1-hsiangkao@aol.com
> Fixes: 8a9e8046f170 ("AOSP: erofs-utils: add fs_config support")
> Reported-by: Yue Hu <huyue2@yulong.com> Signed-off-by: Gao Xiang
> <hsiangkao@aol.com> ---
> changes since v2:
> - fix IS_ROOT misuse reported by Jianan, very sorry about this since
> I know little about canned fs_config.
>
> (please kindly test again...)
>
> lib/inode.c | 39 +++++++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/lib/inode.c b/lib/inode.c
> index 0c4839d..e6159c9 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -696,32 +696,43 @@ int erofs_droid_inode_fsconfig(struct
> erofs_inode *inode, /* filesystem_config does not preserve file type
> bits */ mode_t stat_file_type_mask = st->st_mode & S_IFMT;
> unsigned int uid = 0, gid = 0, mode = 0;
> - char *fspath;
> + const char *fspath;
> + char *decorated = NULL;
>
> inode->capabilities = 0;
> + if (!cfg.fs_config_file && !cfg.mount_point)
> + return 0;
> +
> + if (!cfg.mount_point ||
> + /* have to drop the mountpoint for rootdir of canned
> fsconfig */
> + (cfg.fs_config_file && erofs_fspath(path)[0] == '\0')) {
> + fspath = erofs_fspath(path);
> + } else {
> + if (asprintf(&decorated, "%s/%s", cfg.mount_point,
> + erofs_fspath(path)) <= 0)
> + return -ENOMEM;
> + fspath = decorated;
> + }
> +
> if (cfg.fs_config_file)
Whether we can use the first "cfg.fs_config_file" when loading canned
fs-config to reduce/simplify these duplicated calling?
> - canned_fs_config(erofs_fspath(path),
> + canned_fs_config(fspath,
> S_ISDIR(st->st_mode),
> cfg.target_out_path,
> &uid, &gid, &mode,
> &inode->capabilities);
> - else if (cfg.mount_point) {
> - if (asprintf(&fspath, "%s/%s", cfg.mount_point,
> - erofs_fspath(path)) <= 0)
> - return -ENOMEM;
> -
> + else
> fs_config(fspath, S_ISDIR(st->st_mode),
> cfg.target_out_path,
> &uid, &gid, &mode, &inode->capabilities);
> - free(fspath);
> - }
> - st->st_uid = uid;
> - st->st_gid = gid;
> - st->st_mode = mode | stat_file_type_mask;
>
> erofs_dbg("/%s -> mode = 0x%x, uid = 0x%x, gid = 0x%x, "
> "capabilities = 0x%" PRIx64 "\n",
> - erofs_fspath(path),
> - mode, uid, gid, inode->capabilities);
> + fspath, mode, uid, gid, inode->capabilities);
> +
> + if (decorated)
> + free(decorated);
> + st->st_uid = uid;
> + st->st_gid = gid;
> + st->st_mode = mode | stat_file_type_mask;
> return 0;
> }
> #else
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 11:20 ` Yue Hu
@ 2020-12-28 11:32 ` Gao Xiang
2020-12-28 11:39 ` Yue Hu
0 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2020-12-28 11:32 UTC (permalink / raw)
To: Yue Hu; +Cc: Yue Hu, zhangwen, linux-erofs
Hi Yue,
On Mon, Dec 28, 2020 at 07:20:48PM +0800, Yue Hu wrote:
> On Mon, 28 Dec 2020 18:51:46 +0800
> Gao Xiang <hsiangkao@redhat.com> wrote:
...
> > @@ -696,32 +696,43 @@ int erofs_droid_inode_fsconfig(struct
> > erofs_inode *inode, /* filesystem_config does not preserve file type
> > bits */ mode_t stat_file_type_mask = st->st_mode & S_IFMT;
> > unsigned int uid = 0, gid = 0, mode = 0;
> > - char *fspath;
> > + const char *fspath;
> > + char *decorated = NULL;
> >
> > inode->capabilities = 0;
> > + if (!cfg.fs_config_file && !cfg.mount_point)
> > + return 0;
> > +
> > + if (!cfg.mount_point ||
> > + /* have to drop the mountpoint for rootdir of canned
> > fsconfig */
> > + (cfg.fs_config_file && erofs_fspath(path)[0] == '\0')) {
> > + fspath = erofs_fspath(path);
> > + } else {
> > + if (asprintf(&decorated, "%s/%s", cfg.mount_point,
> > + erofs_fspath(path)) <= 0)
> > + return -ENOMEM;
> > + fspath = decorated;
> > + }
> > +
> > if (cfg.fs_config_file)
>
> Whether we can use the first "cfg.fs_config_file" when loading canned
> fs-config to reduce/simplify these duplicated calling?
Not sure what you mean... If you mean why not using some "fs_config_func"
as squashfs did, that is I'd like to 1) avoid such unneeded indirect
function pointers, and 2) no need to introduce such function prototype (I
don't want to maintain such function pointer type) since fs_config and
canned_fs_config implement differently (and it seems they also behave
differently so no need to mix them at all).
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 11:32 ` Gao Xiang
@ 2020-12-28 11:39 ` Yue Hu
0 siblings, 0 replies; 14+ messages in thread
From: Yue Hu @ 2020-12-28 11:39 UTC (permalink / raw)
To: Gao Xiang; +Cc: Yue Hu, zhangwen, linux-erofs
On Mon, 28 Dec 2020 19:32:47 +0800
Gao Xiang <hsiangkao@redhat.com> wrote:
> Hi Yue,
>
> On Mon, Dec 28, 2020 at 07:20:48PM +0800, Yue Hu wrote:
> > On Mon, 28 Dec 2020 18:51:46 +0800
> > Gao Xiang <hsiangkao@redhat.com> wrote:
>
> ...
>
> > > @@ -696,32 +696,43 @@ int erofs_droid_inode_fsconfig(struct
> > > erofs_inode *inode, /* filesystem_config does not preserve file
> > > type bits */ mode_t stat_file_type_mask = st->st_mode & S_IFMT;
> > > unsigned int uid = 0, gid = 0, mode = 0;
> > > - char *fspath;
> > > + const char *fspath;
> > > + char *decorated = NULL;
> > >
> > > inode->capabilities = 0;
> > > + if (!cfg.fs_config_file && !cfg.mount_point)
> > > + return 0;
> > > +
> > > + if (!cfg.mount_point ||
> > > + /* have to drop the mountpoint for rootdir of canned
> > > fsconfig */
> > > + (cfg.fs_config_file && erofs_fspath(path)[0] ==
> > > '\0')) {
> > > + fspath = erofs_fspath(path);
> > > + } else {
> > > + if (asprintf(&decorated, "%s/%s",
> > > cfg.mount_point,
> > > + erofs_fspath(path)) <= 0)
> > > + return -ENOMEM;
> > > + fspath = decorated;
> > > + }
> > > +
> > > if (cfg.fs_config_file)
> >
> > Whether we can use the first "cfg.fs_config_file" when loading
> > canned fs-config to reduce/simplify these duplicated calling?
>
> Not sure what you mean... If you mean why not using some
Sorry for my typo. I mean duplicated 'check' to cfg.fs_config_file.
> "fs_config_func" as squashfs did, that is I'd like to 1) avoid such
> unneeded indirect function pointers, and 2) no need to introduce such
> function prototype (I don't want to maintain such function pointer
> type) since fs_config and canned_fs_config implement differently (and
> it seems they also behave differently so no need to mix them at all).
Ok.
>
> Thanks,
> Gao Xiang
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 10:51 ` [PATCH v2] " Gao Xiang
2020-12-28 11:14 ` Huang Jianan
2020-12-28 11:20 ` Yue Hu
@ 2020-12-28 11:46 ` Yue Hu
2020-12-28 12:43 ` Gao Xiang
2020-12-31 16:31 ` Li GuiFu via Linux-erofs
2020-12-31 16:46 ` Li GuiFu via Linux-erofs
4 siblings, 1 reply; 14+ messages in thread
From: Yue Hu @ 2020-12-28 11:46 UTC (permalink / raw)
To: Gao Xiang; +Cc: Yue Hu, zhangwen, linux-erofs
Hi Xiang,
Works fine to me for canned/non-canned fs_config.
Tested-by: Yue Hu <huyue2@yulong.com>
Thx.
On Mon, 28 Dec 2020 18:51:46 +0800
Gao Xiang <hsiangkao@redhat.com> wrote:
> From: Gao Xiang <hsiangkao@aol.com>
>
> "failed to find [%s] in canned fs_config" was observed by using
> "--fs-config-file" option as reported by Yue Hu [1].
>
> The root cause was that the mountpoint prefix to subdirectories is
> also needed if "--mount-point" presents. However, such prefix cannot
> be added by just using erofs_fspath().
>
> One exception is that the root directory itself needs to be handled
> specially for canned fs_config. For such case, the prefix of the root
> directory has to be dropped instead.
>
> [1]
> https://lkml.kernel.org/r/20201222020430.12512-1-zbestahu@gmail.com
>
> Link:
> https://lore.kernel.org/r/20201226062736.29920-1-hsiangkao@aol.com
> Fixes: 8a9e8046f170 ("AOSP: erofs-utils: add fs_config support")
> Reported-by: Yue Hu <huyue2@yulong.com> Signed-off-by: Gao Xiang
> <hsiangkao@aol.com> ---
> changes since v2:
> - fix IS_ROOT misuse reported by Jianan, very sorry about this since
> I know little about canned fs_config.
>
> (please kindly test again...)
>
> lib/inode.c | 39 +++++++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/lib/inode.c b/lib/inode.c
> index 0c4839d..e6159c9 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -696,32 +696,43 @@ int erofs_droid_inode_fsconfig(struct
> erofs_inode *inode, /* filesystem_config does not preserve file type
> bits */ mode_t stat_file_type_mask = st->st_mode & S_IFMT;
> unsigned int uid = 0, gid = 0, mode = 0;
> - char *fspath;
> + const char *fspath;
> + char *decorated = NULL;
>
> inode->capabilities = 0;
> + if (!cfg.fs_config_file && !cfg.mount_point)
> + return 0;
> +
> + if (!cfg.mount_point ||
> + /* have to drop the mountpoint for rootdir of canned
> fsconfig */
> + (cfg.fs_config_file && erofs_fspath(path)[0] == '\0')) {
> + fspath = erofs_fspath(path);
> + } else {
> + if (asprintf(&decorated, "%s/%s", cfg.mount_point,
> + erofs_fspath(path)) <= 0)
> + return -ENOMEM;
> + fspath = decorated;
> + }
> +
> if (cfg.fs_config_file)
> - canned_fs_config(erofs_fspath(path),
> + canned_fs_config(fspath,
> S_ISDIR(st->st_mode),
> cfg.target_out_path,
> &uid, &gid, &mode,
> &inode->capabilities);
> - else if (cfg.mount_point) {
> - if (asprintf(&fspath, "%s/%s", cfg.mount_point,
> - erofs_fspath(path)) <= 0)
> - return -ENOMEM;
> -
> + else
> fs_config(fspath, S_ISDIR(st->st_mode),
> cfg.target_out_path,
> &uid, &gid, &mode, &inode->capabilities);
> - free(fspath);
> - }
> - st->st_uid = uid;
> - st->st_gid = gid;
> - st->st_mode = mode | stat_file_type_mask;
>
> erofs_dbg("/%s -> mode = 0x%x, uid = 0x%x, gid = 0x%x, "
> "capabilities = 0x%" PRIx64 "\n",
> - erofs_fspath(path),
> - mode, uid, gid, inode->capabilities);
> + fspath, mode, uid, gid, inode->capabilities);
> +
> + if (decorated)
> + free(decorated);
> + st->st_uid = uid;
> + st->st_gid = gid;
> + st->st_mode = mode | stat_file_type_mask;
> return 0;
> }
> #else
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 11:46 ` Yue Hu
@ 2020-12-28 12:43 ` Gao Xiang
0 siblings, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2020-12-28 12:43 UTC (permalink / raw)
To: Yue Hu, Huang Jianan; +Cc: Yue Hu, linux-erofs, zhangwen
On Mon, Dec 28, 2020 at 07:46:56PM +0800, Yue Hu wrote:
> Hi Xiang,
>
> Works fine to me for canned/non-canned fs_config.
>
> Tested-by: Yue Hu <huyue2@yulong.com>
>
Okay, got it. thanks you all for this :)
(also another link for indirect call: https://lwn.net/Articles/774743/
, so as long as some switch table, I'd like to avoid such usage...)
Thanks,
Gao Xiang
> Thx.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 10:51 ` [PATCH v2] " Gao Xiang
` (2 preceding siblings ...)
2020-12-28 11:46 ` Yue Hu
@ 2020-12-31 16:31 ` Li GuiFu via Linux-erofs
2020-12-31 16:50 ` Gao Xiang
2020-12-31 16:46 ` Li GuiFu via Linux-erofs
4 siblings, 1 reply; 14+ messages in thread
From: Li GuiFu via Linux-erofs @ 2020-12-31 16:31 UTC (permalink / raw)
To: Gao Xiang, linux-erofs, Yue Hu, Huang Jianan; +Cc: Yue Hu
Thank for your works, Yue Hu, Jianan, Gao Xiang
On 2020/12/28 18:51, Gao Xiang wrote:
> From: Gao Xiang <hsiangkao@aol.com>
>
> "failed to find [%s] in canned fs_config" was observed by using
> "--fs-config-file" option as reported by Yue Hu [1].
>
> The root cause was that the mountpoint prefix to subdirectories is
> also needed if "--mount-point" presents. However, such prefix cannot
> be added by just using erofs_fspath().
>
> One exception is that the root directory itself needs to be handled
> specially for canned fs_config. For such case, the prefix of the root
> directory has to be dropped instead.
>
> [1] https://lkml.kernel.org/r/20201222020430.12512-1-zbestahu@gmail.com
>
> Link: https://lore.kernel.org/r/20201226062736.29920-1-hsiangkao@aol.com
> Fixes: 8a9e8046f170 ("AOSP: erofs-utils: add fs_config support")
> Reported-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@aol.com>
> ---
> changes since v2:
> - fix IS_ROOT misuse reported by Jianan, very sorry about this since
> I know little about canned fs_config.
>
> (please kindly test again...)
>
> lib/inode.c | 39 +++++++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/lib/inode.c b/lib/inode.c
> index 0c4839d..e6159c9 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -696,32 +696,43 @@ int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
> /* filesystem_config does not preserve file type bits */
> mode_t stat_file_type_mask = st->st_mode & S_IFMT;
> unsigned int uid = 0, gid = 0, mode = 0;
> - char *fspath;
> + const char *fspath;
> + char *decorated = NULL;
>
> inode->capabilities = 0;
> + if (!cfg.fs_config_file && !cfg.mount_point)
> + return 0;
> +
> + if (!cfg.mount_point ||
> + /* have to drop the mountpoint for rootdir of canned fsconfig */
> + (cfg.fs_config_file && erofs_fspath(path)[0] == '\0')) {
> + fspath = erofs_fspath(path);
> + } else {
> + if (asprintf(&decorated, "%s/%s", cfg.mount_point,
> + erofs_fspath(path)) <= 0)
> + return -ENOMEM;
> + fspath = decorated;
> + }
> +
erofs_fspath has been written for three times, and called always.
What do you think refact it ? Do it with
fspath = erofs_fspath(path);
if need decorated:
fspath = decorated
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-31 16:31 ` Li GuiFu via Linux-erofs
@ 2020-12-31 16:50 ` Gao Xiang
0 siblings, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2020-12-31 16:50 UTC (permalink / raw)
To: Li GuiFu; +Cc: Yue Hu, linux-erofs, Yue Hu
Hi Guifu,
On Fri, Jan 01, 2021 at 12:31:22AM +0800, Li GuiFu wrote:
> Thank for your works, Yue Hu, Jianan, Gao Xiang
>
> On 2020/12/28 18:51, Gao Xiang wrote:
...
> > diff --git a/lib/inode.c b/lib/inode.c
> > index 0c4839d..e6159c9 100644
> > --- a/lib/inode.c
> > +++ b/lib/inode.c
> > @@ -696,32 +696,43 @@ int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
> > /* filesystem_config does not preserve file type bits */
> > mode_t stat_file_type_mask = st->st_mode & S_IFMT;
> > unsigned int uid = 0, gid = 0, mode = 0;
> > - char *fspath;
> > + const char *fspath;
> > + char *decorated = NULL;
> >
> > inode->capabilities = 0;
> > + if (!cfg.fs_config_file && !cfg.mount_point)
> > + return 0;
> > +
> > + if (!cfg.mount_point ||
> > + /* have to drop the mountpoint for rootdir of canned fsconfig */
> > + (cfg.fs_config_file && erofs_fspath(path)[0] == '\0')) {
> > + fspath = erofs_fspath(path);
> > + } else {
> > + if (asprintf(&decorated, "%s/%s", cfg.mount_point,
> > + erofs_fspath(path)) <= 0)
> > + return -ENOMEM;
> > + fspath = decorated;
> > + }
> > +
> erofs_fspath has been written for three times, and called always.
> What do you think refact it ? Do it with
> fspath = erofs_fspath(path);
> if need decorated:
> fspath = decorated
Yeah, it sounds reasonable. Let's refine this as a improvement of
an individual patch (if needed) since I have to rebuild AOSP again
(that is a bit complex for me now to open my PC...)
Thanks,
Gao Xiang
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] AOSP: erofs-utils: fix sub-directory prefix for canned fs_config
2020-12-28 10:51 ` [PATCH v2] " Gao Xiang
` (3 preceding siblings ...)
2020-12-31 16:31 ` Li GuiFu via Linux-erofs
@ 2020-12-31 16:46 ` Li GuiFu via Linux-erofs
4 siblings, 0 replies; 14+ messages in thread
From: Li GuiFu via Linux-erofs @ 2020-12-31 16:46 UTC (permalink / raw)
To: Gao Xiang, linux-erofs, Yue Hu, Huang Jianan; +Cc: Yue Hu
On 2020/12/28 18:51, Gao Xiang wrote:
> From: Gao Xiang <hsiangkao@aol.com>
>
> "failed to find [%s] in canned fs_config" was observed by using
> "--fs-config-file" option as reported by Yue Hu [1].
>
> The root cause was that the mountpoint prefix to subdirectories is
> also needed if "--mount-point" presents. However, such prefix cannot
> be added by just using erofs_fspath().
>
> One exception is that the root directory itself needs to be handled
> specially for canned fs_config. For such case, the prefix of the root
> directory has to be dropped instead.
>
> [1] https://lkml.kernel.org/r/20201222020430.12512-1-zbestahu@gmail.com
>
> Link: https://lore.kernel.org/r/20201226062736.29920-1-hsiangkao@aol.com
> Fixes: 8a9e8046f170 ("AOSP: erofs-utils: add fs_config support")
> Reported-by: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Gao Xiang <hsiangkao@aol.com>
> ---
> changes since v2:
> - fix IS_ROOT misuse reported by Jianan, very sorry about this since
> I know little about canned fs_config.
>
> (please kindly test again...)
>
> lib/inode.c | 39 +++++++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
It looks good
Reviewed-by: Li Guifu <bluce.lee@aliyun.com>
Thanks,
^ permalink raw reply [flat|nested] 14+ messages in thread