Skip to content

systemd: pid1 crash fix#147354

Closed
martinetd wants to merge 1 commit intoNixOS:masterfrom
martinetd:systemd
Closed

systemd: pid1 crash fix#147354
martinetd wants to merge 1 commit intoNixOS:masterfrom
martinetd:systemd

Conversation

@martinetd
Copy link
Member

Motivation for this change

Fixes #147103 (pid1 crash)

While backporting the patch I noticed we weren't up to date, so also updated along the way.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added the 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. label Nov 25, 2021
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Does rebasing the patches actually do anything? patches can be applied if the hunks have offsets.

@martinetd
Copy link
Member Author

Does rebasing the patches actually do anything? patches can be applied if the hunks have offsets.

Other than removing two patches it didn't seem to do anything, no; this was mostly done to make sure it applies (and some apply method fail on fuzz offset so I wasn't sure about this part)
Happy to remove the rebase noise

@martinetd
Copy link
Member Author

I've removed the rebase altogether, it'll probably need further updating when systemd unfreezes after release while this patch will always stay valid (at least until/if it's also backported upstream)

@martinetd martinetd changed the title systemd: pid1 crash fix + 249.7 update systemd: pid1 crash fix Nov 25, 2021
@ofborg ofborg bot requested review from andir, edolstra, flokli and kloenk November 25, 2021 12:26
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Nov 25, 2021
@arianvp arianvp self-requested a review November 25, 2021 13:03
@arianvp
Copy link
Member

arianvp commented Nov 25, 2021

I've removed the rebase altogether, it'll probably need further updating when systemd unfreezes after release while this patch will always stay valid (at least until/if it's also backported upstream)

I do not think the freeze applies to minor / bugfix bumps.

Copy link
Member

@flokli flokli left a comment

Choose a reason for hiding this comment

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

Like in #146573 (comment), we shouldn't ship these patches downstream, but poke upstream to release it in systemd-stable.

@martinetd
Copy link
Member Author

Like in #146573 (comment), we shouldn't ship these patches downstream, but poke upstream to release it in systemd-stable.

I see. There were other such patches so I didn't think too much about it, I've asked upstream about it in the systemd PR

@flokli
Copy link
Member

flokli commented Nov 25, 2021

Like in #146573 (comment), we shouldn't ship these patches downstream, but poke upstream to release it in systemd-stable.

I see. There were other such patches so I didn't think too much about it, I've asked upstream about it in the systemd PR

For curious ones: That's systemd/systemd#21498 (comment).

@Ma27
Copy link
Member

Ma27 commented Dec 12, 2021

Does rebasing the patches actually do anything? patches can be applied if the hunks have offsets.

For the future, I disagree: keeping patches up-to-date reduces the risk of them being unappliable in the future (if the differences around the change become too large).

@flokli I see this is in systemd-stable, but not yet in a release. Do you think it's sufficient to wait for another stable v249 or should we merge this?

(@martinetd I think this should go to staging btw).

@andir
Copy link
Member

andir commented Dec 13, 2021

Does rebasing the patches actually do anything? patches can be applied if the hunks have offsets.

For the future, I disagree: keeping patches up-to-date reduces the risk of them being unappliable in the future (if the differences around the change become too large).

Adding what I wrote on Matrix some time ago:
We should always (at least on major versions) rebase these patches to avoid running into issues with applying them on the next version.

The issue with patch files vs. a git repository is that the patch files have a limited set of context around each change. If that context isn't suffient anymore the patches fail to apply. My workflow for new systemd versions is typically:

  • checkout the old systemd version in a Git repostiry
  • git am all the patches
  • interactive rebase to new git tag
  • git format-patch and replace the files in the repo.

This has been going smooth so far.

@flokli
Copy link
Member

flokli commented Dec 14, 2021

@flokli I see this is in systemd-stable, but not yet in a release. Do you think it's sufficient to wait for another stable v249 or should we merge this?

Upstream wanted to wait for some other patches, but I haven't seen anything more getting cherry-picked. I poked them for the tag in systemd/systemd#21498 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

systemd (pid 1) segfault on nixos-rebuild switch on nixos-unstable, perhaps unmountable mountpoint related?

6 participants