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.

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

(#2227769) sulogin: fix control lost of the current terminal when default.target… #406

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

lnykryn
Copy link
Member

@lnykryn lnykryn commented Jul 31, 2023

… is rescue.target

When default.target is rescue.target, exiting from the single-user shell results in lost of the control of the current terminal. This is because the operation performed to continue to boot is systemctl default but default.target is now rescue.target and it is already active. Hence, no new process that controls the current terminal is created. Users need to make hardware reset to recover the situation.

This sounds like a bit corner case issue and some might feel configuring default.target as rescue.target is odd because there are several other ways to transition to rescue.mode without configuring default.target to rescue.target such as systemctl rescue or systemd.unit=rescue.target something like that. However, users unfamiliar with systemd operations tend to come up with systemctl set-default rescue.target.

To fix this issue, let's transition to default.target only when default.target is inactive. Otherwise, invoke the single-user shell again to keep control of the current terminal for users.

This new logic depends on whether D-Bus working well. Exiting without any check of result of systemctl default could lead to again the control lost of the current terminal. Hence, add checking results of each D-Bus operations including systemctl default and invoke the single-user shell if they fail.

(cherry picked from commit 937ca8330d11e406b8ef343bead6f4f6244e39c7)

Resolves: #2227769

@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Jul 31, 2023
@github-actions
Copy link

github-actions bot commented Jul 31, 2023

Tracker - 2227769

The following commits meet all requirements

commit upstream
951ed53 - sulogin: fix control lost of the current terminal when default.target … systemd/systemd@937ca83

@systemd-rhel-bot systemd-rhel-bot added the pr/needs-review Formerly needs-review label Jul 31, 2023
@systemd-rhel-bot systemd-rhel-bot changed the title sulogin: fix control lost of the current terminal when default.target… (#2227769) sulogin: fix control lost of the current terminal when default.target… Jul 31, 2023
… is rescue.target

When default.target is rescue.target, exiting from the single-user shell
results in lost of the control of the current terminal. This is because the
operation performed to continue to boot is systemctl default but default.target
is now rescue.target and it is already active. Hence, no new process that
controls the current terminal is created. Users need to make hardware reset to
recover the situation.

This sounds like a bit corner case issue and some might feel configuring
default.target as rescue.target is odd because there are several other ways to
transition to rescue.mode without configuring default.target to rescue.target
such as systemctl rescue or systemd.unit=rescue.target something like
that. However, users unfamiliar with systemd operations tend to come up with
systemctl set-default rescue.target.

To fix this issue, let's transition to default.target only when default.target
is inactive. Otherwise, invoke the single-user shell again to keep control of
the current terminal for users.

This new logic depends on whether D-Bus working well. Exiting without any check
of result of systemctl default could lead to again the control lost of the
current terminal. Hence, add checking results of each D-Bus operations
including systemctl default and invoke the single-user shell if they fail.

(cherry picked from commit 937ca8330d11e406b8ef343bead6f4f6244e39c7)

Resolves: #2227769
Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@systemd-rhel-bot systemd-rhel-bot removed the pr/needs-review Formerly needs-review label Aug 1, 2023
@jamacku
Copy link
Member

jamacku commented Aug 1, 2023

@mrc0mmand any ideas why CentOS CI is failing?

@mrc0mmand
Copy link
Member

@mrc0mmand any ideas why CentOS CI is failing?

That's unrelated, the TEST-35 is just incredibly flaky in RHEL 8.

@mergify mergify bot removed the pr/needs-ci Formerly needs-ci label Aug 1, 2023
@jamacku jamacku merged commit 0117502 into redhat-plumbers:rhel-8.8.0 Aug 1, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants