Skip to content

(#2053273) Let SysV compat. shutdown commands honor inhibitors for root#355

Merged
systemd-rhel-bot merged 4 commits intoredhat-plumbers:masterfrom
dtardon:bz2053273-compat-shutdown-inhibitors
Feb 16, 2023
Merged

(#2053273) Let SysV compat. shutdown commands honor inhibitors for root#355
systemd-rhel-bot merged 4 commits intoredhat-plumbers:masterfrom
dtardon:bz2053273-compat-shutdown-inhibitors

Conversation

@dtardon
Copy link
Member

@dtardon dtardon commented Jan 17, 2023

Resolves: #2053273

@systemd-rhel-bot systemd-rhel-bot added the pr/needs-review Formerly needs-review label Jan 17, 2023
@systemd-rhel-bot systemd-rhel-bot changed the title Let SysV compat. shutdown commands honor inhibitors for root (#2053273) Let SysV compat. shutdown commands honor inhibitors for root Jan 17, 2023
@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Jan 17, 2023
@systemd-rhel-bot systemd-rhel-bot added the tracker/unapproved Formerly needs-acks label Jan 17, 2023
@mergify mergify bot removed the pr/needs-ci Formerly needs-ci label Jan 17, 2023
@systemd-rhel-bot systemd-rhel-bot removed the tracker/unapproved Formerly needs-acks label Jan 20, 2023
@jamacku jamacku requested a review from msekletar January 24, 2023 15:02
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/unapproved Formerly needs-acks labels Jan 29, 2023
@dtardon
Copy link
Member Author

dtardon commented Jan 30, 2023

@msekletar It just occurred to me that there's an easier--and safer--way than trying to make the new code backwards-compatible again. Given that the changes are in a single function halt_main, we could add the whole function under a new name and then call the either old one or the new one as needed. Something like:

static int halt_main(void) {
        if (env_var_is_set())
                return halt_main_new();
        else
                return halt_main_old();
}

@msekletar
Copy link
Member

That sounds much better. Please try to implement it this way so we know how exactly would resulting code look.

lnussel and others added 4 commits January 30, 2023 14:33
The code at this point is not able to tell whether it was called as
halt/poweroff/reboot or shutdown with time "now".
The code also takes a shortcut to skip logind if called as root.
That however means asking shutdown for immediate action won't trigger a
wall message.
As per systemd/systemd#8424 (comment)
all commands should trigger a wall message.
That simplifies the code as we can try logind first always.

(cherry picked from commit adefc87)

Resolves: #2053273
For shutdowns don't fall back to starting the target directly if talking
to logind failed with auth failure. That would just lead to another
polkit auth attempt.

(cherry picked from commit 38d55bf)

Related: #2053273
Currently, the legacy shutdown commands ignore inhibitors and reboot
immediately if run by root. Let's preserve that behavior in RHEL-8 by
default. The new behavior can be turned on by those who want it by
exporting SYSTEMD_NEW_SHUTDOWN=1 .

RHEL-only

Related: #2053273
@dtardon dtardon force-pushed the bz2053273-compat-shutdown-inhibitors branch from a463521 to 0c6a303 Compare January 30, 2023 13:34
@dtardon
Copy link
Member Author

dtardon commented Jan 30, 2023

Done.

@mergify mergify bot added pr/needs-ci Formerly needs-ci and removed pr/needs-ci Formerly needs-ci labels Jan 30, 2023
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/unapproved Formerly needs-acks labels Feb 8, 2023
@jamacku jamacku added this to the RHEL-8.8 milestone Feb 13, 2023
Copy link
Member

@msekletar msekletar 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 Feb 16, 2023
@systemd-rhel-bot systemd-rhel-bot merged commit 7463258 into redhat-plumbers:master Feb 16, 2023
@dtardon dtardon deleted the bz2053273-compat-shutdown-inhibitors branch February 16, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants