[Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[Norbert Lange]

Especially for read-only filesystems it is helpfull to
pre-create all folders for non-volatile paths.

This needs to run under fakeroot to allow setting
uids/gids/perms for the target fs.

systemd-tmpfilesd supports specifiers and target rootfs,
but some specifiers resolve to information from the host,
it is necessary to specially handle (skip) entries that
contain problematic specifiers.

Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
v1->v2
*   add a script to skip/handle specifiers that might
    otherwise take information from the host
v2->v3
*   adopt fakeroot_tmpfiles.sh to current systemd-tmpfilesd (v250),
    several more specifiers are supported directly for our usecase.
---
 package/systemd/fakeroot_tmpfiles.sh | 57 ++++++++++++++++++++++++++++
 package/systemd/systemd.mk           |  9 ++++-
 2 files changed, 64 insertions(+), 2 deletions(-)
 create mode 100644 package/systemd/fakeroot_tmpfiles.sh

diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/systemd/fakeroot_tmpfiles.sh
new file mode 100644
index 0000000000..7e9b02cc0a
--- /dev/null
+++ b/package/systemd/fakeroot_tmpfiles.sh
@@ -0,0 +1,57 @@
+#!/bin/sh
+#
+# The systemd-tmpfiles has the ability to grab information
+# from the filesystem (instead from the running system).
+#
+# However there are a few specifiers that *always* will grab
+# information from the running system examples are %a, %b, %m, %H
+# (Architecture, Boot UUID, Machine UUID, Hostname).
+# 
+# See [1] for historic information.
+#
+# This script will (conservatively) skip tmpfiles lines that have
+# such an specifier to prevent leaking host information.
+#
+# shell expansion is critical to be POSIX compliant,
+# this script wont work with zsh in its default mode for example.
+#
+# The script takes several measures to handle more complex stuff
+# like passing this correctly:
+# f+  "/var/example" - - - - %B\n%o\n%w\n%W%%\n
+#
+# [1] - https://github.com/systemd/systemd/pull/16187
+
+[ -n "${HOST_SYSTEMD_TMPFILES-}" ] ||
+  HOST_SYSTEMD_TMPFILES=systemd-tmpfiles
+
+[ -n "${1-}" -a -d "${1-}"/usr/lib/tmpfiles.d ] ||
+  { echo 1>&2 "$0: need ROOTFS argument"; exit 1; }
+
+${HOST_SYSTEMD_TMPFILES} --no-pager --cat-config --root="$1" |
+  sed -e '/^[[:space:]]*#/d' -e 's,^[[:space:]]*,,' -e '/^$/d' |
+    while read -r line; do
+      # it is allowed to use quotes around arguments,
+      # so let the shell pack the arguments
+      eval "set -- $line"
+
+      # dont output warnings for directories we dont process
+      [ "${2#/dev}" = "${2}" ] && [ "${2#/proc}" = "${2}" ] &&
+        [ "${2#/run}" = "${2}" ] && [ "${2#/sys}" = "${2}" ] &&
+        [ "${2#/tmp}" = "${2}" ] && [ "${2#/mnt}" = "${2}" ] ||
+          continue
+
+      # blank out all specs that are ok to use,
+      # test if some remain. (Specs up to date with v250)
+      if echo "$2 ${7-}" | sed -e 's,%[%BCEgGhLMosStTuUVwW],,g' | grep -v -q '%'; then
+        # no "bad" specifiers, pass the line unmodified
+        eval "printf '%s\n' '$line'"
+      else
+        # warn
+        eval "printf 'ignored spec: %s\n' '$line' 1>&2"
+      fi
+    done |
+\
+TMPDIR= TEMP= TMP= ${HOST_SYSTEMD_TMPFILES} --create --boot --root="$1" \
+  --exclude-prefix=/dev --exclude-prefix=/proc --exclude-prefix=/run --exclude-prefix=/sys \
+  --exclude-prefix=/tmp --exclude-prefix=/mnt \
+  -
diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
index 7bf8018438..e136229c8e 100644
--- a/package/systemd/systemd.mk
+++ b/package/systemd/systemd.mk
@@ -709,10 +709,15 @@ define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
 endef
 SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
 
+define SYSTEMD_CREATE_TMPFILES_HOOK
+	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
+		/bin/sh $(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
+endef
+
 define SYSTEMD_PRESET_ALL
 	$(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
 endef
-SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL
+SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK SYSTEMD_PRESET_ALL
 
 SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV)
 SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV)
@@ -783,7 +788,7 @@ HOST_SYSTEMD_CONF_OPTS = \
 	-Dvconsole=false \
 	-Dquotacheck=false \
 	-Dsysusers=false \
-	-Dtmpfiles=false \
+	-Dtmpfiles=true \
 	-Dimportd=false \
 	-Dhwdb=false \
 	-Drfkill=false \
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[Arnout Vandecappelle]

  Hi Norbert,

  Applied to master, thanks. I have a bunch of stylistic remarks, but it's 
nitpicky so I didn't apply them. The only thing I did is the indentation (4 
spaces in shell scripts).


On 09/01/2022 12:07, Norbert Lange wrote:
[snip]
> diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/systemd/fakeroot_tmpfiles.sh
> new file mode 100644
> index 0000000000..7e9b02cc0a
> --- /dev/null
> +++ b/package/systemd/fakeroot_tmpfiles.sh
> @@ -0,0 +1,57 @@
> +#!/bin/sh
> +#
> +# The systemd-tmpfiles has the ability to grab information
> +# from the filesystem (instead from the running system).
> +#
> +# However there are a few specifiers that *always* will grab
> +# information from the running system examples are %a, %b, %m, %H
> +# (Architecture, Boot UUID, Machine UUID, Hostname).
> +#
> +# See [1] for historic information.
> +#
> +# This script will (conservatively) skip tmpfiles lines that have
> +# such an specifier to prevent leaking host information.

  Instead of skipping, perhaps we should terminate with an error? Or would that 
mean that it never passes with current systemd?

> +#
> +# shell expansion is critical to be POSIX compliant,
> +# this script wont work with zsh in its default mode for example.

  I don't get this... If the script doesn't work with /bin/sh == zsh, maybe use 
/usr/bin/env bash instead? (We already check for bash in dependencies.sh so it's 
fine to require it.)

