diff mbox series

[meta-oe] volatile-binds: Calculate the name of the /var/lib service

Message ID 20230827164918.24785-2-sveyret@gmail.com
State New
Headers show
Series [meta-oe] volatile-binds: Calculate the name of the /var/lib service | expand

Commit Message

Stéphane Veyret Aug. 27, 2023, 4:49 p.m. UTC
By default, /var/lib is bind mounted on /var/volatile/lib. If this is
the case, the recipe adds conditions on systemd-random-seed in the
service file mounting it. But as the VOLATILE_BINDS may be modified,
/var/lib may be mounted elsewhere, for example in /persistent/var/lib.
In this case, the conditions are not set because the service file name
does not match expected one.
This patch automatically records the name of the service mounting
/var/lib, if any, in order to set the condition in the appropriate file.

Signed-off-by: Stéphane Veyret <sveyret@gmail.com>
---
 meta/recipes-core/volatile-binds/volatile-binds.bb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Peter Kjellerstedt Aug. 27, 2023, 9:22 p.m. UTC | #1
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Stéphane Veyret
> Sent: den 27 augusti 2023 18:49
> To: openembedded-core@lists.openembedded.org
> Cc: Stéphane Veyret <sveyret@gmail.com>
> Subject: [OE-core] [meta-oe][PATCH] volatile-binds: Calculate the name of the /var/lib service
> 
> By default, /var/lib is bind mounted on /var/volatile/lib. If this is
> the case, the recipe adds conditions on systemd-random-seed in the
> service file mounting it. But as the VOLATILE_BINDS may be modified,
> /var/lib may be mounted elsewhere, for example in /persistent/var/lib.
> In this case, the conditions are not set because the service file name
> does not match expected one.
> This patch automatically records the name of the service mounting
> /var/lib, if any, in order to set the condition in the appropriate file.
> 
> Signed-off-by: Stéphane Veyret <sveyret@gmail.com>
> ---
>  meta/recipes-core/volatile-binds/volatile-binds.bb | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/recipes-core/volatile-binds/volatile-binds.bb b/meta/recipes-core/volatile-binds/volatile-binds.bb
> index 3fefa9abde..7a3f53c3ed 100644
> --- a/meta/recipes-core/volatile-binds/volatile-binds.bb
> +++ b/meta/recipes-core/volatile-binds/volatile-binds.bb
> @@ -48,6 +48,7 @@ do_compile () {
> 
>          servicefile="${spec#/}"
>          servicefile="$(echo "$servicefile" | tr / -).service"

Those two line can be simplified to:

        servicefile="$(echo ${spec#/} | tr / -).service"

> +        [ "${mountpoint}" = /var/lib ] && var_lib_servicefile=$servicefile

You should use "${localstatedir}/lib" instead of "/var/lib". Actually 
${localstatedir} should be used instead of /var throughout the recipe). 
It is also a good idea to avoid && in constructs like the one above 
due to how shell evaluates and sets the exit status. Also avoid ${...} 
for shell variables in bitbake recipes where possible as they will 
otherwise be included in the task hash. All taken together:

        [ "$mountpoint" != ${localstatedir}/lib ] || var_lib_servicefile=$servicefile

>          sed -e "s#@what@#$spec#g; s#@where@#$mountpoint#g" \
>              -e "s#@whatparent@#${spec%/*}#g; s#@whereparent@#${mountpoint%/*}#g" \
>              -e "s#@avoid_overlayfs@#${@d.getVar('AVOID_OVERLAYFS')}#g" \
> @@ -56,12 +57,12 @@ do_compile () {
>  ${@d.getVar('VOLATILE_BINDS').replace("\\n", "\n")}
>  END
> 
> -    if [ -e var-volatile-lib.service ]; then
> +    if [ -n ${var_lib_servicefile} ] && [ -e ${var_lib_servicefile} ]; then

Variables that may be empty shall always be quoted. And when they are, 
you do not need to test that they have a value before using the -e test:

    if [ -e "$var_lib_servicefile" ]; then

>          # As the seed is stored under /var/lib, ensure that this service runs
>          # after the volatile /var/lib is mounted.
>          sed -i -e "/^Before=/s/\$/ systemd-random-seed.service/" \
>                 -e "/^WantedBy=/s/\$/ systemd-random-seed.service/" \
> -               var-volatile-lib.service
> +               ${var_lib_servicefile}

Here too:

               "$var_lib_servicefile"

>      fi
>  }
>  do_compile[dirs] = "${WORKDIR}"
> --
> 2.41.0

//Peter
Stéphane Veyret Aug. 28, 2023, 5:55 p.m. UTC | #2
Thanks, I submitted a v2 patch with the corrections you suggested.
diff mbox series

Patch

diff --git a/meta/recipes-core/volatile-binds/volatile-binds.bb b/meta/recipes-core/volatile-binds/volatile-binds.bb
index 3fefa9abde..7a3f53c3ed 100644
--- a/meta/recipes-core/volatile-binds/volatile-binds.bb
+++ b/meta/recipes-core/volatile-binds/volatile-binds.bb
@@ -48,6 +48,7 @@  do_compile () {
 
         servicefile="${spec#/}"
         servicefile="$(echo "$servicefile" | tr / -).service"
+        [ "${mountpoint}" = /var/lib ] && var_lib_servicefile=$servicefile
         sed -e "s#@what@#$spec#g; s#@where@#$mountpoint#g" \
             -e "s#@whatparent@#${spec%/*}#g; s#@whereparent@#${mountpoint%/*}#g" \
             -e "s#@avoid_overlayfs@#${@d.getVar('AVOID_OVERLAYFS')}#g" \
@@ -56,12 +57,12 @@  do_compile () {
 ${@d.getVar('VOLATILE_BINDS').replace("\\n", "\n")}
 END
 
-    if [ -e var-volatile-lib.service ]; then
+    if [ -n ${var_lib_servicefile} ] && [ -e ${var_lib_servicefile} ]; then
         # As the seed is stored under /var/lib, ensure that this service runs
         # after the volatile /var/lib is mounted.
         sed -i -e "/^Before=/s/\$/ systemd-random-seed.service/" \
                -e "/^WantedBy=/s/\$/ systemd-random-seed.service/" \
-               var-volatile-lib.service
+               ${var_lib_servicefile}
     fi
 }
 do_compile[dirs] = "${WORKDIR}"