linux-kernel - Re: [PATCH v1 07/23] erofs: add nodev mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9ab680eb-beb2-07eb-eab2-833fde48906d@linux.alibaba.com>
Date:   Wed, 5 Jan 2022 17:04:19 +0800
From:   JeffleXu <jefflexu@...ux.alibaba.com>
To:     dhowells@...hat.com, linux-cachefs@...hat.com, xiang@...nel.org,
        chao@...nel.org, linux-erofs@...ts.ozlabs.org,
        linux-fsdevel@...r.kernel.org, joseph.qi@...ux.alibaba.com,
        bo.liu@...ux.alibaba.com, tao.peng@...ux.alibaba.com,
        gerry@...ux.alibaba.com, eguan@...ux.alibaba.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 07/23] erofs: add nodev mode



On 1/4/22 10:58 PM, Gao Xiang wrote:
> On Tue, Jan 04, 2022 at 10:33:26PM +0800, Gao Xiang wrote:
>> On Mon, Dec 27, 2021 at 08:54:28PM +0800, Jeffle Xu wrote:
>>> Until then erofs is exactly blockdev based filesystem. In other using
>>> scenarios (e.g. container image), erofs needs to run upon files.
>>>
>>> This patch introduces a new nodev mode, in which erofs could be mounted
>>> from a bootstrap blob file containing the complete erofs image.
>>>
>>> The following patch will introduce a new mount option "uuid", by which
>>> users could specify the bootstrap blob file.
>>>
>>> Signed-off-by: Jeffle Xu <jefflexu@...ux.alibaba.com>
>>
>> I think the order of some patches in this patchset can be improved.
>>
>> Take this patch as an example. This patch introduces a new mount
>> option called "uuid", so the kernel will just accept it (which
>> generates a user-visible impact) after this patch but it doesn't
>> actually work.
>>
>> Therefore, we actually have three different behaviors here:
>>  - kernel doesn't support "uuid" mount option completely;
>>  - kernel support "uuid" but it doesn't work;
>>  - kernel support "uuid" correctly (maybe after some random patch);
>>
>> Actually that is bad for bisecting since there are some commits
>> having temporary behaviors. And we don't know which commit
>> actually fully implements this "uuid" mount option.
>>
>> So personally I think the proper order is just like the bottom-up
>> approach, and make sure each patch can be tested / bisected
>> independently.
> 
> Oh, I may misread this patch, but I still think we'd better to
> avoid dead paths "TODO" like this as much as possible.
> 
> Just do in the bottom-up way.
> 

OK, it is better to be put at the latter part of the whole patch set.
Would be fixed in the next version. Thanks.

-- 
Thanks,
Jeffle