Skip to content

Conversation

aafeijoo-suse
Copy link
Contributor

When copy_tree() ends up calling cp, if it fails because there is not enough space, it leaves and incomplete initrd image in the output directory. If we are writing the initrd image directly where a bootloader entry has an initrd configured, the system will fail to boot. So, backup the old initrd image before calling copy_tree(), and restore it if that function throws an exception.

Copy link
Contributor

@DaanDeMeyer DaanDeMeyer left a comment

Choose a reason for hiding this comment

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

This is very messy., there's no guarantee we can put the backup back in place when copying the new one fails. We should copy the new initrd to a temporary file next to the initrd we're replacing and then use rename() to atomically replace the old one.

Copy link
Contributor

@behrmann behrmann left a comment

Choose a reason for hiding this comment

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

I agree with @DaanDeMeyer on how this should be implemented, but generally I do want to see this feature. I've had the exact thing this is supposed to prevent happen to me with dracut a few times. :)

@aafeijoo-suse
Copy link
Contributor Author

We should copy the new initrd to a temporary file next to the initrd we're replacing and then use rename() to atomically replace the old one.

With this approach we are always invalidating an amount of space equal to the initrd size, but ok, I see your point.

@aafeijoo-suse aafeijoo-suse force-pushed the initrd-backup-and-restore-on-error branch 2 times, most recently from 4f690b5 to c4a23ff Compare March 13, 2025 14:26
@DaanDeMeyer
Copy link
Contributor

Let's merge #3591 first and then rebase this on top of that and make use of Path more

Copy link
Contributor

@behrmann behrmann left a comment

Choose a reason for hiding this comment

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

LGTM, #3591 has just been merged, so this just needs the rebase. One small nit

@aafeijoo-suse aafeijoo-suse force-pushed the initrd-backup-and-restore-on-error branch from c4a23ff to 9c88011 Compare March 14, 2025 17:24
When `copy_tree()` ends up calling `cp`, if it fails because there is not enough
space, it leaves an incomplete initrd image in the output directory. If we are
writing the initrd image directly where a bootloader entry has an initrd
configured, the system will fail to boot. So, instead of copying the initrd
image directly to the output, copy it next to it in the same output directory,
and if the copy is successful, replace it.
@aafeijoo-suse aafeijoo-suse force-pushed the initrd-backup-and-restore-on-error branch from 9c88011 to e133298 Compare March 15, 2025 06:22
@behrmann behrmann merged commit 76377d3 into systemd:main Mar 17, 2025
34 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants