On Tue, 22 Dec 2020 18:33:20 +0800 Gao Xiang <[email protected]> wrote:
> On Tue, Dec 22, 2020 at 06:17:51PM +0800, Yue Hu wrote: > > On Tue, 22 Dec 2020 17:59:06 +0800 > > Gao Xiang <[email protected]> wrote: > > > > > On Tue, Dec 22, 2020 at 05:46:23PM +0800, Yue Hu wrote: > > > > On Tue, 22 Dec 2020 17:39:52 +0800 > > > > Gao Xiang <[email protected]> wrote: > > > > > > > > > On Tue, Dec 22, 2020 at 05:30:14PM +0800, Yue Hu wrote: > > > > > > > > > > ... > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > + else if (asprintf(&fspath, "%s/%s", cfg.mount_point, > > > > > > > > > + erofs_fspath(path)) <= 0) > > > > > > > > > > > > > > > > The argument of path will be root directory. And canned > > > > > > > > fs_config for root directory as > > > > > > > > below: > > > > > > > > > > > > > > > > 0 0 755 selabel=u:object_r:rootfs:s0 capabilities=0x0 > > > > > > > > > > > > > > > > So, cannot add mount point to root directory for canned > > > > > > > > fs_config. And what about non-canned > > > > > > > > fs_config? > > > > > > > > > > > > > > Not quite sure what you mean. For non-canned fs_config, we didn't > > > > > > > observed any strange > > > > > > > before (I ported to cuttlefish and hikey960 with boot success, > > > > > > > also as I mentioned before > > > > > > > some other vendors already use it.) > > > > > > > > > > > > > > I think the following commit is only useful for squashfs since > > > > > > > its (non)root inode > > > > > > > workflows are different, so need to add in two difference place. > > > > > > > But mkfs.erofs is not. > > > > > > > https://android.googlesource.com/platform/external/squashfs-tools/+/85a6bc1e52bb911f195c5dc0890717913938c2d1%5E%21/#F0 > > > > > > > > > > > > > > For root inode is erofs, I think erofs_fspath(path) would return > > > > > > > "", so that case > > > > > > > is included as well.... Am I missing something? > > > > > > > > > > > > Yes, erofs_fspath(path) returns "" for root inode. However, the > > > > > > above patch add the mount > > > > > > point to fspath when specify it, so the real path is "vendor/" > > > > > > which does not exist in canned > > > > > > fs_config file. build will report below error: > > > > > > > > > > > > failed to find [/vendor/] in canned fs_config > > > > > > > > > > Hmmm... such design is quite strange for me.... > > > > > > > > :) i checked the squashfs before, seems root directory is handled in > > > > some position. Separated > > > > with sub directory fs_config. so i add the goto code in the 1st patch. > > > > > > > > > > What confuses me a lot is that we didn't get any strange without canned > > > fs_config > > > if mount point prefix is added. I will look into other implementation > > > about this > > > later (Another guess is that relates to Android 10 only?). > > > > maybe relates to dynamic partition(intro from Android 10) which not be > > enabled by some vendors. > > I think some of them use dynamic partition AFAIK, but might not be with QSSI > enabled (I'm not sure, anyway, that is minor...) > > > > > > > > > Yeah, the opensource fs_config implementation might be different from > > > HUAWEI > > > internal fs_config version since such part was not originally written by > > > me and > > > I didn't pay more attention about this part when I was in my previous > > > company. > > > But anyway, this cleanup opensource version is already used for some > > > vendors > > > as well and I don't get such report... And any formal description about > > > this > > > would be helpful for me to understand how fs_config really works.. > > > > Now i'm not familar with fs_config also :) I will continue to check when i > > have > > enough time. > > > > Anyway, i observed the issue in canned fs_config since i'm using it. so i > > decide > > to report it and patch it to upstream to verify if it's a real one. > > Yeah, that is somewhat bad and needs fixing if canned fs_config doesn't work > as expected... > My confusion for now is that how to deal with root dir properly (it seems > make_ext4fs doesn't even care about rootdir fs_config at all if my > understanding > is correct.) > > Also, > https://android.googlesource.com/platform/system/core/+/master/libcutils/fs_config.cpp > https://android.googlesource.com/platform/system/core/+/master/libcutils/canned_fs_config.cpp > > are implemented quite different. So look forward to your test result (I tend > to add prefix for fs_config, but drop prefix for canned_fs_config instead.) It works for canned fs_config i'm using. We can simplify the test enviroment. canned fs_config file content/format (e.g. mount point is vendor) as below: 0 2000 755 selabel=u:object_r:rootfs:s0 capabilities=0x0 vendor/app 0 2000 755 selabel=u:object_r:vendor_app_file:s0 capabilities=0x0 vendor/app/CACertService 0 2000 755 selabel=u:object_r:vendor_app_file:s0 capabilities=0x0 vendor/app/CACertService/CACertService.apk 0 0 644 selabel=u:object_r:vendor_app_file:s0 capabilities=0x0 vendor/app/CACertService/oat 0 2000 755 selabel=u:object_r:vendor_app_file:s0 capabilities=0x0 vendor/app/CACertService/oat/arm64 0 2000 755 selabel=u:object_r:vendor_app_file:s0 capabilities=0x0 The 1st line is for root inode search and the others are for sub inode like search. > > Thanks, > Gao Xiang > > > > > Thx. > > > > > > > > Thanks, > > > Gao Xiang > > > > > > > > > > > > Could you try the following diff? > > > > > > > > Let's me verify. > > > > > > > > > >
