Re: [PATCH v5 09/10] io_uring: add support for IORING_OP_LINKAT

[Pavel Begunkov <asml.silence@gmail.com>]

On 6/23/21 7:09 AM, Dmitry Kadashev wrote:
> On Tue, Jun 22, 2021 at 6:48 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>
>> On 6/3/21 6:18 AM, Dmitry Kadashev wrote:
>>> IORING_OP_LINKAT behaves like linkat(2) and takes the same flags and
>>> arguments.
>>>
>>> In some internal places 'hardlink' is used instead of 'link' to avoid
>>> confusion with the SQE links. Name 'link' conflicts with the existing
>>> 'link' member of io_kiocb.
>>>
>>> Suggested-by: Christian Brauner <christian.brauner@ubuntu.com>
>>> Link: https://lore.kernel.org/io-uring/20210514145259.wtl4xcsp52woi6ab@wittgenstein/
>>> Signed-off-by: Dmitry Kadashev <dkadashev@gmail.com>
>>> ---
>>>  fs/internal.h                 |  2 ++
>>>  fs/io_uring.c                 | 67 +++++++++++++++++++++++++++++++++++
>>>  fs/namei.c                    |  2 +-
>>>  include/uapi/linux/io_uring.h |  2 ++
>>>  4 files changed, 72 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/internal.h b/fs/internal.h
>>> index 3b3954214385..15a7d210cc67 100644
>>> --- a/fs/internal.h
>>> +++ b/fs/internal.h
>>
>> [...]
>>> +
>>> +static int io_linkat(struct io_kiocb *req, int issue_flags)
>>> +{
>>> +     struct io_hardlink *lnk = &req->hardlink;
>>> +     int ret;
>>> +
>>> +     if (issue_flags & IO_URING_F_NONBLOCK)
>>> +             return -EAGAIN;
>>> +
>>> +     ret = do_linkat(lnk->old_dfd, lnk->oldpath, lnk->new_dfd,
>>> +                             lnk->newpath, lnk->flags);
>>
>> I'm curious, what's difference b/w SYMLINK and just LINK that
>> one doesn't use old_dfd and another does?
> 
> Symlink's content does not have to exist, it's pretty much an arbitrary string.
> E.g. try `ln -s http://example.com/ foo` :)
> 
>> Can it be supported/wished by someone in the future?
> 
> I don't really know. I guess it could be imagined that someone wants to try and
> resolve the full target name against some dfd. But to me the whole idea looks
> inherently problematic. Accepting the old dfd feels like the path is going to
> be resolved, and historically it is not the case, and we'd need a special dfd
> value to mean "do not resolve", and AT_FDCWD won't work for this (since it
> means "resolve against the CWD", not "do not resolve").

I see, I don't know it good enough to reason, but have to throw the question
into the air, ...

>> In that case I'd rather reserve and verify a field for old_dfd for both, even
>> if one won't really support it for now.
> 
> If I understand you correctly, at this point you mean just checking that
> old_dfd is not set (i.e. == -1)? I'll add a check.

... and we have all 5.14 to fix it and other parts if needed, so let's
leave it as is

-- 
Pavel Begunkov