Re: [PATCH v3 04/13] fanotify: define the structures to report a unique file identifier

[Amir Goldstein <amir73il@gmail.com>]

> > struct fanotify_event_info_fid {
> >         struct fanotify_event_info_header hdr;
> >         ....
> >
> > Seems more appropriate name than the shorter fanotify_event_fid name
> > that you suggested.
>
> Fine by me.

FYI, at the moment the uapi struct looks like this:

struct fanotify_event_info_header {
        __u8 info_type;
        __u8 pad;
        __u16 len;
};

struct fanotify_event_info_fid {
        struct fanotify_event_info_header hdr;
        __kernel_fsid_t fsid;
        struct file_handle fh;
};

But for my global watch prototype [1], I also defined this struct internally:

struct fanotify_event_fid {
       __kernel_fsid_t fsid;
       struct file_handle fh;
};

To be used as key to hash the object in userspace.
This key is often generated by open_by_handle_at() and statfs() from
application and not received from fanotify.

I wonder if it would be useful to include this definition in fanotify uapi
headers and then:

struct fanotify_event_info_fid {
        struct fanotify_event_info_header hdr;
        struct fanotify_event_fid fid;
};

[1] https://github.com/amir73il/inotify-tools/commits/fanotify_fid-v4

>
> > It bothers me a bit not to use struct file_handle in the definition,
> > but I don't with to start moving struct file_handle into uapi.
> > I am a bit lost trying to understand which uapi include file I need
> > to include in fanotify.h if I want to use it. fcntl.h?
>
...

> > struct fanotify_fid {
> >         __kernel_fsid_t fsid;
> >         u8 handle_bytes;
> >         u8 handle_type;
> >         u16 pad;
> >         unsigned char f_handle[0];
> > };
> >
> > Will let you know when I have something ready.

I ended up putting fh_type;fh_len in a 64bit alignment gap in found in
fanotify_event struct and now I use fh_type to determine if the event
info is path (0), fid (>0) or none (FILEID_INVALID).

I pushed the reworked fanotify_dirent [2] branch and I am quite content
with the result.
For comparison, also pushed fanotify_fid-v3 [3] and fanotify_fid-v4 [4],
which correspond to the last patch you posted review on (cached fsid).

[2] https://github.com/amir73il/inotify-tools/commits/fanotify_dirent
[3] https://github.com/amir73il/inotify-tools/commits/fanotify_fid-v3
[4] https://github.com/amir73il/inotify-tools/commits/fanotify_fid-v4

>
> > AFAICT, this rework should not affect the rest of the patches in the
> > series (i.e. cached fsid
> > and dirent events), so if you have time, you don't need to wait on my
> > rework to continue
> > review of v3.
>
> OK, thanks for info. I'm slowly crunching through the patches but it
> takes time and I have also other things to do...
>

I'm well aware of that and thanks for taking the time to crunch this far.
I hope you will like the changes in v4.

The remaining 4 fanotify_dirent patches, did have some rebase conflicts
over fanotify_fid-v4, but the logic remained mostly unchanged.

I will post the entire v4 patch set next week, so you may continue the
review when you have time.

Thanks!
Amir.