Re: [PATCH v4 15/15] fanotify: report FAN_ONDIR to listener with FAN_REPORT_FID

[Amir Goldstein <amir73il@gmail.com>]

On Sat, Jan 5, 2019 at 11:49 AM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> On Sat, Jan 05, 2019 at 09:59:42AM +0200, Amir Goldstein wrote:
> > ...
> > > > > > +     /*
> > > > > > +      * Unlike legacy fanotify events (open/access/close), dirent events
> > > > > > +      * for subdir entries (mkdir/rmdir) will be reported regardless if
> > > > > > +      * user requested FAN_ONDIR, but the FAN_ONDIR flag itself will only
> > > > > > +      * be reported if the user asked for it.
> > > > > > +      */
> > > > > >       if (event_mask & FS_ISDIR &&
> > > > > > +         !(event_mask & ALL_FSNOTIFY_DIRENT_EVENTS) &&
> > > > >
> > > > > I disagree with this. It just seems inconsistent for dirent events for
> > > > > directories to get reported without FAN_ONDIR. I understand there's not
> > > > > great use for not reporting directory dirent events but it's not like
> > > > > adding FAN_ONDIR to the mark mask is that big deal for userspace. And it
> > > > > makes the API more consistent. You could possibly remind the reader in the
> > > > > manpage that FAN_ONDIR is required to get all dirent events.
> > > >
> > > > I see your point.
> > > > I have no problem with requiring FAN_ONDIR for mkdir events.
> > > > I believe the strongest argument should be which way is easier
> > > > to document/understand.
> > > >
> > > > Matthew, if you agree that it looks easier to document Jan's proposal,
> > > > please go a head with this and we will see how man page looks like
> > > > before making the final decision.
> > >
> > > To be fair, for the sake of clarity and consistency with the existing API I do
> > > believe it would make it easier for the API consumer to comprehend what Jan has
> > > suggested. Simple, in order to receive any events of type dirent, one must
> > > supply FAN_ONDIR as part of their mark mask.
> > >
> >
> > But that was not the suggestion.
> >
> > The debate is whether or not user needs to specify (for example)
> >  FAN_ONDIR|FAN_CREATE in order to get mkdir events.
>
> And I'm agreeing with the fact that I think this ^ i.e. FAN_ONDIR | FAN_CREATE
> is the way to go moving forward. However, there is still a small part of me
> that thinks doing it this way seems a little weird and solely supplying
> FAN_CREATE for example should be sufficient in order to get these type of
> dirent events. I don't know why, but for whatever reason I have a feeling of
> uncertainty about this.
>

I think that is sufficient consensus for requiring FAN_ONDIR to
get mkdir/rmdir events, so this is what I will do.
As Jan wrote, it's quite easy to document this weirdness and ease of
documentation is the important thing.

> > The three of us understanding FAN_ONDIR intuitively different is what makes
> > me unease.
> >
> > The purpose of my alternative suggestion was to dis-disambiguate which inode
> > each flag refers to.
> >
> > It should be clear that FAN_DIRENT_ISDIR does not refer to the modified
> > directry but to the created/deleted/renamed subdir.
> > We will avoid making a change of behavior making FAN_ONDIR an out flag.
>
> Yeah, so maybe using FAN_DIRENT_ISDIR is indeed the solution. I don't really
> have any objections with it at this particular point. Let's see whether Jan has
> looked at it from a different perspective and can share his opinion.
>

Nah. It's going to complicated rather than clarify IMO.
Just need to document that FAN_ONDIR is reported only with FAN_REPORT_FID.
If that is too weird, we can propose a new explicit init flag FAN_REPORT_ONDIR.
Do you guys thing that is necessary?

Thanks,
Amir.