Hi Amir! On Sat 06-11-21 18:29:39, Amir Goldstein wrote: > On Fri, Oct 29, 2021 at 2:40 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > This patch set follows up on the discussion on FAN_REPORT_TARGET_FID [1] > > from 3 months ago. > > > > With FAN_REPORT_PIDFD in 5.15 and FAN_FS_ERROR on its way to 5.16, > > I figured we could get an early (re)start of the discussion on > > FAN_REPORT_TARGET_FID towards 5.17. > > > > The added information in dirent events solves problems for my use case - > > It helps getting the following information in a race free manner: > > 1. fid of a created directory on mkdir > > 2. from/to path information on rename of non-dir > > > > I realize those are two different API traits, but they are close enough > > so I preferred not to clutter the REPORT flags space any further than it > > already is. The single added flag FAN_REPORT_TARGET_FID adds: > > 1. child fid info to CREATE/DELETE/MOVED_* events > > 2. new parent+name info to MOVED_FROM event > > > > Instead of going the "inotify way" and trying to join the MOVED_FROM/ > > MOVED_TO events using a cookie, I chose to incorporate the new > > parent+name intomation only in the MOVED_FROM event. > > I made this choice for several reasons: > > 1. Availability of the moved dentry in the hook and event data > > 2. First info record is the old parent+name, like FAN_REPORT_DFID_NAME > > 3. Unlike, MOVED_TO, MOVED_FROM was useless for applications that use > > DFID_NAME info to statat(2) the object as we suggested > > > > I chose to reduce testing complexity and require all other FID > > flags with FAN_REPORT_TARGET_FID and there is a convenience > > macro FAN_REPORT_ALL_FIDS that application can use. > > Self comment - Don't use ALL_ for macro names in uapi... > There are 3 comment of "Deprecated ..." for ALL flags in fanotify.h alone... Yeah, probably the ALL_FIDS is not worth the possible confusion when we add another FID flag later ;) > 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. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
