On Thu 27-04-23 15:28:23, Amir Goldstein wrote:

s_export_op

On Thu, Apr 27, 2023 at 2:48 PM Jan Kara [off-list ref] wrote:

quoted

On Tue 25-04-23 16:01:05, Amir Goldstein wrote:

quoted

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@gmail.com>

...

quoted

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()).

Makes sense.

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.

Yeah, I agree with your judgement. I just wanted to make sure I understand
everything properly.

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR