diff mbox series

systemd: fix journald persistency when VOLATILE_LOG_DIR is false

Message ID 20241024-systemd-create-log-dirs-v1-1-23cba4a734ab@geanix.com
State New
Headers show
Series systemd: fix journald persistency when VOLATILE_LOG_DIR is false | expand

Commit Message

Esben Haabendal Oct. 24, 2024, 11:41 a.m. UTC
Commit 18d46e11d85d ("systemd: fix dead link /var/log/README") broke the
default systemd behavior when VOLATILE_LOG_DIR is set to false.

By not creating the /var/log/journal directory, the default configured
systemd (Storage=auto) will silently revert to just doing volatile
logging (/run/journal).

The problem fixed with the above mentioned commit was specifically related
to configurations where VOLATILE_LOG_DIR is true, in which case it should
be perfectly fine to not create the /var/log/journal directory, so we
should be able to keep the fix in place, while not breaking behavior for
non VOLATILE_LOG_DIR configurations.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 meta/recipes-core/systemd/systemd_256.6.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: 8278d82d8203212bb159eb3805d4a5617c5370df
change-id: 20241024-systemd-create-log-dirs-6f74021fc529

Best regards,

Comments

Richard Purdie Oct. 24, 2024, 12:58 p.m. UTC | #1
On Thu, 2024-10-24 at 13:41 +0200, Esben Haabendal via lists.openembedded.org wrote:
> Commit 18d46e11d85d ("systemd: fix dead link /var/log/README") broke the
> default systemd behavior when VOLATILE_LOG_DIR is set to false.
> 
> By not creating the /var/log/journal directory, the default configured
> systemd (Storage=auto) will silently revert to just doing volatile
> logging (/run/journal).
> 
> The problem fixed with the above mentioned commit was specifically related
> to configurations where VOLATILE_LOG_DIR is true, in which case it should
> be perfectly fine to not create the /var/log/journal directory, so we
> should be able to keep the fix in place, while not breaking behavior for
> non VOLATILE_LOG_DIR configurations.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
>  meta/recipes-core/systemd/systemd_256.6.bb | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-core/systemd/systemd_256.6.bb b/meta/recipes-core/systemd/systemd_256.6.bb
> index 68f15ab065dd5985e4e5cefc2cd005dba5ac9c7f..88ee0c8a7f89726fc9be489f3fc419f9feb37251 100644
> --- a/meta/recipes-core/systemd/systemd_256.6.bb
> +++ b/meta/recipes-core/systemd/systemd_256.6.bb
> @@ -254,7 +254,7 @@ EXTRA_OEMESON += "-Dnobody-user=nobody \
>                    -Dsystem-uid-max=999 \
>                    -Dsystem-alloc-gid-min=101 \
>                    -Dsystem-gid-max=999 \
> -                  -Dcreate-log-dirs=false \
> +                  ${@ '-Dcreate-log-dirs=false' if oe.types.boolean(d.getVar('VOLATILE_LOG_DIR')) else '' } \
>                    ${@bb.utils.contains('DISTRO_FEATURES', 'zeroconf', '-Ddefault-mdns=no -Ddefault-llmnr=no', '', d)} \
>                    "
>  
> 

There were recent changes in this area so that is not the correct way
to do this in master.

meta/conf/bitbake.conf:BB_RENAMED_VARIABLES[VOLATILE_LOG_DIR] = "is obsolete and no longer supported"

