dracutdevs / dracut

dracut the event driven initramfs infrastructure
https://github.com/dracutdevs/dracut/wiki
GNU General Public License v2.0
597 stars 396 forks source link

fix(overlayfs): Split overlayfs tree creation from the mount hook #2525

Closed davidcassany closed 10 months ago

davidcassany commented 11 months ago

This commit splits the creation of required overlayfs underlaying directories and the actual overlayfs mount. This way it is still possible to mount the overlayfs with the generated sysroot.mount that dmsquash-live creates.

The overlayfs tree is created in a pre-mount hook so it is executed before sysroot.mount is started. Otherwise sysroot.mount starts and fails before mount hooks are executed.

Changes

Checklist

Part of #2232

davidcassany commented 11 months ago

On a Fedora 38 Workstation Live it can be seen an ugly sysroot.mount failure which is pretty ugly if one uses the rd.live.overlay.overlayfs flag. The image still boots (thanks to the mount hook executed later on), however this is likely to uncover other boot related issues, I would have expected the boot to fail if something as sensitive as sysroot.mount unit actually fails.

These are the logs of a Fedora Workstation 38 live iso booted including the rd.live.overlay.overlayfs=1: image

aafeijoo-suse commented 11 months ago

@LaszloGombos this PR conflicts with #2269 @Conan-Kudo since you were involved in #1934, maybe you can help with this review.

Conan-Kudo commented 11 months ago

@davidcassany Can you please fix your commit messages to follow the Conventional Commit style? https://www.conventionalcommits.org/en/v1.0.0/

davidcassany commented 11 months ago

@davidcassany Can you please fix your commit messages to follow the Conventional Commit style? https://www.conventionalcommits.org/en/v1.0.0/

I think now it matches the convention

LaszloGombos commented 11 months ago

@LaszloGombos this PR conflicts with #2269

Thanks for flagging this @aafeijoo-suse . Did you mean conflicting as in only one or the other PR should land, or did you mean that if this PR lands than the other PR needs to be rebase ?

As I understand this PR does work at pre-mount phase. https://github.com/dracutdevs/dracut/pull/2269 is doing work at pre-pivot, so I do not see the conceptual conflict between the two PRs.

davidcassany commented 11 months ago

Thanks for flagging this @aafeijoo-suse . Did you mean conflicting as in only one or the other PR should land, or did you mean that if this PR lands than the other PR needs to be rebase ?

Indeed both PRs apply changes in modue-setup.sh which will conflict, even it will be pretty straight forward to adapt. Then on the functional side I don't think they do harm each other even it is true that #2269 is recalling the overlayfs mount script at pre-pivot stage and this current PR changes this script to only mount and the creation of the underlaying required directories is applied on an earlier pre-mount hook stage.

I understand this should not be a problem as pre-mount hook will be executed on an earlier stage and I don't expect rd.live.overlay.overlayfs argument to change after initqueue and between pre-mount stage and pre-pivot stage.

In any case worth having a look close look.

@LaszloGombos is there any special care I should take regarding the tests? I am wondering to what extend I should look at the failing ones, I am a bit confused by this huge test matrix based on many distros. I have the feeling some test failures might not be related to this PR changes.

LaszloGombos commented 11 months ago

I have the feeling some test failures might not be related to this PR changes.

That is correct. Project does not have a green CI and all failures exists independent from this PR.

davidcassany commented 10 months ago

Hi @aafeijoo-suse @LaszloGombos I am wondering, is there anything missing in this PR? or can I help somehow to push it forward?

I am aware there are many PRs in the queue, I just want to double check I do not miss anything and if there is anything I can do to help to move it forward. After all this is a rather simple PR fixing a regression.

davidcassany commented 9 months ago

Just saw it got merged. @LaszloGombos, @Conan-Kudo and @aafeijoo-suse , thank you all.