Re: [RFC] xfs: opting in or out of online repair

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Jul 26, 2024 at 08:33:02AM +1000, Dave Chinner wrote:
> On Thu, Jul 25, 2024 at 04:14:13PM +0200, Christoph Hellwig wrote:
> > On Thu, Jul 25, 2024 at 12:05:26PM +1000, Dave Chinner wrote:
> > > Maybe I'm missing something important - this doesn't feel like
> > > on-disk format stuff. Why would having online repair enabled make
> > > the fileystem unmountable on older kernels?
> > 
> > Yes, that's the downside of the feature flag.
> > 
> > > Hmmm. Could this be implemented with an xattr on the root inode
> > > that says "self healing allowed"?
> > 
> > The annoying thing about stuff in the public file system namespace
> > is that chowning the root of a file system to a random user isn't
> > that uncommon, an that would give that user more privileges than
> > intended.  So it could not hust be a normal xattr but would have
> > to be a privileged one,
> 
> 
> I'm not sure I understand what the problem is. We have a generic
> xattr namespace for this sort of userspace sysadmin info already.
> 
> $ man 7 xattr
> ....
> Trusted extended attributes
>        Trusted extended attributes are visible and accessible only
>        to processes that have the CAP_SYS_ADMIN capability.
>        Attributes in this class are used to implement mechanisms
>        in user space (i.e., outside the kernel) which keep
>        information in extended attributes to which ordinary
>        processes should not have access.
> 
> > and with my VFS hat on I'd really like
> > to avoid creating all these toally overloaded random non-user
> > namespace xattrs that are a complete mess.
> 
> There's no need to create a new xattr namespace at all here.
> Userspace could manipulate a trusted.xfs.self_healing xattr to do
> exactly what we need. It's automatically protected by
> CAP_SYS_ADMIN in the init namespace, hence it provides all the
> requirements that have been presented so far...

<nod> Ok, how about an ATTR_ROOT xattr "xfs.self_healing" that can be
one of "none", "check", or "repair".  No xattr means "check".

> > One option would be an xattr on the metadir root (once we merge
> > that, hopefully for 6.12).  That would still require a new ioctl
> > or whatever interface to change (or carve out an exception to
> > the attr by handle interface), but it would not require kernel
> > and tools to fully understand it.
> 
> That seems awfully complex. It requires a new on-disk
> filesystem format and a new user API to support storing this
> userspace only information. I think this is more work than Darrick's
> original compat flag idea.
> 
> This is information that is only relevant to a specific userspace
> utility, and it can maintain that information itself without needing
> to modify the on-disk format. Indeed, the information doesn't need
> to be in the filesystem at all - it could just as easily be stored
> in a config file in /etc/xfs/ that the xfs-self-healing start
> scripts parse, yes? The config is still privileged information
> requiring root to modify it, so it's no different to a trusted xattr
> except for where the config information is stored.

Sysadmins can already do that via systemctl mask, as I pointed out in
the earlier thread.  I think between that and the xattr we're covered.

> Userspace package config information doesn't belong in the on-disk
> format. It belongs in userspace configuration files (i.e. as file
> data) or in trusted named xattrs (file metadata).

Sounds reasonable to me.

--D

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
>