package/systemd: unbreak a few stuf..

Hello All!

This series, starting with a patch from Trent, is an attempt to fix most of the known systemd issues, especially with respect to the DBus socket location and the rest of the factory stuff.

It should supersed all the previous attempts.

Some work is probably still needed to clean up all the mess. The most important issue left over is that the /var state might be inconsistent, should the filesystem generation fails (e.g. too-small a read-only ext2 filesystem is attempted). This would be fixed by my other series about the handling of filesystems...

At least, this is probably just enough for the release.

Regards,
Yann E. MORIN.

The following changes since commit b1aa2148b0365776a17af3d8e22edd3622d475e4

core: drop no-longer used C.UTF-8 locale option (2018-03-04 11:59:16 +0100)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to 12fe4b30ba04733e7c58451fd2e44ca796465db2

support/tests: enhance the runtime systemd tests (2018-03-04 17:57:13 +0100)

----------------------------------------------------------------
Trent Piepho (1):
package/skeleton-init-systemd: work around for /var/lib not populating

Yann E. MORIN (2):
package/skeleton-systemd: invert factory logic support/tests: enhance the runtime systemd tests

 package/skeleton-init-systemd/skeleton-init-systemd.mk | 17 ++++++++++-------
 support/testing/tests/init/test_systemd.py             | 12 ++++++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

From: Trent Piepho <[email protected]>

When using a RO root with systemd, it is intended that /var/lib should be populated at boot time by tmpfiles system mirroring it from /usr/share/factory/var/lib.

However, this will only happen if /var/lib does not already exist at the time systemd-tmpfiles runs. If it does exist, then tmpfiles will (silently) skip it and do nothing.

It turns out /var/lib will exist, because some part of systemd creates /var/lib/systemd/catalog on boot before tmpfiles runs.

