Bug#759001: [systemd-devel] sysv-generator: doesn't handle /etc/insserv/overrides or /etc/chkconfig.d

Christian Seiler christian at iwakd.de
Tue Feb 17 12:04:56 GMT 2015


Am 2015-02-17 12:31, schrieb Michael Biebl:
> Am 17.02.2015 um 01:08 schrieb Christian Seiler:
>
>> Only minor issue is that SourcePath= is a single-valued entry in
>> systemd, so I can't just add the override if it exists, and
>> FragmentPath= is only ever dynamically filled by systemd when 
>> parsing,
>> so I can't set that either. It's mostly cosmetic, but systemctl 
>> status
>> for example won't show that the file was overwritten.
>>
>> (Alternatively, one could ONLY use /etc/insserv/overrides/$NAME as
>> SourcePath=, of course, I don't have a strong opinion on either one 
>> of
>> these solutions.)
>
> Hm, right. It would be nice to show that information in "systemctl
> status". Doesn't look like we could use DropInPaths= either.

Thinking about it, perhaps we could use Documentation=? Just add a
Documentation=file:///etc/insserv/overrides/$NAME? Then at least
systemctl status will show the information. Not perfect, but
probably better than nothing...

> Somehow I think we should make clear though, that this is a 
> transitional
> measure and parsing of insserv overrides is not going to be supported
> forever.

Then perhaps write out a INFO priority log message (the lowest one
that's still higher than DEBUG), saying that the override was used
but is deprecated? And add a logcheck-ignore rule for this message,
since this is something the administrator should see when looking
at the logs, but not be reminded of every time systemctl
daemon-reload is executed.

Thoughts?

>>> We are pretty late into the freeze though, so this would
>>> require an ack from our release managers.
>>
>> Do you want me to ask for pre-approval?
>
> Yes, that would be appreciated.
> Please ask them, if such a patch would be accepted
> given the current limitations of unsetting orderings/dependencies via
> drop-ins.

Ok, will do, once we've cleared up the above.

>> +                        override_fpath = 
>> strjoin(SYSV_OVERRIDE_PATH, de->d_name, NULL);
>> +                        if (!override_fpath)
>> +                                return log_oom();
>> +
>> +                        if (stat(override_fpath, &st) < 0) {
>> +                                free(override_fpath);
>> +                                override_fpath = NULL;
>> +                        }
>
>
>> -                        name = fpath = NULL;
>> +                        name = fpath = override_fpath = NULL;
>
> With _cleanup_free_, I think explicitly freeing override_fpath is not
> necessary.

It is, since we want to make sure that override_fpath is unset.
If we set it directly to NULL, we leak memory. Admittedly, the
whole generator doesn't properly clean up after itself, it never
frees the list of units, but the code I wrote is correct in the
way I wrote it.

Christian




More information about the Pkg-systemd-maintainers mailing list