Zbigniew, I was planning to add you to the Cc here, but I see that you have already done that yourself.

I'm fine with the suggested change, but I wanted to touch base with you first to make sure that this is the right thing to do. Below is somewhat of a braindump of my understanding of how all the bits fit together:

plymouth consists of the plymouthd daemon and the plymouth commandline client which can be called to make the daemon do various things

plymouth installs a lot of systemd .service files, on boot things start with the already mentioned plymouth-start.service which does 2 things it starts the daemon and it tells the daemon to take over the console and show the splash (the ExecStartPost=-/usr/bin/plymouth show-splash).

Then there are various .service files which are mostly really more triggers for certain events, e.g. there is /lib/systemd/system/plymouth-switch-root.service:

[Unit]
Description=Plymouth switch root service
DefaultDependencies=no
ConditionPathExists=/etc/initrd-release
Before=initrd-switch-root.service

[Service]
Type=oneshot
ExecStart=-/usr/bin/plymouth update-root-fs --new-root-dir=/sysroot
StandardInput=null
StandardOutput=null
StandardError=null

Which runs from the initrd just before switching root and which uses the plymouth cmdline client to tell the daemon to change its concept of the fs-root to /sysroot. Note these triggers are already OneShot but they are not 
marked RemainAfterExit=yes.

###

During boot the following "trigger" services should happen sometime after plymouth-start.service has run:
/lib/systemd/system/plymouth-switch-root.service
/lib/systemd/system/plymouth-read-write.service

Then when boot is finishing up, either the special plymouth-quit.service will run:
/lib/systemd/system/plymouth-quit.service

Which tells the plymouthd daemon to quit; or if gdm is used, then gdm will tell plymouth to quit as part of a flicker free handover of the console from plymouth to gdm, to this goal gdm.service blocks plymouth-quit.service from running:

# replaces plymouth-quit since it quits plymouth on its own
Conflicts=plymouth-quit.service
After=plymouth-quit.service

Then once plymouthd is asked to quit one way or the other, there is a service which waits for plymouthd to actually quit, which goal is to make systemd not consider the boot complete until the daemon has quit:
/lib/systemd/system/plymouth-quit-wait.service

###

Note that we do something similar to what we do at boot at shutdown/reboot, e.g. we also have:

/lib/systemd/system/plymouth-reboot.service

Which gets run on reboot and does almost the same as plymouth-start.service:
ExecStart=/usr/sbin/plymouthd --mode=reboot --attach-to-session
ExecStartPost=-/usr/bin/plymouth show-splash

The only difference is plymouthd is started in a different mode.

Likewise for other variants of reboot/shutdown/kexec new kernel we also have:

/lib/systemd/system/plymouth-halt.service
/lib/systemd/system/plymouth-kexec.service
/lib/systemd/system/plymouth-poweroff.service

###

I have the feeling that adding 

Type=oneshot
RemainAfterExit=yes

To see all installed plymouth .service file is the right thing to do. Notice that with the exception of the .service files which start plymouthd on boot or reboot/halt/etc.:

/lib/systemd/system/plymouth-halt.service
/lib/systemd/system/plymouth-kexec.service
/lib/systemd/system/plymouth-poweroff.service
/lib/systemd/system/plymouth-reboot.service
/lib/systemd/system/plymouth-start.service

All files are already marked Type=oneshot

So the change would be to:
1. Also mark the service files which start plymouthd on boot or reboot/halt/etc. as Type=oneshot; and
2. Add RemainAfterExit=yes to all .service files including the ones which just send a a command to plymouthd, such as: plymouth-switch-root.service, plymouth-read-write.service and plymouth-quit.service

I'm not 100% sure that adding RemainAfterExit=yes is also correct for plymouth-quit-wait.service ?

To see all installed plymouth .service files do :
$EDITOR /lib/systemd/system/plymouth-*.service

###

TL;DR: Zbigniew, since all the plymouth .service files should only run once during a (re)boot-cycle lifetime I believe that the right thing is probably to add: Type=oneshot; and RemainAfterExit=yes to all plymouth .service
files. Do you agree that this is the correct thing to do ?

If you add "RemainAfterExit=yes" to plymouth-read-write.service it will run once only.
Without it it will run twice.

To verify, run journalctl -b -u plymouth-read-write.service

A bit of background from the systemd side: when starting a service, systemd walks the full dependency
tree recursively, even for services and targets which are already started. So if e.g. at some
point we have a job like httpd.service/start, we'll go into all the deps of that, including
usually sysinit.target, and then local-fs.target, and call a start job for any unit in that
tree which isn't running (or hasn't run in case of Type=oneshot/RemainAfterExit=yes units).

