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

(#2036853) Fix unit table overflow by mounts, resulting in "Argument list too long" when trying to start a unit #244

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Dec 22, 2021

This is a backport of systemd/systemd#10980 (commit systemd/systemd@ba0d56f only) to address issues like systemd/systemd#10874. The backport was almost clean, except for

  • a trivial conflict in src/core/mount.c, function mount_load_proc_self_mountinfo, due to local commit ca634ba (that changed variable names and removed = 0 from int r);
  • due to the same commit, int k is no longer used, so I had to remove it.

This also backports systemd/systemd#15222 as it helps a lot to figure out what is going on. This is not required and can be removed.

This should fix https://bugzilla.redhat.com/show_bug.cgi?id=2028153 (a non-parse-able mount in mountinfo resulted in piling up inactive mount units, up to MANAGER_MAX_NAMES, after which it becomes impossible to start any units).

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2036853

@systemd-rhel-bot systemd-rhel-bot added pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review tracker/missing Formerly needs-bz labels Dec 22, 2021
@kolyshkin
Copy link
Contributor Author

This also backports systemd/systemd#15222 as it helps a lot to figure out what is going on. This is not required and can be removed.

Removed, as it uses functionality (SYNTHETIC_ERRNO) not backported to this codebase (i.e. systemd/systemd#10871), and I am not sure what the local rules are wrt
non-trivial backports.

@lgtm-com
Copy link

lgtm-com bot commented Dec 22, 2021

This pull request introduces 1 alert when merging db8d26b into de7125d - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@kolyshkin
Copy link
Contributor Author

@msekletar @dtardon PTAL

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.

Hi, Could you please amend commit message with line:

Resolves: #2028153

@jamacku jamacku requested a review from dtardon January 3, 2022 07:46
@jamacku jamacku changed the title [#2028153] Fix unit table overflow by mounts, resulting in "Argument list too long" when trying to start a unit (#2028153) Fix unit table overflow by mounts, resulting in "Argument list too long" when trying to start a unit Jan 3, 2022
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/missing Formerly needs-bz labels Jan 3, 2022
@kolyshkin
Copy link
Contributor Author

Hi, Could you please amend commit message with line:

Resolves: #2028153

Done, thanks for your review!

@kolyshkin kolyshkin requested a review from jamacku January 3, 2022 21:51
@systemd-rhel-bot systemd-rhel-bot added pr/needs-ci Formerly needs-ci and removed pr/needs-ci Formerly needs-ci labels Jan 3, 2022
If we can't process a specific line in /proc/self/mountinfo we should
log about it (which we do), but this should not affect other lines, nor
further processing of mount units. Let's keep these failures local.

Fixes: #10874

Cherry picked from commit ba0d56f.
Trivial conflict in src/core/mount.c, function mount_load_proc_self_mountinfo,
due to local commit ca634ba. Also, due to the same commit, int k
is no longer used and is thus removed.

Resolves: #2036853

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@jamacku jamacku changed the title (#2028153) Fix unit table overflow by mounts, resulting in "Argument list too long" when trying to start a unit (#2036853) Fix unit table overflow by mounts, resulting in "Argument list too long" when trying to start a unit Jan 4, 2022
@systemd-rhel-bot systemd-rhel-bot added the pr/needs-ci Formerly needs-ci label Jan 4, 2022
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.

@kolyshkin I'm sorry, for my confusion, we need a bug for RHEL8 to properly track these changes, I cloned the original bug and updated Resolves in commit.

@systemd-rhel-bot systemd-rhel-bot removed the pr/needs-ci Formerly needs-ci label Jan 4, 2022
@jamacku jamacku added this to the RHEL-8.6 milestone Jan 6, 2022
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 Jan 7, 2022
@kolyshkin kolyshkin requested a review from jamacku January 7, 2022 22:54
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