On Tue, Dec 12 2017, Amir Goldstein wrote: > On Mon, Dec 11, 2017 at 11:52 PM, NeilBrown <neilb@xxxxxxxx> wrote: >> On Mon, Dec 11 2017, Amir Goldstein wrote: >> >>> On Mon, Dec 11, 2017 at 8:04 AM, NeilBrown <neilb@xxxxxxxx> wrote: >>>> If a filesystem does not set sb->s_export_op, then it >>>> does not support filehandles and export_fs_encode_fh() >>>> and exportfs_encode_inode_fh() should not be called. >>>> They will use export_encode_fh() is which is a default >>>> that uses inode number generation number, but in general >>>> they may not be stable. >>>> >>>> So change exportfs_encode_inode_fh() to return FILEID_INVALID >>>> if called on an unsupported Filesystem. Currently only >>>> notify/fdinfo can do that. >>>> >>> >>> I wish you would leave this check to the caller, maybe add a helper >>> exportfs_can_decode_fh() for callers to use. >>> >>> Although there are no current uses for it in-tree, there is value in >>> being able to encode a unique file handle even when it cannot be >>> decoded back to an open file. >>> >>> I am using this property in my fanotify super block watch patches, >>> where the object identifier on the event is an encoded file handle >>> of the object, which delegates tracking filesystem objects to >>> userspace and prevents fanotify from keeping elevated refcounts >>> on inodes and dentries. >>> >>> There are quite a few userspace tools out there that are checking >>> that st_ino hasn't changed on a file between non atomic operations. >>> Those tools (or others) could benefit from a unique file handle if >>> we ever decide to provide a relaxed version of name_to_handle_at(). >> >> If the filesystem doesn't define ->s_export_op, then you really cannot >> trust anything beyond the inode number (and maybe not even that), and >> the inode number is already easily available. >> What actual value do you think you get from this pretend-file-handle >> on filesystems that don't support file handles? >> > > Sorry, I misread your patch. In my mind I thought you wanted to > eliminate the default export_encode_fh if there was no fh_to_dentry > operation like do_sys_name_to_handle() does. Just in my head... I see... I would have said that fh_to_dentry was mandatory if s_export_op was set, and Documentation/filesystems/nfs/Exporting agrees with me. But I do see that sys_name_to_handle() and nfsd explicitly test for it as well as for s_export_op. It appears that cifs sets s_export_op, but doesn't provide fh_to_dentry, and it is unique in this. But the CIFS_NFSD_EXPORT config option is marked as BROKEN, so that probably doesn't matter. So for all current filesystems, filehandles are only reported if they are usable for dentry lookup... > > FWIW, according to Pavel, if fdinfo would not export file handle > in case !fh_to_dentry op would probably be desirable, because > criu has no need for file handles that cannot be decoded. Yes, it was good to have that confirmed - thanks for getting that checked. NeilBrown
Attachment:
signature.asc
Description: PGP signature
