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

(#2158167) Better handle session leader information #401

Merged
merged 2 commits into from
Jul 20, 2023
Merged

(#2158167) Better handle session leader information #401

merged 2 commits into from
Jul 20, 2023

Conversation

msekletar
Copy link
Member

@msekletar msekletar commented Jul 5, 2023 •
edited by github-actions bot
Loading

No description provided.

@yuwata @msekletar
login: add a missing error check for session_set_leader()
8cb6e2a 
session_set_leader() may fail. If it fails, then manager_start_scope()
will trigger assertion.

This may be related to RHBZ#1663704.

(cherry picked from commit fe3ab84)

Related: #2158167
@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Jul 5, 2023
@github-actions
Copy link

github-actions bot commented Jul 5, 2023 •
edited
Loading

Tracker - 2158167

The following commits meet all requirements

commit upstream
8cb6e2a - login: add a missing error check for session_set_leader() systemd/systemd@fe3ab84
systemd/systemd-stable@fe3ab84
5f37ec0 - logind: reset session leader if we know for a fact that it is gone rhel-only

@systemd-rhel-bot systemd-rhel-bot added pr/needs-review Formerly needs-review tracker/unapproved Formerly needs-acks labels Jul 5, 2023
@mergify mergify bot removed the pr/needs-ci Formerly needs-ci label Jul 5, 2023
@jamacku jamacku added this to the RHEL-8.9 milestone Jul 5, 2023
@msekletar msekletar requested a review from dtardon July 10, 2023 11:17
@msekletar
Copy link
Member Author

@dtardon review of this PR will require extra attention as this is RHEL-only change

keszybz
keszybz reviewed Jul 10, 2023
View reviewed changes
Copy link
Contributor

@keszybz keszybz left a comment

Choose a reason for hiding this comment

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

IIUC, this patch will not do anything bad. I haven't looked at this code in a while, so I might be missing obvious things, but I left some comments where I think things can be improved/clarifier.

src/login/logind-dbus.c Outdated Show resolved Hide resolved
src/login/logind-session.c Show resolved Hide resolved
@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Jul 12, 2023
@msekletar
Copy link
Member Author

@keszybz updated. PTAL.

@mergify mergify bot removed the pr/needs-ci Formerly needs-ci label Jul 12, 2023
@systemd-rhel-bot systemd-rhel-bot removed the tracker/unapproved Formerly needs-acks label Jul 13, 2023
keszybz
keszybz approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@keszybz keszybz 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 Jul 14, 2023
jamacku
jamacku approved these changes Jul 20, 2023
View reviewed changes
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 merged commit ab53c4f into redhat-plumbers:main Jul 20, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dtardon dtardon dtardon left review comments

@keszybz keszybz keszybz approved these changes

@jamacku jamacku jamacku approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
RHEL-8.9
Development

Successfully merging this pull request may close these issues.
6 participants
@msekletar @keszybz @dtardon @jamacku @systemd-rhel-bot @yuwata