> +#
> +# The script takes several measures to handle more complex stuff
> +# like passing this correctly:
> +# f+  "/var/example" - - - - %B\n%o\n%w\n%W%%\n
> +#
> +# [1] - https://github.com/systemd/systemd/pull/16187

  The end of that is that Lennart merged something which does (AFAIU) part of 
what you need done. Would it be possible to rebase your changes on top of that?

> +
> +[ -n "${HOST_SYSTEMD_TMPFILES-}" ] ||
> +  HOST_SYSTEMD_TMPFILES=systemd-tmpfiles
> +
> +[ -n "${1-}" -a -d "${1-}"/usr/lib/tmpfiles.d ] ||

  I don't understand why you need "${1-}" and not simply "$1". In this context 
(with the quotes around it) there is no difference at all between the two, right?

  Also, I prefer

ROOTFS="$1"

and use $ROOTFS further down; IMHO that's more readable.


> +  { echo 1>&2 "$0: need ROOTFS argument"; exit 1; }
> +
> +${HOST_SYSTEMD_TMPFILES} --no-pager --cat-config --root="$1" |
> +  sed -e '/^[[:space:]]*#/d' -e 's,^[[:space:]]*,,' -e '/^$/d' |
> +    while read -r line; do
> +      # it is allowed to use quotes around arguments,
> +      # so let the shell pack the arguments
> +      eval "set -- $line"
> +
> +      # dont output warnings for directories we dont process
> +      [ "${2#/dev}" = "${2}" ] && [ "${2#/proc}" = "${2}" ] &&
> +        [ "${2#/run}" = "${2}" ] && [ "${2#/sys}" = "${2}" ] &&
> +        [ "${2#/tmp}" = "${2}" ] && [ "${2#/mnt}" = "${2}" ] ||
> +          continue
> +
> +      # blank out all specs that are ok to use,
> +      # test if some remain. (Specs up to date with v250)
> +      if echo "$2 ${7-}" | sed -e 's,%[%BCEgGhLMosStTuUVwW],,g' | grep -v -q '%'; then
> +        # no "bad" specifiers, pass the line unmodified
> +        eval "printf '%s\n' '$line'"
> +      else
> +        # warn
> +        eval "printf 'ignored spec: %s\n' '$line' 1>&2"
> +      fi
> +    done |
> +\
> +TMPDIR= TEMP= TMP= ${HOST_SYSTEMD_TMPFILES} --create --boot --root="$1" \
> +  --exclude-prefix=/dev --exclude-prefix=/proc --exclude-prefix=/run --exclude-prefix=/sys \
> +  --exclude-prefix=/tmp --exclude-prefix=/mnt \

  All these excludes are already excluded above from the "directories we don't 
process" block, so maybe it can be removed?

  OTOH, if we can later get upstream to no longer have those ignored specs, we 
can remove the preprocessing and it's handy to have the exclude statements already.

  Anyway, I added a comment to the comment block in the beginning to explain why 
they're excluded:

# tmpfs directories (/tmp, /proc, ...) are skipped since they're not
# relevant for the rootfs image.

> +  -
> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
> index 7bf8018438..e136229c8e 100644
> --- a/package/systemd/systemd.mk
> +++ b/package/systemd/systemd.mk
> @@ -709,10 +709,15 @@ define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>   endef
>   SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>   
> +define SYSTEMD_CREATE_TMPFILES_HOOK
> +	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> +		/bin/sh $(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)

  Since the script has a shebang line, I remove the /bin/sh here and made it 
executable.

> +endef
> +
>   define SYSTEMD_PRESET_ALL
>   	$(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
>   endef
> -SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL
> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK SYSTEMD_PRESET_ALL

  We normally add the hook immediately after the definition of the hook, so I 
changed that as well.


  Regards,
  Arnout

  PS There's no need to keep Maxime in Cc, he's no longer involved in Buildroot 
at all.

>   
>   SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV)
>   SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV)
> @@ -783,7 +788,7 @@ HOST_SYSTEMD_CONF_OPTS = \
>   	-Dvconsole=false \
>   	-Dquotacheck=false \
>   	-Dsysusers=false \
> -	-Dtmpfiles=false \
> +	-Dtmpfiles=true \
>   	-Dimportd=false \
>   	-Dhwdb=false \
>   	-Drfkill=false \
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[Arnout Vandecappelle]

On 09/01/2022 12:07, Norbert Lange wrote:
> Especially for read-only filesystems it is helpfull to
> pre-create all folders for non-volatile paths.
> 
> This needs to run under fakeroot to allow setting
> uids/gids/perms for the target fs.
> 
> systemd-tmpfilesd supports specifiers and target rootfs,
> but some specifiers resolve to information from the host,
> it is necessary to specially handle (skip) entries that
> contain problematic specifiers.
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>

  Applied to master, thanks.

  Regards,
  Arnout

