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

--force should not remove the output when it fails #3083

Closed
aafeijoo-suse opened this issue Sep 30, 2024 · 9 comments · Fixed by #3096
Closed

--force should not remove the output when it fails #3083

aafeijoo-suse opened this issue Sep 30, 2024 · 9 comments · Fixed by #3096
Labels

Comments

@aafeijoo-suse
Copy link
Contributor

mkosi commit the issue has been seen with

main

Used host distribution

openSUSE Tumbleweed

Used target distribution

No response

Linux kernel version used

No response

CPU architectures issue was seen on

None

Unexpected behaviour you saw

Maybe for building OS images this is not critical, but for building initrds it does not look good... Maybe keep a backup copy of the current output image file or dir until mkosi exists with success, or delay removal until the end.

Used mkosi config

No response

mkosi output

# ls -l /boot/initrd-6.10.7-1-default
-rw------- 1 root root 29540805 Sep 30 16:10 /boot/initrd-6.10.7-1-default
# mkosi-initrd -O /boot -o initrd-6.10.7-1-default
‣ Removing output files of default image…
...
‣  Creating cpio archive /var/tmp/mkosi-workspace-0ihot0ak/staging/initrd-6.10.7-1-default.cpio…
‣  Compressing /var/tmp/mkosi-workspace-0ihot0ak/staging/initrd-6.10.7-1-default.cpio with zstd
‣ Could not rename /var/tmp/mkosi-workspace-0ihot0ak/staging/initrd-6.10.7-1-default.cpio.zst to /boot/initrd-6.10.7-1-default.cpio.zst as they are located on different devices, falling back to copying
cp: cannot create regular file '/work/boot/initrd-6.10.7-1-default.cpio.zst': Read-only file system
‣ "cp --recursive --no-dereference --preserve=mode,links,timestamps,ownership,xattr --reflink=auto --copy-contents /work/var/tmp/mkosi-workspace-0ihot0ak/staging/initrd-6.10.7-1-default.cpio.zst /work/boot/initrd-6.10.7-1-default.cpio.zst" returned non-zero exit code 1.
‣ "mkosi --force --directory '' --format cpio --output initrd-6.10.7-1-default --output-dir /boot --extra-tree /usr/lib/modules/6.10.7-1-default:/usr/lib/modules/6.10.7-1-default --extra-tree=/usr/lib/firmware:/usr/lib/firmware '--remove-files=/usr/lib/firmware/*-ucode' '--kernel-modules-exclude=.*' --kernel-modules-include=host --build-sources '' --include=mkosi-initrd --workspace-dir=/var/tmp --package-cache-dir=/var --cache-only=metadata --output-mode=600 --include /usr/lib/mkosi-initrd --include /etc/mkosi-initrd --sandbox-tree /tmp/tmpu592h6np" returned non-zero exit code 1.
# ls -l /boot/initrd-6.10.7-1-default
ls: cannot access '/boot/initrd-6.10.7-1-default': No such file or directory
@aafeijoo-suse
Copy link
Contributor Author

Another side-issue, this happens when the output dir is a btrfs subvolume, the original file can be removed, but cp fails to copy the new file claiming that it is a Read-only file system

@DaanDeMeyer
Copy link
Contributor

Delaying removal until the end probably wouldn't work since the very last operation, copying the new initrd to the output dir could fail with ENOSPC.

Would you be OK with handling this in mkosi-initrd? When given an output name, we suffix it with .tmp and rename it to the final output on successful completion of mkosi?

@aafeijoo-suse
Copy link
Contributor Author

Would you be OK with handling this in mkosi-initrd? When given an output name, we suffix it with .tmp and rename it to the final output on successful completion of mkosi?

Yes, that sounds good to me.

@DaanDeMeyer
Copy link
Contributor

@aafeijoo-suse The failure to copy is because we remount a bunch of directories read-only in mkosi to avoid messing with the host system when running as root. Seems that logic should be a little smarter

@aafeijoo-suse
Copy link
Contributor Author

Would you be OK with handling this in mkosi-initrd? When given an output name, we suffix it with .tmp and rename it to the final output on successful completion of mkosi?

Hmm, a quick test with this approach doesn't seem good enough when the output is compressed or there are multiple output files:

-rw------- 1 root root  48150266 Oct  1 10:14 initrd1.tmp.cpio.zst
lrwxrwxrwx 1 root root        14 Oct  1 10:14 initrd1 -> initrd1.tmp.cpio.zst
-rw------- 1 root root  76151296 Oct  1 10:18 initrd2.tmp.efi
-rw------- 1 root root  14768496 Oct  1 10:18 initrd2.tmp.vmlinuz
-rw------- 1 root root  48266362 Oct  1 10:18 initrd2.tmp.initrd
lrwxrwxrwx 1 root root        14 Oct  1 10:18 initrd2 -> initrd2.tmp.efi

@DaanDeMeyer
Copy link
Contributor

Would you be OK with handling this in mkosi-initrd? When given an output name, we suffix it with .tmp and rename it to the final output on successful completion of mkosi?

Hmm, a quick test with this approach doesn't seem good enough when the output is compressed or there are multiple output files:

-rw------- 1 root root  48150266 Oct  1 10:14 initrd1.tmp.cpio.zst
lrwxrwxrwx 1 root root        14 Oct  1 10:14 initrd1 -> initrd1.tmp.cpio.zst
-rw------- 1 root root  76151296 Oct  1 10:18 initrd2.tmp.efi
-rw------- 1 root root  14768496 Oct  1 10:18 initrd2.tmp.vmlinuz
-rw------- 1 root root  48266362 Oct  1 10:18 initrd2.tmp.initrd
lrwxrwxrwx 1 root root        14 Oct  1 10:18 initrd2 -> initrd2.tmp.efi

Hmm yeah so mkosi is fundamentally a tool that produces multiple output files. Though if we're just building an initrd we definitely shouldn't be producing that many files, only the extra symlink really

@aafeijoo-suse
Copy link
Contributor Author

Hmm yeah so mkosi is fundamentally a tool that produces multiple output files. Though if we're just building an initrd we definitely shouldn't be producing that many files, only the extra symlink really

I don't know how to handle this in mkosi-initrd only, it only knows the name passed via --output. Adding a new option in mkosi?

@DaanDeMeyer
Copy link
Contributor

Hmm yeah so mkosi is fundamentally a tool that produces multiple output files. Though if we're just building an initrd we definitely shouldn't be producing that many files, only the extra symlink really

I don't know how to handle this in mkosi-initrd only, it only knows the name passed via --output. Adding a new option in mkosi?

This isn't trivial to handle in mkosi either. We have many implicit checks that rely on the fact that we empty the output directory first. You can probably handle it in mkosi-initrd by outputting to a temporary directory and then moving everything into place.

@aafeijoo-suse
Copy link
Contributor Author

This isn't trivial to handle in mkosi either. We have many implicit checks that rely on the fact that we empty the output directory first. You can probably handle it in mkosi-initrd by outputting to a temporary directory and then moving everything into place.

This is exactly what I'm doing everywhere when I call mkosi-initrd from other places :)
I'll take a look at it, thanks.

aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 2, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 2, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 2, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
aafeijoo-suse added a commit to aafeijoo-suse/mkosi that referenced this issue Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants