[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTikHyYZYfX7sg=edsC6uykM8gje+JsBUxpbYZ1q1@mail.gmail.com>
Date: Mon, 29 Nov 2010 19:51:26 +0300
From: Alexey Zaytsev <alexey.zaytsev@...il.com>
To: Eric Paris <eparis@...hat.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@...hos.com>,
Scott Hassan <hassan@...funk.com>, Jan Kara <jack@...e.cz>,
"agruen@...bit.com" <agruen@...bit.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"stefan@...ttcher.org" <stefan@...ttcher.org>,
Al Viro <viro@...iv.linux.org.uk>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 4/4] fanotify: Expose the file changes to the user
On Mon, Nov 29, 2010 at 19:14, Eric Paris <eparis@...hat.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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists