[Virtio-fs] [RFC PATCH 0/7] Inotify support in FUSE and virtiofs

[Amir Goldstein]

> > I understand that part. But at the same time, remote fsnotify API will
> > probably evolve as you keep on adding more functionality. What if there
> > is another notification mechanism tomorrow say newfancynotify(), we
> > might have to modify remote fsnoitfy again to accomodate that.
> >
> > IOW, fsnotify seems to be just underlying plumbing and whatever you
> > add today might not be enough to support tomorrow's features. That's
> > why I wanted to start with a minimal set of functionality and add
> > more to it later.
> >
>
> I do want to start with minimal functionality.
> I did not request that you implement more functionality than what inotify
> provides.
>
> TBH, I can't even remember the specific details that made me say
> "this is remote inotify not remote fsnotify", but there were such details.
> I remember inotify rename cookie being one of them.
>

Let me repeat my concern about the rename cookie API.

As Jan has convinced me to create a new event FS_RENAME
for fanotify, there is no harm in FUSE passing the rename cookie
along with FS_MOVE events, but the FUSE out args:

struct fuse_notify_fsnotify_out {
 uint64_t inode;
 uint64_t mask;
 uint32_t namelen;
 uint32_t cookie;
};

How do you expect to extend it when somebody wants to implement
the FS_RENAME event that carries two names and two dir inodes
(and no cookie)?
Will they need to use a new out struct? a union?

Unlike inotify, fanotify can sometimes report the inode of the child
along with inode of the parent in events (e.g. FS_OPEN) when watching
a parent directory.

How will this out args struct be extended if we would want to pass that
information?

There is no need to address this now. I just want to know that the
design you suggest is extendable to future needs and if it is not,
I would prefer to reserve enough fields in the FUSE event
struct for the common needs of all current fsnotify backends,
just as we pass all the relevant info to the fsnotify_XXX() hooks.

> I guess this discussion is not very productive at this point as none of us
> are saying anything very specific about what should and should not
> be done, so let me try to suggest something -
>
> Try to see if you could replace the server side implementation with
> fanotify even if you use CAP_SYS_ADMIN for the experiment.
> fanotify should be almost a drop-in replacement for inotify at this point
> If you think that you cannot make this experiment with your current
> protocol and vfs extensions then you must have done something wrong
> and tied the protocol and/or the vfs API to inotify terminology.
>

So I did this thought experiment myself.
I did not find any obvious issues with implementing the backend as
fanotify. If anything, mapping fhandle to inode is even a bit easier
than mapping wd to inode if you know the fhandle encoding.

For event reporting, besides generalizing the out args, implementation
would be similar. Since FUSE uses nodeid to identify the object in
events, there is no problem for fanotify to report fhandle and inotify
to report wd.

For setting watches, protocol seems generic enough, although
I dislike how fuse_fsnotify_update_mark() masks out inotify
specific flags.

inotify back should mask out its own private flags when calling
into a generic vfs API for registering remote watches.
Also, masking out FS_EVENT_ON_CHILD is pretty weird, because
requesting a watch on children is functionally completely different
than requesting to watch the directory itself.

I think I gave you enough comments already for a new revision.
Are there any open questions left unanswered?

And please grep your kernel patches for "inotify" and "remote inotify"
in particular. When this term is used in comments and commit messages,
it is very likely that either the code or documentation is inaccurate.

Thanks,
Amir.