-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 lvm extension and fs-cryptroot extension #7527
fix lvm extension and fs-cryptroot extension #7527
Conversation
c24567d
to
8cf5289
Compare
…ck of the physical partition/uuid and rootdevice/uuid seperately
8cf5289
to
c60d751
Compare
…naming scheme; cleanup
c60d751
to
e5280ab
Compare
Thank you for looking into this. Can you check if documentation needs updating? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
It seems this method worsen loop device management: |
Did you perhaps had time to inspect this deeper? We have troubles making daily images after this PR. Loop management was IIRC pain since ever while this change made it a lot more unstable https://paste.armbian.de/ikiduqifum |
That's annoying - maybe I have a hunch. I will have a look at it in my lunch break. |
Description (hint: further down is a TLDR)
The lvm extension replaces the value of
$rootdevice
(defined in partitioning:280) with a reference to the device mapper to the rootfs partition on lvm, but in partitioning.sh:303 - where the uuid of the filesystem which contains the rootfs is needed, ${LOOP}p2 was used (instead of $rootdevice), soblkid
could not find the UUID and the script failed.stacktrace
While working on that Issue I stumbled over leftovers from a/the previous executions and had to remove the lvm volume group + loopdevices manually so I added a cleanup handler to the extension in case of errors.
In the meantime i.e. revelations/discoveries while creating this PR:
Seems this is a regression bug as this line of code was already adapted in #7217
But reintroduced in 8175192 while fixing the fs-cryptroot extension (from the commit naming - as I stumbled over the messed up UUID myself multiple weeks ago and it was working after git pull this seems reasonable)
Plan: I will try to make both extensions work as they seem to interfere with each other:
TLDR: report: fixing lvm + cryptroot
context: With the merge of the fs-cryptroot extension fix about 3 weeks ago the lvm extension broke as they both "shared" the rootdevice variable which resulted in a wrong UUID assigned to crypttab/lvm.
solution: adding another variable to keep track of the "actual"/outmost/first/.. root partition.
solution: moved loop device creation logic for
${SDCARD}.raw
fromlvm.post_create_partitions
into the default behaviour, if no LOOP device is set by an extension viapost_create_partitions
hook. This enabled the lvm extension to accept a different device than only the device pointing to${SDCARD}.raw
- e.g. a device mapper from fs-cryptroot ->added cleanup handlers to both extensions to cleanup volume groups/close LUKS in case of an error (as manually removing leftovers was a bit tedious ;-)) - reduced hardcoded cryptroot extension cleanup code.
solution: move export into a hook to use the same naming scheme as for .img for the exported private key.
maybe this https://armbian.atlassian.net/browse/AR-2489 can be closed afterwards?
How Has This Been Tested?
only lvm
./compile.sh BOARD=rock-3a RELEASE=bookworm BRANCH=vendor DEB_COMPRESS=xz KERNEL_CONFIGURE=no KERNEL_GIT=shallow BUILD_DESKTOP=no BUILD_MINIMAL=no ENABLE_EXTENSIONS="lvm"
cryptroot
./compile.sh BOARD=rock-3a RELEASE=bookworm BRANCH=vendor DEB_COMPRESS=xz KERNEL_CONFIGURE=no KERNEL_GIT=shallow BUILD_DESKTOP=no BUILD_MINIMAL=no CRYPTROOT_ENABLE=yes CRYPTROOT_PASSPHRASE=1234 ENABLE_EXTENSIONS=""
lvm+cryptroot
./compile.sh BOARD=rock-3a RELEASE=bookworm BRANCH=vendor DEB_COMPRESS=xz KERNEL_CONFIGURE=no KERNEL_GIT=shallow BUILD_DESKTOP=no BUILD_MINIMAL=no CRYPTROOT_ENABLE=yes CRYPTROOT_PASSPHRASE=1234 ENABLE_EXTENSIONS="lvm"
bare
./compile.sh BOARD=rock-3a RELEASE=bookworm BRANCH=vendor DEB_COMPRESS=xz KERNEL_CONFIGURE=no KERNEL_GIT=shallow BUILD_DESKTOP=no BUILD_MINIMAL=no
Checklist: