All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	"linux-man@vger.kernel.org" <linux-man@vger.kernel.org>,
	Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Andreas Dilger <adilger@dilger.ca>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: For review: open_by_name_at(2) man page
Date: Tue, 18 Mar 2014 09:00:07 +1100	[thread overview]
Message-ID: <20140318090007.3adee3d0@notabene.brown> (raw)
In-Reply-To: <53271B69.3000305@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4127 bytes --]

On Mon, 17 Mar 2014 16:57:29 +0100 "Michael Kerrisk (man-pages)"
<mtk.manpages@gmail.com> wrote:

> Hi Aneesh, (and others)
> 
> Below is a man page I've written for name_to_handle_at(2) and
> open_by_name_at(2). Would you be willing to review it please,
> and let me know of any corrections/improvements?
> 
> Thanks,
> 
> Michael

Thanks for writing this Michael.  The fact that I can only find very small
points to comment on reflects the high quality...


> Otherwise,
> .IR dirfd
> must be a file descriptor that refers to a directory, and
  ^^^^^^^
> .I pathname
> is interpreted relative to that directory.

As you clarify later, "must be" is not correct.  Maybe this is just an issue
of style, in which case you should obviously keep a consistent style across
man pages, but to me it sounds wrong.  I would use "is generally" or similar.



> The
> .IR mountdirfd
> argument is a file descriptor for a directory under
> the mount point with respect to which
> .IR handle
> should be interpreted.

mountdirfd does not have to be for a directory.  It can be for any object in
the filesystem.  And I would say "in", not "under".
If /foo and /foo/bar are both mountpoints, and I want to look up a
filehandle for the filesystem mounted at /foo, then opening "/foo/bar"
wouldn't work even though /foo/bar is "under" /foo.  And opening "/foo" would
work even though "/foo" is not under "/foo/" (is it?).

  The
  .IR mountfd
  argument is a file descriptor for any object (file, directory, etc.) in the
  filesystem with respect to which
  .IR handle
  should be interpreted.

??



> .B ESTALE
> The specified
> .I handle
> is no longer valid.

ESTALE is also returned if the filesystem does not support file-handle ->
file mappings.
On filesystems which don't provide export_operations (/sys /proc ubifs
romfs cramfs nfs coda ... several others) name_to_handle_at will produce a
generic handle using the 32 bit inode and 32 bit i_generation.
open_by_name_at given this (or any) filehandle will fail with ESTALE.
I don't know how best to include this in the documentation.  Maybe a note
earlier noting that some filesystems do not support open_by_name_at(), and
you cannot programatically determine which do except by trying.
At the same time note that a file handle can become in valid if a file is
deleted or for any other reason as determined by the filesystem, and that the
error is the same as for when the filesystem doesn't support open_by_name_at.


> For example, one can use the device name in the fifth field of the
> .I mountinfo
> record to search for the corresponding device UUID via the symbolic links in
> .IR /dev/disks/by-uuid .
> (A more comfortable way of obtaining the UUID is to use the
> .\" e.g., http://stackoverflow.com/questions/6748429/using-libblkid-to-find-uuid-of-a-partition
> .BR libblkid (3)
> library, which uses the
> .I /sys
> filesystem to obtain the same information.)

Does it?  My understanding from "man libblkid" (it is a while since I've read
the code) is that it either uses info in /dev/disks/by-* or reads directly
from the block devices (maybe using /sys to find them?) and interprets the
superblock to extract a UUID.



> Now delete and re-create the file with the same inode number;
> .BR open_by_handle_at ()
> recognizes that the file referred to by the file handle no longer exists.
> 
> .in +4n
> .nf
> $ \fBstat \-\-printf="%i\\n" cecilia.txt\fP       # Display inode number 
> 4072121
> $ \fBecho 'Warum?' > cecilia.txt\fP
> $ \fBstat \-\-printf="%i\\n" cecilia.txt\fP       # Check inode number
> 4072121
> $ \fBsudo ./t_open_by_handle_at < fh\fP
> open_by_handle_at: Stale NFS file handle

Something is very wrong here.
   echo foo > somefile
does not "delete and re-create" the file.  It opens and truncates.
That operation should not invalidate the filehandle on any sane filesystem.


>     if (argc > 1)
>         mount_fd = open(argv[1], O_RDONLY | O_DIRECTORY);

O_DIRECTORY is not appropriate, as mentioned earlier.


Thanks,
NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

  reply	other threads:[~2014-03-17 22:00 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-17 15:57 For review: open_by_name_at(2) man page Michael Kerrisk (man-pages)
2014-03-17 22:00 ` NeilBrown [this message]
2014-03-18  9:43   ` Christoph Hellwig
2014-03-18 12:37     ` Michael Kerrisk (man-pages)
     [not found]       ` <53283DFB.6040105-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-03-18 22:24         ` NeilBrown
2014-03-18 22:24           ` NeilBrown
2014-03-19  9:09           ` Michael Kerrisk (man-pages)
     [not found]   ` <20140318090007.3adee3d0-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2014-03-18 12:35     ` Michael Kerrisk (man-pages)
2014-03-18 12:35       ` Michael Kerrisk (man-pages)
2014-03-18 13:07       ` Christoph Hellwig
2014-03-18 13:30         ` Michael Kerrisk (man-pages)
2014-03-18  9:37 ` Christoph Hellwig
2014-03-18 12:41   ` Michael Kerrisk (man-pages)
2014-03-18 12:55 ` For review: open_by_name_at(2) man page [v2] Michael Kerrisk (man-pages)
     [not found]   ` <53284233.3050800-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-03-19  4:13     ` NeilBrown
2014-03-19  4:13       ` NeilBrown
     [not found]       ` <20140319151349.33a76023-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2014-03-19  9:09         ` Michael Kerrisk (man-pages)
2014-03-19  9:09           ` Michael Kerrisk (man-pages)
     [not found]           ` <53295ED0.7070304-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-03-19 14:14             ` For review: open_by_handle_at(2) man page [v3] Michael Kerrisk (man-pages)
2014-03-19 14:14               ` Michael Kerrisk (man-pages)
2014-03-19  6:42   ` For review: open_by_name_at(2) man page [v2] Mike Frysinger
2014-03-19 13:11     ` Michael Kerrisk (man-pages)

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=20140318090007.3adee3d0@notabene.brown \
    --to=neilb@suse.de \
    --cc=adilger@dilger.ca \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-man@vger.kernel.org \
    --cc=mtk.manpages@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.