10:50 p.m.
Hi Philipp,
On Tue, Aug 16, 2022 at 6:23 PM Philipp Rudo <prudo(a)redhat.com> wrote:
Hi Tao,
On Tue, 16 Aug 2022 16:57:09 +0800
Tao Liu <ltao(a)redhat.com> wrote:
> Previously kexec-tools will pass "--compress zstd" to dracut. It
> will make dracut to decide whether: a) call mksquashfs to make a
> zstd format squash-root.img, b) call cmd zstd to make a initramfs.
>
> Currently dracut has decoupled the compressor for dracut and
> dracut-squash, So in this patch, we will pass the compressor seperately.
Please mention the exact dracut version instead of just "currently".
FWIW this should do
In dracut 057 the compressor for dracut and dracut-squash was
decoupled. So in this patch pass the compressor separately.
OK, agreed.
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
>
> v1 -> v2:
> 1) Add a checker if dracut doesn't have "--squash-compressor"
> option.
> 2) Removed the "Recommends: zstd" in spec file. Since it is optional
> and won't break any function if not present.
>
> ---
> kdump-lib.sh | 7 ++++++-
> kexec-tools.spec | 1 -
> mkdumprd | 7 +++----
> mkfadumprd | 4 +++-
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index d362e84..77a51d6 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -37,6 +37,11 @@ is_zstd_command_available()
> [[ -x "$(command -v zstd)" ]]
> }
>
> +dracut_have_squash_compressor_option()
> +{
> + ! $(dracut --squash-compressor 2>&1 | grep -q "unrecognized
option")
> +}
multiple thoughts on this one
1) when we bump the minimum dracut version to 057 this function won't
be needed anymore as we can assume --squash-compressor to be present.
2) this function could easily be generalized in case that is needed.
I.e.
dracut_has_option()
{
! $(dracut "$1" 2>&1 | grep -q "unrecognized option")
}
Great suggestion! I think it is much better.
> +
> perror_exit()
> {
> derror "$@"
> @@ -458,7 +463,7 @@ is_wdt_active()
> have_compression_in_dracut_args()
> {
> [[ "$(kdump_get_conf_val dracut_args)" =~ \
> -
(^|[[:space:]])--(gzip|bzip2|lzma|xz|lzo|lz4|zstd|no-compress|compress)([[:space:]]|$) ]]
> +
(^|[[:space:]])--(gzip|bzip2|lzma|xz|lzo|lz4|zstd|no-compress|compress|squash-compressor)([[:space:]]|$)
]]
> }
>
> # If "dracut_args" contains "--mount" information, use it
> diff --git a/kexec-tools.spec b/kexec-tools.spec
> index 24c616d..7e8fa9c 100644
> --- a/kexec-tools.spec
> +++ b/kexec-tools.spec
> @@ -70,7 +70,6 @@ Requires: dracut >= 050
> Requires: dracut-network >= 050
> Requires: dracut-squash >= 050
> Requires: ethtool
> -Recommends: zstd
> Recommends: grubby
> Recommends: hostname
> BuildRequires: make
> diff --git a/mkdumprd b/mkdumprd
> index 3e250e0..ae2e790 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -432,10 +432,9 @@ done <<< "$(kdump_read_conf)"
> handle_default_dump_target
>
> if ! have_compression_in_dracut_args; then
> - # Here zstd is set as the default compression method. If squash module
> - # is available for dracut, libzstd will be used by mksquashfs. If
> - # squash module is unavailable, command zstd will be used instead.
> - if is_squash_available || is_zstd_command_available; then
> + if is_squash_available && dracut_have_squash_compressor_option; then
> + add_dracut_arg "--squash-compressor" "zstd"
> + elif is_zstd_command_available; then
> add_dracut_arg "--compress" "zstd"
> fi
I don't believe this will work as you expect. At least not without
bumping dracut to 057. For that case there could be the situation where
is_squash_available && !dracut_has_option --squash-compressor &&
!is_zsdt_command_available
in the old code this situation added --compress zsdt but the new code
doesn't add anything.
The previous implementation is problematic, the current modification
here is to correct it,
so the is_squash_available && !dracut_has_option --squash-compressor
&& !is_zsdt_command_available case is left unprocessed on purpose.
Actually, the situation when we want to call zstd compression is:
1) If squash function OK, we want dracut to invoke mksquashfs to make
a zstd format squash-root.img within initramfs.
2) If squash function is not OK, and cmd zstd presents, we want dracut
to invoke cmd zstd to make a a zstd format initramfs.
is_zstd_command_available check can handle case 2 completely.
However, for the is_squash_available check, it cannot handle case 1
completely. It only checks if the kernel supports squashfs, it doesn't
check whether the squash module has been added by dracut when making
initramfs. In fact, in kexec-tools we are unable to do the check,
there are multiple ways to forbit dracut to load a module, such as
"dracut -o module" and "omit_dracutmodules in dracut.conf".
So for the previous implementation, when squash dracut module is
omitted, is_squash_available check will still pass, so "--compress
zstd" will be appended to dracut cmdline, and it will call cmd zstd to
do the compression. However cmd zstd may not exist, so it fails.
The previous "--compress zstd" is ambiguous, after the intro of
"--squash-compressor", "--squash-compressor" only effect for
mksquashfs and "--compress" only effect for specific cmd.
So for the is_squash_available && !dracut_has_option
--squash-compressor && !is_zsdt_command_available case, we just leave
it to be handled the default way.
Thanks,
Tao Liu
Thanks
Philipp
> fi
> diff --git a/mkfadumprd b/mkfadumprd
> index 86dfcee..f3fd282 100644
> --- a/mkfadumprd
> +++ b/mkfadumprd
> @@ -64,7 +64,9 @@ fi
>
> # Same as setting zstd in mkdumprd
> if ! have_compression_in_dracut_args; then
> - if is_squash_available || is_zstd_command_available; then
> + if is_squash_available && dracut_have_squash_compressor_option; then
> + _dracut_isolate_args+=(--squash-compressor zstd)
> + elif is_zstd_command_available; then
> _dracut_isolate_args+=(--compress zstd)
> fi
> fi