Hi Kairui,
On Fri, Jan 14, 2022 at 4:22 PM Kairui Song <ryncsn(a)gmail.com> wrote:
Tao Liu <ltao(a)redhat.com> 于2022年1月14日周五 15:50写道:
>
> Previously if users have not specified the compress method for
> kdump initramfs, "--compress zstd" will be appended to dracut cmdline.
>
> It will make dracut to decide: a) if dracut module squash is added,
> libzstd will be used, b) otherwise cmd zstd will be used. However in
> kexec-tools we cannot guarantee dracut module squash is added, for
> example users can set omit_dracutmodules in dracut.conf and etc, so
> is_squash_available is not able to check is libzstd or cmd zstd
> been used at runtime.
>
> This patch will give "--compress zstd" a try first, if fails, it will
> just proceed the default compression method, in order to avoid the
> error prone zstd/libzstd dependence check.
>
> Fixes: 7de4a0d ("Set zstd as recommented for kexec-tools")
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
>
> v1 -> v2:
> Modified the commit message
> Notify user through ddebug if zstd-try fails, and redirect the error
> message to ddebug as well.
>
> ---
> kdump-lib.sh | 5 -----
> mkdumprd | 23 ++++++++++++++---------
> mkfadumprd | 9 +++++++--
> 3 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 2ea51be..6415dda 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -31,11 +31,6 @@ is_squash_available()
> done
> }
>
> -is_zstd_command_available()
> -{
> - [[ -x "$(command -v zstd)" ]]
> -}
> -
> perror_exit()
> {
> derror "$@"
> diff --git a/mkdumprd b/mkdumprd
> index 593ec77..0a81dc7 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -431,15 +431,6 @@ 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
> - add_dracut_arg "--compress" "zstd"
> - fi
> -fi
> -
> if [[ -n $extra_modules ]]; then
> add_dracut_arg "--add-drivers" "$extra_modules"
> fi
> @@ -457,6 +448,20 @@ if ! is_fadump_capable; then
> add_dracut_arg "--no-hostonly-default-device"
> fi
>
> +if ! have_compression_in_dracut_args; then
> + # Here we will first try to compress in zstd, if fails we will proceed
> + # the default compress method.
> + dracut "${dracut_args[@]}" "--compress" "zstd"
"$@" 2> >(ddebug)
> + _rc=$?
> +
> + if [[ $_rc -eq 0 ]]; then
> + sync
> + exit $_rc
> + else
> + ddebug "Failback to default compress method"
> + fi
> +fi
> +
> dracut "${dracut_args[@]}" "$@"
>
> _rc=$?
> diff --git a/mkfadumprd b/mkfadumprd
> index 86dfcee..0d40975 100644
> --- a/mkfadumprd
> +++ b/mkfadumprd
> @@ -64,8 +64,13 @@ fi
>
> # Same as setting zstd in mkdumprd
> if ! have_compression_in_dracut_args; then
> - if is_squash_available || is_zstd_command_available; then
> - _dracut_isolate_args+=(--compress zstd)
> + dracut --force --quiet "${_dracut_isolate_args[@]}"
"--compress" "zstd" "$@" "$TARGET_INITRD" 2>
>(ddebug)
> + _rc=$?
> +
> + if [[ $_rc -eq 0 ]]; then
> + exit $_rc
> + else
> + ddebug "Failback to default compress method for fadump"
> fi
> fi
>
This will make dracut try again if it fails with any reason, even if
not caused by an absent compressor, which doesn't seem a good
behavior...
Thanks for the comments!
There are 3 ways:
a.) If the 1st dracut try (with --compress zstd) fails, we report
everything to users and stop.
b.) If the 1st dracut try (with --compress zstd) fails, we don't
report anything, just continue the 2nd dracut try.
c.) If the 1st dracut try (with --compress zstd) fails, we do error
message identification. If error is produced by zstd/libzstd, we
filter out it and continue the 2nd dracut try. If not, we report it
and stop.
For a, not good.
For b, this patch.
For c, the ideal plan, however we cannot perfectly identify out
zstd/libzstd errors. "zstd: command not found" is the one we ensure
belongs to zstd/libzstd, but how about the other messages? If we do
not report them and continue 2nd try, it becomes case b; if we report
them and stop here, users may get confused by zstd/libzstd related
messages, since it is unexpected and no longer seamless.
My thought is, since zstd is relatively new compared to the default
compressor(zlib/gzip), I think it is better to not go too far at one
step. We can switch to way c later after some inspection. As for b, it
just takes a little more time, no other side effects.
Any ideas?
Thanks,
Tao Liu