[PATCH v1 1/2] erofs-utils: Relax the hardchecks on the blocksize

[Gao Xiang]

On 2023/8/31 07:40, Sandeep Dhavale wrote:
> On Wed, Aug 30, 2023 at 4:34 PM Gao Xiang <hsiangkao at linux.alibaba.com> wrote:
>>
>> Hi Sandeep,
>>
>> On 2023/8/31 07:16, Sandeep Dhavale wrote:
>>> As erofs-utils supports different block sizes upto
>>> EROFS_MAX_BLOCK_SIZE, relax the checks so same tools
>>> can be used to create images for platforms where
>>> page size can be greater than 4096.
>>>
>>> Signed-off-by: Sandeep Dhavale <dhavale at google.com>
>>> ---
>>>    lib/namei.c | 2 --
>>>    mkfs/main.c | 9 +++++----
>>>    2 files changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/namei.c b/lib/namei.c
>>> index 2bb1d4c..45dbcd3 100644
>>> --- a/lib/namei.c
>>> +++ b/lib/namei.c
>>> @@ -144,8 +144,6 @@ int erofs_read_inode_from_disk(struct erofs_inode *vi)
>>>                vi->u.chunkbits = sbi->blkszbits +
>>>                        (vi->u.chunkformat & EROFS_CHUNK_FORMAT_BLKBITS_MASK);
>>>        } else if (erofs_inode_is_data_compressed(vi->datalayout)) {
>>> -             if (erofs_blksiz(vi->sbi) != EROFS_MAX_BLOCK_SIZE)
>>> -                     return -EOPNOTSUPP;
>>>                return z_erofs_fill_inode(vi);
>>>        }
>>>        return 0;
>>> diff --git a/mkfs/main.c b/mkfs/main.c
>>> index c03a7a8..37bf658 100644
>>> --- a/mkfs/main.c
>>> +++ b/mkfs/main.c
>>> @@ -550,10 +550,11 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
>>>                cfg.c_dbg_lvl = EROFS_ERR;
>>>                cfg.c_showprogress = false;
>>>        }
>>> -     if (cfg.c_compr_alg[0] && erofs_blksiz(&sbi) != EROFS_MAX_BLOCK_SIZE) {
>>> -             erofs_err("compression is unsupported for now with block size %u",
>>> -                       erofs_blksiz(&sbi));
>>> -             return -EINVAL;
>>> +     if (cfg.c_compr_alg[0] && erofs_blksiz(&sbi) != getpagesize()) {
>>> +             erofs_warn("subpage blocksize with compression is not yet "
>>> +                     "supported. Compressed image will only work with "
>>> +                     "arch pagesize = blocksize = %u bytes",
>>> +                     erofs_blksiz(&sbi));
>>>        }
>>
>> Thanks for the patches.
>>
>> I'm fine to relax  EROFS_MAX_BLOCK_SIZE check, yet could we
>> add a check as erofs_blksiz <= EROFS_MAX_BLOCK_SIZE somewhere?
>>
>> Otherwise, we could suffer from stack overflow
>> (if EROFS_MAX_BLOCK_SIZE is outdated or somewhat small...)
>>
> Hi Gao,
> There is already a check in place for -b option to validate the block
> size passed is within range [512..EROFS_MAX_BLOCK_SIZE] in function
> mkfs_parse_options_cfg()
> 
> Please correct me if I misunderstood your comment.

Oh, sorry I didn't check the current code and thanks for your pointing
out!  Now I have no more questions.

Will apply soon.

Thanks,
Gao Xiang