Re: [PATCH 0/7] Report more information in fanotify dirent events
flat view
From: Amir Goldstein <amir73il@gmail.com>
Date: 2021-11-16 06:59:45
Also in:
linux-fsdevel
quoted
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.quoted
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).quoted
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.