Discussion:
System units packaging and rpmlint
(too old to reply)
Michael Scherer
2013-05-18 21:44:48 UTC
Permalink
Hi,

After the last discussion on fedora-devel about boot times[1], I have
been looking at the various units on my disk and if I could code
something in rpmlint ( tool to verify a rpm against a set of guidelines
) to have packagers follow some best practices.

So I have been writing various checks that emit Warning/Errors, and
would like to get some feedback on a few assumption I made when writing
the checks :

- pid file should be in /run, or in a subdirectory of /run
I have seen a few service who still use /var/run, but can I safely
assume that anything booting with systemd would have /run?
(and so warn if something is using /run for that)

- unit file should be in %_unitdir, which is on /usr/lib for
distribution with merged /usr ( at least, on Fedora and Mageia )

So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?


- we should avoid as much as possible to use Type=forking when we can
avoid it.

This one is likely the one that will be met with resistance from
packagers, so before adding it as warning, I would like to be sure that
I am not totally wrong.

A standard daemon will fork ( likely twice ) in the background, do
various stuff and then write the pidfile on /run.

By not going in the background, we can :
- avoid forking 1 or 2 times for nothing,
- avoid taking memory for /run, and avoid taking a inode

So that's should be a little bit better in most case, or do I miss
anything ?
( ie, something like "this is so negligible that we shouldn't care" )

If there is other good argument to add in the explanation of the error,
that would help to convince the users.


- if using Type=forking, it is better to use PIDFile,
While systemd seems to support fine to guess the main pid, I think this
should be avoided when possible , according to
http://lists.freedesktop.org/archives/systemd-devel/2011-June/002690.html .
So does it make sense to send a warning if there is a service that do
not use PIDFile and of type Forking ?

