From: Matthew Bobrowski <mbobrowski@mbobrowski.org>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
linux-api@vger.kernel.org
Subject: Re: [PATCH v4 15/15] fanotify: report FAN_ONDIR to listener with FAN_REPORT_FID
Date: Sat, 5 Jan 2019 20:49:20 +1100 [thread overview]
Message-ID: <20190105094918.GD25434@lithium.mbobrowski.org> (raw)
In-Reply-To: <CAOQ4uxjnh2+phE92r6Xs6psJ29acomg6uo8oYji0VdNhhhrr5g@mail.gmail.com>
On Sat, Jan 05, 2019 at 09:59:42AM +0200, Amir Goldstein wrote:
> ...
> > > > > + /*
> > > > > + * Unlike legacy fanotify events (open/access/close), dirent events
> > > > > + * for subdir entries (mkdir/rmdir) will be reported regardless if
> > > > > + * user requested FAN_ONDIR, but the FAN_ONDIR flag itself will only
> > > > > + * be reported if the user asked for it.
> > > > > + */
> > > > > if (event_mask & FS_ISDIR &&
> > > > > + !(event_mask & ALL_FSNOTIFY_DIRENT_EVENTS) &&
> > > >
> > > > I disagree with this. It just seems inconsistent for dirent events for
> > > > directories to get reported without FAN_ONDIR. I understand there's not
> > > > great use for not reporting directory dirent events but it's not like
> > > > adding FAN_ONDIR to the mark mask is that big deal for userspace. And it
> > > > makes the API more consistent. You could possibly remind the reader in the
> > > > manpage that FAN_ONDIR is required to get all dirent events.
> > >
> > > I see your point.
> > > I have no problem with requiring FAN_ONDIR for mkdir events.
> > > I believe the strongest argument should be which way is easier
> > > to document/understand.
> > >
> > > Matthew, if you agree that it looks easier to document Jan's proposal,
> > > please go a head with this and we will see how man page looks like
> > > before making the final decision.
> >
> > To be fair, for the sake of clarity and consistency with the existing API I do
> > believe it would make it easier for the API consumer to comprehend what Jan has
> > suggested. Simple, in order to receive any events of type dirent, one must
> > supply FAN_ONDIR as part of their mark mask.
> >
>
> But that was not the suggestion.
>
> The debate is whether or not user needs to specify (for example)
> FAN_ONDIR|FAN_CREATE in order to get mkdir events.
And I'm agreeing with the fact that I think this ^ i.e. FAN_ONDIR | FAN_CREATE
is the way to go moving forward. However, there is still a small part of me
that thinks doing it this way seems a little weird and solely supplying
FAN_CREATE for example should be sufficient in order to get these type of
dirent events. I don't know why, but for whatever reason I have a feeling of
uncertainty about this.
> The three of us understanding FAN_ONDIR intuitively different is what makes
> me unease.
>
> The purpose of my alternative suggestion was to dis-disambiguate which inode
> each flag refers to.
>
> It should be clear that FAN_DIRENT_ISDIR does not refer to the modified
> directry but to the created/deleted/renamed subdir.
> We will avoid making a change of behavior making FAN_ONDIR an out flag.
Yeah, so maybe using FAN_DIRENT_ISDIR is indeed the solution. I don't really
have any objections with it at this particular point. Let's see whether Jan has
looked at it from a different perspective and can share his opinion.
--
Matthew Bobrowski
next prev parent reply other threads:[~2019-01-05 9:49 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-02 11:38 [PATCH v4 00/15] fanotify: add support for more event types Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 01/15] fsnotify: annotate directory entry modification events Amir Goldstein
2019-01-03 15:41 ` Jan Kara
2019-01-03 16:31 ` Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 02/15] fsnotify: send all event types to super block marks Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 03/15] fsnotify: move mask out of struct fsnotify_event Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 04/15] fanotify: rename struct fanotify_{,perm_}event_info Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 05/15] fanotify: open code fill_event_metadata() Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 06/15] fanotify: encode file identifier for FAN_REPORT_FID Amir Goldstein
2019-01-03 16:18 ` Jan Kara
2018-12-02 11:38 ` [PATCH v4 07/15] fanotify: copy event fid info to user Amir Goldstein
2019-01-03 17:13 ` Jan Kara
2019-01-04 3:58 ` Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 08/15] fanotify: enable FAN_REPORT_FID init flag Amir Goldstein
2018-12-08 9:26 ` Amir Goldstein
2018-12-10 16:20 ` Jan Kara
2018-12-11 6:12 ` Matthew Bobrowski
2018-12-11 6:58 ` Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 09/15] fanotify: cache fsid in fsnotify_mark_connector Amir Goldstein
2019-01-04 9:38 ` Jan Kara
2019-01-04 9:54 ` Jan Kara
2018-12-02 11:38 ` [PATCH v4 10/15] vfs: add vfs_get_fsid() helper Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 11/15] fanotify: use vfs_get_fsid() helper instead of vfs_statfs() Amir Goldstein
2019-01-04 9:55 ` Jan Kara
2018-12-02 11:38 ` [PATCH v4 12/15] fanotify: check FS_ISDIR flag instead of d_is_dir() Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 13/15] fanotify: support events with data type FSNOTIFY_EVENT_INODE Amir Goldstein
2019-01-04 10:17 ` Jan Kara
2019-01-04 11:12 ` Amir Goldstein
2018-12-02 11:38 ` [PATCH v4 14/15] fanotify: add support for create/attrib/move/delete events Amir Goldstein
2019-01-04 10:26 ` Jan Kara
2018-12-02 11:38 ` [PATCH v4 15/15] fanotify: report FAN_ONDIR to listener with FAN_REPORT_FID Amir Goldstein
2019-01-04 10:57 ` Jan Kara
2019-01-04 11:42 ` Amir Goldstein
2019-01-04 12:18 ` Jan Kara
2019-01-04 12:39 ` Amir Goldstein
2019-01-05 0:34 ` Matthew Bobrowski
2019-01-05 8:18 ` Amir Goldstein
2019-01-07 7:40 ` Amir Goldstein
2019-01-04 12:19 ` Jan Kara
2019-01-04 23:46 ` Matthew Bobrowski
2019-01-05 7:59 ` Amir Goldstein
2019-01-05 9:49 ` Matthew Bobrowski [this message]
2019-01-07 7:37 ` Amir Goldstein
2019-01-04 11:00 ` [PATCH v4 00/15] fanotify: add support for more event types Jan Kara
2019-01-07 7:46 ` Amir Goldstein
2019-01-09 14:02 ` Jan Kara
2019-01-09 15:34 ` Amir Goldstein
2019-01-10 7:49 ` Amir Goldstein
2019-01-10 9:22 ` Jan Kara
2019-01-10 9:50 ` Amir Goldstein
2019-01-10 11:43 ` Jan Kara
2019-01-10 11:55 ` Amir Goldstein
2019-01-10 8:53 ` Jan Kara
2019-01-10 10:10 ` Amir Goldstein
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190105094918.GD25434@lithium.mbobrowski.org \
--to=mbobrowski@mbobrowski.org \
--cc=amir73il@gmail.com \
--cc=jack@suse.cz \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).