Added dm-verity setup to initoverlayfs-install script #71
Conversation
Signed-off-by: Martin McConnell <mmcconne@redhat.com>
martinmcconnell
requested review from
ericcurtin,
rhatdan,
dougsland and
masneyb
as code owners
|
@martinmcconnell probably you missed to setup username and email in git and use |
|
Also, we just merged a patch, you might need a rebase as well. |
| @@ -37,6 +37,30 @@ exec_erofs() { | |||
| popd | |||
| rm -f "${INITRAMFS_DIR}/initoverlayfs-$kver.img" | |||
| mkfs.erofs $erofs_compression "${INITRAMFS_DIR}/initoverlayfs-$kver.img" ${INITRAMFS_DUMP_DIR} | |||
|
|
|||
| generate_dm_verity_hash "${INITRAMFS_DIR}/initoverlayfs-$kver.img" "${INITRAMFS_DIR}/verity_table.img" $INITOVERLAYFS_CONF | |||
There was a problem hiding this comment.
just wondering if there are extra spaces in the indentation here.
There was a problem hiding this comment.
Weird that indentation is not in my file. Newline is there but can be removed if requested.
Should I close this pull request and fix the issues?
There was a problem hiding this comment.
@martinmcconnell as there is a patch merged recently might be easier to you instead of rebase manually.
| echo "root_hash=$verity_root_hash" >> "$conf_path" | ||
| # remove tempfile | ||
| echo "dm-verity setup complete" | systemd-cat -t initoverlayfs-setup | ||
| # rm -f verity_output.txt |
There was a problem hiding this comment.
probably need to remove this comment?
| echo "Confirmation: dm-verity setup completed successfully." | ||
| rm -f verity_output.txt | ||
| else | ||
| echo"Warning: dm-verity error" |
There was a problem hiding this comment.
we need a space after echo.
There was a problem hiding this comment.
question, should we fail here?
There was a problem hiding this comment.
Note the top of this script has:
set -ex
we automatically should fail when:
veritysetup format
has a non-zero exit code anyway.
| @@ -37,6 +37,30 @@ exec_erofs() { | |||
| popd | |||
| rm -f "${INITRAMFS_DIR}/initoverlayfs-$kver.img" | |||
| mkfs.erofs $erofs_compression "${INITRAMFS_DIR}/initoverlayfs-$kver.img" ${INITRAMFS_DUMP_DIR} | |||
|
|
|||
| generate_dm_verity_hash "${INITRAMFS_DIR}/initoverlayfs-$kver.img" "${INITRAMFS_DIR}/verity_table.img" $INITOVERLAYFS_CONF | |||
| if journalctl -b -o short-monotonic | grep -qi "dm-verity setup for initoverlayfs complete"; then | |||
There was a problem hiding this comment.
optional: can we use long options instead of -b (--boot) and -o (--output), it makes easy to review.
| local conf_path="$3" | ||
|
|
||
| # Generate dm-verity hash for the EROFS image | ||
| veritysetup format "$image_path" "$hash_table_path" &> verity_output.log |
There was a problem hiding this comment.
can we use /tmp/verity_output.log? Probably TMPDIR is not set at this stage to use it. Also, I would suggest create a constant for /tmp/verity_output.log as it's used in more than place.
There was a problem hiding this comment.
Lets just not create a logging system and leave the processes dump to stdout/stderr like all the other processes in this file...
| else | ||
| CAT="cat" | ||
| fi | ||
| mkdir -p "${INITRAMFS_DUMP_DIR}" |
|
|
||
| echo " - File path: ${file_path}" | ||
| echo " - Decompressor: $CAT" | ||
| file_path="${INITRAMFS_DIR}/initramfs-$kver.img" |
There was a problem hiding this comment.
might be indentation too?
There was a problem hiding this comment.
This all has to be reverted in fact detect_initramfs should not change
|
Thanks Doug, I'll get to it. will close this shortly. |
| @@ -27,8 +27,8 @@ detect_path_initramfs() { | |||
|
|
|||
| done | |||
|
|
|||
| # on first build, like in osbuild, there will be no prior initrd to detect | |||
| INITRAMFS_DIR="/boot" | |||
| echo "Cannot detect initramfs path, aborting..." | |||
There was a problem hiding this comment.
This needs to be changed back
|
This whole PR, should be just one line:
Many of these changes were brought in accidentally I believe from working off an older version of the code, that's ok, we can fix it :) |
No description provided.