- some package install directly file in /usr/lib/systemd/system/*.wants
There is some special case ( like plymouth ), but usually, that
shouldn't be done directly in the package, but better done in %post, and
in /etc ?

IE, is this right to make a warning or a error when that occurs ?


I was also interested into checking the syntax of systemd file, but
since systemd is moving quite fast, I doubt to be able to keep a up to
date parser of unit/service/timer/snapshot files. And duplicating code
of systemd in python do not seems like a smart move.

I didn't found any way to reuse systemd code, but I think that a tool
like desktop-file-validate would be quite useful for all distributions.


[1] http://lists.fedoraproject.org/pipermail/devel/2013-May/182697.html
--
Michael Scherer
David Strauss
2013-05-19 06:50:52 UTC
Permalink
I'm skipping to the questions I can answer.
Post by Michael Scherer
- we should avoid as much as possible to use Type=forking when we can
avoid it.
This one is likely the one that will be met with resistance from
packagers, so before adding it as warning, I would like to be sure that
I am not totally wrong.
A standard daemon will fork ( likely twice ) in the background, do
various stuff and then write the pidfile on /run.
- avoid forking 1 or 2 times for nothing,
- avoid taking memory for /run, and avoid taking a inode
So that's should be a little bit better in most case, or do I miss
anything ?
( ie, something like "this is so negligible that we shouldn't care" )
That's not correct. Doing Type=simple services is generally
undesirable because they lack proper support for dependent units only
starting after startup completes (versus when startup begins, which is
all systemd knows about a Type=simple unit). Type=notify is the most
desirable because it gives the benefits you list for Type=simple but
still allows proper dependency effects. However, Type=notify requires
use of systemd APIs from the daemon's own code.

If a service is socket-activiated, though, dependencies on the service
itself are generally unnecessary, as units can depend on the socket
instead. In such situations, Type=simple is quite good, with
Type=notify offering almost only academic advantages over Type=simple.
Post by Michael Scherer
- if using Type=forking, it is better to use PIDFile,
While systemd seems to support fine to guess the main pid, I think this
should be avoided when possible , according to
http://lists.freedesktop.org/archives/systemd-devel/2011-June/002690.html .
So does it make sense to send a warning if there is a service that do
not use PIDFile and of type Forking ?
Absolutely. While the guessing behavior is solid, any packager should
be able to set this explicitly.
Post by Michael Scherer
- some package install directly file in /usr/lib/systemd/system/*.wants
There is some special case ( like plymouth ), but usually, that
shouldn't be done directly in the package, but better done in %post, and
in /etc ?
No, the systemd ideal is that packages should not ever install systemd
units or settings into /etc. Lennart's long-term goal is for daemons
to not install anything by default to /etc, but I think our scope of
authority in this discussion ends with systemd-related matters.
Post by Michael Scherer
IE, is this right to make a warning or a error when that occurs ?
It should be an error.
Post by Michael Scherer
I was also interested into checking the syntax of systemd file, but
since systemd is moving quite fast, I doubt to be able to keep a up to
date parser of unit/service/timer/snapshot files. And duplicating code
of systemd in python do not seems like a smart move.
It should be possible to invoke something in systemd to sanity-check
the included unit files.

Is it okay if the Python invokes a subprocess? If not, I work to add
unit file sanity checking to the C APIs and extend our Python
bindings.
Post by Michael Scherer
I didn't found any way to reuse systemd code, but I think that a tool
like desktop-file-validate would be quite useful for all distributions.
Agreed.

--
David Strauss
| ***@davidstrauss.net
| +1 512 577 5827 [mobile]
Andrey Borzenkov
2013-05-19 07:10:53 UTC
Permalink
В Sat, 18 May 2013 23:50:52 -0700
Post by David Strauss
Post by Michael Scherer
- some package install directly file in /usr/lib/systemd/system/*.wants
There is some special case ( like plymouth ), but usually, that
shouldn't be done directly in the package, but better done in %post, and
in /etc ?
No, the systemd ideal is that packages should not ever install systemd
units or settings into /etc. Lennart's long-term goal is for daemons
to not install anything by default to /etc, but I think our scope of
authority in this discussion ends with systemd-related matters.
Post by Michael Scherer
IE, is this right to make a warning or a error when that occurs ?
It should be an error.
Do you mean, "installing directly under /usr/lib/systemd/system/*.wants
should be an error"? It was the actual question.

I do not think there is single answer in all cases. IMHO it is
perfectly valid to use /usr/lib/systemd/system/*.wants to express
inter-services dependencies. If service A needs services B and C to
work, they should be installed as /usr/lib/systemd/A.wants, so starting
A also transparently starts them.
David Strauss
2013-05-19 07:22:07 UTC
Permalink
Post by Andrey Borzenkov
Do you mean, "installing directly under /usr/lib/systemd/system/*.wants
should be an error"? It was the actual question.
I do not think there is single answer in all cases. IMHO it is
perfectly valid to use /usr/lib/systemd/system/*.wants to express
inter-services dependencies. If service A needs services B and C to
work, they should be installed as /usr/lib/systemd/A.wants, so starting
A also transparently starts them.
An RPM installing systemd-related files to /etc should produce an
error. RPMs should install their systemd files to /usr/lib/systemd (or
equivalent if that's not the unified systemd installed file location).

--
David Strauss
| ***@davidstrauss.net
| +1 512 577 5827 [mobile]
David Strauss
2013-05-19 07:28:11 UTC
Permalink
Post by Andrey Borzenkov
Do you mean, "installing directly under /usr/lib/systemd/system/*.wants
should be an error"? It was the actual question.
Wait, do you mean versus putting WantedBy= into the unit? That depends
on packaging policy. RPM packaging policy is to not have services
start by default, so I'd recommend using WantedBy= so systemd computes
the dependencies at the time the administrator enables the service --
if she enables it at all.
Post by Andrey Borzenkov
If service A needs services B and C to
work, they should be installed as /usr/lib/systemd/A.wants, so starting
A also transparently starts them.
If service A *needs* B and C, then service A should use Requires= and
possibly After= for B and C, not "wants." A "want" in system still
allows the transaction to complete even with dependency failure. We
generally use "wants" for startup and shutdown targets so the system
can still boot to those stages even with a service failing.

--
David Strauss
| ***@davidstrauss.net
| +1 512 577 5827 [mobile]
David Strauss
2013-05-19 07:29:49 UTC
Permalink
I hopped in IRC just now if you'd like to chat.
Lennart Poettering
2013-06-06 07:34:50 UTC
Permalink
Post by Andrey Borzenkov
В Sat, 18 May 2013 23:50:52 -0700
Post by David Strauss
Post by Michael Scherer
- some package install directly file in /usr/lib/systemd/system/*.wants
There is some special case ( like plymouth ), but usually, that
shouldn't be done directly in the package, but better done in %post, and
in /etc ?
No, the systemd ideal is that packages should not ever install systemd
units or settings into /etc. Lennart's long-term goal is for daemons
to not install anything by default to /etc, but I think our scope of
authority in this discussion ends with systemd-related matters.
Post by Michael Scherer
IE, is this right to make a warning or a error when that occurs ?
It should be an error.
Do you mean, "installing directly under /usr/lib/systemd/system/*.wants
should be an error"? It was the actual question.
I do not think there is single answer in all cases. IMHO it is
perfectly valid to use /usr/lib/systemd/system/*.wants to express
inter-services dependencies. If service A needs services B and C to
work, they should be installed as /usr/lib/systemd/A.wants, so starting
A also transparently starts them.
I'd recommend specifying this in Wants= or Requires= in A.service...

Lennart
--
Lennart Poettering - Red Hat, Inc.
Michael Scherer
2013-05-19 09:21:09 UTC
Permalink
Post by David Strauss
I'm skipping to the questions I can answer.
Post by Michael Scherer
- we should avoid as much as possible to use Type=forking when we can
avoid it.
This one is likely the one that will be met with resistance from
packagers, so before adding it as warning, I would like to be sure that
I am not totally wrong.
A standard daemon will fork ( likely twice ) in the background, do
various stuff and then write the pidfile on /run.
- avoid forking 1 or 2 times for nothing,
- avoid taking memory for /run, and avoid taking a inode
So that's should be a little bit better in most case, or do I miss
anything ?
( ie, something like "this is so negligible that we shouldn't care" )
That's not correct. Doing Type=simple services is generally
undesirable because they lack proper support for dependent units only
starting after startup completes (versus when startup begins, which is
all systemd knows about a Type=simple unit). Type=notify is the most
desirable because it gives the benefits you list for Type=simple but
still allows proper dependency effects. However, Type=notify requires
use of systemd APIs from the daemon's own code.
In this case, that's Type=simple vs Type=forking. IE, is Type=forking
better in term of having the startup being complete in the general
case ? ( of course, there is always exception, but ).

Type=notify is better, but that requires patching daemon.

However, I could check if the binary is linked with libsystemd-daemon
and trigger a warning for using Type=notify.
Post by David Strauss
If a service is socket-activiated, though, dependencies on the service
itself are generally unnecessary, as units can depend on the socket
instead. In such situations, Type=simple is quite good, with
Type=notify offering almost only academic advantages over Type=simple.
Socket activated is another beast, and I do not know how I can check if
a binary can be socket activated to trigger a warning.
Post by David Strauss
Post by Michael Scherer
- some package install directly file in /usr/lib/systemd/system/*.wants
There is some special case ( like plymouth ), but usually, that
shouldn't be done directly in the package, but better done in %post, and
in /etc ?
No, the systemd ideal is that packages should not ever install systemd
units or settings into /etc. Lennart's long-term goal is for daemons
to not install anything by default to /etc, but I think our scope of
authority in this discussion ends with systemd-related matters.
Post by Michael Scherer
IE, is this right to make a warning or a error when that occurs ?
It should be an error.
For /etc, yes, that's planned.

But as said in the thread, my issue is :

A package ship /usr/lib/systemd/system/foo.wants/bar.service as a link
to /usr/lib/systemd/system/bar.service

instead of using WantedBy=bar.service

Is there one technical reason to prefer the former to the latter ?
I would be tempted to mandate to use WantedBy because that would be more
consistant with most packages, and hardcoding that setting on the FS
forbid to override it in /etc, but maybe that's the goal.
Post by David Strauss
Post by Michael Scherer
I was also interested into checking the syntax of systemd file, but
since systemd is moving quite fast, I doubt to be able to keep a up to
date parser of unit/service/timer/snapshot files. And duplicating code
of systemd in python do not seems like a smart move.
It should be possible to invoke something in systemd to sanity-check
the included unit files.
Is it okay if the Python invokes a subprocess?
It is ok. Bonus point for a parsable output, but just having a exit code
would be ok for integration and let people run the binary and read
errors.
Post by David Strauss
If not, I work to add
unit file sanity checking to the C APIs and extend our Python
bindings.
I think since lintian ( a similar tool for debian package ) is in perl,
so having it in a external binary would be better for them.

Now, from a deployment point of view, desktop-file-validate was
backported on stable version for our builders at Mageia, and since the
tool was self contained, that was ok, but pulling over a complete
systemd backport on the server would likely be refused by any sysadmins,
so if the tool could be compiled as a standalone without interfering
with existing systemd installation, that would be nice.
( if not, I guess they can use some chroot tricks as i do for others
quality checking tools )
--
Michael Scherer
Mathieu Bridon
2013-05-21 02:34:54 UTC
Permalink
Post by Michael Scherer
Post by David Strauss
Post by Michael Scherer
- we should avoid as much as possible to use Type=forking when we can
avoid it.
[... snip ...]
Post by Michael Scherer
Post by David Strauss
That's not correct. Doing Type=simple services is generally
undesirable because they lack proper support for dependent units only
starting after startup completes
[... snip ...]
Post by Michael Scherer
In this case, that's Type=simple vs Type=forking. IE, is Type=forking
better in term of having the startup being complete in the general
case ? ( of course, there is always exception, but ).
Type=notify is better, but that requires patching daemon.
However, I could check if the binary is linked with libsystemd-daemon
and trigger a warning for using Type=notify.
Post by David Strauss
If a service is socket-activiated, though, dependencies on the service
itself are generally unnecessary, as units can depend on the socket
instead. In such situations, Type=simple is quite good, with
Type=notify offering almost only academic advantages over Type=simple.
Socket activated is another beast, and I do not know how I can check if
a binary can be socket activated to trigger a warning.
It's not really another beast in fact, it's all quite related.

What David was trying to say is that depending on the service type,
systemd will wait for a specific event to consider the service as
"started", and proceed with the units ordered after it.

For a Type=forking service, systemd will consider that the service is
started as soon as the parent process exits. That's the signal for "this
service is now running and ready to handle requests, other services
which need it can be started.

For a Type=dbus service, that signal is that the BusName=org.foo.bar
appeared on the Bus.

For a Type=notify service, that signal is that the service notified
systemd.

For a Type=simple service, there is no such signal, systemd starts the
service and considers it ready right away, which can cause trouble if
the service takes a long time to be actually ready.

That's why Type=simple is generally not desirable, unless the service
happens to be socket-activated.

So I don't think you can have such a binary "forking vs simple" point of
view. The actual task that is provided by the service needs to be taken
into account, as well as how long it takes to be ready, do other
services need it, etc...
Post by Michael Scherer
Post by David Strauss
Post by Michael Scherer
- some package install directly file in /usr/lib/systemd/system/*.wants
There is some special case ( like plymouth ), but usually, that
shouldn't be done directly in the package, but better done in %post, and
in /etc ?
No, the systemd ideal is that packages should not ever install systemd
units or settings into /etc. Lennart's long-term goal is for daemons
to not install anything by default to /etc, but I think our scope of
authority in this discussion ends with systemd-related matters.
Post by Michael Scherer
IE, is this right to make a warning or a error when that occurs ?
It should be an error.
For /etc, yes, that's planned.
A package ship /usr/lib/systemd/system/foo.wants/bar.service as a link
to /usr/lib/systemd/system/bar.service
instead of using WantedBy=bar.service
Is there one technical reason to prefer the former to the latter ?
I would be tempted to mandate to use WantedBy because that would be more
consistant with most packages, and hardcoding that setting on the FS
forbid to override it in /etc, but maybe that's the goal.
Exactly.

Some units really shouldn't ever be disabled, and as such they are valid
candidates for installing directly
a /usr/lib/systemd/system/foo.wants/bar.unit
--
Mathieu
Andrey Borzenkov
2013-05-19 06:58:26 UTC
Permalink
В Sat, 18 May 2013 23:44:48 +0200
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
For current openSUSE (12.3 and above) standard location is /usr/lib
Zbigniew Jędrzejewski-Szmek
2013-05-19 13:45:43 UTC
Permalink
Post by Michael Scherer
Hi,
After the last discussion on fedora-devel about boot times[1], I have
been looking at the various units on my disk and if I could code
something in rpmlint ( tool to verify a rpm against a set of guidelines
) to have packagers follow some best practices.
I'm not sure if this was mentioned on fedora-devel, but it would
be great to add a check for file permissions on unit files. They
should be root:root 0644 exactly. We now have [1] as the offical
statement.

Zbyszek

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions
T.C. Hollingsworth
2013-05-21 02:58:09 UTC
Permalink
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
Shouldn't rpmlint just make sure the spec uses "%{_unitdir}"? Nothing
except the systemd package should hardcode this directory.

(Or if the check has to be on the binary RPM side, just `rpm --eval
'%_unitdir'` for the right location?)

-T.C.
Frederic Crozat
2013-05-21 08:29:32 UTC
Permalink
Post by T.C. Hollingsworth
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
Shouldn't rpmlint just make sure the spec uses "%{_unitdir}"? Nothing
except the systemd package should hardcode this directory.
(Or if the check has to be on the binary RPM side, just `rpm --eval
'%_unitdir'` for the right location?)
Yes, I agree with this suggestion. This way, even if a new rpmlint
release was used on a "old" openSUSE (where /lib/systemd/system was
used), it would still work.
--
Frederic Crozat <***@suse.com>
SUSE
Michael Scherer
2013-05-21 21:28:40 UTC
Permalink
Post by T.C. Hollingsworth
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
Shouldn't rpmlint just make sure the spec uses "%{_unitdir}"? Nothing
except the systemd package should hardcode this directory.
Parsing spec is not something I will attempt
It cannot been done in a robust way ( or at least, wasn't last time I
looked ).
Post by T.C. Hollingsworth
(Or if the check has to be on the binary RPM side, just `rpm --eval
'%_unitdir'` for the right location?)
I already do that, but I look in that directory to see if there is a
file for systemd ( I was using *.service, but since dbus use the same
suffix ). So by definition, i cannot detect out of that directory file
to warn about them ( unless for well know error like in /etc ).
--
Michael Scherer
Lennart Poettering
2013-06-06 07:33:23 UTC
Permalink
Post by Michael Scherer
Hi,
After the last discussion on fedora-devel about boot times[1], I have
been looking at the various units on my disk and if I could code
something in rpmlint ( tool to verify a rpm against a set of guidelines
) to have packagers follow some best practices.
It would be fantastic to get rpmlint updated to do a a couple of systemd
unit file checks.
Post by Michael Scherer
So I have been writing various checks that emit Warning/Errors, and
would like to get some feedback on a few assumption I made when writing
- pid file should be in /run, or in a subdirectory of /run
I have seen a few service who still use /var/run, but can I safely
assume that anything booting with systemd would have /run?
(and so warn if something is using /run for that)
Yes, systemd strictly requires /run to be around.
Post by Michael Scherer
- unit file should be in %_unitdir, which is on /usr/lib for
distribution with merged /usr ( at least, on Fedora and Mageia )
Correct.
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
I'd recommend adding a warning for all units that aren't located in
%_unitdir.
Post by Michael Scherer
- we should avoid as much as possible to use Type=forking when we can
avoid it.
Well, the rule should be, in order of preference:

1) for bus services, please use Type=dbus and specify BusName=

2) for socket-activatable services, please use Type=simple

3) for one-shot services use Type=oneshot

4) for other services use Type=notify if they support it

5) otherwise use Type=forking, but if you do please make sure to specify
PIDFile=, too.

I figure the only things rpmlint could check here is that Type=dbus has
BusName=, and Type=forking has PIDFile=, everything else is probably
nothing we could automatically check.
Post by Michael Scherer
- some package install directly file in /usr/lib/systemd/system/*.wants
There is some special case ( like plymouth ), but usually, that
shouldn't be done directly in the package, but better done in %post, and
in /etc ?
Installing something into /usr/lib/systemd/system/*.wants is only OK for
very special system components, but not for normal packages. I think
it's OK to add a check for this, and then tell the Plymouth maintainers
(and similar low-level folks) to just ignore it.
Post by Michael Scherer
I was also interested into checking the syntax of systemd file, but
since systemd is moving quite fast, I doubt to be able to keep a up to
date parser of unit/service/timer/snapshot files. And duplicating code
of systemd in python do not seems like a smart move.
There have been requests to add a unit file validator to systemd. It
shouldn't be too hard to do that actually, simply reusing systemd's own
parsing code, instead of writing a second parser. I'd be happy to merge
a patch that adds "systemd-validate" or so, which instantiates a
throw-away Manager object (as defined by systemd's core), then tries to
load specified unit files into it, and shows all warnings, then freeing
things immediately.

Other things I'd like to see checked:

Packages should *never* install symlinks or unit files into
/etc/systemd/...

Packages should *never* install their own preset files. That's something
for fedora-release.rpm and systemd.rpm, but nobody else. (the various
display managers ended up doing this, which is really broken).

The unit files should have 644 access mode.

Packages should only install symlinks in
/usr/lib/systemd/system/*.wants/ and they should point to unit files in
/usr/lib/systemd/system/.

Service files with WantedBy=sysinit.target, WantedBy=local-fs.target or
WantedBy=basic.target should have DefaultDependencies=no.

Lennart
--
Lennart Poettering - Red Hat, Inc.
David Greaves
2013-06-16 18:18:45 UTC
Permalink
Post by Lennart Poettering
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
I'd recommend adding a warning for all units that aren't located in
%_unitdir.
Would it make sense to add:

%define _userunitdir %{_libdir}/systemd/user/

at this time?

David
--
"Don't worry, you'll be fine; I saw it work in a cartoon once..."
Andrey Borzenkov
2013-06-17 02:34:33 UTC
Permalink
В Sun, 16 Jun 2013 19:18:45 +0100
Post by David Greaves
Post by Lennart Poettering
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
I'd recommend adding a warning for all units that aren't located in
%_unitdir.
%define _userunitdir %{_libdir}/systemd/user/
_libdir is most likely /usr/lib64 on 64 bit systems.
Post by David Greaves
at this time?
David
Lennart Poettering
2013-06-17 16:38:08 UTC
Permalink
Post by David Greaves
Post by Lennart Poettering
Post by Michael Scherer
So I planned to warn if the unit are directly in /lib, but I know there
is some distribution that didn't choose this path yet. So when /usr is
not merged, what is the canonical location ( ie, for Opensuse, Mandriva,
since they are both rpm based ) ?
I'd recommend adding a warning for all units that aren't located in
%_unitdir.
%define _userunitdir %{_libdir}/systemd/user/
at this time?
I added this now to the macros.systemd file we ship upstream.

Lennart
--
Lennart Poettering - Red Hat, Inc.
Continue reading on narkive:
Loading...