Re: [PATCH v1 0/5] io_uring: add xattr support

Clay Harris
2021-12-01T19:52:59Z

On Wed, Dec 01 2021 at 13:19:03 +0100, Stefan Metzmacher quoth thus:

> Hi Stefan,
>
> > On 11/29/21 5:08 PM, Clay Harris wrote:
> >> On Mon, Nov 29 2021 at 14:12:52 -0800, Stefan Roesch quoth thus:
> >>
> >>> This adds the xattr support to io_uring. The intent is to have a more
> >>> complete support for file operations in io_uring.
> >>>
> >>> This change adds support for the following functions to io_uring:
> >>> - fgetxattr
> >>> - fsetxattr
> >>> - getxattr
> >>> - setxattr
> >>
> >> You may wish to consider the following.
> >>
> >> Patching for these functions makes for an excellent opportunity
> >> to provide a better interface. Rather than implement fXetattr
> >> at all, you could enable io_uring to use functions like:
> >>
> >> int Xetxattr(int dfd, const char *path, const char *name,
> >> [const] void *value, size_t size, int flags);
> >>
> >> Not only does this simplify the io_uring interface down to two
> >> functions, but modernizes and fixes a deficit in usability.
> >> In terms of io_uring, this is just changing internal interfaces.
> >>
> >> Although unnecessary for io_uring, it would be nice to at least
> >> consider what parts of this code could be leveraged for future
> >> Xetxattr2 syscalls.
> >
> > Clay,
> >
> > while we can reduce the number of calls to 2, providing 4 calls will
> > ease the adoption of the interface.
> >
> > If you look at the userspace interface in liburing, you can see the
> > following function signature:
> >
> > static inline void io_uring_prep_fgetxattr(struct io_uring_sqe *sqe,
> > int fd,
> > const char *name,
> > const char *value,
> > size_t len)
> >
> > This is very similar to what you proposed.
>
> What's with lsetxattr and lgetxattr, why are they missing.
Do any filesystems even support xattrs on symbolic links?

> I'd assume that even 6 helper functions in liburing would be able
> to use just 2 low level iouring opcodes.
>
> *listxattr is also missing, are there plans for them?
>
> metze