[PATCH 2/2] erofs-utils: rebuild: set the appropriate `dev` field for dirs

[Hongzhen Luo]

On 2024/11/27 17:31, Gao Xiang wrote:
>
>
> On 2024/11/27 17:28, Hongzhen Luo wrote:
>> Currently, setting a default `dev` value (i.e., 0) for directories
>> during parsing tar files can lead to the following error:
>>
>> <E> erofs: bogus i_mode (0) @ nid 0
>> <E> erofs: failed to read inode @ 0
>>
>> Consider the following incremental build scenario, where the path
>> "dir1/dir2" is currently being parsed in tarerofs_parse_tar() and
>> the directory "dir1" has never appeared before. 
>> erofs_rebuild_get_dentry()
>> will call erofs_rebuild_mkdir() to allocate a new inode for "dir1",
>> with its `dev` field set to 0 by default.
>>
>> During the dump tree phase, since `dir1->dev` matches `sbi->dev` 
>> (both are 0),
>> erofs_rebuild_load_basedir() will be called to read the contents of 
>> directory
>> "dir1" from the disk. However, since there is no information for the 
>> new directory
>> "dir1" on the disk, the above error occurs.
>>
>> This patch resolves the above issue by setting the appropriate value
>> for the directory's `dev` field during the tar file parsing phase.
>>
>> Fixes: f64d9d02576b ("erofs-utils: introduce incremental builds")
>> Signed-off-by: Hongzhen Luo <hongzhen at linux.alibaba.com>
>> ---
>>   include/erofs/rebuild.h | 1 +
>>   lib/rebuild.c           | 7 +++++--
>>   lib/tar.c               | 2 ++
>>   3 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/erofs/rebuild.h b/include/erofs/rebuild.h
>> index b37bc80e8a3c..b71cfdf1fff4 100644
>> --- a/include/erofs/rebuild.h
>> +++ b/include/erofs/rebuild.h
>> @@ -22,6 +22,7 @@ struct erofs_rebuild_getdentry_ctx {
>>       bool *whout;
>>       bool *opq;
>>       bool to_head;
>> +    int dev;
>>   };
>>     struct erofs_dentry *erofs_rebuild_get_dentry(struct 
>> erofs_rebuild_getdentry_ctx *ctx);
>> diff --git a/lib/rebuild.c b/lib/rebuild.c
>> index 58f1701b3721..3fcb5c92b562 100644
>> --- a/lib/rebuild.c
>> +++ b/lib/rebuild.c
>> @@ -27,7 +27,7 @@
>>   #endif
>>     static struct erofs_dentry *erofs_rebuild_mkdir(struct 
>> erofs_inode *dir,
>> -                        const char *s)
>> +                        const char *s, int dev)
>>   {
>>       struct erofs_inode *inode;
>>       struct erofs_dentry *d;
>> @@ -47,6 +47,7 @@ static struct erofs_dentry 
>> *erofs_rebuild_mkdir(struct erofs_inode *dir,
>>       inode->i_gid = getgid();
>>       inode->i_mtime = inode->sbi->build_time;
>>       inode->i_mtime_nsec = inode->sbi->build_time_nsec;
>> +    inode->dev = dev;
>
> Why not just use inode->dir->dev?
>
> Thanks,
> Gao Xiang

Yes, I didn't see that; this makes it much simpler. I'll give it a try 
and send another version of the patch later.

---

Thanks,

Hongzhen Luo