lkml.org 
[lkml]   [2010]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 4/4] fanotify: Expose the file changes to the user
    From
    On Mon, Nov 29, 2010 at 19:14, Eric Paris <eparis@redhat.com> 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.
    >
    > 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. */
    > };
    >

    Yep, except mask should go after options_offset to avoid the gap, and
    let's make both vers and options_offset u8, just in case we need the
    other 2 bytes in the future:

    struct fanotify_event_metadata {
    __u32 event_len; /* Including the options */
    __u8 vers;
    __u8 options_offset; /* Aka header length */
    __u16 reserved;
    __aligned_u64 mask;
    __s32 fd;
    __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 about3
    > structure lengths?

    Actually, with event_len being __u32, and mask moved back, we are
    already abi-compatible. The users would see the wrong version, but
    it's always > 2, so everyone happy. Together with the hypothetical
    huge options, this fully justifies spending 2 bytes on event_len to
    me.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2010-11-29 17:59    [W:0.025 / U:28.108 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site