Re: [PATCH v3] fs: introduce getfsxattrat and setfsxattrat syscalls

Pali Rohár
2025-02-25T20:34:45Z

On Tuesday 25 February 2025 07:59:26 Darrick J. Wong wrote:
> On Tue, Feb 25, 2025 at 12:24:08PM +0100, Christian Brauner wrote:
> > On Tue, Feb 25, 2025 at 11:40:51AM +0100, Arnd Bergmann wrote:
> > > On Tue, Feb 25, 2025, at 11:22, Christian Brauner wrote:
> > > > On Tue, Feb 25, 2025 at 09:02:04AM +0100, Arnd Bergmann wrote:
> > > >> On Mon, Feb 24, 2025, at 12:32, Christian Brauner wrote:
> > > >>
> > > >> The ioctl interface relies on the existing behavior, see
> > > >> 0a6eab8bd4e0 ("vfs: support FS_XFLAG_COWEXTSIZE and get/set of
> > > >> CoW extent size hint") for how it was previously extended
> > > >> with an optional flag/word. I think that is fine for the syscall
> > > >> as well, but should be properly documented since it is different
> > > >> from how most syscalls work.
> > > >
> > > > If we're doing a new system call I see no reason to limit us to a
> > > > pre-existing structure or structure layout.
> > >
> > > Obviously we could create a new structure, but I also see no
> > > reason to do so. The existing ioctl interface was added in
> > > in 2002 as part of linux-2.5.35 with 16 bytes of padding, half
> > > of which have been used so far.
> > >
> > > If this structure works for another 23 years before we run out
> > > of spare bytes, I think that's good enough. Building in an
> > > incompatible way to handle potential future contents would
> > > just make it harder to use for any userspace that wants to
> > > use the new syscalls but still needs a fallback to the
> > > ioctl version.
> >
> > The fact that this structure has existed since the dawn of time doesn't
> > mean it needs to be retained when adding a completely new system call.
> >
> > People won't mix both. They either switch to the new interface because
> > they want to get around the limitations of the old interface or they
> > keep using the old interface and the associated workarounds.
> >
> > In another thread they keep arguing about new extensions for Windows
> > that are going to be added to the ioctl interface and how to make it fit
> > into this. That just shows that it's very hard to predict from the
> > amount of past changes how many future changes are going to happen. And
> > if an interface is easy to extend it might well invite new changes that
> > people didn't want to or couldn't make using the old interface.
>
> Agreed, I don't think it's hard to enlarge struct fsxattr in the
> existing ioctl interface; either we figure out how to make the kernel
> fill out the "missing" bytes with an internal getfsxattr call, or we
> make it return some errno if we would be truncating real output due to
> struct size limits and leave a note in the manpage that "EL3HLT means
> use a bigger structure definition"
>
> Then both interfaces can plod along for another 30 years. :)
>
> --D

For Windows attributes, there are for sure needed new 11 bits for
attributes which can be both get and set, additional 4 bits for get-only
attributes, and plus there are 9 reserved bits (which Windows can start
using it and exporting over NTFS or SMB). And it is possible that
Windows can reuse some bits which were previously assigned for things
which today does not appear on NTFS.

I think that fsx_xflags does not have enough free bits for all these
attributes. So it would be really nice to design API/ABI in away which
can be extended for new fields.

Also another two points, for this new syscalls. I have not looked at the
current changes (I was added to CC just recently), but it would be nice:

1) If syscall API allows to operate on the symlink itself. This is
because NTFS and also SMB symlink also contains attributes. ioctl
interface currently does not support to get/set these symlink
attributes.

2) If syscall API contains ability to just change subset of attributes.
And provide an error reporting to userspace if userspace application
is trying to set attribute which is not supported by the filesystem.
This error reporting is needed for possible "cp -a" or possible
"rsync" implementation which informs when some metadata cannot be
backup/restored. There are more filesystems which supports only
subset of attributes, this applies also for windows attributes.
For example UDF fs supports only "hidden" attribute.