[arch-projects] [INITSCRIPTS][PATCH] Load sysctl config files from sysctl.d

[Seblu]

On Wed, Aug 31, 2011 at 12:19 PM, Dave Reisner <d at 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".

>> +# 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.

About nullglob i'm not sure it's the good way. I would you experience
about this, because it can cause cross damage.

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

Regards,

-- 
Sébastien Luttringer
www.seblu.net