On Wed, Jul 14, 2021 at 07:58:17AM +0100, Christoph Hellwig wrote:
> > + /*
> > + * Now that we've recovered any pending superblock feature bit
> > + * additions, we can finish setting up the attr2 behaviour for the
> > + * mount. If no attr2 mount options were specified, the we use the
> > + * behaviour specified by the superblock feature bit.
> > + */
> > + if (!(mp->m_flags & (XFS_MOUNT_ATTR2|XFS_MOUNT_NOATTR2)) &&
>
> Missing spaces around the |.
>
> > + if ((mp->m_flags & (XFS_MOUNT_ATTR2|XFS_MOUNT_NOATTR2)) ==
> > + (XFS_MOUNT_ATTR2|XFS_MOUNT_NOATTR2)) {
> > + xfs_warn(mp, "attr2 and noattr2 cannot both be specified.");
> > + return -EINVAL;
> > + }
>
> Same here.
>
> > +
> > +
> > if ((mp->m_flags & XFS_MOUNT_NOALIGN) &&
>
> Double empty line.
Will fix.
> More importantly I wonder if we can simplify this further:
>
> - completely ignore the attr2 option (we already warn about it being
> deprecated) and remove XFS_MOUNT_ATTR2
> - just check for XFS_MOUNT_NOATTR2 do disable automatically switching
> to v2 attrs
That's what happens later in the patchset. The XFS_FEAT_ATTR2 is
set when either the mount option or the on-disk sb flag is set, and
it is overridden after log recovery (which can set the SB flag) if
the XFS_FEAT_NOATTR2 feature has been specified.
> And maybe as a service to the user warn when the noattr2 option is
> specified on a file system that already has v2 attrs.
That can be added, but I don't see the point - it will jsut add a
new warning that can be ignored to every mount for such users.
<shrug>
See what others think, and I'll do whatever falls out...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
