On Mon, Jul 12, 2021 at 2:10 PM Jan Kara <jack@xxxxxxx> wrote:
>
> Hi Amir!
>
> On Sun 11-07-21 20:02:29, Amir Goldstein wrote:
> > I am struggling with an attempt to extend the fanotify API and
> > I wanted to ask your opinion before I go too far in the wrong direction.
> >
> > I am working with an application that used to use inotify rename
> > cookies to match MOVED_FROM/MOVED_TO events.
> > The application was converted to use fanotify name events, but
> > the rename cookie functionality was missing, so I am carrying
> > a small patch for FAN_REPORT_COOKIE.
> >
> > I do not want to propose this patch for upstream, because I do
> > not like this API.
> >
> > What I thought was that instead of a "cookie" I would like to
> > use the child fid as a way to pair up move events.
> > This requires that the move events will never be merged and
> > therefore not re-ordered (as is the case with inotify move events).
> >
> > My thinking was to generalize this concept and introduce
> > FAN_REPORT_CHILD_FID flag. With that flag, dirent events
> > will report additional FID records, like events on a non-dir child
> > (but also for dirent events on subdirs).
>
> I'm starting to get lost in what reports what so let me draw a table here:
>
> Non-directories
> DFID FID CHILD_FID
> ACCESS/MODIFY/OPEN/CLOSE/ATTRIB parent self self
> CREATE/DELETE/MOVE - - -
> DELETE_SELF/MOVE_SELF x self self
> ('-' means cannot happen, 'x' means not generated)
>
> Directories
> DFID FID CHILD_FID
> ACCESS/MODIFY/OPEN/CLOSE/ATTRIB self self self
> CREATE/DELETE/MOVE self self target
> DELETE_SELF/MOVE_SELF x self self
>
> Did I get this right?
I am not sure if the columns in your table refer to group flags
or to info records types? or a little bit of both, but I did not
mean for CHILD_FID to be a different record type.
Anyway, the only complexity missing from the table is that
for events reporting a single record with fid of a directory,
(i.e. self event on dir or dirent event) the record type depends
on the group flags.
FAN_REPORT_FID => FAN_EVENT_INFO_TYPE_FID
FAN_REPORT_DIR_FID => FAN_EVENT_INFO_TYPE_DFID
>
> I guess "CHILD_FID" seems somewhat confusing as it isn't immediately clear
> from the name what it would report e.g. for open of a non-directory.
I agree it is a bit confusing. FWIW for events on a non-dir child (not dirent)
FAN_REPORT_FID and FAN_REPORT_CHILD_FID flags yield the exact
same event info.
> Maybe
> we could call it "TARGET_FID"? Also I'm not sure it needs to be exclusive
> with FID. Sure it doesn't make much sense to report both FID and CHILD_FID
> but does the exclusivity buy us anything? I guess I don't have strong
> opinion either way, I'm just curious.
>
FAN_REPORT_TARGET_FID sounds good to me.
You are right. I don't think that exclusivity buys us anything.
> > There are other benefits from FAN_REPORT_CHILD_FID which are
> > not related to matching move event pairs, such as the case described
> > in this discussion [2], where I believe you suggested something along
> > the lines of FAN_REPORT_CHILD_FID.
> >
> > [2] https://lore.kernel.org/linux-fsdevel/CAOQ4uxhEsbfA5+sW4XPnUKgCkXtwoDA-BR3iRO34Nx5c4y7Nug@xxxxxxxxxxxxxx/
>
> Yes, I can see FAN_REPORT_CHILD_FID (or however we call it) can be useful
> at times (in fact I think we made a mistake that we didn't make reported
> FID to always be what you now suggest as CHILD_FID, but we found that out
> only when DFID+NAME implementation settled so that train was long gone).
Yes, we did. FAN_REPORT_TARGET_FID is also about trying to make amends.
We could have just as well called it FAN_REPORT_FID_V2, but no ;-)
> So I have no problem with that functionality as such.
>
Good, so I will try to see that I can come up with sane semantics that
also result in a sane man page.
> > Either FAN_REPORT_CHILD_FID would also prevent dirent events
> > from being merged or we could use another flag for that purpose,
> > but I wasn't able to come up with an idea for a name for this flag :-/
> >
> > I sketched this patch [1] to implement the flag and to document
> > the desired semantics. It's only build tested and I did not even
> > implement the merge rules listed in the commit message.
> >
> > [1] https://github.com/amir73il/linux/commits/fanotify_child_fid
>
> WRT changes to merging: Whenever some application wants to depend on the
> ordering of events I'm starting to get suspicious.
I completely agree with that sentiment.
But note that the application does NOT require event ordering.
I was proposing the strict ordering of MOVE_ events as a method
to allow for matching of MOVE_ pairs of the same target as
a *replacement* for the inotify rename cookie method.
> What is it using these events for?
The application is trying to match MOVE_ event pairs.
It's a best effort situation - when local file has been renamed,
a remote rename can also be attempted while verifying that the
recorded fid of the source (in remote file) matches the fid of the
local target.
> How is renaming different from linking a file into a new dir
> and unlinking it from the previous one which is a series of events that
> could be merged?
It is different because renames are common operations that actual people
often do and link+unlink are less common so we do not care to optimize
them. Anyway, as many other applications, our application does not
support syncing hardlinks to remote location, so link+unlink would be
handled as plain copy+delete and dedup of copied file is handled
is handled by the remote sync protocol.
As a matter of fact, a rename could also be (and sometimes is) handled
as copy+delete. In that case, the remote content would be fine but the
logs and change history would be inaccurate.
BTW, in my sketch commit message I offer to prevent merge of all dirent
events, not only MOVE_, because I claim that there is not much to be
gained from merging the CREATE/DELETE of a specific TARGET_FID
event with other events as there are normally very few of those events.
However, while I can argue why it is useful to avoid merge of dirent events,
it's not as easy for me to come up with a name for that flag not to
easily explain the semantics in man page :-/
so any help with coming up with simplified semantics would be appreciated.
> Also fanotify could still be merging events happening
> after rename to events before rename. Can the application tolerate that?
Yes. The application treats the name of the file as a property that
can be synced regardless of the file's data and metadata and it doesn't
need to be synced to remote in the same order that changes happened.
The destination is "eventually consistent" with the source.
> Inotify didn't do this because it is always merging only to the last event
> in the queue.
>
> When we were talking about FID events in the past (in the context of
> directory events) we always talked about application just maintaining a set
> of dirs (plus names) to look into. And that is safe and sound. But what you
> talk about now seems rather fragile at least from the limited information I
> have about your usecase...
>
It's true. The application uses the DFID as the main key for tracking changes -
i.e. which directories need to be synced.
Rename between directories is a case where syncing individual directories
looses information. It does not loose data, only the accuracy of reported
change history - IOW, its a minor functionality gap, but one that product
people will not be willing to waiver.
P.S. unlike rename of non-dir, rename of directories and large directory
trees specifically must be identified and handled as a remote rename,
but that is easy to achieve with MOVE_SELF, because as I wrote, the
application uses DFID as the key for tracking changes, so MOVE_SELF
of directory will carry a DFID whose "remote path" is stored in a db.
Thanks,
Amir.
