supporting functions missing from inotify patch

supporting functions missing from inotify patch

[Steve French]

It looks like a couple of exports and a key supporting function are
missing from the inotify patch that went into mainline yesterday.

I don't see an inode operation for registering inotify events in the fs
(there is a file operation for dir_notify to register its events).  In
create_watch in fs/inotify.c I expected to see something like:

if(inode->inotify_create)
	ret = inode->inotify_create(inode, mask);

(a similar change would be needed in the INOTIFY_IGNORE path)

Without this, inotify will be broken except on local filesystems.
Cluster and network filesystems (nfs, cifs etc.) won't know when they
need to start calling back on remote directory change notifications if
they aren't called at the time of the ioctl(INOTIFY_WATCH)

I also don't see exports for 
	fsnotify_access
	fsnotify_modify

Without these exports, network and cluster filesystems can't notify the
local system about changes.

Tracking local changes to files/directories is somewhat useful by itself
on non-networked systems.  But if directory and file change notification
for remote/cluster filesystems is broken that limits its usefullness a
lot.

Re: supporting functions missing from inotify patch

[Robert Love]

On Wed, 2005-07-13 at 13:43 -0500, Steve French wrote:

> I don't see an inode operation for registering inotify events in the fs
> (there is a file operation for dir_notify to register its events).  In
> create_watch in fs/inotify.c I expected to see something like:

Why not use the existing dir_notify method?  No point in adding another.

Add inotify hooks as needed to your filesystem's dir_notify.

> I also don't see exports for 
> 	fsnotify_access
> 	fsnotify_modify
> 
> Without these exports, network and cluster filesystems can't notify the
> local system about changes.

Eh?

They are in <linux/fsnotify.h>.

	Robert Love

Re: supporting functions missing from inotify patch

[Steve French]

>> I don't see an inode operation for registering inotify events in the fs
>> (there is a file operation for dir_notify to register its events).  In
>> create_watch in fs/inotify.c I expected to see something like:
>Why not use the existing dir_notify method?  No point in adding another.

I did not think that inotify_add_watch called dir_notify.  I don't see a path in which 
calls to add a new inotify watch end up in a call to fcntl_dirnotify or file->dir_notify 
This is for the case in which an app only calls inotify ioctl - ie does not [also] do a call 
to dnotify. 

Without such a call - an app that does your new ioctl to add a watch on a file or directory will
not cause the network/cluster fs to turn on notification on the server since the watch
will be not seen by the client filesystem.

>> I also don't see exports for 
>> 	fsnotify_access
>> 	fsnotify_modify
>> 
>> Without these exports, network and cluster filesystems can't notify the
>> local system about changes.
>>
>Eh?
>
>They are in <linux/fsnotify.h>.

OK - you exported a common underlying function
	inotify_inode_queue_event
under the inline functions which the network/cluster fs would call to notify of remote changes.
That makes sense. I had missed that.

Re: supporting functions missing from inotify patch

[Robert Love]

On Wed, 2005-07-13 at 15:21 -0500, Steve French wrote:

> I did not think that inotify_add_watch called dir_notify.  I don't see a path in which 
> calls to add a new inotify watch end up in a call to fcntl_dirnotify or file->dir_notify 
> This is for the case in which an app only calls inotify ioctl - ie does not [also] do a call 
> to dnotify. 

No, you are right, they do not, right now.

Is dir_notify suitable for inotify and your uses?  In the 10 months of
inotify development, I had hoped that a remote filesystem developer
would add support so we could test it.  But there is no rush to get this
hook added, so its okay.

The problem with dir_notify is that the args parameter is dnotify flags.
Those don't map directly to inotify flags.

What I'd like is

	(a) a patch adding the requisite inotify hook (really, 4 lines)
	(b) a filesystem successfully using the hook

> Without such a call - an app that does your new ioctl to add a watch on a file or directory will
> not cause the network/cluster fs to turn on notification on the server since the watch
> will be not seen by the client filesystem.

It is a system call, now.  ;-)

> OK - you exported a common underlying function
> 	inotify_inode_queue_event
> under the inline functions which the network/cluster fs would call to notify of remote changes.
> That makes sense. I had missed that.

Nod.

	Robert Love

Re: supporting functions missing from inotify patch

[Steve French]

> Is dir_notify suitable for inotify and your uses?
The six dir_notify flags obviously map better to the network protocol
which cifs can request (and which Samba server needs to respond to
various network filesystem clients) but the 11 IN_ flags do not seem
that different.

>The problem with dir_notify is that the args parameter is 
> dnotify flags. Those don't map directly to inotify flags.

They almost map (11 down to 6) -  Not sure how 
	IN_MOVE_FROM and IN_MOVE_TO
map to DN_RENAME or what DNOTIFY event delete self triggerred (inotify
splits IN_DELETE_SELF out).

The mapping of the local calls to the network filesystem wire protocol
also will be interesting.

The corresponding network filesystem protocol defines a few events that
don't make sense to Linux (changing dos attributes for a file e.g.) but
I don't think that that matters much.  More important is whether it can
handle the IN_ events 

[from fs/cifs/cifspdu.h]
/* Completion Filter flags for Notify */
#define FILE_NOTIFY_CHANGE_FILE_NAME    0x00000001
#define FILE_NOTIFY_CHANGE_DIR_NAME     0x00000002
#define FILE_NOTIFY_CHANGE_NAME         0x00000003
#define FILE_NOTIFY_CHANGE_ATTRIBUTES   0x00000004
#define FILE_NOTIFY_CHANGE_SIZE         0x00000008
#define FILE_NOTIFY_CHANGE_LAST_WRITE   0x00000010
#define FILE_NOTIFY_CHANGE_LAST_ACCESS  0x00000020
#define FILE_NOTIFY_CHANGE_CREATION     0x00000040
#define FILE_NOTIFY_CHANGE_EA           0x00000080
#define FILE_NOTIFY_CHANGE_SECURITY     0x00000100
#define FILE_NOTIFY_CHANGE_STREAM_NAME  0x00000200
#define FILE_NOTIFY_CHANGE_STREAM_SIZE  0x00000400
#define FILE_NOTIFY_CHANGE_STREAM_WRITE 0x00000800

Which can cause the server to report back "actions" in the notification
events sent to the client:
#define FILE_ACTION_ADDED               0x00000001
#define FILE_ACTION_REMOVED             0x00000002
#define FILE_ACTION_MODIFIED            0x00000003
#define FILE_ACTION_RENAMED_OLD_NAME    0x00000004
#define FILE_ACTION_RENAMED_NEW_NAME    0x00000005
#define FILE_ACTION_ADDED_STREAM        0x00000006
#define FILE_ACTION_REMOVED_STREAM      0x00000007
#define FILE_ACTION_MODIFIED_STREAM     0x00000008

I don't see a way to do mapping of IN_OPEN, IN_CLOSE_WRITE and IN_CLOSE_NOWRITE
to anything that makes sense to the server but the rest may be doable.