Version: (using KDE 4.1.3) OS: Linux Installed from: Fedora RPMs KDirWatch uses inotify by default if it is available, but inotify does not detect changes to files on NFS mounts unless those changes originate from the system where the inotify watch is running. It is possible to override the watch method in kdeglobals with [DirWatch] preferredMethod=Stat Which will detect changes on NFS filesystems, but means that Stat is used for all watches, even on local filesystems, which is much less efficient than iNotify, and takes slightly longer to detect changes. In the attached patch I have added a new config key "nfsPreferredMethod" to the [DirWatch} section which allows the runtime configuration of a different watch method for network filesystems to the default one used for local filesystems. If no "nfsPreferredMethod" is specified then "preferredMethod" (or the first available builtin method) is used for NFS filesystems. Additionally if nfsPreferredMethod is the same as preferredMethod the mount-point check is skipped. I had also considered whether the default method order for NFS filesystems should be changed to move FAM and/or Stat ahead of iNotify so that NFS change detection works out of the box, but I'm not sure whether that's such a good idea given that iNotify does work on NFS for the single user case.
Created attachment 29374 [details] Add nfsPreferredMethod config key to KDirwatch
Nice patch, thanks. Are you sure about this change, though? - if (useINotify(e)) return; + if (useINotify(e)); return; While I'm asking about that, can I also suggest a helper function (file-static) for converting a string to a WatchMethod enum, in order to reduce the duplication between the two "read from config" bits of code? Thanks.
Created attachment 29389 [details] Add nfsPreferredMethod config key to KDirwatch
Yes, your are right that iNotify change shouldn't be there, I've fixed it in the new version that I just attached. I've also added a static method methodFromString for the common bit of the config mapping, my C++ is a little rusty, but just declaring the function static limits it to the file scope right? is that what you had in mind?
Yep, the static function looks good (should take a const QString& though). I almost committed this, but I realized I have one more question :-) I noticed that the default is "same method for nfs as elsewhere", which doesn't really fix the bug (it still requires users to read this code and add a key to a config file), right? If NFS mounts don't work with inotify, shouldn't there be logic like if (slowmounted) { if (nfspreferredmethod was set in config file) use that else if (m_preferredMethod == inotify) use stat else use m_preferredMethod } ?
Changing the default order for NFS was something I was considered, but the situation is a bit more complex: iNotify will detect changes on NFS when they originate from the local system - which makes it a reasonable choice if the NFS filesystem is only expected to be modified from a single client in general, for example when user home directories are NFS mounted. FAM (the original one), supports a client-server mode where it can run a watch daemon on the server and propagate events to the client, but gamin which Fedora (and I believe most current Linux distos) now use instead of FAM doesn't support this feature. I'm not sure other platforms though. Stat is the only method that is guaranteed to detect remote changes on NFS, but it has a fairly high overhead, can take up to 5 seconds (by default) to detect changes and from reading some gamin bug reports (gamin polls on NFS by default too) can end up placing a significant load on NFS servers with a large number of clients. I guess the question is which is the best trade off for an NFS default, and I'm not sure of the answer, which is why I took the approach of making it configurable, in my particular case I have less than 20 users who need to see changes on shared NFS mounts, and don't have a FAM server, so Stat is the best choice for me.
Good point about NFS-mounted homes, a default value of inotify makes sense indeed. We just need to document this setting on userbase.kde.org, then.
SVN commit 898321 by dfaure: Add nfsPreferredMethod config key to KDirwatch, patch by Travers Carter. This allows to configure a different method for watching NFS-mounted directories, but the default is to simply use the same method as normally, so this patch is a safe no-op by default. BUG: 177892 M +48 -40 kdirwatch.cpp M +1 -1 kdirwatch_p.h WebSVN link: http://websvn.kde.org/?view=rev&revision=898321
Committed, I'll let you add something to the userbase wiki so that others can figure this out :-)