The fix used here is to also create tmpfiles entries for the contents of /var/lib/* and /var/lib/systemd/*. This way, when those directories already exist, the entire tree is not skipped and instead the not-yet-existing contents of /var/lib and /var/lib/systemd will be still be mirrored from the factory dir.

And if /var/lib/systemd, or a prefix of that, stops getting created and does not exist, it'll still mirror properly.

It does cause some warnings from systemd:
systemd[1]: Starting Create Volatile Files and Directories...
systemd-tmpfiles[148]: [/etc/tmpfiles.d/var-factory.conf:7] Duplicate line for path "/var/lib/systemd", ignoring. systemd-tmpfiles[148]: [/etc/tmpfiles.d/var-factory.conf:8] Duplicate line for path "/var/lib/systemd/coredump", ignoring.

But they can be ignored.

IMHO, I think a better solution would be for systemd-tmpfiles to gain a "merge tree" operation that is like "C" but doesn't abort if the destination exists, but rather merges the source into it.

---
 package/skeleton-init-systemd/skeleton-init-systemd.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index a2d4e8c4b3..6a0527fde2 100644
@@ -40,7 +40,10 @@ endef
 define SKELETON_INIT_SYSTEMD_PRE_ROOTFS_VAR
 	rm -f $(TARGET_DIR)/var
 	mkdir $(TARGET_DIR)/var
-	for i in $(TARGET_DIR)/usr/share/factory/var/*; do \
+	for i in $(TARGET_DIR)/usr/share/factory/var/* \
+		 $(TARGET_DIR)/usr/share/factory/var/lib/* \
+		 $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do \
+		[ -e "$${i}" ] || continue; \
 		j="$${i#$(TARGET_DIR)/usr/share/factory}"; \
 		if [ -L "$${i}" ]; then \
 			printf "L+! %s - - - - %s\n" \

2.14.1

Hello;

Hello,

Note: the patches have already been applied, and my comments were not meant to prevent them from being applied.

But annoying. Lots of people are going to wonder what's happening, and find them bogus.

What is "C" ? What is needed for this "better solution" ?

Thomas

Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

Thomas, All,

On 2018-03-04 21:24 +0100, Thomas Petazzoni spake thusly:

Thaks for applying! :-)

Yes, I have a few isdeas on how to fix them...

But later.

    $ man tmpfiles.d
    [--SNIP--]
        C
           Recursively copy a file or directory, if the destination
           files or directories do not exist yet. Note that this command
           will not descend into subdirectories if the destination
           directory already exists. Instead, the entire copy operation
           is skipped.  If the argument is omitted, files from the source
           directory /usr/share/factory/ with the same name are copied.
           Does not follow symlinks.
    [--SNIP--]

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

This indicates that there is another tmpfile.d line somewhere for the same path. In this case, they are in systemd.conf, part of systemd's installed files.

One could just have the buildroot script avoid the lines for these specific paths, but then what if systemd removes them from its tmpfiles.d configuration? Add in some fancy grep command to check a path against /usr/lib/tmpfiles.d/* and see if it appears anywhere?

That last sentence is the problem. It seems to me that merging the source into the destination would be more useful. So why wasn't that done? Maybe it ends up being a really bad idea for some reason that is not obvious to me? Or maybe the person who coded this originally didn't need to merge directories and "do nothing" was an easy way to avoid thinking about that problem?

So what would be needed is to convince the systemd guys that "merge tree" should be added to tmpfiles and then have someone code it. Looking at the existing infrastructure in systemd for filesystem tree ops, it seems like a lot of what is needed is there. But clearly the timeframe for that is much too long for the buildroot release at hand now.

The thing I don't like the current situation is that the "create /var tree" process needs to know each and every path in the /var that is created on boot by something else. If buildroot duplicates something from elsewhere, we get those warnings. If buildroot fails to account for something that is elsewhere, the whole operation can stop working. Too fragile!

Currently, we handle the factory by redirectoring /var with a symlink at build time, and with some trickery during the filesystem generation, depending on whether we need to remount the filesystem read-write or not.

However, this is causing wquite some pain with latest systemd, now that they have moved their dbus socket to /run instead of /var/run.

As such, trying to play tricks with /var/run as a symlink is difficult, because at times it is in .usr/share/factory/var/run (during build) and then it is in /var/run (at runtime). So a relative symlink is not possible. But an absolute symlink is not possible either, because we are installing out-pf-tree.

Oh the joys of cross-compilation... :-)

We fix all this mess by making /var a real directory from the onset, so that we can use the runtime-expected layout even during the build.

Then, during filesystem generation, we move /var away to the factory, and populate it as we used to do. This still requires a post-fs hook to restore /var after the filesystem generation.

This leaves a situation that, should the filesystem generation fails, /var will be left in an inconsistent state. But that is not worse than what we already had anyway.

---
Note: even with this, we can't move the /var directory back to the common skeleton, becasue the sysv skeleton has a slew of symlinks back to /tmp anyway. So /var has to stay a per-skeleton directory. Pfeew...

---
 package/skeleton-init-systemd/skeleton-init-systemd.mk | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

index 6a0527fde2..ff64205cbe 100644
@@ -19,7 +19,6 @@ ifeq ($(BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW),y)
 
 define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
 	echo "/dev/root / auto rw 0 1" >$(TARGET_DIR)/etc/fstab
-	mkdir -p $(TARGET_DIR)/var
 endef
 
 else

@@ -31,15 +30,14 @@ else
 # back there by the tmpfiles.d mechanism.
 define SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW
 	mkdir -p $(TARGET_DIR)/etc/systemd/tmpfiles.d
-	mkdir -p $(TARGET_DIR)/usr/share/factory/var
-	ln -s usr/share/factory/var $(TARGET_DIR)/var
 	echo "/dev/root / auto ro 0 1" >$(TARGET_DIR)/etc/fstab
 	echo "tmpfs /var tmpfs mode=1777 0 0" >>$(TARGET_DIR)/etc/fstab
 endef
 
 define SKELETON_INIT_SYSTEMD_PRE_ROOTFS_VAR
-	rm -f $(TARGET_DIR)/var
-	mkdir $(TARGET_DIR)/var
+	rm -rf $(TARGET_DIR)/usr/share/factory/var
+	mv $(TARGET_DIR)/var $(TARGET_DIR)/usr/share/factory/var
+	mkdir -p $(TARGET_DIR)/var
 	for i in $(TARGET_DIR)/usr/share/factory/var/* \
 		 $(TARGET_DIR)/usr/share/factory/var/lib/* \
 		 $(TARGET_DIR)/usr/share/factory/var/lib/systemd/*; do \

@@ -59,7 +57,7 @@ SKELETON_INIT_SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SKELETON_INIT_SYSTEMD_PRE_ROOTFS_V
 
 define SKELETON_INIT_SYSTEMD_POST_ROOTFS_VAR
 	rm -rf $(TARGET_DIR)/var
-	ln -s usr/share/factory/var $(TARGET_DIR)/var
+	mv $(TARGET_DIR)/usr/share/factory/var $(TARGET_DIR)/var
 endef
 SKELETON_INIT_SYSTEMD_ROOTFS_POST_CMD_HOOKS += SKELETON_INIT_SYSTEMD_POST_ROOTFS_VAR
 

@@ -68,6 +66,8 @@ endif
 define SKELETON_INIT_SYSTEMD_INSTALL_TARGET_CMDS
 	mkdir -p $(TARGET_DIR)/home
 	mkdir -p $(TARGET_DIR)/srv
+	mkdir -p $(TARGET_DIR)/var
+	ln -s ../run $(TARGET_DIR)/var/run
 	$(SKELETON_INIT_SYSTEMD_ROOT_RO_OR_RW)
 endef
 

2.14.1

The symlink does work ok if the patches to fix it are applied. Both the build time ${TARGET_DIR}/var/run and run time /var/run work.

However...

I think this a better way. It was I was alluding to on IRC as a system that wasn't tied to work-arounds for specific directories in var. But I was troubled by...

I wondered if there was a way to get fakeroot to also "fake" the movement of the files in addition to modification of owner and group, but it appears there is not. I do not think it would be impossible for fakeroot to gain this ability.

Recent systemd bump has broken DBus dameon and DBus applications can no longer find the daemon. So we want to catch those kind of failures early.

We also want to check that the system as a whole is stable: no unit should be failed.

Finally, ensure that we cn read the jounrnal, even when we are doing our tricks on read-only systems.

---
 support/testing/tests/init/test_systemd.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

index 48fac1490f..a324ba8569 100644
@@ -21,6 +21,18 @@ class InitSystemSystemdBase(InitSystemBase):
     def check_init(self):
         super(InitSystemSystemdBase, self).check_init("/lib/systemd/systemd")
 
+        # Test all units are OK
+        output, _ = self.emulator.run("systemctl --no-pager --failed --no-legend")
+        self.assertEqual(len(output), 0)
+
+        # Test we can reach the DBus daemon
+        _, exit_code = self.emulator.run("busctl --no-pager")
+        self.assertEqual(exit_code, 0)
+
+        # Test we can read at least one line from the journal
+        output, _ = self.emulator.run("journalctl --no-pager --lines 1 --quiet")
+        self.assertEqual(len(output), 1)
+
 
 class TestInitSystemSystemdRoNetworkd(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \

2.14.1

Indeed - Committed series, thanks!

Bye, Peter Korsgaard