On Wed, Jan 9, 2019 at 4:02 PM Jan Kara <jack@xxxxxxx> wrote: > > On Mon 07-01-19 09:46:37, Amir Goldstein wrote: > > On Fri, Jan 4, 2019 at 1:00 PM Jan Kara <jack@xxxxxxx> wrote: > > > On Sun 02-12-18 13:38:11, Amir Goldstein wrote: > > > > This is the 4th revision of patch series to add support for filesystem > > > > change monitoring to fanotify. > > > > It incorporates the changes you requested in review of v3 FAN_REPORT_FID > > > > patches. > > > > The complete work is available on fanotify_dirent branch [1] on my tree. > > > > > > > > The combined functionality of FAN_MARK_FILESYSTEM, FAN_REPORT_FID and > > > > dirent modification events is demonstrated with a prototype of global > > > > filesystem monitor based on inotify-tools [2]. > > > > > > > > In your review of v3 patched you only got as far as patch v3 9/13. > > > > Because this patch marks the end of the FAN_REPORT_FID sub series, > > > > I found it best to re-post the entire series with the changes you > > > > requested thus far. For convenience of review, I pushed branches > > > > fanotify_fid-v3 [3] and fanotify_fid-v4 [4] with the work you > > > > reviewed so far and its re-worked version. > > > > > > > > One thing that we discussed and I did NOT do is move struct file_handle > > > > to uapi headers. This got complicated due to existing definitions in > > > > glibc header files and I realized we could do without it. > > > > > > > > I have added the vfs_get_fsid() helper as you requested, but since it > > > > wasn't required by the patch set, I added it as two new cleanup patches > > > > at the end of the FAN_REPORT_FID series, so you will be able to stage > > > > the feature with or without the VFS change. > > > > > > So overall the series looks very good. I've had only some smaller comments > > > / disagreements. So once we settle those please resend the series and I'll > > > pick it up to my tree. > > > > > > > Jan, > > > > I addressed all your comments, rebased on v5.0-rc1 and pushed to: > > https://github.com/amir73il/linux/commits/fanotify_dirent > > > > When I get an ACK from you on the FAN_ONDIR issue, I will post the v5 series. > > The gist is that FAN_ONDIR an input flag is required to produce mkdir/rmdir > > events (as you suggested) and FAN_ONDIR is reported as output flag > > for FAN_REPORT_FID group for all events that occur on directories > > and never reported to non FAN_REPORT_FID group. > > Agreed. But thinking about it a bit more and looking at your patch on > Github I think your solution doesn't quite work in presence of event > merging since an event with FAN_ONDIR can get merged with event without > FAN_ONDIR, can't it? That seems to be generally a problem with > should_merge() for direntry events that it doesn't take ISDIR flag into > account... > Right... good catch! will fix should_merge(). Do you want me to re-post the series or just push to Github? Thanks, Amir.
