diff mbox series

[V2] systemd: do not delete systemd-getty-generator

Message ID 20250319064016.1119702-1-Qi.Chen@windriver.com
State New
Headers show
Series [V2] systemd: do not delete systemd-getty-generator | expand

Commit Message

ChenQi March 19, 2025, 6:40 a.m. UTC
From: Chen Qi <Qi.Chen@windriver.com>

Some BSPs don't set SERIAL_CONSOLES. They need systemd-getty-generator[1]
be there to work. This generator has been there for a few previous
releases, we don't see any problem it brings up. So let's keep it.

[1] https://www.freedesktop.org/software/systemd/man/latest/systemd-getty-generator.html

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/recipes-core/systemd/systemd_257.3.bb | 6 ------
 1 file changed, 6 deletions(-)

Comments

Ross Burton March 25, 2025, 5:50 p.m. UTC | #1
On 19 Mar 2025, at 06:40, Qi.Chen@windriver.com wrote:
> Some BSPs don't set SERIAL_CONSOLES. They need systemd-getty-generator[1]
> be there to work. This generator has been there for a few previous
> releases, we don't see any problem it brings up. So let's keep it.

This is a wonderfully convoluted and not-by-design situation.  Looking at eg, styhead.  The getty generator isn’t enabled by default:

$ bitbake-getvar --value -r systemd PACKAGECONFIG|grep getty || echo not found
not found

In styhead if the getty generator isn’t enabled then we delete the serial-getty units that the generator would create symlinks to, so the generator can’t work.

if ${@bb.utils.contains('PACKAGECONFIG', 'serial-getty-generator', 'false', 'true', d)}; then
    # Provided by a separate recipe
    rm ${D}${systemd_system_unitdir}/serial-getty* -f
fi

