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

[Jan Kara <jack@suse.cz>]

On Fri 04-01-19 13:42:33, Amir Goldstein wrote:
> On Fri, Jan 4, 2019 at 12:57 PM Jan Kara <jack@suse.cz> wrote:
> > The reporting of FAN_ONDIR when the event was mkdir / rmdir could be useful
> > I guess. E.g. when implementing recursive watching of a directory. Or what
> > is your intended usecase? It should be said explicitely in the changelog.
> 
> Recursive watch of directory tree is certainly a use case that could benefit
> from "mkdir" events. I will add that to commit message.

Hum, so it does not seem like you had any particular usecase in mind? :)
Before complicating the interface with reporting FAN_ONDIR in some cases
I'd prefer we considered whether the usecases are worth it. So let me start
that: Reporting FAN_ONDIR can distinguish between file/directory
creation/deletion. That can save userspace some rescans of the changed
directory if it is interested only in subdirectory creation / deletion (if
the application is interested only in file changes it just does not set
FAN_ONDIR and that's it).

Another motivation is the compatibility with inotify and it's IN_ISDIR flag
I guess.

Hum, OK, I guess the complication is worth it.

> > > diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> > > index e9d45387089f..f5f86566c277 100644
> > > --- a/include/linux/fanotify.h
> > > +++ b/include/linux/fanotify.h
> > > @@ -61,13 +61,16 @@
> > >  #define FANOTIFY_PERM_EVENTS (FAN_OPEN_PERM | FAN_ACCESS_PERM | \
> > >                                FAN_OPEN_EXEC_PERM)
> > >
> > > +/* Events types that may be reported from vfs */
> > > +#define FANOTIFY_EVENT_TYPES (FANOTIFY_EVENTS | \
> > > +                              FANOTIFY_PERM_EVENTS)
> > > +
> > >  /* Extra flags that may be reported with event or control handling of events */
> > >  #define FANOTIFY_EVENT_FLAGS (FAN_EVENT_ON_CHILD | FAN_ONDIR)
> > >
> > >  /* Events that may be reported to user */
> > > -#define FANOTIFY_OUTGOING_EVENTS     (FANOTIFY_EVENTS | \
> > > -                                      FANOTIFY_PERM_EVENTS | \
> > > -                                      FAN_Q_OVERFLOW)
> > > +#define FANOTIFY_OUTGOING_EVENTS     (FANOTIFY_EVENT_TYPES | \
> > > +                                      FAN_Q_OVERFLOW | FAN_ONDIR)
> > >
> > >  #define ALL_FANOTIFY_EVENT_BITS              (FANOTIFY_OUTGOING_EVENTS | \
> > >                                        FANOTIFY_EVENT_FLAGS)
> >
> > I don't like this renaming. FAN_ONDIR essentially becomes the same type of
> > thing as FAN_EVENT_ON_CHILD - i.e., an event flag. So I'd just leave these
> > defines as is...
> >
> 
> Sorry. I don't understand what you mean.
> FAN_EVENT_ON_CHILD is not in FANOTIFY_OUTGOING_EVENTS
> FAN_ONDIR is in FANOTIFY_OUTGOING_EVENTS after this change.
> copy_event_to_user() masks out with FANOTIFY_OUTGOING_EVENTS.
> Do you not like the new group definition FANOTIFY_EVENT_TYPES?

Sorry, I've got confused and thought that FAN_EVENT_ON_CHILD gets reported
to userspace. I don't like the FANOTIFY_EVENT_TYPES name and
FANOTIFY_OUTGOING_EVENTS becomes somewhat a misnomer after adding FAN_ONDIR
there. So how about renaming FANOTIFY_OUTGOING_EVENTS to
FANOTIFY_OUTGOING_MASK and have FANOTIFY_OUTGOING_EVENTS what your
FANOTIFY_EVENT_TYPES is?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR