Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 22 Aug 2019 17:08:27 +0800
From:   Gao Xiang <gaoxiang25@huawei.com>
To:     Richard Weinberger <richard.weinberger@gmail.com>
CC:     linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Richard Weinberger <richard@nod.at>,
        <linux-erofs@lists.ozlabs.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: erofs: Question on unused fields in on-disk structs
Message-ID: <20190822090827.GB193349@architecture4>
References: <1323459733.69859.1566234633793.JavaMail.zimbra@nod.at>
 <20190819204504.GB10075@hsiangkao-HP-ZHAN-66-Pro-G1>
 <CAFLxGvxr2UMeVa29M9pjLtWMFPz7w6udRV38CRxEF1moyA9_Rw@mail.gmail.com>
 <20190821220251.GA3954@hsiangkao-HP-ZHAN-66-Pro-G1>
 <CAFLxGvzLPgD22pVOV_jz1EvC-c7YU_2dEFbBt4q08bSkZ3U0Dg@mail.gmail.com>
 <20190822090541.GA193349@architecture4>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20190822090541.GA193349@architecture4>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Aug 22, 2019 at 05:05:41PM +0800, Gao Xiang wrote:

[]

> > 
> > But be very sure that existing erofs filesystems actually have this field
> > set to 0 or something other which is always the same.
> > Otherwise you cannot use the field anymore because it could be anything.
> > A common bug is that the mkfs program keeps such unused fields
> > uninitialized and then it can be a more or less random value without
> > notice.
> 
> Why? In my thought, the logic is that
>  - v4.3, "features" that kernel can handle is 0, so chksum is unused (DONTCARE field)
>    and chksum field could be anything, but the kernel doesn't care.

- sorry, I meant linux <= v5.3. add a word....

Thanks,
Gao Xiang

> 
>  - later version, add an extra compat feature to "features" to indicate SB_CHKSUM
>     is now valid, such as EROFS_FEATURE_SB_CHKSUM (rather than requirements, it's
>     incompat), so the kernel can check the checksum like that:
> 
>     if (feature & EROFS_FEATURE_SB_CHKSUM) {	/* chksum is set */
>         if (chk crc32c and no match) {
>              return -EFSBADCRC;
> 	}
>         go ahead
>     } else {
>         /* still don't care chksum field but print the following warning to kmsg */
>         warnln("You are mounting a image without super_block chksum, please take care!!!!");
> 
>         or maybe we can even refuse mount these images, except for some mount option
>         such as "force-mount".
>     }
> 
>  That is also what F2FS did recently, refer the following commit
>    commit d440c52d3151("f2fs: support superblock checksum")
> 
> > 
> > > Or maybe you mean these reserved fields? I have no idea all other
> > > filesystems check these fields to 0 or not... But I think it should
> > > be used with some other flag is set rather than directly use, right?
> > 
> > Basically you want a way to know when a field shall be used and when not.
> > Most filesystems have version/feature fields. Often multiple to denote different
> > levels of compatibility.
> 
> On-disk inode has i_advise field, and super_block has
> "features" and "requirements" fields. we can use some of them
> or any combinations.
> 
> Thanks,
> Gao Xiang
> 
> > 
> > -- 
> > Thanks,
> > //richard