Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

units: also swap ordering of systemd-exit.service vs exit.target and friends #32895

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

units: also swap ordering of systemd-exit.service vs exit.target and friends #32895

wants to merge 1 commit into from

Conversation

yuwata
Copy link
Member

@yuwata yuwata commented May 17, 2024 •
edited

Follow-up for 4263d76 (#32880).

Otherwise, the targets cannot be used as a synchronization point.

@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label May 17, 2024
@yuwata yuwata requested a review from poettering May 17, 2024 12:00
@github-actions GitHub Actions
Copy link

Important

An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released.

@arvidjaar
Copy link
Contributor

targets cannot be used as a synchronization point

Is not final.target intended for it? What unit needs to survive final.target but be stopped before e.g. reboot.target? Arguably, systemd-journald.service should be stopped before final.target as well, not be an exception that survives it.

@yuwata
Copy link
Member Author

yuwata commented May 17, 2024 •
edited

systemd-journald.service should be running at the very last stage of shutdown process. To make journald stopped only on soft-reboot, it is necessary to make soft-reboot.target is a usable synchronization point.

BTW, it may not be necessary that journald is stopped on soft-reboot. Calling journalctl --smart-relinquish-var before soft-reboot.target may be sufficient.

@bluca
Copy link
Member

bluca commented May 17, 2024

softreboot is new enough and there were actual issues, but I don't think we should change these ones after RC2, unless there's a specific bug that is being solved. Could you please split the manpage change in a separate PR?

@yuwata
Copy link
Member Author

yuwata commented May 17, 2024

softreboot is new enough and there were actual issues, but I don't think we should change these ones after RC2, unless there's a specific bug that is being solved. Could you please split the manpage change in a separate PR?

Okay. But I'd like to wait for @poettering's comment about the swap of the ordering done in #32880.

YHNdnzj
YHNdnzj reviewed May 17, 2024
View reviewed changes
units/kexec.target Outdated Show resolved Hide resolved
units/halt.target Outdated Show resolved Hide resolved
units/exit.target Outdated Show resolved Hide resolved
@YHNdnzj
Copy link
Member

YHNdnzj commented May 17, 2024

So yeah, I think this should be exclusive to soft-reboot, which is realistically more similar to switch-root rather than normal shutdown.

If one day an actual issue pops up with the current ordering, we can always revisit things. All normal services should conflict with shutdown.target and gets stopped, anyway.

YHNdnzj added a commit to YHNdnzj/systemd that referenced this pull request May 17, 2024
@YHNdnzj
man/soft-reboot: order surviving services before shutdown.target
a01541b 
Prompted by systemd#32895

Rather than ordering with each power operation targets,
ordering against shutdown.target which is a valid
synchronization point. This has no effect if soft-reboot
is being performed.
@YHNdnzj YHNdnzj mentioned this pull request May 17, 2024
Merged
@YHNdnzj
Copy link
Member

YHNdnzj commented May 17, 2024

I opened #32897, to ensure that units that should survive soft-reboot are stopped at correct spot on normal shutdown. It shouldn't be necessary to swap the orders like this PR does.

keszybz pushed a commit that referenced this pull request May 17, 2024
@YHNdnzj @keszybz
man/soft-reboot: order surviving services before shutdown.target
3acc318 
Prompted by #32895

Rather than ordering with each power operation targets,
ordering against shutdown.target which is a valid
synchronization point. This has no effect if soft-reboot
is being performed.
@YHNdnzj YHNdnzj added needs-rebase and removed please-review PR is ready for (re-)review by a maintainer documentation labels May 18, 2024
@yuwata
units: also swap ordering of systemd-exit.service vs exit.target and …
8e021d8 
…friends

Otherwise, the targets cannot be used as a synchronization point.

Follow-up for 4263d76.
@github-actions github-actions bot added documentation please-review PR is ready for (re-)review by a maintainer and removed needs-rebase labels May 18, 2024
@yuwata
Copy link
Member Author

yuwata commented May 18, 2024

So yeah, I think this should be exclusive to soft-reboot, which is realistically more similar to switch-root rather than normal shutdown.

If one day an actual issue pops up with the current ordering, we can always revisit things. All normal services should conflict with shutdown.target and gets stopped, anyway.

Maybe. But not sure. People may want to do something different on e.g. reboot.

@yuwata yuwata mentioned this pull request May 21, 2024
Merged
@YHNdnzj
Copy link
Member

YHNdnzj commented May 22, 2024

(need to include reverts for reverts in #32975)

@YHNdnzj YHNdnzj added this to the v257 milestone May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YHNdnzj YHNdnzj YHNdnzj left review comments

@poettering poettering Awaiting requested review from poettering

Assignees
No one assigned
Labels
documentation needs-rebase please-review PR is ready for (re-)review by a maintainer units
Milestone
v257
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@yuwata @arvidjaar @bluca @YHNdnzj