On Tue, 22 Dec 2020 11:44:55 +0800
Gao Xiang <[email protected]> wrote:

> Hi Yue,
> 
> On Tue, Dec 22, 2020 at 10:04:30AM +0800, Yue Hu wrote:
> > From: Yue Hu <[email protected]>
> > 
> > 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.)

Hi Xiang,

It's been observed under QC/QSSI platform with dynamic partition.

such as product.img/product_filesystem_config.txt:

```txt
 0 0 755 selabel=u:object_r:rootfs:s0 capabilities=0x0
product/app 0 0 755 selabel=u:object_r:system_file:s0 capabilities=0x0
product/app/CalculatorGoogle 0 0 755 selabel=u:object_r:system_file:s0 
capabilities=0x0
```

product_filesystem_config.txt should be from below build:

$(call fs_config,$(zip_root)/PRODUCT,product/) > 
$(zip_root)/META/product_filesystem_config.txt

Do not observe this issue in squashfs, so i check related code of squashfs, 
squashfs have
considered the mount point, also ext4 does. So, erofs should be same as one 
long used before.

After this patch, build & boot are working fine by test.

Here's a change from mksqushfs: Allow passing fs_config file for generating 
mksquashfs

> 
> > 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... :(

Refer to change: mksquashfs: Run android_fs_config() on the root inode

I think erofs of AOSP has this issue also. Am i right?

Thx.

> 
> Thanks,
> Gao Xiang
> 
> > 
> > Signed-off-by: Yue Hu <[email protected]>
> > ---
> >  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
> >   
> 

Reply via email to