[systemd-devel] [PATCH] correctly mark system reboot

Discussion:

Andrey Borzenkov

2011-03-02 08:41:26 UTC

It is expected that system will put "reboot" in wtmp to mark
when it starts coming up. This is looked for by "who -b" and is
used by "last" to calculate correct time spent by various programs.
Add systemd-update-utmp-reboot.service which is started as soon
as possible after local-fs.target to mark reboot.

Signed-off-by: Andrey Borzenkov <***@gmail.com>

---
Makefile.am | 4 ++++
units/systemd-update-utmp-reboot.service.in | 16 ++++++++++++++++
2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1841ad5..54786e7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -258,6 +258,7 @@ nodist_systemunit_DATA = \
units/systemd-modules-load.service \
units/systemd-vconsole-setup.service \
units/systemd-remount-api-vfs.service \
+ units/systemd-update-utmp-reboot.service \
units/systemd-update-utmp-runlevel.service \
units/systemd-update-utmp-shutdown.service \
units/systemd-random-seed-save.service \
@@ -303,6 +304,7 @@ EXTRA_DIST = \
units/systemd-modules-load.service.in \
units/systemd-vconsole-setup.service.in \
units/systemd-remount-api-vfs.service.in \
+ units/systemd-update-utmp-reboot.service \
units/systemd-update-utmp-runlevel.service.in \
units/systemd-update-utmp-shutdown.service.in \
units/systemd-random-seed-save.service.in \
@@ -1276,6 +1278,7 @@ install-data-hook:
systemd-tmpfiles-setup.service \
systemd-sysctl.service \
systemd-ask-password-console.path \
+ systemd-update-utmp-reboot.service \
cryptsetup.target && \
$(LN_S) ../dev-hugepages.automount dev-hugepages.automount && \
$(LN_S) ../dev-mqueue.automount dev-mqueue.automount && \
@@ -1288,6 +1291,7 @@ install-data-hook:
$(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service && \
$(LN_S) ../systemd-sysctl.service systemd-sysctl.service && \
$(LN_S) ../systemd-ask-password-console.path systemd-ask-password-console.path && \
+ $(LN_S) ../systemd-update-utmp-reboot.service systemd-update-utmp-reboot.service && \
$(LN_S) ../cryptsetup.target cryptsetup.target )
( cd $(DESTDIR)$(systemunitdir)/basic.target.wants && \
rm -f systemd-tmpfiles-clean.timer && \
diff --git a/units/systemd-update-utmp-reboot.service.in b/units/systemd-update-utmp-reboot.service.in
new file mode 100644
index 0000000..e6ccda3
--- /dev/null
+++ b/units/systemd-update-utmp-reboot.service.in
@@ -0,0 +1,16 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Notify Audit System and Update UTMP about System Reboot
+DefaultDependencies=no
+Wants=local-fs.target
+After=local-fs.target systemd-tmpfiles-setup.service
+
+[Service]
+Type=oneshot
+ExecStart=@rootlibexecdir@/systemd-update-utmp reboot

--
tg: (fc7a744..) upstream/utmp-reboot (depends on: origin/master)

Andrey Borzenkov

2011-03-02 09:12:50 UTC

Permalink

It is expected that system will put "reboot" in wtmp to mark
when it starts coming up. This is looked for by "who -b" and is
used by "last" to calculate correct time spent by various programs.
Add systemd-update-utmp-reboot.service which is started as soon
as possible after local-fs.target to mark reboot.

V2: fix copy'n'paste typo in EXTA_DIST file name

Signed-off-by: Andrey Borzenkov <***@gmail.com>

---
Makefile.am | 4 ++++
units/systemd-update-utmp-reboot.service.in | 16 ++++++++++++++++
2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1841ad5..a55e598 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -258,6 +258,7 @@ nodist_systemunit_DATA = \
units/systemd-modules-load.service \
units/systemd-vconsole-setup.service \
units/systemd-remount-api-vfs.service \
+ units/systemd-update-utmp-reboot.service \
units/systemd-update-utmp-runlevel.service \
units/systemd-update-utmp-shutdown.service \
units/systemd-random-seed-save.service \
@@ -303,6 +304,7 @@ EXTRA_DIST = \
units/systemd-modules-load.service.in \
units/systemd-vconsole-setup.service.in \
units/systemd-remount-api-vfs.service.in \
+ units/systemd-update-utmp-reboot.service.in \
units/systemd-update-utmp-runlevel.service.in \
units/systemd-update-utmp-shutdown.service.in \
units/systemd-random-seed-save.service.in \
@@ -1276,6 +1278,7 @@ install-data-hook:
systemd-tmpfiles-setup.service \
systemd-sysctl.service \
systemd-ask-password-console.path \
+ systemd-update-utmp-reboot.service \
cryptsetup.target && \
$(LN_S) ../dev-hugepages.automount dev-hugepages.automount && \
$(LN_S) ../dev-mqueue.automount dev-mqueue.automount && \
@@ -1288,6 +1291,7 @@ install-data-hook:
$(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service && \
$(LN_S) ../systemd-sysctl.service systemd-sysctl.service && \
$(LN_S) ../systemd-ask-password-console.path systemd-ask-password-console.path && \
+ $(LN_S) ../systemd-update-utmp-reboot.service systemd-update-utmp-reboot.service && \
$(LN_S) ../cryptsetup.target cryptsetup.target )
( cd $(DESTDIR)$(systemunitdir)/basic.target.wants && \
rm -f systemd-tmpfiles-clean.timer && \
diff --git a/units/systemd-update-utmp-reboot.service.in b/units/systemd-update-utmp-reboot.service.in
new file mode 100644
index 0000000..e6ccda3
--- /dev/null
+++ b/units/systemd-update-utmp-reboot.service.in
@@ -0,0 +1,16 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Notify Audit System and Update UTMP about System Reboot
+DefaultDependencies=no
+Wants=local-fs.target
+After=local-fs.target systemd-tmpfiles-setup.service
+
+[Service]
+Type=oneshot
+ExecStart=@rootlibexecdir@/systemd-update-utmp reboot

--
tg: (fc7a744..) upstream/utmp-reboot (depends on: origin/master)

Lennart Poettering

2011-03-02 21:31:27 UTC

Permalink

Post by Andrey Borzenkov
It is expected that system will put "reboot" in wtmp to mark
when it starts coming up. This is looked for by "who -b" and is
used by "last" to calculate correct time spent by various programs.
Add systemd-update-utmp-reboot.service which is started as soon
as possible after local-fs.target to mark reboot.

Hmm, systemd-update-utmp-runlevel.service should normally do that
implicitly. When /lib/systemd/systemd-update-utmp is called with the
"runlevel" argument then it will add the "reboot" entry if necessary
automatically, followed by the "runlevel" entry.

Are you suggesting that this automatic logic isn't working correctly?

Lennart

--
Lennart Poettering - Red Hat, Inc.

Andrey Borzenkov

2011-03-03 04:51:52 UTC

Permalink

On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering

Post by Lennart Poettering

Hmm, systemd-update-utmp-runlevel.service should normally do that
implicitly. When /lib/systemd/systemd-update-utmp is called with the
"runlevel" argument then it will add the "reboot" entry if necessary
automatically, followed by the "runlevel" entry.
Are you suggesting that this automatic logic isn't working correctly?

On my notebook "reboot" line is never added. What is funny, it appears
that on my test VM which has stripped down installation "reboot" does
actually appear. Which suggests some race condition or uninitialized
variable somewhere.

Any suggestions how to debug it further?

Andrey Borzenkov

2011-03-03 06:14:17 UTC

Permalink

Post by Andrey Borzenkov
On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering

Post by Lennart Poettering

Hmm, systemd-update-utmp-runlevel.service should normally do that
implicitly. When /lib/systemd/systemd-update-utmp is called with the
"runlevel" argument then it will add the "reboot" entry if necessary
automatically, followed by the "runlevel" entry.
Are you suggesting that this automatic logic isn't working correctly?

Well, the problem is, on my notebook it *does* find previous runlevel
entry and so never triggers on_reboot:

Mar 3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 0/0
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Previous runlevel: 0
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Current runlevel: 53
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 0/0

Because it works on stripped down installation, it appears that some
package puts this entry there on startup.

I really do not know how to find who is messing up with utmp; it may
be anything. So it looks like one of

- use explicit unit for "reboot" (and remove current magic from on_runlevel).

- call on_reboot() if previous run-level found was 0.

Personally I find explicit unit to be better understandable and easier
to debug than hiding some magic inside binary that no one is aware of.

Lennart Poettering

2011-03-03 15:20:23 UTC

Permalink

Post by Andrey Borzenkov

Post by Andrey Borzenkov
On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering

Post by Lennart Poettering

Hmm, systemd-update-utmp-runlevel.service should normally do that
implicitly. When /lib/systemd/systemd-update-utmp is called with the
"runlevel" argument then it will add the "reboot" entry if necessary
automatically, followed by the "runlevel" entry.
Are you suggesting that this automatic logic isn't working correctly?

Well, the problem is, on my notebook it *does* find previous runlevel
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 0/0
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Previous runlevel: 0
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Current runlevel: 53
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
Mar 3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 0/0
Because it works on stripped down installation, it appears that some
package puts this entry there on startup.
I really do not know how to find who is messing up with utmp; it may
be anything. So it looks like one of
- use explicit unit for "reboot" (and remove current magic from on_runlevel).
- call on_reboot() if previous run-level found was 0.
Personally I find explicit unit to be better understandable and easier
to debug than hiding some magic inside binary that no one is aware of.

Hmm, here's a guess: the code relies that utmp is emptied? Maybe this
doesn't happen on your system?

Normally systemd-update-utmp-runlevel.service is run after
systemd-tmpfiles-setup.service, but maybe this doesn't work on your system?

Lennart

--
Lennart Poettering - Red Hat, Inc.

Andrey Borzenkov

2011-03-03 15:25:38 UTC

Permalink

On Thu, Mar 3, 2011 at 6:20 PM, Lennart Poettering

Post by Lennart Poettering

Post by Andrey Borzenkov

Post by Andrey Borzenkov
On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering

Post by Lennart Poettering

Hmm, systemd-update-utmp-runlevel.service should normally do that
implicitly. When /lib/systemd/systemd-update-utmp is called with the
"runlevel" argument then it will add the "reboot" entry if necessary
automatically, followed by the "runlevel" entry.
Are you suggesting that this automatic logic isn't working correctly?

Hmm, here's a guess: the code relies that utmp is emptied? Maybe this
doesn't happen on your system?

Fixed in patch I am about to send.

Post by Lennart Poettering
Normally systemd-update-utmp-runlevel.service is run after
systemd-tmpfiles-setup.service, but maybe this doesn't work on your system?

it does.

I still believe that "one unit - one task" is better for understanding
and easier for debugging :)

Lennart Poettering

2011-03-03 15:21:26 UTC

Permalink

Post by Andrey Borzenkov
On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering

Post by Lennart Poettering

Hmm, systemd-update-utmp-runlevel.service should normally do that
implicitly. When /lib/systemd/systemd-update-utmp is called with the
"runlevel" argument then it will add the "reboot" entry if necessary
automatically, followed by the "runlevel" entry.
Are you suggesting that this automatic logic isn't working correctly?

Might want to check the activation timestamps shown in "systemctl show
systemd-tmpfiles-setup.service systemd-update-utmp-runlevel.service".

Lennart

--
Lennart Poettering - Red Hat, Inc.