> ---
> v1->v2
> *   add a script to skip/handle specifiers that might
>      otherwise take information from the host
> v2->v3
> *   adopt fakeroot_tmpfiles.sh to current systemd-tmpfilesd (v250),
>      several more specifiers are supported directly for our usecase.
> ---
>   package/systemd/fakeroot_tmpfiles.sh | 57 ++++++++++++++++++++++++++++
>   package/systemd/systemd.mk           |  9 ++++-
>   2 files changed, 64 insertions(+), 2 deletions(-)
>   create mode 100644 package/systemd/fakeroot_tmpfiles.sh
> 
> diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/systemd/fakeroot_tmpfiles.sh
> new file mode 100644
> index 0000000000..7e9b02cc0a
> --- /dev/null
> +++ b/package/systemd/fakeroot_tmpfiles.sh
> @@ -0,0 +1,57 @@
> +#!/bin/sh
> +#
> +# The systemd-tmpfiles has the ability to grab information
> +# from the filesystem (instead from the running system).
> +#
> +# However there are a few specifiers that *always* will grab
> +# information from the running system examples are %a, %b, %m, %H
> +# (Architecture, Boot UUID, Machine UUID, Hostname).
> +#
> +# See [1] for historic information.
> +#
> +# This script will (conservatively) skip tmpfiles lines that have
> +# such an specifier to prevent leaking host information.
> +#
> +# shell expansion is critical to be POSIX compliant,
> +# this script wont work with zsh in its default mode for example.
> +#
> +# The script takes several measures to handle more complex stuff
> +# like passing this correctly:
> +# f+  "/var/example" - - - - %B\n%o\n%w\n%W%%\n
> +#
> +# [1] - https://github.com/systemd/systemd/pull/16187
> +
> +[ -n "${HOST_SYSTEMD_TMPFILES-}" ] ||
> +  HOST_SYSTEMD_TMPFILES=systemd-tmpfiles
> +
> +[ -n "${1-}" -a -d "${1-}"/usr/lib/tmpfiles.d ] ||
> +  { echo 1>&2 "$0: need ROOTFS argument"; exit 1; }
> +
> +${HOST_SYSTEMD_TMPFILES} --no-pager --cat-config --root="$1" |
> +  sed -e '/^[[:space:]]*#/d' -e 's,^[[:space:]]*,,' -e '/^$/d' |
> +    while read -r line; do
> +      # it is allowed to use quotes around arguments,
> +      # so let the shell pack the arguments
> +      eval "set -- $line"
> +
> +      # dont output warnings for directories we dont process
> +      [ "${2#/dev}" = "${2}" ] && [ "${2#/proc}" = "${2}" ] &&
> +        [ "${2#/run}" = "${2}" ] && [ "${2#/sys}" = "${2}" ] &&
> +        [ "${2#/tmp}" = "${2}" ] && [ "${2#/mnt}" = "${2}" ] ||
> +          continue
> +
> +      # blank out all specs that are ok to use,
> +      # test if some remain. (Specs up to date with v250)
> +      if echo "$2 ${7-}" | sed -e 's,%[%BCEgGhLMosStTuUVwW],,g' | grep -v -q '%'; then
> +        # no "bad" specifiers, pass the line unmodified
> +        eval "printf '%s\n' '$line'"
> +      else
> +        # warn
> +        eval "printf 'ignored spec: %s\n' '$line' 1>&2"
> +      fi
> +    done |
> +\
> +TMPDIR= TEMP= TMP= ${HOST_SYSTEMD_TMPFILES} --create --boot --root="$1" \
> +  --exclude-prefix=/dev --exclude-prefix=/proc --exclude-prefix=/run --exclude-prefix=/sys \
> +  --exclude-prefix=/tmp --exclude-prefix=/mnt \
> +  -
> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
> index 7bf8018438..e136229c8e 100644
> --- a/package/systemd/systemd.mk
> +++ b/package/systemd/systemd.mk
> @@ -709,10 +709,15 @@ define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>   endef
>   SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>   
> +define SYSTEMD_CREATE_TMPFILES_HOOK
> +	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> +		/bin/sh $(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
> +endef
> +
>   define SYSTEMD_PRESET_ALL
>   	$(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
>   endef
> -SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL
> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK SYSTEMD_PRESET_ALL
>   
>   SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV)
>   SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV)
> @@ -783,7 +788,7 @@ HOST_SYSTEMD_CONF_OPTS = \
>   	-Dvconsole=false \
>   	-Dquotacheck=false \
>   	-Dsysusers=false \
> -	-Dtmpfiles=false \
> +	-Dtmpfiles=true \
>   	-Dimportd=false \
>   	-Dhwdb=false \
>   	-Drfkill=false \
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[Norbert Lange]

Am So., 9. Jan. 2022 um 15:49 Uhr schrieb Arnout Vandecappelle <arnout@mind.be>:
>
>   Hi Norbert,
>
>   Applied to master, thanks. I have a bunch of stylistic remarks, but it's
> nitpicky so I didn't apply them. The only thing I did is the indentation (4
> spaces in shell scripts).
>
>
> On 09/01/2022 12:07, Norbert Lange wrote:
> [snip]
> > diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/systemd/fakeroot_tmpfiles.sh
> > new file mode 100644
> > index 0000000000..7e9b02cc0a
> > --- /dev/null
> > +++ b/package/systemd/fakeroot_tmpfiles.sh
> > @@ -0,0 +1,57 @@
> > +#!/bin/sh
> > +#
> > +# The systemd-tmpfiles has the ability to grab information
> > +# from the filesystem (instead from the running system).
> > +#
> > +# However there are a few specifiers that *always* will grab
> > +# information from the running system examples are %a, %b, %m, %H
> > +# (Architecture, Boot UUID, Machine UUID, Hostname).
> > +#
> > +# See [1] for historic information.
> > +#
> > +# This script will (conservatively) skip tmpfiles lines that have
> > +# such an specifier to prevent leaking host information.
>
>   Instead of skipping, perhaps we should terminate with an error? Or would that
> mean that it never passes with current systemd?

Nope, there are for example directories created for each boot with an boot uuid,
those should be skipped.
systemd-tmpfilesd will create those on the real boot, so its no error,
just something
you cant do ahead of time.

>
> > +#
> > +# shell expansion is critical to be POSIX compliant,
> > +# this script wont work with zsh in its default mode for example.
>
>   I don't get this... If the script doesn't work with /bin/sh == zsh, maybe use
> /usr/bin/env bash instead? (We already check for bash in dependencies.sh so it's
> fine to require it.)

Sorry, worded badly. the script should run with any shell, as they will use a
POSIX compliant mode if run as /bin/sh.
You cant copy paste the lines into a zsh terminal, as then it would not work
due to some differences in parameter expansion.

>
> > +#
> > +# The script takes several measures to handle more complex stuff
> > +# like passing this correctly:
> > +# f+  "/var/example" - - - - %B\n%o\n%w\n%W%%\n
> > +#
> > +# [1] - https://github.com/systemd/systemd/pull/16187
>
>   The end of that is that Lennart merged something which does (AFAIU) part of
> what you need done. Would it be possible to rebase your changes on top of that?

That's more or less what I did with v3 of the buildroot patch. Lennart
did his own
Implementation of handling a staged rootfs. The things he did not
include, he did
so on purpose (as noted in the PR).

Notably, you will end up creating some directories containing the boot UUID
of your running system.

Filtering out the offending lines seems to me the smallest amount of pain.

>
> > +
> > +[ -n "${HOST_SYSTEMD_TMPFILES-}" ] ||
> > +  HOST_SYSTEMD_TMPFILES=systemd-tmpfiles
> > +
> > +[ -n "${1-}" -a -d "${1-}"/usr/lib/tmpfiles.d ] ||
>
>   I don't understand why you need "${1-}" and not simply "$1". In this context
> (with the quotes around it) there is no difference at all between the two, right?

