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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.