linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Enrico Weigelt, metux IT consult" <lkml@metux.net>
To: Peng Tao <bergwolf@gmail.com>
Cc: Alessio Balsini <balsini@android.com>,
	Peng Tao <tao.peng@linux.alibaba.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Amir Goldstein <amir73il@gmail.com>
Subject: Re: [PATCH RFC] fuse: add generic file store
Date: Mon, 21 Jun 2021 21:05:14 +0200	[thread overview]
Message-ID: <65fc0313-b01b-9882-d716-d5a2911222b7@metux.net> (raw)
In-Reply-To: <CA+a=Yy5rnqLqH2iR-ZY6AUkNJy48mroVV3Exmhmt-pfTi82kXA@mail.gmail.com>

On 17.06.21 15:23, Peng Tao wrote:

>> Just keeping fd's open while a server restarts ?
>> If that's what you want, I see much wider use far outside of fuse,
>> and that might call for some more generic approach - something like
>> Plan9's /srv filesystem.
>>
> 1. keeping FDs across userspace restart

if application needs to be rewritten for that anyways, there're other
ways to achieve this, w/o touching the kernel at all - exec() doesn't
automatically close fd's (unless they're opened w/ O_CLOEXEC)

> 2. help save FD in the FUSE fd passthrough use case as implemented by
> Alessio Balsini

you mean this one ?

https://lore.kernel.org/lkml/20210125153057.3623715-1-balsini@android.com

I fail to see why an extra fd store within the fuse device is necessary
for that - I'd just let the fuse daemon(s) reply the open request with
the fd it already holds.

I'd hate to run into situations where even killing all processes holding
some file open leads to a situation where it remains open inside the
kernel, thus blocking e.g. unmounting. I already see operators getting
very angy ... :o

by the way: alessio's approach is limited to simple read/write
operations anyways - other operations like ioctl() don't seem to work
easily that way.

and for the creds switching: I tend to believe that cases where a fs or
device looks at the calling process' creds in operations on an already
open fd, it's most likely a bad implementation.

yes, some legacy drivers actually do check for CAP_SYS_ADMIN e.g. for
low level hardware configuration (e.g. IO and IRQ on ISA bus), but I
wonder whether these are use at all in the our use cases and should be
ever allowed to non-root.

do you have any case where you really need to use the opener's creds ?
(after the fd is already open)

>> Does FUSE actually manipulate the process' fd table directly, while
>> in the open() callback ?
> 
> hmm, you are right. The open() callback cannot install FD from there.
> So in order for your use case to work, the VFS layer needs to be
> changed to transparently replace an empty file struct with another
> file struct that is prepared by the file system somewhere else. It is
> really beyond the current RFC patch's scope IMHO.

Exactly. That's where I'm struggling right now. Yet have to find out
whether I could just copy from one struct file into another (probably
some refcnt'ing required). And that still has some drawback: fd state
like file position won't be shared.

I've been thinking about changing the vfs_open() chain so that it
doesn't pass in an existing/prepared struct file, but instead returns
one, which is allocated further down the chain, right before the fs'
open operation is called. Then we could add another variant that
returns struct file. If the new one is present, it will be called,
otherwise a new struct file is allocated, the old variant is called
on the newly allocated one, and finally return this one.

this is a bigger job to do ...


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

  reply	other threads:[~2021-06-21 19:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-01  8:58 [PATCH RFC] fuse: add generic file store Peng Tao
2021-06-02 15:50 ` Alessio Balsini
2021-06-07  7:46   ` Peng Tao
2021-06-07 15:32   ` Enrico Weigelt, metux IT consult
2021-06-08  2:58     ` Peng Tao
2021-06-08 10:49       ` Enrico Weigelt, metux IT consult
2021-06-08 12:41         ` Peng Tao
2021-06-09 12:54           ` Enrico Weigelt, metux IT consult
2021-06-11 12:46             ` Peng Tao
2021-06-15 18:50               ` Enrico Weigelt, metux IT consult
2021-06-16 10:20                 ` Peng Tao
2021-06-16 16:09                   ` Enrico Weigelt, metux IT consult
2021-06-17 13:23                     ` Peng Tao
2021-06-21 19:05                       ` Enrico Weigelt, metux IT consult [this message]
2021-06-22  6:46                         ` Peng Tao
2021-06-24 14:19                           ` Enrico Weigelt, metux IT consult

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=65fc0313-b01b-9882-d716-d5a2911222b7@metux.net \
    --to=lkml@metux.net \
    --cc=amir73il@gmail.com \
    --cc=balsini@android.com \
    --cc=bergwolf@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=tao.peng@linux.alibaba.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).