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

[Stefan Roesch <shr@fb.com>]

Hi Stefan,

On 12/1/21 4:19 AM, Stefan Metzmacher wrote:
> 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.
> 
> I'd assume that even 6 helper functions in liburing would be able
> to use just 2 low level iouring opcodes.
>

I'm open to also adding lsetxattr and lgetxattr. Do you have a use case in mind?
 
> *listxattr is also missing, are there plans for them?
> 

*listxattr is currently not planned.

> metze
>