This does make me wonder about testing :(.

Cheers,

Richard
Esben Haabendal Oct. 24, 2024, 2:29 p.m. UTC | #2
"Richard Purdie" <richard.purdie@linuxfoundation.org> writes:

> On Thu, 2024-10-24 at 13:41 +0200, Esben Haabendal via lists.openembedded.org wrote:
>> Commit 18d46e11d85d ("systemd: fix dead link /var/log/README") broke the
>> default systemd behavior when VOLATILE_LOG_DIR is set to false.
>> 
>> By not creating the /var/log/journal directory, the default configured
>> systemd (Storage=auto) will silently revert to just doing volatile
>> logging (/run/journal).
>> 
>> The problem fixed with the above mentioned commit was specifically related
>> to configurations where VOLATILE_LOG_DIR is true, in which case it should
>> be perfectly fine to not create the /var/log/journal directory, so we
>> should be able to keep the fix in place, while not breaking behavior for
>> non VOLATILE_LOG_DIR configurations.
>> 
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  meta/recipes-core/systemd/systemd_256.6.bb | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/meta/recipes-core/systemd/systemd_256.6.bb b/meta/recipes-core/systemd/systemd_256.6.bb
>> index 68f15ab065dd5985e4e5cefc2cd005dba5ac9c7f..88ee0c8a7f89726fc9be489f3fc419f9feb37251 100644
>> --- a/meta/recipes-core/systemd/systemd_256.6.bb
>> +++ b/meta/recipes-core/systemd/systemd_256.6.bb
>> @@ -254,7 +254,7 @@ EXTRA_OEMESON += "-Dnobody-user=nobody \
>>                    -Dsystem-uid-max=999 \
>>                    -Dsystem-alloc-gid-min=101 \
>>                    -Dsystem-gid-max=999 \
>> -                  -Dcreate-log-dirs=false \
>> +                  ${@ '-Dcreate-log-dirs=false' if oe.types.boolean(d.getVar('VOLATILE_LOG_DIR')) else '' } \
>>                    ${@bb.utils.contains('DISTRO_FEATURES', 'zeroconf', '-Ddefault-mdns=no -Ddefault-llmnr=no', '', d)} \
>>                    "
>>  
>> 
>
> There were recent changes in this area so that is not the correct way
> to do this in master.
>
> meta/conf/bitbake.conf:BB_RENAMED_VARIABLES[VOLATILE_LOG_DIR] = "is obsolete and no longer supported"
>
> This does make me wonder about testing :(.

Guilty as charged. Testing was on scarthgap. I currently does not have a
working test setup for master.

I guess I will have to wait with this until I can do proper testing
then.

I will resend this patch against scarthgap though, as it is valid and
tested there.

For master and styhead, I need to change to check
FILESYSTEM_PERMS_TABLES instead.

/Esben
Richard Purdie Oct. 24, 2024, 10:02 p.m. UTC | #3
On Thu, 2024-10-24 at 16:29 +0200, Esben Haabendal wrote:
> "Richard Purdie" <richard.purdie@linuxfoundation.org> writes:
> 
> > On Thu, 2024-10-24 at 13:41 +0200, Esben Haabendal via lists.openembedded.org wrote:
> > > Commit 18d46e11d85d ("systemd: fix dead link /var/log/README") broke the
> > > default systemd behavior when VOLATILE_LOG_DIR is set to false.
> > > 
> > > By not creating the /var/log/journal directory, the default configured
> > > systemd (Storage=auto) will silently revert to just doing volatile
> > > logging (/run/journal).
> > > 
> > > The problem fixed with the above mentioned commit was specifically related
> > > to configurations where VOLATILE_LOG_DIR is true, in which case it should
> > > be perfectly fine to not create the /var/log/journal directory, so we
> > > should be able to keep the fix in place, while not breaking behavior for
> > > non VOLATILE_LOG_DIR configurations.
> > > 
> > > Signed-off-by: Esben Haabendal <esben@geanix.com>
> > > ---
> > >  meta/recipes-core/systemd/systemd_256.6.bb | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/meta/recipes-core/systemd/systemd_256.6.bb b/meta/recipes-core/systemd/systemd_256.6.bb
> > > index 68f15ab065dd5985e4e5cefc2cd005dba5ac9c7f..88ee0c8a7f89726fc9be489f3fc419f9feb37251 100644
> > > --- a/meta/recipes-core/systemd/systemd_256.6.bb
> > > +++ b/meta/recipes-core/systemd/systemd_256.6.bb
> > > @@ -254,7 +254,7 @@ EXTRA_OEMESON += "-Dnobody-user=nobody \
> > >                    -Dsystem-uid-max=999 \
> > >                    -Dsystem-alloc-gid-min=101 \
> > >                    -Dsystem-gid-max=999 \
> > > -                  -Dcreate-log-dirs=false \
> > > +                  ${@ '-Dcreate-log-dirs=false' if oe.types.boolean(d.getVar('VOLATILE_LOG_DIR')) else '' } \
> > >                    ${@bb.utils.contains('DISTRO_FEATURES', 'zeroconf', '-Ddefault-mdns=no -Ddefault-llmnr=no', '', d)} \
> > >                    "
> > >  
> > > 
> > 
> > There were recent changes in this area so that is not the correct way
> > to do this in master.
> > 
> > meta/conf/bitbake.conf:BB_RENAMED_VARIABLES[VOLATILE_LOG_DIR] = "is obsolete and no longer supported"
> > 
> > This does make me wonder about testing :(.
> 
> Guilty as charged. Testing was on scarthgap. I currently does not have a
> working test setup for master.
> 
> I guess I will have to wait with this until I can do proper testing
> then.
> 
> I will resend this patch against scarthgap though, as it is valid and
> tested there.

Our backport policy is such that it needs to be fixed in master first.

> For master and styhead, I need to change to check
> FILESYSTEM_PERMS_TABLES instead.

Correct, yes.

Cheers,

Richard
Esben Haabendal Oct. 25, 2024, 6:05 a.m. UTC | #4
Richard Purdie <richard.purdie@linuxfoundation.org> writes:

> On Thu, 2024-10-24 at 16:29 +0200, Esben Haabendal wrote:
>> "Richard Purdie" <richard.purdie@linuxfoundation.org> writes:
>> 
>> > On Thu, 2024-10-24 at 13:41 +0200, Esben Haabendal via lists.openembedded.org wrote:
>> > > Commit 18d46e11d85d ("systemd: fix dead link /var/log/README") broke the
>> > > default systemd behavior when VOLATILE_LOG_DIR is set to false.
>> > > 
>> > > By not creating the /var/log/journal directory, the default configured
>> > > systemd (Storage=auto) will silently revert to just doing volatile
>> > > logging (/run/journal).
>> > > 
>> > > The problem fixed with the above mentioned commit was specifically related
>> > > to configurations where VOLATILE_LOG_DIR is true, in which case it should
>> > > be perfectly fine to not create the /var/log/journal directory, so we
>> > > should be able to keep the fix in place, while not breaking behavior for
>> > > non VOLATILE_LOG_DIR configurations.
>> > > 
>> > > Signed-off-by: Esben Haabendal <esben@geanix.com>
>> > > ---
>> > >  meta/recipes-core/systemd/systemd_256.6.bb | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > > 
>> > > diff --git a/meta/recipes-core/systemd/systemd_256.6.bb b/meta/recipes-core/systemd/systemd_256.6.bb
>> > > index 68f15ab065dd5985e4e5cefc2cd005dba5ac9c7f..88ee0c8a7f89726fc9be489f3fc419f9feb37251 100644
>> > > --- a/meta/recipes-core/systemd/systemd_256.6.bb
>> > > +++ b/meta/recipes-core/systemd/systemd_256.6.bb
>> > > @@ -254,7 +254,7 @@ EXTRA_OEMESON += "-Dnobody-user=nobody \
>> > >                    -Dsystem-uid-max=999 \
>> > >                    -Dsystem-alloc-gid-min=101 \
>> > >                    -Dsystem-gid-max=999 \
>> > > -                  -Dcreate-log-dirs=false \
>> > > +                  ${@ '-Dcreate-log-dirs=false' if oe.types.boolean(d.getVar('VOLATILE_LOG_DIR')) else '' } \
>> > >                    ${@bb.utils.contains('DISTRO_FEATURES', 'zeroconf', '-Ddefault-mdns=no -Ddefault-llmnr=no', '', d)} \
>> > >                    "
>> > >  
>> > > 
>> > 
>> > There were recent changes in this area so that is not the correct way
>> > to do this in master.
>> > 
>> > meta/conf/bitbake.conf:BB_RENAMED_VARIABLES[VOLATILE_LOG_DIR] = "is obsolete and no longer supported"
>> > 
>> > This does make me wonder about testing :(.
>> 
>> Guilty as charged. Testing was on scarthgap. I currently does not have a
>> working test setup for master.
>> 
>> I guess I will have to wait with this until I can do proper testing
>> then.
>> 
>> I will resend this patch against scarthgap though, as it is valid and
>> tested there.
>
> Our backport policy is such that it needs to be fixed in master first.

Ok. Then it will have to wait a bit then.

>> For master and styhead, I need to change to check
>> FILESYSTEM_PERMS_TABLES instead.
>
> Correct, yes.

/Esben
diff mbox series

Patch

diff --git a/meta/recipes-core/systemd/systemd_256.6.bb b/meta/recipes-core/systemd/systemd_256.6.bb
index 68f15ab065dd5985e4e5cefc2cd005dba5ac9c7f..88ee0c8a7f89726fc9be489f3fc419f9feb37251 100644
--- a/meta/recipes-core/systemd/systemd_256.6.bb
+++ b/meta/recipes-core/systemd/systemd_256.6.bb
@@ -254,7 +254,7 @@  EXTRA_OEMESON += "-Dnobody-user=nobody \
                   -Dsystem-uid-max=999 \
                   -Dsystem-alloc-gid-min=101 \
                   -Dsystem-gid-max=999 \
-                  -Dcreate-log-dirs=false \
+                  ${@ '-Dcreate-log-dirs=false' if oe.types.boolean(d.getVar('VOLATILE_LOG_DIR')) else '' } \
                   ${@bb.utils.contains('DISTRO_FEATURES', 'zeroconf', '-Ddefault-mdns=no -Ddefault-llmnr=no', '', d)} \
                   "