On Fri, Jul 16, 2021 at 12:47 PM Jan Kara <jack@xxxxxxx> wrote:
>
> On Fri 16-07-21 09:53:15, Matthew Bobrowski wrote:
> > On Wed, Jul 14, 2021 at 03:09:56PM +0300, Amir Goldstein wrote:
> > > I am still debating with myself between adding a new event type
> > > (FAN_RENAME), adding a new report flag (FAN_REPORT_TARGET_FID)
> > > that adds info records to existing MOVE_ events or some combination.
> >
> > Well, if we went with adding a new event FAN_RENAME and specifying that
> > resulted in the generation of additional
> > FAN_EVENT_INFO_TYPE_DFID_NAME_{FROM,TO} information record types for an
> > event, wouldn't it be weird as it doesn't follow the conventional mechanism
> > of a listener asking for additional information records? As in,
> > traditionally we'd intialize the notification group with a flag and then
> > that flag controls whether or not one is permitted to receive events of a
> > particular type that may or may not include information records?
> >
> > Maybe a combination approach is needed in this instance, but this doesn't
> > necessarily simplify things when attempting to document the API semantics
> > IMO.
>
> So there are couple of ways how to approach this I guess. One is that we
> add a flag like FAN_REPORT_SECONDARY_DFID and FAN_REPORT_SECONDARY_NAME
> which would add another DFID(+NAME) record of new type to rename event. In
> principle these could be added to MOVED_FROM and/or MOVED_TO events
> (probably both are useful). But I'd find the naming somewhat confusing and
> difficult to sensibly describe.
>
> That's why I think it may be clearer to go with new FAN_RENAME event that
> will be triggered when the directory is on either end of rename(2) (source
> or target). If DFID(+NAME) is enabled for the group, the event would report
> both source and target DFIDs (and names if enabled) instead of one. I don't
> think special FAN_REPORT_? flag to enable the second DFID would be useful
> in this case (either for clarity or enabling some functionality).
>
I agree that FAN_RENAME without any new REPORT flag is possible.
Still, I would like to at least try to come up with some UAPI that is more
compatible with existing semantics and simlifies them rather than creating
more special cases.
For example, if FAN_REPORT_ALL_FIDS would start reporting all the
relevant fid records for all events, then nothing would change for
FAN_OPEN etc, but FAN_CREATE etc would start reporting the
target fid and FAN_MOVED_* would start reporting source, target and self
fids or dfid/name records and then FAN_MOVED_* pair can be "merged"
because they carry the exact same information.
(I suppose FAN_MOVE_SELF could be "merged" with them as well)
When I write "merged" I mean queued as a single recorded event
depending on backend flags, like we do with event ON_CHILD and
event on self for inotify vs. fanotify.
With this scheme, listeners that only set FAN_MOVED_FROM in
mask would get all relevant information and users that only set
FAN_MOVED_TO would get all relevant information and we avoid
the spam of different event formats for the same event in case
users set FAN_MOVED|FAN_RENAME|FAN_MOVE_SELF in the mask.
That just leaves the question of HOW to describe the info records
in a consistent way.
I was thinking about:
#define FAN_EVENT_INFO_OF_SELF 1
#define FAN_EVENT_INFO_OF_SOURCE 2
#define FAN_EVENT_INFO_OF_TARGET 3
struct fanotify_event_info_header {
__u8 info_type; /* The type of object of the info record */
__u8 info_of; /* The subject of the info record */
__u16 len;
};
The existing info_type values determine HOW that info can be used.
For example, FAN_EVENT_INFO_TYPE_DFID can always be resolved
to a linked path if directory is not dead and reachable, while
FAN_EVENT_INFO_TYPE_FID is similar, but it could resolve to an
unknown path even if the inode is linked.
Most events will only report records OF_SELF or OF_TARGET or both.
FAN_MOVED_* will also report records OF_SOURCE.
One way to think about it is that we are encoding all the information
found in man page about what the info record is describing, depending
on the event type and making it available in the event itself, so application
does not need to code the semantics of the man page w.r.t info records
meaning for different event types.
w.r.t backward compatibility of event parsers, naturally, the REPORT_ALL
flag protects us from regressions, but also adapting to the new scheme is
pretty easy. In particular, if application ignored the info_of field the only
event where things could go wrong is FAN_MOVED_TO because the
first info record is not what it used to be (assuming that we report the
info of source dirent first).
Thoughts?
Amir.
