linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: "Aneesh Kumar K. V" <aneesh.kumar@linux.vnet.ibm.com>
Cc: Andreas Dilger <andreas.dilger@oracle.com>,
	Neil Brown <neilb@suse.de>, Christoph Hellwig <hch@infradead.org>,
	Jonathan Corbet <corbet@lwn.net>, Serge Hallyn <serue@us.ibm.com>,
	linux-fsdevel@vger.kernel.org, Steven French <sfrench@us.ibm.com>,
	Philippe Deniel <philippe.deniel@CEA.FR>,
	"linux-kernel@vger.kernel.org Mailinglist" 
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH -V7 3/9] vfs: Add name to file handle conversion support
Date: Sat, 15 May 2010 07:00:23 +0100	[thread overview]
Message-ID: <20100515060023.GC31073@ZenIV.linux.org.uk> (raw)
In-Reply-To: <874oi9enoe.fsf@linux.vnet.ibm.com>

On Sat, May 15, 2010 at 11:01:13AM +0530, Aneesh Kumar K. V wrote:

> We need to have a file system identifier to identify a file system with
> respect to which file_id need to be decoded.

Obviously.

> We can either do it in
> userspace via statfs or any other encoding or let the kernel fill the
> field for userspace. Now having kernel providing both file_id and file
> system id implies we can get an 'unique' handle with one syscall. (Not
> unique if we have multiple file system with same UUID. Userspace
> file server can find out whether all the mount point it is exporting
> have a unique UUID during startup.) 

Or it can decide which UUID to use by parsing its analog of /etc/exports, or
any number of other schemes.  Sure.

> Now if we have UUID as a part of file handle, adding the check make
> sure that we are passing the right file handle with the correct mountdirfd.
> If they don't match return -ESTALE. 

Pardon?  Your variant needs to choose mountdirfd somehow and it has to be
done by UUID found in fhandle it has received.  Whatever userland code will
be doing that, you
	a) need to open() these suckers at some point
	b) do _NOT_ want to reopen them on each incoming fhandle, for obvious
efficiency reasons
	c) will have very few opens compared to the amount of fhandles you
are going to handle (number of filesystems vs. number of requested accesses
to them)
	d) can be sure that identity of fs hosting an opened directory will
_not_ change while it's open, so the checks concerning the validity of
UUID -> mountdirfd mapping you need to do should be done only once per
opening mountdirfd

So why the devil would you repeat them on each open-by-fhandle in a variant
that gets mountdirfd from caller?
 
> Adding UUID as a part of file handle also help us to get a 16 byte UUID
> of the file system in userspace in a simple syscall interface, rather
> than using file system specific libraries like libext2fs.

Your choice of fsid encoding is orthogonal to my question; again, the question
is "why would you want to have open-by-fhandle do anything with fsid, if it
gets filesystem identified by opened file descriptor passed by userland anyway?"

I don't think that UUID is always a good choice, BTW, but that's a separate
issue.  For one thing, not every fs _has_ UUID, so while that might be a
reasonable default, it doesn't make sense to hardwire it into the design.
For another, you might want a different policy (e.g. you might want to
allow explicitly configured "this fsid value corresponds to whatever's mounted
on that path" for some userland server), so again it doesn't make any sense
to hardwire that one.

But that's a completely independent issue; whether you use hardwire UUID into
the thing or not, rechecking that yes, mountdirfd is on the same fs as it
used to back when we'd opened it, is pointless.  It *will* be on the same fs,
so checking that we'd got the right filesystem should be done once per
opening mountdirfd, not once per opening file by fhandle.

  reply	other threads:[~2010-05-15  6:00 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-12 15:50 [PATCH -V7 0/8] Generic name to handle and open by handle syscalls Aneesh Kumar K.V
2010-05-12 15:50 ` [PATCH -V7 1/9] exportfs: Return the minimum required handle size Aneesh Kumar K.V
2010-05-12 15:50 ` [PATCH -V7 2/9] vfs: Add uuid based vfsmount lookup Aneesh Kumar K.V
2010-05-12 15:50 ` [PATCH -V7 3/9] vfs: Add name to file handle conversion support Aneesh Kumar K.V
2010-05-12 21:49   ` Andreas Dilger
2010-05-12 22:43     ` Neil Brown
2010-05-13  6:17       ` Aneesh Kumar K. V
2010-05-13  7:11         ` Neil Brown
2010-05-13  8:30           ` Andreas Dilger
2010-05-13  8:47             ` Neil Brown
2010-05-13 14:21             ` Aneesh Kumar K. V
2010-05-13 18:17               ` Aneesh Kumar K. V
2010-05-13 22:54                 ` Andreas Dilger
2010-05-14 17:25                   ` Al Viro
2010-05-14 18:18                     ` Aneesh Kumar K. V
2010-05-14 18:40                       ` Al Viro
2010-05-15  5:31                         ` Aneesh Kumar K. V
2010-05-15  6:00                           ` Al Viro [this message]
2010-05-15 15:28                             ` Aneesh Kumar K. V
2010-05-13  0:20     ` Dave Chinner
2010-05-13  6:23       ` Aneesh Kumar K. V
2010-05-13  7:31         ` Dave Chinner
2010-05-13  5:56     ` Aneesh Kumar K. V
2010-05-13 14:24     ` Aneesh Kumar K. V
2010-05-12 15:50 ` [PATCH -V7 4/9] vfs: Add open by file handle support Aneesh Kumar K.V
2010-05-12 23:44   ` Neil Brown
2010-05-13  6:09     ` Dave Chinner
2010-05-13  6:37       ` Aneesh Kumar K. V
2010-05-14 10:41         ` Dave Chinner
2010-05-12 15:50 ` [PATCH -V7 5/9] vfs: Add freadlink syscall Aneesh Kumar K.V
2010-05-13  1:43   ` Neil Brown
2010-05-13  6:25     ` Aneesh Kumar K. V
2010-05-13  6:56       ` Neil Brown
2010-05-13  7:34         ` Aneesh Kumar K. V
2010-05-13  8:09           ` Neil Brown
2010-05-14 11:18         ` Aneesh Kumar K. V
2010-05-12 15:50 ` [PATCH -V7 6/9] ext4: Add get_fsid callback Aneesh Kumar K.V
2010-05-13  3:11   ` Dave Chinner
2010-05-13  6:32     ` Aneesh Kumar K. V
2010-05-14  1:44       ` Dave Chinner
2010-05-15  6:09         ` Aneesh Kumar K. V
2010-05-14 17:32   ` Coly Li
2010-05-14 18:21     ` Aneesh Kumar K. V
2010-05-14 19:08       ` Coly Li
2010-05-12 15:50 ` [PATCH -V7 7/9] x86: Add new syscalls for x86_32 Aneesh Kumar K.V
2010-05-12 15:50 ` [PATCH -V7 8/9] x86: Add new syscalls for x86_64 Aneesh Kumar K.V
2010-05-12 15:50 ` [PATCH -V7 9/9] ext3: Add get_fsid callback Aneesh Kumar K.V

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=20100515060023.GC31073@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=andreas.dilger@oracle.com \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=corbet@lwn.net \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=philippe.deniel@CEA.FR \
    --cc=serue@us.ibm.com \
    --cc=sfrench@us.ibm.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).