lkml.org 
[lkml]   [2010]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 4/4] fanotify: Expose the file changes to the user
Date
On Monday 29 Nov 2010 16:14:54 Eric Paris wrote:
> On Fri, 2010-11-26 at 10:11 +0000, Tvrtko Ursulin wrote:
> > On Monday 22 Nov 2010 00:37:21 Alexey Zaytsev wrote:
> > > struct fanotify_event_metadata {
> > >
> > > - __u16 event_len;
> > > + __u16 event_len; /* Including the options */
> > >
> > > __u8 vers;
> > >
> > > - __u8 reserved;
> > > + __u8 options_offset; /* Aka header length */
> > >
> > > __s32 fd;
> > > __aligned_u64 mask;
> > > __s32 pid;
> > >
> > > + /* Options go here. */
> > >
> > > };
> >
> > Also, options_offset is, if I understood it correctly, basically the
> > lenght of fanotify_event_metadata. As such, isn't that field redundant
> > since the lenght is implied from the protocol version?
>
> Ok, the way I envision the interface is the fanotify_event_metadata is
> going to be information that is common for all event types. If a
> particular event type (in the case being brought forth MODIFY and
> CLOSE_WRITE) needs more information it will use the optional headers.
> I'd still like to be able to add generic info, ex: the uid of the
> requesting process, to the generic event metadata. We might also
> someday want to use the optional headers for things like the inotify
> move cookie. Why waste space in every event when only the move type
> events cared about this cookie?
>
> This necessitates options_offset. Lets say I compile my userspace app
> as version 3. I know that fanotify_event_metadata is 14 bytes long in
> version 3. Thus I start looking at byte 15 for the first optional
> header. But the kernel is version 4 and fanotify_event_metadata is
> actually 18 bytes long. Whoops, userspace should be looking at byte 19
> for the header but accidentally looked at byte 15. options_offset will
> allow a version 3 userspace to run on a version 4 kernel while skipping
> bytes 15-18 which it doesn't understand.

I see now what you want to do here. I assumed metadata is fixed for ever and
all expansions will come via options. That is why I said offset field is
redundant. If you want to leave option for changing metadata open then yes, I
agree it is needed.

> At the end of all of this discussion what did people finally agree on?
> Something like this?
>
> struct fanotify_opt_hdr {
> __u16 type;
> __u16 len;
> /* Payload goes here. */
> };
>
> struct fanotify_event_metadata {
> __u32 event_len; /* Including the options */
> __u16 vers;
> __u16 options_offset; /* Aka header length */
> __s32 fd;
> __aligned_u64 mask;
> __s32 pid;
> /* Options go here. */
> };
>
> We could be ABI compatible if we let vers as a __u32 and added
> options_offset at the end of the structure... Do we have strong
> feelings on either side about maintaining compatibility or about
> structure lengths?

No strong feelings here on either. I do not really think maintaining
compatibility with an unreleased protocol is worth it. And I would not like to
limit any field to 8-bits if possible.

Tvrtko

Sophos Limited, The Pentagon, Abingdon Science Park, Abingdon, OX14 3YP, United Kingdom.
Company Reg No 2096520. VAT Reg No GB 991 2418 08.


\
 
 \ /
  Last update: 2010-11-29 18:15    [W:0.222 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site