Re: erofs: Question on unused fields in on-disk structs

From: Richard Weinberger <richard.weinberger@gmail.com>
To: Gao Xiang <hsiangkao@aol.com>
Cc: Richard Weinberger <richard@nod.at>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-erofs@lists.ozlabs.org,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: erofs: Question on unused fields in on-disk structs
Date: Thu, 22 Aug 2019 10:33:01 +0200	[thread overview]
Message-ID: <CAFLxGvzLPgD22pVOV_jz1EvC-c7YU_2dEFbBt4q08bSkZ3U0Dg@mail.gmail.com> (raw)
In-Reply-To: <20190821220251.GA3954@hsiangkao-HP-ZHAN-66-Pro-G1>

On Thu, Aug 22, 2019 at 12:03 AM Gao Xiang <hsiangkao@aol.com> wrote:
>
> Hi Richard,
>
> On Wed, Aug 21, 2019 at 11:37:30PM +0200, Richard Weinberger wrote:
> > Gao Xiang,
> >
> > On Mon, Aug 19, 2019 at 10:45 PM Gao Xiang via Linux-erofs
> > <linux-erofs@lists.ozlabs.org> wrote:
> > > > struct erofs_super_block has "checksum" and "features" fields,
> > > > but they are not used in the source.
> > > > What is the plan for these?
> > >
> > > Yes, both will be used laterly (features is used for compatible
> > > features, we already have some incompatible features in 5.3).
> >
> > Good. :-)
> > I suggest to check the fields being 0 right now.
> > Otherwise you are in danger that they get burned if an mkfs.erofs does not
> > initialize the fields.
>
> Sorry... I cannot get the point...

Sorry for being unclear, let me explain in more detail.

> super block chksum could be a compatible feature right? which means
> new kernel can support it (maybe we can add a warning if such image
> doesn't have a chksum then when mounting) but old kernel doesn't
> care it.

Yes. But you need some why to indicate that the chksum field is now
valid and must be used.

The features field can be used for that, but you don't use it right now.
I recommend to check it for being 0, 0 means then "no features".
If somebody creates in future a erofs with more features this code
can refuse to mount because it does not support these features.

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.

> 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.

-- 
Thanks,
//richard