Yes, I typically write my scripts to not fault on "set -e". No
difference if this mode is not enabled.

>
>   Also, I prefer
>
> ROOTFS="$1"
>
> and use $ROOTFS further down; IMHO that's more readable.

Yeah, its a reasonably simple script tho ;)

>
>
> > +  { echo 1>&2 "$0: need ROOTFS argument"; exit 1; }
> > +
> > +${HOST_SYSTEMD_TMPFILES} --no-pager --cat-config --root="$1" |
> > +  sed -e '/^[[:space:]]*#/d' -e 's,^[[:space:]]*,,' -e '/^$/d' |
> > +    while read -r line; do
> > +      # it is allowed to use quotes around arguments,
> > +      # so let the shell pack the arguments
> > +      eval "set -- $line"
> > +
> > +      # dont output warnings for directories we dont process
> > +      [ "${2#/dev}" = "${2}" ] && [ "${2#/proc}" = "${2}" ] &&
> > +        [ "${2#/run}" = "${2}" ] && [ "${2#/sys}" = "${2}" ] &&
> > +        [ "${2#/tmp}" = "${2}" ] && [ "${2#/mnt}" = "${2}" ] ||
> > +          continue
> > +
> > +      # blank out all specs that are ok to use,
> > +      # test if some remain. (Specs up to date with v250)
> > +      if echo "$2 ${7-}" | sed -e 's,%[%BCEgGhLMosStTuUVwW],,g' | grep -v -q '%'; then
> > +        # no "bad" specifiers, pass the line unmodified
> > +        eval "printf '%s\n' '$line'"
> > +      else
> > +        # warn
> > +        eval "printf 'ignored spec: %s\n' '$line' 1>&2"
> > +      fi
> > +    done |
> > +\
> > +TMPDIR= TEMP= TMP= ${HOST_SYSTEMD_TMPFILES} --create --boot --root="$1" \
> > +  --exclude-prefix=/dev --exclude-prefix=/proc --exclude-prefix=/run --exclude-prefix=/sys \
> > +  --exclude-prefix=/tmp --exclude-prefix=/mnt \
>
>   All these excludes are already excluded above from the "directories we don't
> process" block, so maybe it can be removed?

Yep. doesn't hurt having them tho

>
>   OTOH, if we can later get upstream to no longer have those ignored specs, we
> can remove the preprocessing and it's handy to have the exclude statements already.

upstream does just do the wrong thing *for us* on those specs and its
unlikely to change,
the way it is now makes sense for running in containers.

>   Anyway, I added a comment to the comment block in the beginning to explain why
> they're excluded:
>
> # tmpfs directories (/tmp, /proc, ...) are skipped since they're not
> # relevant for the rootfs image.
>
> > +  -
> > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
> > index 7bf8018438..e136229c8e 100644
> > --- a/package/systemd/systemd.mk
> > +++ b/package/systemd/systemd.mk
> > @@ -709,10 +709,15 @@ define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
> >   endef
> >   SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
> >
> > +define SYSTEMD_CREATE_TMPFILES_HOOK
> > +     HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> > +             /bin/sh $(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
>
>   Since the script has a shebang line, I remove the /bin/sh here and made it
> executable.

Sure

>
> > +endef
> > +
> >   define SYSTEMD_PRESET_ALL
> >       $(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
> >   endef
> > -SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL
> > +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK SYSTEMD_PRESET_ALL
>
>   We normally add the hook immediately after the definition of the hook, so I
> changed that as well.

Ok

>   Regards,
>   Arnout
>
>   PS There's no need to keep Maxime in Cc, he's no longer involved in Buildroot
> at all.
>
> >
> >   SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV)
> >   SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV)
> > @@ -783,7 +788,7 @@ HOST_SYSTEMD_CONF_OPTS = \
> >       -Dvconsole=false \
> >       -Dquotacheck=false \
> >       -Dsysusers=false \
> > -     -Dtmpfiles=false \
> > +     -Dtmpfiles=true \
> >       -Dimportd=false \
> >       -Dhwdb=false \
> >       -Drfkill=false \
> >

Maybe its easier with an example, these are the lines I get when
running buildroot:

PATH="/tmp/ZBuild/host/bin:/tmp/ZBuild/host/sbin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
FAKEROOTDONTTRYCHOWN=1 /tmp/ZBuild/host/bin/fakeroot --
/tmp/ZBuild/build/buildroot-fs/tar/fakeroot
rootdir=/tmp/ZBuild/build/buildroot-fs/tar/target
table='/tmp/ZBuild/build/buildroot-fs/full_devices_table.txt'
ignored spec: h /var/log/journal/%m - - - - +C
ignored spec: x /var/tmp/systemd-private-%b-*
ignored spec: X /var/tmp/systemd-private-%b-*/tmp
ignored spec: x  /var/lib/systemd/coredump/.#core*.%b*
ignored spec: z /var/log/journal/%m 2755 root systemd-journal - -
ignored spec: z /var/log/journal/%m/system.journal 0640 root systemd-journal - -


Norbert
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[yann.morin]

Norbert, All,

On 2022-01-09 12:07 +0100, Norbert Lange spake thusly:
> Especially for read-only filesystems it is helpfull to
> pre-create all folders for non-volatile paths.

I'm revisiting this old change of yours, and I have some issues
understanding the original issue this is supposed to fix...

Indeed, for read-only filesystems, we do generate a factory for /var.
This code is guarded by a condition on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW,
so indeed, if BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is set, we do not
create the factory. But if it is set, then we do create the factory:

  - move /var away to /usr/share/factory/var
  - register its content as a tmpfiles.d:
    https://github.com/buildroot/buildroot/blob/master/package/skeleton-init-systemd/skeleton-init-systemd.mk#L37

  - mount a tmpfs on /var:
    https://github.com/buildroot/buildroot/blob/master/package/skeleton-init-systemd/skeleton-init-systemd.mk#L31

Now with this change of yours, /var is no longer empty in the generated
filesystem, and this is in fact useless because a tmpfs is mounted over
anyway.

I've just tried reverting this change, and my /var is still correctly
populated at boot.

What was your original issue that prompted running tmpfiles at build
time? Did you have BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW still enabled by
chance?

(Hint: I think we should just hide away read-only filesystems when
BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is set).

