Re: [PATCH 03/28] xfs: define the on-disk format for the metadir feature

Darrick J. Wong
2024-10-15T18:26:31Z

On Tue, Oct 15, 2024 at 07:21:23AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 15, 2024 at 03:11:58PM +1100, Dave Chinner wrote:
> > > +enum xfs_metafile_type {
> > > + XFS_METAFILE_UNKNOWN, /* unknown */
> > > + XFS_METAFILE_DIR, /* metadir directory */
> > > + XFS_METAFILE_USRQUOTA, /* user quota */
> > > + XFS_METAFILE_GRPQUOTA, /* group quota */
> > > + XFS_METAFILE_PRJQUOTA, /* project quota */
> > > + XFS_METAFILE_RTBITMAP, /* rt bitmap */
> > > + XFS_METAFILE_RTSUMMARY, /* rt summary */
> > > +
> > > + XFS_METAFILE_MAX
> > > +} __packed;
> >
> > Ok, so that's all the initial things that we want to support. How do
> > we handle expanding this list of types in future? i.e. does it
> > require incompat or rocompat feature bit protection, new inode
> > flags, and/or something else?
>
> New incompat flag.
>
> >
> >
> > > @@ -812,7 +844,10 @@ struct xfs_dinode {
> > > __be16 di_mode; /* mode and type of file */
> > > __u8 di_version; /* inode version */
> > > __u8 di_format; /* format of di_c data */
> > > - __be16 di_onlink; /* old number of links to file */
> > > + union {
> > > + __be16 di_onlink; /* old number of links to file */
> > > + __be16 di_metatype; /* XFS_METAFILE_* */
> > > + } __packed; /* explicit packing because arm gcc bloats this up */
> >
> > That's a nasty landmine. Does anything bad happen to the log dinode
> > with the same compilers?
>
> The log_dinode just has di_metatype because we never log di_onlink.
>
> TBH I wonder if just changing the field to be just di_metatype
> and adding a comment that for v1 inodes it is used for the nlink
> field might be a cleaner.

I think so. packed unions are gross.

--D