Re: [PATCH 1/5] xfs: struct xfs_sb is no longer tied to the on-disk format

Dave Chinner
2015-02-02T19:30:38Z

On Mon, Feb 02, 2015 at 12:41:02AM -0800, Christoph Hellwig wrote:
> > /*
> > - * Superblock - in core version. Must match the ondisk version below.
> > - * Must be padded to 64 bit alignment.
> > - */
> > -typedef struct xfs_sb {
> > - __uint32_t sb_magicnum; /* magic number == XFS_SB_MAGIC */
> > - __uint32_t sb_blocksize; /* logical block size, bytes */
>
> > -static inline int xfs_sb_version_hasfinobt(xfs_sb_t *sbp)
> > +static inline int xfs_sb_version_hasfinobt(struct xfs_sb *sbp)
>
> So xfs_format.h now requires struct xfs_sb to be defined before it
> can be included? I guess we need to move these macros around as well.

Yes, that's why I moved it to xfs_super.h, which is included from
xfs_linux.h. I just wanted to move it from xfs_format.h to
/somewhere/ to demonstrate that it isn't part of the on-disk format
anymore.

> > diff --git a/fs/xfs/xfs_super.h b/fs/xfs/xfs_super.h
> > index 2b830c2..a02236b 100644
> > --- a/fs/xfs/xfs_super.h
> > +++ b/fs/xfs/xfs_super.h
> > @@ -61,6 +61,87 @@ struct xfs_mount;
> > struct xfs_buftarg;
> > struct block_device;
> >
> > +/*
> > + * Superblock - in core version. This does not have ot match the size and shape
> > + * of the on-disk superblock, but must contain all the fields that we use in the
> > + * on-disk superblock.
> > + */
> > +struct xfs_sb {
>
> Is this really the right header? xfs_super.h only really is for bits
> related to linux super block operastions.

No, probably not, but it was expedient and good enough for an RFC
and sanity testing. I thought about xfs_mount.h, but then realised
that just introduces a new dependency between xfs_mount.h and
libxfs, which is something I'm trying to reduce.

> I'd expect to move it close to stuct xfs_mount, and maybe even merge
> it into that in the long run.

I guess moving the structure there is fine, but we still want all
the version functions to be shared with userspace, which then makes
for an interesting set of dependencies. Any other ideas?

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com