Doing things like this increases robustness, because new dependencies will often be started
after being configured even without explicit restarting of targets, and if things fail, they
will often be started again. OTOH, it makes pid1 go through the whole unit tree every time
something is started. It also has the downside that units will be started if they are part
of the dep tree even in cases where we don't expect. This has been discussed before, and
I think it'd be interesting to explore if we can change this behaviour, but it'd be a very
risky change to fundamentals and I'm not even sure if it would make things better. So for the
foreseeable future this will not change.

Any unit which is part of the dependency tree and should be started just once should
have RemainAfterExit=yes.

(In reply to Hans de Goede from comment #1)
> 2. Add RemainAfterExit=yes to all .service files including the ones which
> just send a a command to plymouthd, such as: plymouth-switch-root.service,
> plymouth-read-write.service and plymouth-quit.service

Yeah, that sounds correct.

> 1. Also mark the service files which start plymouthd on boot or
> reboot/halt/etc. as Type=oneshot; and

Hmm, they currently have Type=forking, which means that systemd expects a different process to
stay behind after the process from ExecStart= has exited. With Type=oneshot systemd expects
no processes after the control process exits. So just changing Type=forking→oneshot without
changes to the code normally doesn't work.

But do those units actually follow the forking protocol? If they simply exit after sending
the command to plymouthd, then Type=oneshot with RemainAfterExit=yes sounds appropriate.

> I'm not 100% sure that adding RemainAfterExit=yes is also correct for
> plymouth-quit-wait.service ?

Yeah, I think so.

BTW, plymouth-halt.service, plymouth-kexec.service, plymouth-poweroff.service are
all identical, so they could just be aliases to one service. Not that it matters much.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> (In reply to Hans de Goede from comment #1)
> > 2. Add RemainAfterExit=yes to all .service files including the ones which
> > just send a a command to plymouthd, such as: plymouth-switch-root.service,
> > plymouth-read-write.service and plymouth-quit.service
> 
> Yeah, that sounds correct.

Ok.

> > 1. Also mark the service files which start plymouthd on boot or
> > reboot/halt/etc. as Type=oneshot; and
> 
> Hmm, they currently have Type=forking, which means that systemd expects a
> different process to
> stay behind after the process from ExecStart= has exited. With Type=oneshot
> systemd expects
> no processes after the control process exits. So just changing
> Type=forking→oneshot without
> changes to the code normally doesn't work.
> 
> But do those units actually follow the forking protocol? If they simply exit
> after sending
> the command to plymouthd, then Type=oneshot with RemainAfterExit=yes sounds
> appropriate.

The units with Type=forking are the ones which actually start plymouthd
at boot / (shutdown|reboot|etc) :
/lib/systemd/system/plymouth-halt.service
/lib/systemd/system/plymouth-kexec.service
/lib/systemd/system/plymouth-poweroff.service
/lib/systemd/system/plymouth-reboot.service
/lib/systemd/system/plymouth-start.service

These all have some variant of:

ExecStart=/usr/sbin/plymouthd --mode=XXXX --pid-file=/run/plymouth/pid
ExecStartPost=-/usr/bin/plymouth show-splash
Type=forking

And plymouthd does fork itself when started, it has a cmdline option
to not do that, but then I'm not sure if the ExecStartPost will ever
run? And if ExecStartPost does run, it might run to early before
plymouthd has started listening for commands from boot-clients...

So we likely need to keep this as Type=forking, will adding just
RemainAfterExit=yes then be enough to stop systemd from restarting it
even after it has exited (e.g. systemd rescanning deps after gdm has
told plymouthd to quit and it thus has exited) ?

> > I'm not 100% sure that adding RemainAfterExit=yes is also correct f
> > plymouth-quit-wait.service ?
> 
> Yeah, I think so.

Ok.

> BTW, plymouth-halt.service, plymouth-kexec.service,
> plymouth-poweroff.service are
> all identical, so they could just be aliases to one service. Not that it
> matters much.

They are not 100% identical, they all have a:

Before=systemd-XXXX.service

line which differs per file, e.g. plymouth-poweroff.service has "Before=systemd-poweroff.service"

> And plymouthd does fork itself when started

Oh, right. I don't know how I missed the call to plymouthd there.

> if the ExecStartPost will ever
> run? And if ExecStartPost does run, it might run to early before
> plymouthd has started listening for commands from boot-clients...

ExecStartPost will be executed after the ExecStart line is considered done,
which in case of Type=forking means that the called process returns. If
plymouthd implements the protocol properly then it should be already
listening on any sockets before that exit.

>  Type=forking, will adding just RemainAfterExit=yes then be enough to stop
> systemd from restarting it even after it has exited

Yes.

> They are not 100% identical, they all have a:
> 
> Before=systemd-XXXX.service
> 
> line which differs per file, e.g. plymouth-poweroff.service has "Before=systemd-poweroff.service"

Right. That would have to be coalesced:
Before=systemd-poweroff.service systemd-kexec.service systemd-halt.service.
But this is a side track... OK, please ignore this subthread ;)

(In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
> ExecStartPost will be executed after the ExecStart line is considered done,
> which in case of Type=forking means that the called process returns. If
> plymouthd implements the protocol properly then it should be already
> listening on any sockets before that exit.

Right, my remark referred to changing to Type=oneshot combined with
specifying a cmdline parameter to plymouthd which stops it from forking,
which would be racy if systemd would run ExecStartPost once the ExecStart
process is "running", but since systemd waits until ExecStart is
considered done, using Type=oneshot + disabling plymouthd forking
will not work at all. IOW this is all noise please ignore.

> >  Type=forking, will adding just RemainAfterExit=yes then be enough to stop
> > systemd from restarting it even after it has exited
> 
> Yes.

Ok, so summarizing what basically needs to happen is add:

RemainAfterExit=yes

To all plymouth .service files and then everything should work properly. Correct?

I've just pushed a plymouth update for F32+ this morning fixing a crash related to DP-MST docks, which is intended to go into F32 beta.

I assume you would like these plymouth changes to land for F32, so that the systemd package can drop the revert which Adam Williamson added?  I think it would be best to land the addition of "RemainAfterExit=yes" to the plymouth .service files after the beta has been spun, Zbigniew, would that work for you?

Then once the new plymouth build is done and that plymouth hits the stable F32 repo you can do a systemd build with the revert dropped.

(In reply to Hans de Goede from comment #6)
> Ok, so summarizing what basically needs to happen is add:
> 
> RemainAfterExit=yes
> 
> To all plymouth .service files and then everything should work properly.
> Correct?

That's my understanding too.

> I assume you would like these plymouth changes to land for F32, so that the
> systemd package can drop the revert which Adam Williamson added?  I think it
> would be best to land the addition of "RemainAfterExit=yes" to the plymouth
> .service files after the beta has been spun,

Yep, sounds good.

> Then once the new plymouth build is done and that plymouth hits the stable
> F32 repo you can do a systemd build with the revert dropped.

"I assume you would like these plymouth changes to land for F32, so that the systemd package can drop the revert which Adam Williamson added?"

I would like to stress again that *just* changing plymouth will *not* be sufficient to drop that revert. Just in a default install (not even considering any complex cases), the bug in question affected 17 services, listed in this comment:

https://bugzilla.redhat.com/show_bug.cgi?id=1803293#c7

*all* of those need to be checked and considered before the revert is removed.

(In reply to Adam Williamson from comment #8)
> "I assume you would like these plymouth changes to land for F32, so that the
> systemd package can drop the revert which Adam Williamson added?"
> 
> I would like to stress again that *just* changing plymouth will *not* be
> sufficient to drop that revert. Just in a default install (not even
> considering any complex cases), the bug in question affected 17 services,
> listed in this comment:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1803293#c7
> 
> *all* of those need to be checked and considered before the revert is
> removed.

Ok, I will leave that up to the systemd folks. I have some other plymouth fixes which I want to get in post-beta anyways so I'll likely do a plymouth build regardless.

(In reply to Hans de Goede from comment #9)
> (In reply to Adam Williamson from comment #8)
> > "I assume you would like these plymouth changes to land for F32, so that the
> > systemd package can drop the revert which Adam Williamson added?"
> > 
> > I would like to stress again that *just* changing plymouth will *not* be
> > sufficient to drop that revert. Just in a default install (not even
> > considering any complex cases), the bug in question affected 17 services,
> > listed in this comment:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1803293#c7
> > 
> > *all* of those need to be checked and considered before the revert is
> > removed.
> 
> Ok, I will leave that up to the systemd folks. I have some other plymouth
> fixes which I want to get in post-beta anyways so I'll likely do a plymouth
> build regardless.

I tested with the changed plymouth service files and downgraded systemd to systemd-245~rc1-3.fc32.x86_64

System boots up perfectly.

(In reply to Hans de Goede from comment #6)

...

> Then once the new plymouth build is done and that plymouth hits the stable
> F32 repo you can do a systemd build with the revert dropped.

Are there any reason for not applying this change before the beta period for Fedora 32 ends?

I've been running with this change for a while now both on my main Fedora 31 and testing Fedora 32,
and I see no negative effects what so ever.

(In reply to Adam Williamson from comment #8)
> "I assume you would like these plymouth changes to land for F32, so that the
> systemd package can drop the revert which Adam Williamson added?"
> 
> I would like to stress again that *just* changing plymouth will *not* be
> sufficient to drop that revert. Just in a default install (not even
> considering any complex cases), the bug in question affected 17 services,
> listed in this comment:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1803293#c7
> 
> *all* of those need to be checked and considered before the revert is
> removed.

I only see issues with systemd-vconsole-setup, which -- by the way -- is wanted by plymouth-start.

Normally systemd-vconsole-setup as activated by a udev rule, so nothing really needs to want this service for it to be activated.

(In reply to Villy Kruse from comment #11)
> (In reply to Hans de Goede from comment #6)
> 
> ...
> 
> > Then once the new plymouth build is done and that plymouth hits the stable
> > F32 repo you can do a systemd build with the revert dropped.
> 
> Are there any reason for not applying this change before the beta period for
> Fedora 32 ends?

I did not say the beta period had to end, I said that the beta had to be "spun" IOW that the beta-freeze has to be released and that we entered the final (non beta blocking) bug-fixing phase of F32. Since we have a workaround fixing this was not a beta blocker and making these changes during the freeze thus would not have been the right thing to do.

(In reply to Villy Kruse from comment #12)
> I only see issues with systemd-vconsole-setup, which -- by the way -- is
> wanted by plymouth-start.
> 
> Normally systemd-vconsole-setup as activated by a udev rule, so nothing
> really needs to want this service for it to be activated.

systemd-vconsole-setup the service is not activated by a udev rule! /usr/lib/systemd/systemd-vconsole-setup, the binary, is run by a udev rule so that if some consoles (e.g. serial ones) show up later during boot they get configured as they show up.

Yes the udev rule will call /usr/lib/systemd/systemd-vconsole-setup for VT1 / /dev/tty1, but we need to be sure that this is done before plymouth starts, hence the "Wants" + "After", there is nothing wrong with this.

It would seem that the fix for systemd-vconsole-setup.service would be the same as for plymouth-start and friends add a "RemainAfterExit=yes" to the .service file. Although I wonder if that does not break "systemctl restart systemd-vconsole-setup.service" which is recommended by "man systemd-vconsole-setup" in case of making changes to /etc/vconsole.conf

(In reply to Hans de Goede from comment #13)
>

> 
> It would seem that the fix for systemd-vconsole-setup.service would be the
> same as for plymouth-start and friends add a "RemainAfterExit=yes" to the
> .service file. Although I wonder if that does not break "systemctl restart
> systemd-vconsole-setup.service" which is recommended by "man
> systemd-vconsole-setup" in case of making changes to /etc/vconsole.conf


That is already in the git repository


diff --git a/units/systemd-vconsole-setup.service.in b/units/systemd-vconsole-setup.service.in
index f4178f495a..9042521c9d 100644
--- a/units/systemd-vconsole-setup.service.in
+++ b/units/systemd-vconsole-setup.service.in
@@ -16,4 +16,5 @@ ConditionPathExists=/dev/tty0
 
 [Service]
 Type=oneshot
+RemainAfterExit=yes
 ExecStart=@rootlibexecdir@/systemd-vconsole-setup

> That is already in the git repository

Yep, we changed it because of the same issue.

> Although I wonder if that does not break "systemctl restart systemd-vconsole-setup.service" which is recommended by
> "man systemd-vconsole-setup" in case of making changes to /etc/vconsole.conf

Restart still works, even with RemainAfterExit=yes. (This is the same as a for a service with
Type=notify/forking/simple which is constantly running. It can be restarted at any time. But
a simple "start" job does nothing. Setting RemainAfterExit=yes makes a Type=oneshot service behave
like a constantly running service, except that nothing is running.)

(In reply to Hans de Goede from comment #13)
> (In reply to Villy Kruse from comment #11)
> > (In reply to Hans de Goede from comment #6)
> > 
> > ...
> > 
> > > Then once the new plymouth build is done and that plymouth hits the stable
> > > F32 repo you can do a systemd build with the revert dropped.
> > 
> > Are there any reason for not applying this change before the beta period for
> > Fedora 32 ends?
> 
> I did not say the beta period had to end, I said that the beta had to be
> "spun" IOW that the beta-freeze has to be released and that we entered the
> final (non beta blocking) bug-fixing phase of F32. Since we have a
> workaround fixing this was not a beta blocker and making these changes
> during the freeze thus would not have been the right thing to do.
> 

Sorry,  I thought "{ost-beta" meant the end of the beta testing period.

Upstream merge-req for this is here:

https://gitlab.freedesktop.org/plymouth/plymouth/-/merge_requests/99

There are a few other minor plymouth issues I want to try and fix, I plan to do a Fedora build with this and the other fixes added tomorrow.

I hit the bodhi bug where bodhi does not add a comment when creating the update (for some reason), a bodhi update with a fixed plymouth is here:
https://bodhi.fedoraproject.org/updates/FEDORA-2020-5e5f6af77e

FEDORA-2020-5e5f6af77e has been pushed to the Fedora 32 testing repository.
In short time you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2020-5e5f6af77e`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-5e5f6af77e

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

FEDORA-2020-5e5f6af77e has been pushed to the Fedora 32 stable repository.
If problem still persists, please make note of it in this bug report.