On Fri, Feb 21, 2025 at 5:34 PM Theodore Ts'o <tytso@xxxxxxx> wrote: > > I think a few people were talking past each other, because there are two > fileds in struct fileattr --- flags, and fsx_xflags. The flags field > is what was originally used by FS_IOC_EXT2_[GS]ETFLAGS, which later > started getting used by many other file systems, starting with > resierfs and btrfs, and so it became FS_IOC_[GS]ETFLAGS. The bits in > that flags word were both the ioctl ABI and the on-disk encoding, and > because we were now allowing multiple file systems to allocate bits, > and we needed to avoid stepping on each other (for example since btrfs > started using FS_NOCOW_FL, that bit position wouldn't be used by ext4, > at least not for a publically exported flag). > > So we started running out of space in the FS_FLAG_*_FL namespace, and > that's why we created FS_IOC_[GS]ETXATTR and the struct fsxattr. The > FS_XFLAG_*_FL space has plenty of space; there are 14 unassigned bit > positions, by my count. > > As far as the arguments about "proper interface design", as far as > Linux is concerned, backwards compatibility trumps "we should have > done if it differently". The one and only guarantee that we have that > FS_IOC_GETXATTR followed by FS_IOC_SETXATTR will work. Nothing else. > > The use case of "what if a backup program wants to backup the flags > and restore on a different file system" is one that hasn't been > considered, and I don't think any backup programs do it today. For > that matter, some of the flags, such as the NODUMP flag, are designed > to be instructions to a dump/restore system, and not really one that > *should* be backed up. Again, the only semantic that was guaranteed > is GETXATTR or GETXATTR followed by SETXATTR. > Thanks for chiming in, Ted! Notes from the original author of FS_IOC_EXT2_[GS]ETFLAGS are valuable. > We can define some new interface for return what xflags are supported > by a particular file system. This could either be the long-debated, > but never implemented statfsx() system call. Or it could be extending > what gets returned by FS_IOC_GETXATTR by using one of the fs_pad > fields in struct fsxattr. This is arguably the simplest way of > dealing with the problem. > That is also what I think. fsx_xflags_mask semantics for GET are pretty clear and follows the established pattern of stx_attributes_mask Even if it is not mandatory for userspace, it can be useful. I asked Dave if he objects to fsx_xflags_mask and got silence, so IMO, if Pali wants to implement fsx_xflags_mask for the API I see no reason to resist it. > I suppose the field could double as the bitmask field when > FS_IOC_SETXATTR is called, but that just seems to be an overly complex > set of semantics. If someone really wants to do that, I wouldn't > really complain, but then what we would actually call the field > "flags_supported_on_get_bitmask_on_set" would seem a bit wordy. :-) If we follow the old rule of SET after GET should always work then fsx_xflags_mask will always be a superset of fsx_xflags, so I think it would be sane to return -EINVAL in the case of (fsx_xflags_mask && fsx_xflags & ~fsx_xflags_mask), which is anyway the correct behavior for filesystems when the user is trying to set flags that the filesystem does not support. As far as I could see, all in-tree filesystems behave this way when the user is trying to set unsupported flags either via FS_IOC_SETFLAGS or via FS_IOC_SETXATTR except xfs, which ignores unsupported fsx_xflags from FS_IOC_SETXATTR. Changing the behavior of xfs to return -EINVAL for unsupported fsx_xflags if fsx_xflags_mask is non zero is NOT going to break UAPI, because as everyone keeps saying, the only guarantee from FS_IOC_SETXATTR was that FS_IOC_SETXATTR after FS_IOC_GETXATTR works and that guarantee will not be broken. Thanks, Amir.
