On Wed, Aug 31, 2011 at 01:02:11PM +0200, Seblu wrote:
On Wed, Aug 31, 2011 at 12:19 PM, Dave Reisner <d@falconindy.com> wrote:
NOTE: maybe this feature should be added as a daemon for procps package allowing user to reload configs files by calling rc.d reload procps
What's the "off" version of this setting:
vm.swappiness=0
I have no idea how you intend to "turn off" sysctl setting as part of a "restart", nor can I figure out why this might be needed. I doesn't want turn off a feature. Just reload the config files. Before we do this by calling sysctl -p. Now we have more than one file to load and sysctl -p doesn't make the jobs. So an helper which reload all files is handy. Debian have this "feature".
Seems like a fix for something that's awkwardly broken elsewhere. If this is split off to its own script, a user can "reload" this at their leisure.
+# Load sysctl variables (new style) +# http://0pointer.de/public/systemd-man/sysctl.d.html +declare -a sysctl_d=( + /usr/lib/sysctl.d + /etc/sysctl.d + /run/sysctl.d +)
You enabled nullglob, so if you declare this array as globs of *.conf files, none of the directory checking is necessary and we get a singular loop over an array of files. It'd be nice to actually check that what we're adding is a regular file, though, as opposed to something insane like a fifo sneaking in. ok i will add this check. About all inside sysctl_d, we need (as spec require it), to order file in this directory order an overload them if there are the same name. If we do like this it will be difficult to reorder correctly after.
I'm aware of the spec. Globs expand in alphapbetical order (sorted according to the C locale) but the directory order will be preserved. With nullglob enabled, we get the directory existance check for free. This is exactly what we do in arch-tmpfiles and it works quite well.
About nullglob i'm not sure it's the good way. I would you experience about this, because it can cause cross damage.
Sorry, I have no idea what this means.
We've already uniq'd the array, so this should't require multiple invocations:
(( ${#fragments[*]} )) && cat "${fragments[@]}" | sysctl -q -p - the first load all valid files and doesn't stop at first error and show a pretty message warning: /etc/sysctl.d/toto toto.conf(1): invalid syntax, continuing...
I don't really follow what you're trying to say here either. $ sysctl -p - this.isnt.a.real.key=3 error: "this.isnt.a.real.key" is an unknown key vm.swappiness=0 error: permission denied on key 'vm.swappiness' bogus warning: -(3): invalid syntax, continuing... We only get a useful warning (tying it back to a file) on bad syntax and never for permission denied (unlikely) or invalid keys. This is garbage, really. I, personally, don't think this is worth preserving in any way so we may as well avoid the needless forking. If we wanted to take the time to get proper error messages, we'd need to traverse /proc/sys ourselves (or patch procps).
I should remove to have a correct ouput on error &>/dev/null
+done # Start daemons for daemon in "${DAEMONS[@]}"; do case ${daemon:0:1} in diff --git a/tmpfiles.conf b/tmpfiles.conf index 7dd1358..61081d0 100644 --- a/tmpfiles.conf +++ b/tmpfiles.conf @@ -4,6 +4,7 @@
D /tmp 1777 root root D /run/daemons 0755 root root +D /run/sysctl.d 0755 root root
By the time this runs, it's too late to care about creating this directory. Further, if /run/sysctl.d does exist and someone dropped a .conf in it, you just wiped it out, as D will clean out an existing directory.
ok
General: There's no need to make your variable names so cryptic. ok
I'm also not sure why this needs to be embedded into /etc/rc.multi as opposed to living in its own script or a function (where you could namespace properly). I agree with this. This is no race condition on this loading which need it before running the daemons.
I think the better way is to add a daemon in propcs package with directories. Because this package own sysctl and sysctl.conf. Why scatter more than necessary? Find a good place to create /run/sysctl.d Add in default rc.conf, procps as first lauched daemon (procps is in the base group)
It's about not adding random scripts to other packages, even as dead as procps is upstream. We didn't add arch-tmpfiles to the filesystem package. And please, no more "daemons" who don't actually have long running processes. d P.S. I'm still not really sold on the need for this.