linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bernd Schubert <bernd.schubert@fastmail.fm>
To: "Jean-Pierre André" <jean-pierre.andre@wanadoo.fr>,
	linux-fsdevel@vger.kernel.org, "Vivek Goyal" <vgoyal@redhat.com>,
	"Miklos Szeredi" <miklos@szeredi.hu>
Cc: fuse-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 1/3] FUSE: Implement atomic lookup + create
Date: Wed, 11 May 2022 12:08:53 +0200	[thread overview]
Message-ID: <f512b616-e252-205a-d8e5-4ea7fef53edc@fastmail.fm> (raw)
In-Reply-To: <a712f535-7e34-4967-d335-f3680f9c4b6f@wanadoo.fr>



On 5/7/22 12:42, Jean-Pierre André wrote:
> Bernd Schubert wrote on 5/6/22 8:45 PM:
>>
>>
>> On 5/6/22 19:07, Vivek Goyal wrote:
>>> I looked at fuse_lowlevel API and passthrough_ll.c and there is no
>>> assumption whether FUSE_LOOKUP has already been called by client
>>> before calling FUSE_CREATE. Similarly, I looked at virtiofs code
>>> and I can't see any such assumption there as well.
>>
>> The current linux kernel code does this right now, skipping the lookup 
>> just changes behavior.  Personally I would see it as bug if the 
>> userspace implementation does not handle EEXIST for FUSE_CREATE. 
>> Implementation developer and especially users might see it differently 
>> if a kernel update breaks/changes things out of the sudden. 
>> passthrough_ll.c is not the issue here, it handles it correctly, but 
>> what about the XYZ other file systems out there - do you want to check 
>> them one by one, including closed source ones? And wouldn't even a 
>> single broken application count as regression?
>>
>>>
>>> https://github.com/qemu/qemu/blob/master/tools/virtiofsd/passthrough_ll.c 
>>>
>>>
>>> So I am sort of lost. May be you can help me understsand this.
>>
>> I guess it would be more interesting to look at different file systems 
>> that are not overlay based. Like ntfs-3g - I have not looked at the 
>> code yet, but especially disk based file system didn't have a reason 
>> so far to handle EEXIST. And
> 
> AFAIK ntfs-3g proper does not keep a context across calls and does
> not know what LOOKUP was preparing a CREATE. However this may have
> consequences in the high level of libfuse for managing the file tree.

I don't think high level is effected. I'm really just scared to break

> 
> The kernel apparently issues a LOOKUP to decide whether issuing a
> CREATE (create+open) or an OPEN. If it sent blindly a CREATE,
> ntfs-3g would return EEXIST if the name was already present in
> the directory.

Ok, so ntfs-3g is ok - which leaves N-1 file systems to check...

> 
> For a test, can you suggest a way to force ignore of such lookup
> within libfuse, without applying your kernel patches ? Is there a
> way to detect the purpose of a lookup ? (A possible way is to
> hardcode a directory inode within which the lookups return ENOENT).


What I mean is that we need to check the code or test N file systems - 
if ntfs-3g handles it, N-1 are left...

I we all agree on that there is no issue in skipping the lookup - fine 
with me - it slightly simplifies the patches.


Thanks,
Bernd

  reply	other threads:[~2022-05-11 10:09 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-02 10:25 [PATCH v4 0/3] FUSE: Implement atomic lookup + open/create Dharmendra Singh
2022-05-02 10:25 ` [PATCH v4 1/3] FUSE: Implement atomic lookup + create Dharmendra Singh
2022-05-03 12:43   ` Vivek Goyal
2022-05-03 14:13   ` Vivek Goyal
2022-05-03 19:53   ` Vivek Goyal
2022-05-03 20:48     ` Bernd Schubert
2022-05-04  4:26     ` Dharmendra Hans
2022-05-04 14:47       ` Vivek Goyal
2022-05-04 15:46         ` Bernd Schubert
2022-05-04 17:31           ` Vivek Goyal
2022-05-05  4:51         ` Dharmendra Hans
2022-05-05 14:26           ` Vivek Goyal
2022-05-06  5:34             ` Dharmendra Hans
2022-05-06 14:12               ` Vivek Goyal
2022-05-06 16:41                 ` Bernd Schubert
2022-05-06 17:07                   ` Vivek Goyal
2022-05-06 18:45                     ` Bernd Schubert
2022-05-07 10:42                       ` Jean-Pierre André
2022-05-11 10:08                         ` Bernd Schubert [this message]
2022-05-02 10:25 ` [PATCH v4 2/3] FUSE: Implement atomic lookup + open Dharmendra Singh
2022-05-04 18:20   ` Vivek Goyal
2022-05-05  6:39     ` Dharmendra Hans
2022-05-02 10:25 ` [PATCH v4 3/3] FUSE: Avoid lookup in d_revalidate() Dharmendra Singh
2022-05-04 20:39   ` Vivek Goyal
2022-05-04 21:05     ` Bernd Schubert
2022-05-05  5:49     ` Dharmendra Hans
2022-05-04 19:18 ` [PATCH v4 0/3] FUSE: Implement atomic lookup + open/create Vivek Goyal
2022-05-05  6:12   ` Dharmendra Hans
2022-05-05 12:54     ` Vivek Goyal
2022-05-05 15:13       ` Bernd Schubert
2022-05-05 19:59         ` Vivek Goyal
2022-05-11  9:40           ` Miklos Szeredi
2022-05-11  9:59             ` Bernd Schubert
2022-05-11 17:21             ` Vivek Goyal
2022-05-11 19:30               ` Vivek Goyal
2022-05-12  8:16                 ` Dharmendra Hans
2022-05-12 15:24                   ` Vivek Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f512b616-e252-205a-d8e5-4ea7fef53edc@fastmail.fm \
    --to=bernd.schubert@fastmail.fm \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=jean-pierre.andre@wanadoo.fr \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).