Regards,
Yann E. MORIN.

> This needs to run under fakeroot to allow setting
> uids/gids/perms for the target fs.
> 
> systemd-tmpfilesd supports specifiers and target rootfs,
> but some specifiers resolve to information from the host,
> it is necessary to specially handle (skip) entries that
> contain problematic specifiers.
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> v1->v2
> *   add a script to skip/handle specifiers that might
>     otherwise take information from the host
> v2->v3
> *   adopt fakeroot_tmpfiles.sh to current systemd-tmpfilesd (v250),
>     several more specifiers are supported directly for our usecase.
> ---
>  package/systemd/fakeroot_tmpfiles.sh | 57 ++++++++++++++++++++++++++++
>  package/systemd/systemd.mk           |  9 ++++-
>  2 files changed, 64 insertions(+), 2 deletions(-)
>  create mode 100644 package/systemd/fakeroot_tmpfiles.sh
> 
> diff --git a/package/systemd/fakeroot_tmpfiles.sh b/package/systemd/fakeroot_tmpfiles.sh
> new file mode 100644
> index 0000000000..7e9b02cc0a
> --- /dev/null
> +++ b/package/systemd/fakeroot_tmpfiles.sh
> @@ -0,0 +1,57 @@
> +#!/bin/sh
> +#
> +# The systemd-tmpfiles has the ability to grab information
> +# from the filesystem (instead from the running system).
> +#
> +# However there are a few specifiers that *always* will grab
> +# information from the running system examples are %a, %b, %m, %H
> +# (Architecture, Boot UUID, Machine UUID, Hostname).
> +# 
> +# See [1] for historic information.
> +#
> +# This script will (conservatively) skip tmpfiles lines that have
> +# such an specifier to prevent leaking host information.
> +#
> +# shell expansion is critical to be POSIX compliant,
> +# this script wont work with zsh in its default mode for example.
> +#
> +# The script takes several measures to handle more complex stuff
> +# like passing this correctly:
> +# f+  "/var/example" - - - - %B\n%o\n%w\n%W%%\n
> +#
> +# [1] - https://github.com/systemd/systemd/pull/16187
> +
> +[ -n "${HOST_SYSTEMD_TMPFILES-}" ] ||
> +  HOST_SYSTEMD_TMPFILES=systemd-tmpfiles
> +
> +[ -n "${1-}" -a -d "${1-}"/usr/lib/tmpfiles.d ] ||
> +  { echo 1>&2 "$0: need ROOTFS argument"; exit 1; }
> +
> +${HOST_SYSTEMD_TMPFILES} --no-pager --cat-config --root="$1" |
> +  sed -e '/^[[:space:]]*#/d' -e 's,^[[:space:]]*,,' -e '/^$/d' |
> +    while read -r line; do
> +      # it is allowed to use quotes around arguments,
> +      # so let the shell pack the arguments
> +      eval "set -- $line"
> +
> +      # dont output warnings for directories we dont process
> +      [ "${2#/dev}" = "${2}" ] && [ "${2#/proc}" = "${2}" ] &&
> +        [ "${2#/run}" = "${2}" ] && [ "${2#/sys}" = "${2}" ] &&
> +        [ "${2#/tmp}" = "${2}" ] && [ "${2#/mnt}" = "${2}" ] ||
> +          continue
> +
> +      # blank out all specs that are ok to use,
> +      # test if some remain. (Specs up to date with v250)
> +      if echo "$2 ${7-}" | sed -e 's,%[%BCEgGhLMosStTuUVwW],,g' | grep -v -q '%'; then
> +        # no "bad" specifiers, pass the line unmodified
> +        eval "printf '%s\n' '$line'"
> +      else
> +        # warn
> +        eval "printf 'ignored spec: %s\n' '$line' 1>&2"
> +      fi
> +    done |
> +\
> +TMPDIR= TEMP= TMP= ${HOST_SYSTEMD_TMPFILES} --create --boot --root="$1" \
> +  --exclude-prefix=/dev --exclude-prefix=/proc --exclude-prefix=/run --exclude-prefix=/sys \
> +  --exclude-prefix=/tmp --exclude-prefix=/mnt \
> +  -
> diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk
> index 7bf8018438..e136229c8e 100644
> --- a/package/systemd/systemd.mk
> +++ b/package/systemd/systemd.mk
> @@ -709,10 +709,15 @@ define SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>  endef
>  SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_RM_CATALOG_UPDATE_SERVICE
>  
> +define SYSTEMD_CREATE_TMPFILES_HOOK
> +	HOST_SYSTEMD_TMPFILES=$(HOST_DIR)/bin/systemd-tmpfiles \
> +		/bin/sh $(SYSTEMD_PKGDIR)/fakeroot_tmpfiles.sh $(TARGET_DIR)
> +endef
> +
>  define SYSTEMD_PRESET_ALL
>  	$(HOST_DIR)/bin/systemctl --root=$(TARGET_DIR) preset-all
>  endef
> -SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL
> +SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_CREATE_TMPFILES_HOOK SYSTEMD_PRESET_ALL
>  
>  SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV)
>  SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV)
> @@ -783,7 +788,7 @@ HOST_SYSTEMD_CONF_OPTS = \
>  	-Dvconsole=false \
>  	-Dquotacheck=false \
>  	-Dsysusers=false \
> -	-Dtmpfiles=false \
> +	-Dtmpfiles=true \
>  	-Dimportd=false \
>  	-Dhwdb=false \
>  	-Drfkill=false \
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
| +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
|      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[yann.morin]

Norbert,All,

On 2022-07-04 08:41 +0200, MORIN Yann INNOV/IT-S spake thusly:
> On 2022-01-09 12:07 +0100, Norbert Lange spake thusly:
> > Especially for read-only filesystems it is helpfull to
> > pre-create all folders for non-volatile paths.
> I'm revisiting this old change of yours, and I have some issues
> understanding the original issue this is supposed to fix...
> 
> Indeed, for read-only filesystems, we do generate a factory for /var.
> This code is guarded by a condition on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW,
> so indeed, if BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is set, we do not
> create the factory. But if it is set, then we do create the factory:

*if it is _unset_, then we do create the factory.

>   - move /var away to /usr/share/factory/var
>   - register its content as a tmpfiles.d:
>     https://github.com/buildroot/buildroot/blob/master/package/skeleton-init-systemd/skeleton-init-systemd.mk#L37

