[PATCH v2] erofs-utils: mkfs: support flatdev for multi-blob images

[Jingbo Xu]

On 9/15/23 5:21 PM, Gao Xiang wrote:
> 
> 
> On 2023/9/15 16:16, Jingbo Xu wrote:
>> Since kernel commit 8b465fecc35a ("erofs: support flattened block device
>> for multi-blob images"), the flatdev feature has been introduced to
>> support mounting multi-blobs container image as a single block device.
>>
>> To enable this feature, the mapped_blkaddr of each device slot needs to
>> be set properly to the offset of the device in the flat address space.
>>
>> The kernel side requires a non-empty device tag to mount an image in
>> flatdev mode.  The uuid of the source image is used as the corresponding
>> device tag in rebuild mode.
>>
>> Signed-off-by: Jingbo Xu <jefflexu at linux.alibaba.com>
>> ---
>> v2: drop "-Eflatdev" option; always set mapped_blkaddr in device slot
>> ---
>>   include/erofs/internal.h |  1 +
>>   lib/blobchunk.c          |  8 ++++++--
>>   lib/super.c              |  1 +
>>   mkfs/main.c              | 17 +++++++++++++----
>>   4 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
>> index 19b912b..616cd3a 100644
>> --- a/include/erofs/internal.h
>> +++ b/include/erofs/internal.h
>> @@ -54,6 +54,7 @@ extern struct erofs_sb_info sbi;
>>   struct erofs_buffer_head;
>>     struct erofs_device_info {
>> +    u8 tag[64];
>>       u32 blocks;
>>       u32 mapped_blkaddr;
>>   };
>> diff --git a/lib/blobchunk.c b/lib/blobchunk.c
>> index aca616e..a599f3a 100644
>> --- a/lib/blobchunk.c
>> +++ b/lib/blobchunk.c
>> @@ -410,20 +410,24 @@ int erofs_mkfs_dump_blobs(struct erofs_sb_info
>> *sbi)
>>       }
>>         if (sbi->extra_devices) {
>> -        unsigned int i;
>> +        unsigned int i, ret;
>> +        erofs_blk_t nblocks;
>>   +        nblocks = erofs_mapbh(NULL);
>>           pos_out = erofs_btell(bh_devt, false);
>>           i = 0;
>>           do {
>>               struct erofs_deviceslot dis = {
>> +                .mapped_blkaddr = cpu_to_le32(nblocks),
>>                   .blocks = cpu_to_le32(sbi->devs[i].blocks),
>>               };
>> -            int ret;
>>   +            memcpy(dis.tag, sbi->devs[i].tag, sizeof(dis.tag));
>>               ret = dev_write(sbi, &dis, pos_out, sizeof(dis));
>>               if (ret)
>>                   return ret;
>>               pos_out += sizeof(dis);
>> +            nblocks += sbi->devs[i].blocks;
>>           } while (++i < sbi->extra_devices);
>>           bh_devt->op = &erofs_drop_directly_bhops;
>>           erofs_bdrop(bh_devt, false);
>> diff --git a/lib/super.c b/lib/super.c
>> index ce97278..f952f7e 100644
>> --- a/lib/super.c
>> +++ b/lib/super.c
>> @@ -65,6 +65,7 @@ static int erofs_init_devices(struct erofs_sb_info
>> *sbi,
>>             sbi->devs[i].mapped_blkaddr =
>> le32_to_cpu(dis.mapped_blkaddr);
>>           sbi->devs[i].blocks = le32_to_cpu(dis.blocks);
>> +        memcpy(sbi->devs[i].tag, dis.tag, sizeof(dis.tag));
>>           sbi->total_blocks += sbi->devs[i].blocks;
>>           pos += EROFS_DEVT_SLOT_SIZE;
>>       }
>> diff --git a/mkfs/main.c b/mkfs/main.c
>> index 4fa2d92..9327b6f 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -765,8 +765,15 @@ static void erofs_mkfs_default_options(void)
>>       sbi.feature_compat = EROFS_FEATURE_COMPAT_SB_CHKSUM |
>>                    EROFS_FEATURE_COMPAT_MTIME;
>>   -    /* generate a default uuid first */
>> -    erofs_uuid_generate(sbi.uuid);
>> +    /*
>> +     * Generate a default uuid first.  In rebuild mode the uuid of the
>> +     * source image is used as the device slot's tag.  The kernel will
>> +     * identify the tag as empty and fail the mount if its first byte is
>> +     * zero.  Apply this constraint to uuid to work around it.
>> +     */
>> +    do {
>> +        erofs_uuid_generate(sbi.uuid);
>> +    } while (!sbi.uuid[0]);
> 
> I don't think we need this, since uuid need to be parsed to string.

Alright.

> 
>>   }
>>     /* https://reproducible-builds.org/specs/source-date-epoch/ for
>> more details */
>> @@ -822,7 +829,7 @@ static int erofs_rebuild_load_trees(struct
>> erofs_inode *root)
>>       struct erofs_sb_info *src;
>>       unsigned int extra_devices = 0;
>>       erofs_blk_t nblocks;
>> -    int ret;
>> +    int ret, idx;
>>         list_for_each_entry(src, &rebuild_src_list, list) {
>>           ret = erofs_rebuild_load_tree(root, src);
>> @@ -854,7 +861,9 @@ static int erofs_rebuild_load_trees(struct
>> erofs_inode *root)
>>           else
>>               nblocks = src->primarydevice_blocks;
>>           DBG_BUGON(src->dev < 1);
>> -        sbi.devs[src->dev - 1].blocks = nblocks;
>> +        idx = src->dev - 1;
>> +        sbi.devs[idx].blocks = nblocks;
>> +        memcpy(sbi.devs[idx].tag, src->uuid, sizeof(src->uuid));
> 
> Should we use string conversion instead? Should we use uuid only
> if sbi.devs[idx].tag is empty?


sbi.devs[idx].tag is always empty as sbi.devs[] is newly allocated from
calloc().

-- 
Thanks,
Jingbo