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

[Amir Goldstein <amir73il@gmail.com>]

> > 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?

Thanks,
Amir.