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

fix(multipath): drop dependency on systemd-udev-settle.service #1553

Closed
wants to merge 1 commit into from

Conversation

jlebon
Copy link
Contributor

@jlebon jlebon commented Jun 30, 2021

Rawhide systemd warns about this now:

udevadm[445]: systemd-udev-settle.service is deprecated. Please fix
multipathd-configure.service, multipathd.service not to pull it in.

Similarly to #1552, we don't actually need that dependency anyway here.
multipathd-configure.service just outputs a file in /etc and shouldn't
need to probe devices, and multipathd itself by nature is designed to
handle dynamic udev changes.

So just drop it.

Rawhide systemd warns about this now:

    udevadm[445]: systemd-udev-settle.service is deprecated. Please fix
    multipathd-configure.service, multipathd.service not to pull it in.

Similarly to dracutdevs#1552, we don't actually need that dependency anyway here.
`multipathd-configure.service` just outputs a file in /etc and shouldn't
need to probe devices, and `multipathd` itself by nature is designed to
handle dynamic udev changes.

So just drop it.
@github-actions github-actions bot added modules Issue tracker for all modules multipath Issues related to the multipath module labels Jun 30, 2021
@jlebon
Copy link
Contributor Author

jlebon commented Jun 30, 2021

/cc @bmarzins

I've tested this on Fedora CoreOS 34 on top of multipath.

@bmarzins
Copy link
Contributor

bmarzins commented Jul 1, 2021

Did you test this with a filesystem that is necessary for bootup on top of a multipath device? The main reason that multipath still depends on systemd-udev-settle.service has to do with transitioning out of the initramfs, and needing the path devices that it is currently using to have been re-dicovered before multipathd starts up again. You can see the multipath issue here: opensvc/multipath-tools#3 with a comment by @mwilck explaining the problem. But this issue doesn't apply to multipathd initially starting up in the initramfs. So, it needs proper testing to verify, but I don't see an issue with removing the udev-settle dependency here.

@mwilck
Copy link
Contributor

mwilck commented Jul 1, 2021

I've mixed feelings about this. @bmarzins is right that the argument I made in opensvc/multipath-tools#3 only applies after initramfs processing is finished. OTOH, changing the boot ordering for multipath is risky; we have lots of evidence for that. Is it reasonable to do that just because of this questionable warning? Thorough testing on multipath-enabled systems would be a requirement IMO.

BTW if we change this, we might be better off starting multipathd before systemd-udev-trigger.service. That way it will not see any maps on startup, and add paths as they get discovered. This is how it works for iSCSI. IMO it would be more reasonable than starting it after trigger but before settle, IOW while udev is scanning devices.

Copy link
Contributor

@mwilck mwilck left a comment

Choose a reason for hiding this comment

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

This change will cause multipathd startup to race with udev device probing. That's not healthy. If we must drop the dependency on systemd-udev-settle.service, we should change the After= dependency on systemd-udev-trigger.service to Before=.

@jlebon
Copy link
Contributor Author

jlebon commented Jul 5, 2021

The test I did on FCOS 34 was with root on multipath, and it did work though it was a somewhat artificial set up using qemu-nbd and qemu-kvm. I'm not in a position to test this more extensively. We don't personally need this fixed immediately. Since it's already tracked upstream, let's close it.

@jlebon jlebon closed this Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules Issue tracker for all modules multipath Issues related to the multipath module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants