[Buildroot] [PATCH 0/3] package/systemd: unbreak a few stuf..

[Buildroot] [PATCH 0/3] package/systemd: unbreak a few stuf..

[Yann E. MORIN]

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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/3] package/skeleton-init-systemd: work around for /var/lib not populating

[Yann E. MORIN]

From: Trent Piepho <tpiepho@impinj.com>

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.

Signed-off-by: Trent Piepho <tpiepho@impinj.com>
[yann.morin.1998 at free.fr: slight rework of commit title]
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 package/skeleton-init-systemd/skeleton-init-systemd.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
index a2d4e8c4b3..6a0527fde2 100644
--- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
+++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
@@ -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

[Buildroot] [PATCH 1/3] package/skeleton-init-systemd: work around for /var/lib not populating

[Adam Duskett]

Hello;

On Sun, Mar 4, 2018 at 12:06 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> From: Trent Piepho <tpiepho@impinj.com>
>
> 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.
>
> Signed-off-by: Trent Piepho <tpiepho@impinj.com>
> [yann.morin.1998 at free.fr: slight rework of commit title]
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
>  package/skeleton-init-systemd/skeleton-init-systemd.mk | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> index a2d4e8c4b3..6a0527fde2 100644
> --- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
> +++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
> @@ -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
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Tested-by: Adam Duskett <aduskett@gmail.com>

[Buildroot] [PATCH 1/3] package/skeleton-init-systemd: work around for /var/lib not populating

[Thomas Petazzoni]

Hello,

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

On Sun,  4 Mar 2018 18:06:15 +0100, Yann E. MORIN wrote:

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

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

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

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

[Buildroot] [PATCH 1/3] package/skeleton-init-systemd: work around for /var/lib not populating

[Yann E. MORIN]

Thomas, All,

On 2018-03-04 21:24 +0100, Thomas Petazzoni spake thusly:
> Note: the patches have already been applied, and my comments were not
> meant to prevent them from being applied.

Thaks for applying! :-)

> On Sun,  4 Mar 2018 18:06:15 +0100, Yann E. MORIN wrote:
> > 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.
> 
> But annoying. Lots of people are going to wonder what's happening, and
> find them bogus.

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

But later.

> > 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
> 
> What is "C" ? What is needed for this "better solution" ?

    $ 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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/3] package/skeleton-init-systemd: work around for /var/lib not populating

[Trent Piepho]

On Sun, 2018-03-04 at 21:29 +0100, Yann E. MORIN wrote:
> On 2018-03-04 21:24 +0100, Thomas Petazzoni spake thusly:
> > On Sun,  4 Mar 2018 18:06:15 +0100, Yann E. MORIN wrote:
> > > 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.
> > 
> > But annoying. Lots of people are going to wonder what's happening, and
> > find them bogus.
> 
> Yes, I have a few isdeas on how to fix them...

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?

> > > 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
> > 
> > What is "C" ? What is needed for this "better solution" ?
> 
>            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.

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!

[Buildroot] [PATCH 2/3] package/skeleton-systemd: invert factory logic

[Yann E. MORIN]

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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Cc: Trent Piepho <tpiepho@impinj.com>
Cc: Adam Duskett <aduskett@gmail.com>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Peter Korsgaard <peter@korsgaard.com>

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

diff --git a/package/skeleton-init-systemd/skeleton-init-systemd.mk b/package/skeleton-init-systemd/skeleton-init-systemd.mk
index 6a0527fde2..ff64205cbe 100644
--- a/package/skeleton-init-systemd/skeleton-init-systemd.mk
+++ b/package/skeleton-init-systemd/skeleton-init-systemd.mk
@@ -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

[Buildroot] [PATCH 2/3] package/skeleton-systemd: invert factory logic

[Trent Piepho]

On Sun, 2018-03-04 at 18:06 +0100, Yann E. MORIN wrote:
> 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.

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

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

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

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

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.

[Buildroot] [PATCH 3/3] support/tests: enhance the runtime systemd tests

[Yann E. MORIN]

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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 support/testing/tests/init/test_systemd.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/support/testing/tests/init/test_systemd.py b/support/testing/tests/init/test_systemd.py
index 48fac1490f..a324ba8569 100644
--- a/support/testing/tests/init/test_systemd.py
+++ b/support/testing/tests/init/test_systemd.py
@@ -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

[Buildroot] [PATCH 0/3] package/systemd: unbreak a few stuf..

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

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

Indeed - Committed series, thanks!

-- 
Bye, Peter Korsgaard