If the generator is disabled then systemd depends on the systemd-serialgetty recipe which creates explicit links for the consoles in SERIAL_CONSOLES.  This recipe in styhead also ships a serial-getty@.service unit template, but the package should be empty if SERIAL_CONSOLES isn’t set.  However, the recipe itself has a default value (SERIAL_CONSOLES ?= "115200;ttyS0”) which provides a value if the BSP doesn’t set it, so the unit files do get installed after all.

I think that your setup potentially doesn’t actually use the getty generator at all, but the bad default of ttyS0 from the systemd-serialgetty recipe.  This no longer exists as the recipe doesn’t set its own default anymore (as, I believe, is correct).

Can you have a look at your actual use case?  I wouldn’t be surprised if it’s actually just using ttyS0 from the configuration, and the BSP could just set that.

In the medium term I really do want to propose switching from SERIAL_CONSOLES to auto-detection by default, but this involves making the same thing idea on sysvinit which is more involved.  I _almost_ have it working, but it didn’t get working in time for M3.

Ross
ChenQi March 26, 2025, 2:55 a.m. UTC | #2
On 3/26/25 01:50, Ross Burton wrote:
> On 19 Mar 2025, at 06:40, Qi.Chen@windriver.com wrote:
>> Some BSPs don't set SERIAL_CONSOLES. They need systemd-getty-generator[1]
>> be there to work. This generator has been there for a few previous
>> releases, we don't see any problem it brings up. So let's keep it.
> This is a wonderfully convoluted and not-by-design situation.  Looking at eg, styhead.  The getty generator isn’t enabled by default:
>
> $ bitbake-getvar --value -r systemd PACKAGECONFIG|grep getty || echo not found
> not found
>
> In styhead if the getty generator isn’t enabled then we delete the serial-getty units that the generator would create symlinks to, so the generator can’t work.
>
> if ${@bb.utils.contains('PACKAGECONFIG', 'serial-getty-generator', 'false', 'true', d)}; then
>      # Provided by a separate recipe
>      rm ${D}${systemd_system_unitdir}/serial-getty* -f
> fi
>
> If the generator is disabled then systemd depends on the systemd-serialgetty recipe which creates explicit links for the consoles in SERIAL_CONSOLES.  This recipe in styhead also ships a serial-getty@.service unit template, but the package should be empty if SERIAL_CONSOLES isn’t set.  However, the recipe itself has a default value (SERIAL_CONSOLES ?= "115200;ttyS0”) which provides a value if the BSP doesn’t set it, so the unit files do get installed after all.
>
> I think that your setup potentially doesn’t actually use the getty generator at all, but the bad default of ttyS0 from the systemd-serialgetty recipe.  This no longer exists as the recipe doesn’t set its own default anymore (as, I believe, is correct).
>
> Can you have a look at your actual use case?  I wouldn’t be surprised if it’s actually just using ttyS0 from the configuration, and the BSP could just set that.

The BSP is using the generator. See output below:

root@xilinx-zynqmp:~# ls -l /run/systemd/generator/getty.target.wants/*
lrwxrwxrwx 1 root root 41 Mar 28 01:00 
/run/systemd/generator/getty.target.wants/serial-getty@ttyPS0.service -> 
/lib/systemd/system/serial-getty@.service
root@xilinx-zynqmp:~#

I think the generator took effect for previous releases because, as you 
said, the systemd-serialgetty@.service template was always installed (by 
a lucky mistake that you've now fixed) and the systemd-getty-generator 
was always installed.

Now, in master branch, this template is always installed too. But we'll 
also need the systemd-getty-generator to be there to do autodetection 
for some BSPs.

Regards,
Qi

>
> In the medium term I really do want to propose switching from SERIAL_CONSOLES to auto-detection by default, but this involves making the same thing idea on sysvinit which is more involved.  I _almost_ have it working, but it didn’t get working in time for M3.
>
> Ross
ChenQi April 2, 2025, 5:14 a.m. UTC | #3
kindly ping

Could we consider merging this patch into both master and walnascar 
branches?

I think this is a correct fix because of the following two reasons:
1) It will fix a potential regression on machines that don't set 
SERIAL_CONSOLES.
2) It will not cause any problem. Because /etc/ takes higher priority 
than /run/systemd/generator/.
     See 
https://manpages.debian.org/testing/systemd/systemd.generator.7.en.html.
     """
     Directory paths for generator output differ by priority: 
.../generator.early has priority higher than
     the admin configuration in /etc/, while .../generator has lower 
priority than /etc/ but higher than
     vendor configuration in /usr/, and .../generator.late has priority 
lower than all other configuration.
     """

Regards,
Qi

On 3/26/25 10:55, Chen Qi via lists.openembedded.org wrote:
> On 3/26/25 01:50, Ross Burton wrote:
>> On 19 Mar 2025, at 06:40, Qi.Chen@windriver.com wrote:
>>> Some BSPs don't set SERIAL_CONSOLES. They need 
>>> systemd-getty-generator[1]
>>> be there to work. This generator has been there for a few previous
>>> releases, we don't see any problem it brings up. So let's keep it.
>> This is a wonderfully convoluted and not-by-design situation. Looking 
>> at eg, styhead.  The getty generator isn’t enabled by default:
>>
>> $ bitbake-getvar --value -r systemd PACKAGECONFIG|grep getty || echo 
>> not found
>> not found
>>
>> In styhead if the getty generator isn’t enabled then we delete the 
>> serial-getty units that the generator would create symlinks to, so 
>> the generator can’t work.
>>
>> if ${@bb.utils.contains('PACKAGECONFIG', 'serial-getty-generator', 
>> 'false', 'true', d)}; then
>>      # Provided by a separate recipe
>>      rm ${D}${systemd_system_unitdir}/serial-getty* -f
>> fi
>>
>> If the generator is disabled then systemd depends on the 
>> systemd-serialgetty recipe which creates explicit links for the 
>> consoles in SERIAL_CONSOLES.  This recipe in styhead also ships a 
>> serial-getty@.service unit template, but the package should be empty 
>> if SERIAL_CONSOLES isn’t set.  However, the recipe itself has a 
>> default value (SERIAL_CONSOLES ?= "115200;ttyS0”) which provides a 
>> value if the BSP doesn’t set it, so the unit files do get installed 
>> after all.
>>
>> I think that your setup potentially doesn’t actually use the getty 
>> generator at all, but the bad default of ttyS0 from the 
>> systemd-serialgetty recipe.  This no longer exists as the recipe 
>> doesn’t set its own default anymore (as, I believe, is correct).
>>
>> Can you have a look at your actual use case?  I wouldn’t be surprised 
>> if it’s actually just using ttyS0 from the configuration, and the BSP 
>> could just set that.
>
> The BSP is using the generator. See output below:
>
> root@xilinx-zynqmp:~# ls -l /run/systemd/generator/getty.target.wants/*
> lrwxrwxrwx 1 root root 41 Mar 28 01:00 
> /run/systemd/generator/getty.target.wants/serial-getty@ttyPS0.service 
> -> /lib/systemd/system/serial-getty@.service
> root@xilinx-zynqmp:~#
>
> I think the generator took effect for previous releases because, as 
> you said, the systemd-serialgetty@.service template was always 
> installed (by a lucky mistake that you've now fixed) and the 
> systemd-getty-generator was always installed.
>
> Now, in master branch, this template is always installed too. But 
> we'll also need the systemd-getty-generator to be there to do 
> autodetection for some BSPs.
>
> Regards,
> Qi
>
>>
>> In the medium term I really do want to propose switching from 
>> SERIAL_CONSOLES to auto-detection by default, but this involves 
>> making the same thing idea on sysvinit which is more involved. I 
>> _almost_ have it working, but it didn’t get working in time for M3.
>>
>> Ross
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#213624): https://lists.openembedded.org/g/openembedded-core/message/213624
> Mute This Topic: https://lists.openembedded.org/mt/111784889/3618072
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/recipes-core/systemd/systemd_257.3.bb b/meta/recipes-core/systemd/systemd_257.3.bb
index 64fb8fe69a..edaa2f33e3 100644
--- a/meta/recipes-core/systemd/systemd_257.3.bb
+++ b/meta/recipes-core/systemd/systemd_257.3.bb
@@ -288,12 +288,6 @@  do_install() {
 	fi
 	install -d ${D}/${base_sbindir}
 
-	if ! ${@bb.utils.contains('PACKAGECONFIG', 'serial-getty-generator', 'true', 'false', d)}; then
-		# Remove the serial-getty generator and instead use explicit services
-		# created by the systemd-serialgetty recipe
-		find ${D} -name \*getty-generator\* -delete
-	fi
-
 	# Provide support for initramfs
 	[ ! -e ${D}/init ] && ln -s ${nonarch_libdir}/systemd/systemd ${D}/init
 	[ ! -e ${D}/${base_sbindir}/udevd ] && ln -s ${nonarch_libdir}/systemd/systemd-udevd ${D}/${base_sbindir}/udevd