Skip to content

Fix races with udev - #446

Merged
ajeddeloh merged 2 commits into
coreos:masterfrom
ajeddeloh:fix-races
Sep 14, 2017
Merged

ajeddeloh merged 2 commits into
coreos:masterfrom
ajeddeloh:fix-races

Conversation

@ajeddeloh

Copy link
Copy Markdown
Contributor

This addresses two races:

  1. We create symlink aliases for devices as soon as they become available, since udevd will frequently delete them moments later. Unfortunately, sometimes the device is deleted before we even get a chance to do that. This PR let Ignition retry the alias creation.
  2. The same deletion behavior can affect other systemd units that depend on the created /dev/* symlinks and Ignition. Running udevadm settle greatly reduces the size of the race. This PR ensures we always settle if we mess with disks, even if we do not create filesystems.

@ajeddeloh

Copy link
Copy Markdown
Contributor Author

/cc @bgilbert

@ajeddeloh
ajeddeloh force-pushed the fix-races branch 2 times, most recently from 504788f to c892950 Compare September 13, 2017 20:46
@ajeddeloh

Copy link
Copy Markdown
Contributor Author

Did testing with udevadm monitor to ensure one second is a reasonable cooldown and modified the comment to reflect that.

Andrew Jeddeloh added 2 commits September 14, 2017 10:52
CreateDeviceAlias exists to create a stable symlink to be used so when
udev deletes and recreates the symlink it owns, Ignition wont fail.
However there is a narrow race creating the symlink: if udev creates
then deletes its symlink fast enough we wont have time to read the
symlink to create our own.

This adds retry logic to rapidly retry creating the symlink, in case we
lose the race, but still fail after a timeout.
Previously we only did a udevadm settle after creating filesystems;
this was added because creating btrfs filesystems was particularly good
at causing these races. Usually only settling after creating filesystems
is ok, because we typically create filesystems after partitioning. But
partitioning can create these kinds of races too, and if we do not
create filesystems we never settle and units dependent on Ignition and a
/dev symlink could find their link deleted out from under them.

This moves the settle up to happen after dealing with the disks stage in
entirety. It could be added as an ExecStartPost directive in the unit
file, but here we can be smart and skip it if the disks stage didn't do
anything.
@ajeddeloh

Copy link
Copy Markdown
Contributor Author

Moved retry logic to more tightly wrap EvaluateSymlink and made it retry more aggressively.

@ajeddeloh
ajeddeloh merged commit 991e38d into coreos:master Sep 14, 2017
@ajeddeloh
ajeddeloh deleted the fix-races branch September 22, 2017 17:22
prestist added a commit to prestist/ignition that referenced this pull request Jan 12, 2026
docs/release-notes: update for release
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants