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@xxxxxxxx>
SUSE Labs, CR
