s_export_op On Thu, Apr 27, 2023 at 2:48 PM Jan Kara <jack@xxxxxxx> wrote: > > On Tue 25-04-23 16:01:05, Amir Goldstein wrote: > > fanotify users do not always need to decode the file handles reported > > with FAN_REPORT_FID. > > > > Relax the restriction that filesystem needs to support NFS export and > > allow reporting file handles from filesystems that only support ecoding > > unique file handles. > > > > For such filesystems, users will have to use the AT_HANDLE_FID of > > name_to_handle_at(2) if they want to compare the object in path to the > > object fid reported in an event. > > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > ... > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > > index 8f430bfad487..a5af84cbb30d 100644 > > --- a/fs/notify/fanotify/fanotify_user.c > > +++ b/fs/notify/fanotify/fanotify_user.c > > @@ -1586,11 +1586,9 @@ static int fanotify_test_fid(struct dentry *dentry) > > * We need to make sure that the file system supports at least > > * encoding a file handle so user can use name_to_handle_at() to > > * compare fid returned with event to the file handle of watched > > - * objects. However, name_to_handle_at() requires that the > > - * filesystem also supports decoding file handles. > > + * objects, but it does not need to support decoding file handles. > > */ > > - if (!dentry->d_sb->s_export_op || > > - !dentry->d_sb->s_export_op->fh_to_dentry) > > + if (!dentry->d_sb->s_export_op) > > return -EOPNOTSUPP; > > So AFAICS the only thing you require is that s_export_op is set to > *something* as exportfs_encode_inode_fh() can deal with NULL ->encode_fh > just fine without any filesystem cooperation. What is the reasoning behind > the dentry->d_sb->s_export_op check? Is there an implicit expectation that > if s_export_op is set to something, the filesystem has sensible > i_generation? Or is it just a caution that you don't want the functionality > to be enabled for unexpected filesystems? A little bit of both. Essentially, I do not want to use the generic encoding unless the filesystem opted-in to say "This is how objects should be identified". The current fs that have s_export_op && !s_export_op->encode_fh practically make that statement because they support NFS export (i.e. they implement fh_to_dentry()). I don't like the implicit fallback to generic encoding, especially when introducing this new functionality of encode_fid(). Before posting this patch set I had two earlier revisions. One that changed the encode_fh() to mandatory and converted all the INO32_GEN fs to explicitly set s_export_op.encode_fh = generic_encode_ino32_fh, And one that marked all the INO32_GEN fs with s_export_op.flags = EXPORT_OP_ENCODE_INO32_GEN in both cases there was no blind fallback to INO32_GEN. But in the end, these added noise without actual value so I dropped them, because the d_sb->s_export_op check is anyway a pretty strong indication for opt-in to export fids. CC exportfs maintainers in case they have an opinion one way or the other. > In either case it would be good > to capture the reasoning either in a comment or the changelog... > Will do. Thanks, Amir.
