On Tue 02-10-18 13:37:13, Amir Goldstein wrote: > On Tue, Oct 2, 2018 at 12:24 PM Jan Kara <jack@xxxxxxx> wrote: > > > > On Mon 01-10-18 17:01:23, Amir Goldstein wrote: > [...] > > > IMO we need to report FAN_OPEN for every open like we do now. > > > and additionally report FAN_OPEN_EXEC for open for exec. > > > > Fully agreed. > > > > > Then user can implement FAN_OPEN_NOEXEC by setting ignore mask > > > with FAN_OPEN_EXEC. > > > > Good point. > > > > > You can make the analogy to the compound event FAN_CLOSE. > > > If user sets a mask to the compound event FAN_CLOSE and sets ignore > > > mask with FAN_CLOSE_WRITE, then user effectively implemented > > > FAN_CLOSE_NOWRITE with similar semantics to implementation of > > > FAN_OPEN_NOEXEC. > > > > > > Similarly, if user requests FAN_OPEN|FAN_CLOSE and then checks > > > (event->mask & FAN_OPEN) or (event->mask & FAN_CLOSE) it has > > > similar meaning. Testing (event->mask & FAN_OPEN_EXEC) and > > > (event->mask & FAN_CLOSE_WRITE) in this case is similarly informative. > > > > > > Honestly, I can't think of an application interested only in > > > FAN_CLOSE_NOWRITE nor only in FAN_OPEN_NOEXEC, but the > > > functionality is available for both. The fact that user *can* implement the > > > former without ignore mask and cannot implement to latter without > > > ignore mask is IMO the neccesary evil we need to carry for historic API > > > decisions. > > > > So it seems we are in full agreement that we don't want "optional event > > flags" as Matthew implemented it currently and rather want new event type > > FAN_OPEN_EXEC? > > > > First, let me try to better define the semantic difference between > "optional event flag" vs. "new event type" (which may be clear to you). > The former may be reported to user even if user did not set the flag > in any mark mask. > The latter may be reported to user only if user set the event type in > a mark mask. > > I am in fact in leaning to the former (as Mathew implemented it), because > I am looking at inotify and my effort to add the "dentry" events to fanotify. > First, my proposal suggests to report the optional event flag FAN_ONDIR, > just like inotify does. Well, we already do deliver FAN_ONDIR event flag if the event was on directory AFAIK. Just with fanotify you also have to explicitely ask for events on directories to be delivered by setting FAN_ONDIR in the mark's mask. > Second, fsnotify_change() reports a combination of event types on a single > event mask (i.e. FS_MODIFY|FS_ATTRIB), which then may cause users > to see an event type they didn't ask for reported. > We could change fsnotify_change() to report 3 separate events for > FS_MODIFY,FS_ATTRIB,FS_ACCESS, but really, I don't see the point. OK. > How badly can a program be written that it opts into EXEC/ONDIR events > in fanotify_init() and doesn't request them in fanotify_mark() and it flips > when those "optional" flags are reported? > Assuming we also properly document that behavior. Yeah, so I'm not so concerned about an applicating getting surprised by additional event being set when it in fact explicitely asked for it. I'm more concerned about the "ease to understand the interface and use it correctly". I.e., the logic of interface design. And in this area, just defining new FAN_OPEN_EXEC event like any other seems to win? No need for special fanotify_init() flags and explanations in the manpage. > BTW, as far as I understand the current man page, I did not find any explicit > statement that says that you CANNOT get an event if you did not ask for it. > FWIW, inotify and fanotify man pages are quite similar, so it may infer that > fanotify inherits the same expectations as one had from inotify. > > Having said all that, I'd like to clarify that I do not object to "new > event type", > I understand why you find it "cleaner". > I just find it less "efficient", because it adds extra calls to > fsnotify() for what > IMO is not a good enough reason. I'm not sure I understand your concern here. Are you concerned that fsnotify_open() would need to do one call for FS_OPEN event and one call for FS_OPEN_EXEC so that we won't "leak" FS_OPEN_EXEC event if user didn't ask for it? If that's your concern, what if we just masked out all "unwanted" events in fanotify_handle_event()? fanotify_should_send_event() does all the masking anyway so it's not like we'd loose any performance with that and with current set of fanotify events it would be completely transparent. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
