[Nepomuk] Review Request 109078: Add a base class for the file watcher back-ends

[Simeon Bird]

> On Feb. 21, 2013, 3:32 p.m., Simeon Bird wrote:
> > So, the patch looks fine, but, while not wanting to be negative, I want to ask if this project is really the right direction?
> > KDirWatch uses either inotify or stat internally, so I don't see how it would be better than using inotify?
> > 
> > fanotify looks quite useful, but we would still need to watch everything with inotify for moves, no?
> > Also I found on the kernel list that fanotify has some fairly large bugs:
> > http://article.gmane.org/gmane.linux.kernel/1441629/match=fanotify
> > http://thread.gmane.org/gmane.linux.kernel/1224800/focus=1439656
> > 
> > Particularly the fact that the first has not been found before suggests to me that not many people are using it, 
> > and that we will hit many more bugs. Of course, bugs can be fixed, but it suggests we should move carefully...
> > 
> > I'm not saying don't do it, I'm just wondering if the wiki is really the right approach, before a lot of code gets written.
> > Sorry if this is undue pooh-pooh.
> > 
> > Simeon
> 
> Vishesh Handa wrote:
>     Hey Simeon
>     
>     I was thinking more along the lines of allowing the users to use any combination of the following - inotify, kdirnotify( a dbus based api ) and fanotify.
>     
>     There are two big problems with inotify -
>     * Non-recusrive watches
>     * Having to watch both the source and destination in order to receive file move events.
>     
>     KDirNotify provides file deletion and move events, but only for applications that use KIO for doing so.
>     Fanotify allows recursive file watches, but does not provide deletion or move events.
>     
>     If one wanted, one could use either - inotify + KDirNotify and avoid having to watch everything. Or maybe fanotify + KDirNotify and then one wouldn't even have to install so many watches.
>     
>     I though it's high time to provide the users with an option to change backends if they want to. Maybe the default could be inotify + KDirWatch? Or we could just stick to the inotify only backend as possible.
>     
>     What do you think?

Hey Vishesh, 

So, I've been thinking about this a bit today. One thing that occurred to me is this: 

inotify has these two limitations, right? And every now and then someone asks the kernel people to fix them, and the discussion goes like this:

Us: We need recursive watches and the ability to avoid watching both the source and destination in order to receive file move events!
Them: No! Recursive watches are a security hole! Denial of service! memory exhaustion! etc etc
<the action becomes general>

But I've never seen *anyone* address the issue of why exactly one needs to watch both the source and destination to receive the file move events, which is actually the *bigger* problem for us. So I wonder if it might be worth contacting the kernel people again with just the *second* request, to see if we can get some motion? Or at least a rationale? I feel people see the first one, go into denial mode and don't read further, and I also think that if we had this, a lot of other problems would be solvable.

My worry with KDirWatch is that users basically have to promise not to move files except with kio, right? So if it were the default, I think we would get lots of complaints from /usr/bin/mv users, no? Or did I misunderstand?

So this leaves fanotify:

- I didn't know that fanotify doesn't catch deletions as well as moves. Does it catch the directory change due to the file unlink? Can we emulate move/deletion detection using this?

- Presumably before something is moved, the directory is opened? Is the idea with kDirWatch is to catch the directory open and stick a watch on the opened directory to catch moves/deletions? If we didn't have to watch both the source and destination directory with inotify, we could do fanotify+(inotify on open directories for moves/deletions) and it would just work perfectly.

- When Martin Steigerwald contacted the kernel people, it was mentioned that fanotify requires root access and can only watch entire mount points, not directories. So one would need a root daemon...

- Apart from this, I strongly suspect that right now fanotify probably does not really work. But, the only way the bugs are going to be found and fixed is if someone uses it, and I guess nepomuk is that someone, so on reflection I agree we should just go with it and see if we can break stuff.

Perhaps a good way to proceed would be to try to get as far as we can with an fanotify+KDirWatch backend? Root daemon notwithstanding, it seems a sensible way of doing this, if we could only make it work. Why not keep the default as the current inotify for 4.11, but have fanotify+kDirWatch as an "experimental" backend, and see how it goes?

What do you think? I don't want to put anyone off - all the problems I've mentioned seem solvable, with a little bit of effort. And I totally agree that having a better file watcher is well worth that.

Simeon


- Simeon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109078/#review27847
-----------------------------------------------------------


On Feb. 21, 2013, 2:05 p.m., Gabriel Poesia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109078/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2013, 2:05 p.m.)
> 
> 
> Review request for Nepomuk and Vishesh Handa.
> 
> 
> Description
> -------
> 
> This is about this task: http://community.kde.org/Projects/Nepomuk/4.11#File_Watcher
> 
> This abstract FileSystemMonitor class is basically the generic interface extracted from KInotify.
> It also has a method to ignore paths based on a RegExpCache filter list, just like IgnoringKInotify.
> 
> 
> Diffs
> -----
> 
>   services/filewatch/filesystemmonitor.h PRE-CREATION 
>   services/filewatch/filesystemmonitor.cpp PRE-CREATION 
>   services/filewatch/test/CMakeLists.txt 8b6ebe6 
>   services/filewatch/test/filesystemmonitortest.h PRE-CREATION 
>   services/filewatch/test/filesystemmonitortest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109078/diff/
> 
> 
> Testing
> -------
> 
> The empty test that includes it compiles.
> 
> 
> Thanks,
> 
> Gabriel Poesia
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20130222/b192710e/attachment.html>