Re: [PATCH 0/7] Report more information in fanotify dirent events

[Jan Kara <jack@suse.cz>]

On Tue 16-11-21 08:59:29, Amir Goldstein wrote:
> > > I like it. However,
> > > If FAN_RENAME can have any combination of old,new,old+new info
> > > we cannot get any with a single new into type
> > > FAN_EVENT_INFO_TYPE_DFID_NAME2
> > >
> > > (as in this posting)
> >
> > We could define only DFID2 and DFID_NAME2 but I agree it would be somewhat
> > weird to have DFID_NAME2 in an event and not DFID_NAME.
> >
> > > We can go with:
> > > #define FAN_EVENT_INFO_TYPE_OLD_DFID_NAME   6
> > > #define FAN_EVENT_INFO_TYPE_NEW_DFID_NAME  7
> > > #define FAN_EVENT_INFO_TYPE_OLD_DFID               8
> > > #define FAN_EVENT_INFO_TYPE_NEW_DFID              9
> > >
> > > Or we can go with:
> > > /* Sub-types common to all three fid info types */
> > > #define FAN_EVENT_INFO_FID_OF_OLD_DIR     1
> > > #define FAN_EVENT_INFO_FID_OF_NEW_DIR    2
> > >
> > > struct fanotify_event_info_header {
> > >        __u8 info_type;
> > >        __u8 sub_type;
> > >        __u16 len;
> > > };
> > >
> > > (as in my wip branch fanotify_fid_of)
> >
> > When we went the way of having different types for FID and DFID, I'd
> > continue with OLD_DFID_NAME, NEW_DFID_NAME, ... and keep the padding byte
> > free for now (just in case there's some extension which would urgently need
> > it).
> >
> > > We could also have FAN_RENAME require FAN_REPORT_NAME
> > > that would limit the number of info types, but I cannot find a good
> > > justification for this requirement.
> >
> > Yeah, I would not force that.
> >
> 
> On second thought and after trying to write a mental man page
> and realizing how ugly it gets, I feel strongly in favor of requiring
> FAN_REPORT_NAME for the FAN_RENAME event.
> 
> My arguments are:
> 1. What is the benefit of FAN_RENAME without names?
>     Is the knowledge that *something* was moved from dir A to dir B
>     that important that it qualifies for the extra man page noise and
>     application developer headache?
> 2. My declared motivation for this patch set was to close the last (?)
>     functional gap between inotify and fanotify, that is, being able to
>     reliably join MOVED_FROM and MOVED_TO events.
>     Requiring FAN_REPORT_NAME still meets that goal.
> 3. In this patch set, FAN_REPORT_NAME is required (for now) for
>     FAN_REPORT_TARGET_FID to reduce implementation and test
>     matrix complexity (you did not object, so I wasn't planning on
>     changing this requirement).
>     The same argument holds for FAN_RENAME
> 
> So let's say this - we can add support for OLD_DFID, NEW_DFID types
> later if we think they may serve a purpose, but at this time, I see no
> reason to complicate the UAPI anymore than it already is and I would
> rather implement only:
> 
> /* Info types for FAN_RENAME */
> #define FAN_EVENT_INFO_TYPE_OLD_DFID_NAME       10
> /* Reserved for FAN_EVENT_INFO_TYPE_OLD_DFID    11 */
> #define FAN_EVENT_INFO_TYPE_NEW_DFID_NAME       12
> /* Reserved for FAN_EVENT_INFO_TYPE_NEW_DFID    13 */
> 
> Do you agree?

I agree the utility of FAN_RENAME without FAN_REPORT_NAME is very limited
so I'm OK with not implementing that at least for now.

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