> > > > > BTW, I did not mention the FAN_RENAME event alternative proposal in this posting
> > > > > not because I object to FAN_RENAME, just because it was simpler to implement
> > > > > the MOVED_FROM alternative, so I thought I'll start with this proposal
> > > > > and see how
> > > > > it goes.
> > > >
> > > > I've read through all the patches and I didn't find anything wrong.
> > > > Thinking about FAN_RENAME proposal - essentially fsnotify_move() would call
> > > > fsnotify_name() once more with FS_RENAME event and we'd gate addition of
> > > > second dir+name info just by FS_RENAME instead of FS_MOVED_FROM &&
> > > > FAN_REPORT_TARGET_FID. Otherwise everything would be the same as in the
> > > > current patch set, wouldn't it? IMHO it looks like a bit cleaner API so I'd
> > > > lean a bit more towards that.
> > >
> > > I grew to like FAN_RENAME better myself as well.
> > > To make sure we are talking about the same thing:
> > > 1. FAN_RENAME always reports 2*(dirfid+name)
> > > 2. FAN_REPORT_TARGET_FID adds optional child fid record to
> > > CREATE/DELETE/RENAME/MOVED_TO/FROM
> > >
>
> Correct, that's what I meant.
>
> > Err, I tried the FAN_RENAME approach and hit a semantic issue:
> > Users can watch a directory inode and get only MOVED_FROM
> > when entries are moved from this directory. Same for MOVED_TO.
> > How would FAN_RENAME behave when setting FAN_RENAME on a directory inode?
> > Should listeners get events on files renamed in and out of that
> > directory?
> >
> > I see several options:
> > 1. Go back to FAN_MOVED_FROM as in this patch set, where semantics are clear
>
> Well, semantics are clear but in principle user does not have access to
> target dir either so the permission problems are the same as with option 2,
> aren't they?
Correct.
>
> > 2. Report FAN_RENAME if either old or new dir is watched (or mount/sb)
> > 3. Report FAN_RENAME only if both old and new dirs are watched (or mount/sb)
> >
> > For option 2, we may need to hide information records, For example,
> > because an unprivileged listener may not have access to old or new
> > directory.
>
> Good spotting. That can indeed be a problem.
>
> > A variant of option 3, is that FAN_RENAME will be an event mask flag
> > that can be added to FAN_MOVE events, to request that if both FROM/TO events
> > are going to be reported, then a single joint event will be reported
> > instead, e.g:
> >
> > #define FAN_MOVE (FAN_MOVED_FROM | FAN_MOVED_TO)
> > #define FAN_RENAME (FAN_MOVE | __FAN_MOVE_JOIN)
> >
> > Instead of generating an extra FS_RENAME event in fsnotify_move(),
> > fsnotify() will search for matching marks on the moved->d_parent->d_inode
> > of MOVED_FROM event add the mark as the FSNOTIFY_OBJ_TYPE_PARENT
> > mark iterator type and then fanotify_group_event_mask() will be able
> > to tell if the
> > event should be reported as FAN_MOVED_FROM, FAN_MOVED_TO or a joint
> > FAN_RENAME.
> >
> > If a group has the FAN_RENAME mask on the new parent dir, then
> > FS_MOVED_TO events can be dropped, because the event was already
> > reported as FAN_MOVED_TO or FAN_RENAME with the FS_MOVED_FROM
> > event.
> >
> > Am I over complicating this?
> > Do you have a better and clearer semantics to propose?
>
> So from API POV I like most keeping FAN_RENAME separate from FAN_MOVED_TO &
> FAN_MOVED_FROM. It would be generated whenever source or target is tagged
> with FAN_RENAME, source info is provided if source is tagged, target info
> is provided when target is tagged (both are provides when both are tagged).
> So it is kind of like FAN_MOVED_FROM | FAN_MOVED_TO but with guaranteed
> merging. This looks like a clean enough and simple to explain API. Sure it
> duplicates FAN_MOVED_FROM & FAN_MOVED_TO a lot but I think the simplicity
> of the API outweights the duplication. Basically FAN_MOVED_FROM &
> FAN_MOVED_TO could be deprecated with this semantics of FAN_RENAME although
> I don't think we want to do it for compatibility reasons.
Well, not only for compatibility.
The ability to request events for files moved into directory ~/inbox/ and files
moved out of directory ~/outbox/ cannot be expressed with FAN_RENAME
alone...
>
> Implementation-wise we have couple of options. Currently the simplest I can
> see is that fsnotify() would iterate marks on both source & target dirs
> (like we already do for inode & parent) when it handles FS_RENAME event. In
Yes. I already have a WIP branch (fan_reanme) using
FSNOTIFY_OBJ_TYPE_PARENT for the target dir mark.
Heads up: I intend to repurpose FS_DN_RENAME, by sending FS_RENAME
to ->handle_inode_event() backends only if (parent_mark == inode_mark).
Duplicating FS_MOVED_FROM I can cope with, but wasting 3 flags for
the same event is too much for me to bare ;-)
> fanotify_handle_event() we will decide which info to report with FAN_RENAME
> event based on which marks in iter_info have FS_RENAME set (luckily mount
> marks are out of question for rename events so it will be relatively
> simple). What do you think?
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 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)
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.
Any preference?
Thanks,
Amir.
