-----------------------------------------------------------
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
> 
>

_______________________________________________
Nepomuk mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/nepomuk

Reply via email to