Note: this tmpfiles.d is currently stored in /etc/tmpfiles.d, but it
really belong to /usr/lib/tmpfiles.d (to later allow for an empty /etc
too).

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
| +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
|      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[Norbert Lange]

[-- Attachment #1: Type: text/plain, Size: 2502 bytes --]

Am Mo., 4. Juli 2022 um 08:46 Uhr schrieb <yann.morin@orange.com>:
>
> Norbert,All,
>
> On 2022-07-04 08:41 +0200, MORIN Yann INNOV/IT-S spake thusly:
> > On 2022-01-09 12:07 +0100, Norbert Lange spake thusly:
> > > Especially for read-only filesystems it is helpfull to
> > > pre-create all folders for non-volatile paths.
> > I'm revisiting this old change of yours, and I have some issues
> > understanding the original issue this is supposed to fix...

Often if a directory is readonly, the service will still start (and
have limited functionality),
but if its missing it wont be even attempted or fails early, taking
down more services that depend on it.

> >
> > Indeed, for read-only filesystems, we do generate a factory for /var.
> > This code is guarded by a condition on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW,
> > so indeed, if BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is set, we do not
> > create the factory. But if it is set, then we do create the factory:
>
> *if it is _unset_, then we do create the factory.

Yeah, and this is an insanely fragile method, which I brought up
multiple times (see [1]),
and prevents other solutions that work above or below your initsystem.

Some versions I personally cooked up:

-   precreate the directories, or add some symlinks to /tmp.
    (I got a debian USB Setup/Rescue stick with Gnome running that way)
-   deal with the issue in the initramfs (mount tmpf/ overlayfs)
-   add a systemd unit + mount creating the overlayfs.

Any of these are just as viable and more robust than that weird
tmpfiles solution,
why is that one forced down to be the only one?

Look for example on the BR2_PACKAGE_SYSTEMD_CATALOGDB config
which depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW to
avoid a huge mess.

>
> >   - move /var away to /usr/share/factory/var
> >   - register its content as a tmpfiles.d:
> >     https://github.com/buildroot/buildroot/blob/master/package/skeleton-init-systemd/skeleton-init-systemd.mk#L37
>
> Note: this tmpfiles.d is currently stored in /etc/tmpfiles.d, but it
> really belong to /usr/lib/tmpfiles.d (to later allow for an empty /etc
> too).

The whole feature belongs behind its own option, or even better: own package.

I attached an archive implementing the systemd unit + mount. Some
equivalent would be possible
for other init systems. You wont need ANY complicated trickery that way,
just unpack that into /usr/lib/systemd/system

Regards, Norbert

[1] - http://lists.busybox.net/pipermail/buildroot/2020-July/287016.html

[-- Attachment #2: rootvar.tar.xz --]
[-- Type: application/x-xz, Size: 680 bytes --]

[-- Attachment #3: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[yann.morin]

Norbert, All,

On 2022-07-04 13:05 +0200, Norbert Lange spake thusly:
> Am Mo., 4. Juli 2022 um 08:46 Uhr schrieb <yann.morin@orange.com>:
> > On 2022-07-04 08:41 +0200, MORIN Yann INNOV/IT-S spake thusly:
> > > On 2022-01-09 12:07 +0100, Norbert Lange spake thusly:
> > > > Especially for read-only filesystems it is helpfull to
> > > > pre-create all folders for non-volatile paths.
> > > I'm revisiting this old change of yours, and I have some issues
> > > understanding the original issue this is supposed to fix...
> Often if a directory is readonly, the service will still start (and
> have limited functionality),
> but if its missing it wont be even attempted or fails early, taking
> down more services that depend on it.

But again, what exactly is broken?

Our var factory is supposed to make that working, so if it is broken,
I'd rather that it is fixed (somehow), rather than some orthogonal
change be implemented. Can you provide an actual defconfig that exhibits
the issue, that the factory does not solve?

And as I explained: with the factory, the entries added in /var are
anyway totally useless because we anyway mount a tmpfs on there. So, in
a sense, this change conflicts with the var factory.

And from http://0pointer.de/blog/projects/stateless.html, I understand
that it it is totally expected that one may want to mount a filesystem,
like a tmpfs, on /var, so what we are doing is perfectly legit.

And as I (my non-work alterego) explained in 26085bbbd500 (system: make
systemd work on a read-only rootfs), people who want persistent /var can
change the fstab (but see below) to something else, to have /var an
actual filesystem.

So, I still do no tunderstand the initial issue your change was trying
to fix...

> > > Indeed, for read-only filesystems, we do generate a factory for /var.
> > > This code is guarded by a condition on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW,
> > > so indeed, if BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is set, we do not
> > > create the factory. But if it is set, then we do create the factory:
> > *if it is _unset_, then we do create the factory.
> Yeah, and this is an insanely fragile method, which I brought up
> multiple times (see [1]),
> and prevents other solutions that work above or below your initsystem.

I agree that BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is fragile. However,
we do not have a bette rsolution to know whether /var is on a r/w
filesystem, or if we need to explicitly mount one there.

We should also expand the help entry for BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
and add some explanations in the manual, probably.

> Some versions I personally cooked up:
> -   precreate the directories, or add some symlinks to /tmp.
>     (I got a debian USB Setup/Rescue stick with Gnome running that way)
> -   deal with the issue in the initramfs (mount tmpf/ overlayfs)
> -   add a systemd unit + mount creating the overlayfs.
> 
> Any of these are just as viable and more robust than that weird
> tmpfiles solution,
> why is that one forced down to be the only one?

First it is not the only one. But we need something, and that is
something minimalist. It is not covering 100% cases, but that is not
possible in a generic way.

Buildroto is all about providing a working base system, that user can
extend and customise at will. But the premises is that it shold work out
of the box.

> Look for example on the BR2_PACKAGE_SYSTEMD_CATALOGDB config
> which depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW to
> avoid a huge mess.

Sorry, but I do not understand what the issue here is. If there are
conflicts with tmpfiles.d entries, we need to find a solution, rather
than hide them under the rug...

> > >   - move /var away to /usr/share/factory/var
> > >   - register its content as a tmpfiles.d:
> > >     https://github.com/buildroot/buildroot/blob/master/package/skeleton-init-systemd/skeleton-init-systemd.mk#L37
> >
> > Note: this tmpfiles.d is currently stored in /etc/tmpfiles.d, but it
> > really belong to /usr/lib/tmpfiles.d (to later allow for an empty /etc
> > too).
> The whole feature belongs behind its own option, or even better: own package.

Sorry, but we need a way to provide a r/w /var by default, for those
packages that expect it to be, and that are not systemd-aware, but do
install stuff in /var at build-time. I don't see how that can be done if
the factory is moved to its own package...

I also don't see why it should be a separate option either. Indeed, on
aread-only rootfs (either by design, like squashfs, or by configuration,
like passing 'ro' on the kernel command line), we still need a r/w /var.

So the best option is to correlate it to BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
even if it feels a bit kludgy...

> I attached an archive implementing the systemd unit + mount. Some
> equivalent would be possible
> for other init systems. You wont need ANY complicated trickery that way,
> just unpack that into /usr/lib/systemd/system

Ah, I see that you switched to var.mout. This is *exactly* the change I
am doing right now, and it is working very nice. Here's my var.mount:

    # SPDX-License-Identifier: LGPL-2.1-or-later
    # Modelled after systemd's tmp.mount

    [Unit]
    Description=Buildroot /var tmpfs
    DefaultDependencies=no
    Conflicts=umount.target
    Before=basic.target local-fs.target umount.target systemd-tmpfiles-setup.service
    After=swap.target

    [Mount]
    What=tmpfs
    Where=/var
    Type=tmpfs
    Options=mode=1777,strictatime,nosuid,nodev,size=50%%,nr_inodes=1m

    [Install]
    WantedBy=basic.target

I can see a few lines that yours has that could be interesting to add
in there, like (but why is that latter one commented out?):
  - ConditionPathIsSymbolicLink=!/var and
  - ConditionPathIsReadWrite=!/var

(we could also maybe use ConditionPathIsMountPoint=!/var)

(also note that systemd will complain that /var is not empty, but will
still proceed to mount a tmpfs on /var anyway).

Thus, people that want to moun tsomething else can ovide a drop-in to
complement our var.mount, or even provide their own that overrides ours.

With my var.mount, and with this change reverted, I validate that all
the processes running on my system do have /var mounted:

    # for i in /proc/*/mountinfo; do grep -q ' /var ' ${i} || echo ${i}; done

The only such process is the one running [kdevtmpfs], but that is a
kernel thread that is running for the /dev devtmpfs mount, and thus is
started before the init process is, and the factory is workign nicely.

I'd be interested to see how the systemd catalogs are broken, though,
and see how we can unbreak them.

Regards,
Yann E. MORIN.

> Regards, Norbert
> 
> [1] - http://lists.busybox.net/pipermail/buildroot/2020-July/287016.html

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
| +33 534.541.179 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
|      yann.morin (at) orange.com      |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3] package/systemd: invoke systemd-tmpfilesd on final image

[Norbert Lange]

Am Mo., 4. Juli 2022 um 14:07 Uhr schrieb <yann.morin@orange.com>:
>
> Norbert, All,
>
> On 2022-07-04 13:05 +0200, Norbert Lange spake thusly:
> > Am Mo., 4. Juli 2022 um 08:46 Uhr schrieb <yann.morin@orange.com>:
> > > On 2022-07-04 08:41 +0200, MORIN Yann INNOV/IT-S spake thusly:
> > > > On 2022-01-09 12:07 +0100, Norbert Lange spake thusly:
> > > > > Especially for read-only filesystems it is helpfull to
> > > > > pre-create all folders for non-volatile paths.
> > > > I'm revisiting this old change of yours, and I have some issues
> > > > understanding the original issue this is supposed to fix...
> > Often if a directory is readonly, the service will still start (and
> > have limited functionality),
> > but if its missing it wont be even attempted or fails early, taking
> > down more services that depend on it.
>
> But again, what exactly is broken?

With this change you can boot into a filesystem even without mounting
a tmpfs on /var.

>
> Our var factory is supposed to make that working, so if it is broken,
> I'd rather that it is fixed (somehow), rather than some orthogonal
> change be implemented. Can you provide an actual defconfig that exhibits
> the issue, that the factory does not solve?

Your factory is not a good solution. I can easily cook up a package with
tmpfiles.d that break it.
Your factory interferes with the solution above to just leave /var alone,
pre-create a few directories and have the system boot (albeit in a
reduced state).

>
> And as I explained: with the factory, the entries added in /var are
> anyway totally useless because we anyway mount a tmpfs on there. So, in
> a sense, this change conflicts with the var factory.

Yeah... but I consider the var factory the problem. Other than "been that way",
is there any argument for this solution?

>
> And from http://0pointer.de/blog/projects/stateless.html, I understand
> that it it is totally expected that one may want to mount a filesystem,
> like a tmpfs, on /var, so what we are doing is perfectly legit.

Yes, but that should haven in the initramfs, there is even code for that in
systemd which mounts an overlayfs on /. Again, your factory runs opposite to
the usually approach to use an overlay.

Besides the argument in the article above would be that you *should not*
have to copy stuff in tmpfiles.d to make it work, but the services
take care of that.

>
> And as I (my non-work alterego) explained in 26085bbbd500 (system: make
> systemd work on a read-only rootfs), people who want persistent /var can
> change the fstab (but see below) to something else, to have /var an
> actual filesystem.
>
> So, I still do no tunderstand the initial issue your change was trying
> to fix...

Well, I thought

"Buildroot is all about providing a working base system, that user can
extend and customise at will."

then I wanted Buildroot to create me the rootfs I can build an erofs from,
mount that and an overlayfs in the initramfs.
Even if the partition holding the overlay would be corrupt I could boot and
ssh into a system. That means having the directories in /var existing,
cause services might bail if not.

So there is no issue  other than finding it more versatile if

-   work is done upfront
-   the rootfs is usable in more context

Note that I have to *enable* REMOUNT_ROOTFS_RW to build a
read-only rootfs.

>
> > > > Indeed, for read-only filesystems, we do generate a factory for /var.
> > > > This code is guarded by a condition on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW,
> > > > so indeed, if BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is set, we do not
> > > > create the factory. But if it is set, then we do create the factory:
> > > *if it is _unset_, then we do create the factory.
> > Yeah, and this is an insanely fragile method, which I brought up
> > multiple times (see [1]),
> > and prevents other solutions that work above or below your initsystem.
>
> I agree that BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW is fragile. However,
> we do not have a bette rsolution to know whether /var is on a r/w
> filesystem, or if we need to explicitly mount one there.

Nope, I meant your specific lets move all stuff into tmpfiles.d
is fragile. Not having an option for that.
I would expect REMOUNT_ROOTFS_RW to *only* effect writing the
/etc/fstab file.

Then have *another* option to mess with buildroots tmpfiles.

>
> We should also expand the help entry for BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
> and add some explanations in the manual, probably.
>
> > Some versions I personally cooked up:
> > -   precreate the directories, or add some symlinks to /tmp.
> >     (I got a debian USB Setup/Rescue stick with Gnome running that way)
> > -   deal with the issue in the initramfs (mount tmpf/ overlayfs)
> > -   add a systemd unit + mount creating the overlayfs.
> >
> > Any of these are just as viable and more robust than that weird
> > tmpfiles solution,
> > why is that one forced down to be the only one?
>
> First it is not the only one. But we need something, and that is
> something minimalist. It is not covering 100% cases, but that is not
> possible in a generic way.
>
> Buildroto is all about providing a working base system, that user can
> extend and customise at will. But the premises is that it shold work out
> of the box.

Yeah, the pain is purely at the guys trying to extend buildroot
without causing a mess with that solution ;)

>
> > Look for example on the BR2_PACKAGE_SYSTEMD_CATALOGDB config
> > which depends on BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW to
> > avoid a huge mess.
>
> Sorry, but I do not understand what the issue here is. If there are
> conflicts with tmpfiles.d entries, we need to find a solution, rather
> than hide them under the rug...

The issue was that symlinks arent handled correctly, some years ago I
knew more about the problem.
hiding under the rug is the move to tmpfiles.d solution that
causes those problems. And the only sane solution would
be to replace it with something more robust IMHO

>
> > > >   - move /var away to /usr/share/factory/var
> > > >   - register its content as a tmpfiles.d:
> > > >     https://github.com/buildroot/buildroot/blob/master/package/skeleton-init-systemd/skeleton-init-systemd.mk#L37
> > >
> > > Note: this tmpfiles.d is currently stored in /etc/tmpfiles.d, but it
> > > really belong to /usr/lib/tmpfiles.d (to later allow for an empty /etc
> > > too).
> > The whole feature belongs behind its own option, or even better: own package.
>
> Sorry, but we need a way to provide a r/w /var by default, for those
> packages that expect it to be, and that are not systemd-aware, but do
> install stuff in /var at build-time. I don't see how that can be done if
> the factory is moved to its own package...
>
> I also don't see why it should be a separate option either. Indeed, on
> aread-only rootfs (either by design, like squashfs, or by configuration,
> like passing 'ro' on the kernel command line), we still need a r/w /var.

It should be an option to to allow for example - mounting an overlayfs -
without all the fallout that moving files around entails.

Do you expect users to be happy if things suddenly break when they
are using their own tmpfiles.d that happen to have unlucky sideeffects,
showing up only after disabling REMOUNT_ROOTFS_RW.

Again you are moving around stuff, copying back in random order
interleaved with operations from buildroot packages and user configs.

>
> So the best option is to correlate it to BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
> even if it feels a bit kludgy...

Heck no. The best option would be to use an overlay, and also have an option
to disable everything outside adding an entry in /etc/fstab.

Unless you want to artificially restrict how Buildroot's can be used,
initramfs and container services can perfectly fine make an read-only
image usable.

>
> > I attached an archive implementing the systemd unit + mount. Some
> > equivalent would be possible
> > for other init systems. You wont need ANY complicated trickery that way,
> > just unpack that into /usr/lib/systemd/system
>
> Ah, I see that you switched to var.mout. This is *exactly* the change I
> am doing right now, and it is working very nice. Here's my var.mount:

its important to do it that way, so mounts are correctly ordered.

>
>     # SPDX-License-Identifier: LGPL-2.1-or-later
>     # Modelled after systemd's tmp.mount
>
>     [Unit]
>     Description=Buildroot /var tmpfs
>     DefaultDependencies=no
>     Conflicts=umount.target
>     Before=basic.target local-fs.target umount.target systemd-tmpfiles-setup.service
>     After=swap.target
>
>     [Mount]
>     What=tmpfs
>     Where=/var
>     Type=tmpfs
>     Options=mode=1777,strictatime,nosuid,nodev,size=50%%,nr_inodes=1m
>
>     [Install]
>     WantedBy=basic.target
>
> I can see a few lines that yours has that could be interesting to add
> in there, like (but why is that latter one commented out?):
>   - ConditionPathIsSymbolicLink=!/var and
>   - ConditionPathIsReadWrite=!/var

Cause the check doesn't do what I want, it seems to go only
on inode permissions. Means if you mount the image read-only
(or use squashfs/erofs) and have the directory "writable" the service wont run

>
> (we could also maybe use ConditionPathIsMountPoint=!/var)
>
> (also note that systemd will complain that /var is not empty, but will
> still proceed to mount a tmpfs on /var anyway).

All trickery and pain can be avoided by using an overlay.
(Wish I could Jedi-mind-trick you)

>
> Thus, people that want to moun tsomething else can ovide a drop-in to
> complement our var.mount, or even provide their own that overrides ours.
>
> With my var.mount, and with this change reverted, I validate that all
> the processes running on my system do have /var mounted:
>
>     # for i in /proc/*/mountinfo; do grep -q ' /var ' ${i} || echo ${i}; done
>
> The only such process is the one running [kdevtmpfs], but that is a
> kernel thread that is running for the /dev devtmpfs mount, and thus is
> started before the init process is, and the factory is workign nicely.
>
> I'd be interested to see how the systemd catalogs are broken, though,
> and see how we can unbreak them.

you shouldn't move the files to the factory, cause the service has its
own tmpfiles.d to create them.

Here lies your crux:

-  You have services that can't deal with an empty /var
-  You have services that can deal with an empty /var and chances are
good that your tmpfiles trickery breaks them now or in the future.
-  You have users that can drop in both, so even if you manage to
if-but your script to fix the packages in buildroot, you cant if-but
all usages

You real want to use overlays *Waves hand*

Regards,
Norbert
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot