[Nepomuk] Review Request 106748: FileWatch: Use KAuth to automatically raise the inotify watch limit if we run out of watches.

[Vishesh Handa]

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

Ship it!


I think this patch contains some other changes that are not required. You probably didn't generate it properly.

Anyway, nicely done. Please make sure to only commit the relevant parts.


cleaner/cleaningjob.h
<http://git.reviewboard.kde.org/r/106748/#comment23843>

    ?


- Vishesh Handa


On May 4, 2013, 2:48 a.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106748/
> -----------------------------------------------------------
> 
> (Updated May 4, 2013, 2:48 a.m.)
> 
> 
> Review request for Nepomuk, Sebastian Trueg and Vishesh Handa.
> 
> 
> Description
> -------
> 
> FileWatch: Use KAuth to automatically raise the inotify watch limit if we run out of watches.
> 
> When we run out of watches, use a KAuth action to double the inotify
> watch limit (by writing to /proc/sys/fs/inotify/max_user_watches).
> At the same time, make the new setting persist across reboots by writing
> /etc/sysctl.d/97-kde-nepomuk-filewatch-inotify.conf.
> 
> If for some reason raising the limit does not work, print a message to
> syslog.
> 
> While the limit is being raised, no new watches will be added, only
> queued.
> 
> ==Potential issues==
> 
> 1. At the moment there is no way to turn this off, except by not using nepomuk or denying the user the requisite kauth permissions. This is the sort of thing that people complain about, but I can't really see any reason to want to do that - you'd be running nepomuk in a "known broken" configuration, which makes no sense.
> 
> 2. the action description string is "To avoid missing file changes, raise the folder watch limit", which could probably be improved.
> 
> 3. The method of making the change persist across reboots is to write a file to /etc/sysctl.d, which is a bit anti-social. (note that if /etc is not writable, it simply does nothing). /etc/sysctl.d should work on all systemd distros, debian (including derivatives such as ubuntu) and gentoo.
> 
> Part of me wants to make this a separate action, but as I understand it this would require a second prompt and a second authorization, which would be a bit annoying. Also, the user's file system isn't going away - if they wanted a larger limit once, they almost certainly want it again, so there are limited reasons for not wanting it permanent. But finer grained permissions are a Good Thing, so I'm not so sure about this.
> 
> 4. If the user has manually set the watch limit to a too-low number in sysctl.conf, it could potentially over-ride the file in /etc/sysctl.d, leading to the prompt appearing on every boot.
> 
> Also, I'd just like to mention that I was quite impressed at how easy to KAuth was to work with.
> 
> 
> Diffs
> -----
> 
>   cleaner/CMakeLists.txt 87bae29f4dfa8eb9c1d5c0414e4661d7d461b726 
>   cleaner/cleaningjob.h eb0548b06daaf07dc7ba4b2ede6fc0b3bc65acdb 
>   cleaner/nepomukcleaner.desktop 35136a0b50f4419dfc40c3c5bfa65e23887fd368 
>   cleaner/nepomukcleaningjob.desktop 6ebb1c96c5abb09c7d0048768729ab4252cde579 
>   libnepomukcore/service/nepomukservice2.desktop 0361017fdb519b2881d1be8151026bfbab917d88 
>   services/fileindexer/indexer/extractorplugin.h 8e264941bfa40ca6ac7d546740c9d6cea84f77ab 
>   services/fileindexer/indexer/nepomukexiv2extractor.desktop aea0ec8b496c96eb853aa0426cc72a14b4eb505c 
>   services/fileindexer/indexer/nepomukextractor.desktop 8e8da0cc3b08b33bd2962b4dcd92207ac717d688 
>   services/fileindexer/indexer/nepomukffmpegextractor.desktop b7b21a65e30a1df9bd6ab4d849ab609313e5d33c 
>   services/fileindexer/indexer/nepomukplaintextextractor.desktop 9653070e96e85d6a1b4def1f421e1a7d05ea61e0 
>   services/fileindexer/indexer/nepomukpopplerextractor.desktop 25aa375e72bbf9777d4d74728af853571d31f90e 
>   services/fileindexer/indexer/nepomuktaglibextractor.desktop f5ca96b784d73fdebfda11762f0cd35280e90b70 
>   services/fileindexer/indexer/test/CMakeLists.txt 6422c895484a30722e882ecdb65beca917bf46df 
>   services/filewatch/CMakeLists.txt 338fe8c2b008b1c898d71934e4de3028c0078fca 
>   services/filewatch/kinotify.h e795371d922d483bce29e9eea03c1eeb97738355 
>   services/filewatch/kinotify.cpp 94babfe437ddfa8c9318b8b29dd8c8a03a4e71b1 
>   services/filewatch/nepomukfilewatch.h 66e0112d909a2abefed48d0959323e7f32a5ff9b 
>   services/filewatch/nepomukfilewatch.cpp fbbf3db619516e296bc1e4aa07f53808fbe4a4c0 
>   services/filewatch/nepomukfilewatch.desktop a0710e3fd399123b23012faec25a5a687ca9eacb 
>   services/filewatch/org.kde.nepomuk.filewatch.actions PRE-CREATION 
>   services/filewatch/raiselimit.h PRE-CREATION 
>   services/filewatch/raiselimit.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106748/diff/
> 
> 
> Testing
> -------
> 
> Compiled, ran, raised and lowered the limit a few times.
> 
> 
> Thanks,
> 
> Simeon Bird
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20130504/526a12d8/attachment-0001.html>