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@xxxxxxxxxxxxx >
