Hi Coiby,
On Mon, Jan 17, 2022 at 2:40 PM Coiby Xu <coxu(a)redhat.com> wrote:
Hi Tao,
On Fri, Jan 14, 2022 at 07:37:38PM +0800, Tao Liu wrote:
>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.
Do you think checking if the squash module would be added to the kdump
initramfs without actually running dracut by looking at /etc/dracut.conf
and the dracut options added by mkdumprd/mkfadumprd would be better?
Thanks for the comments!
I don't think it is a good idea to check dracut configs in
kexec-tools. On one hand, it is dracut's responsibility to handle
what's in the config file instead of kexec-tools, if dracut provides a
config-check function for kexec-tools to use, then it is fine, but
apparently it doesn't have one. On the other hand, according to `man 5
dracut.conf`, there are many more files and locations for dracut
custom configs, we must check those as well, which is more
complicated...
Thanks,
Tao Liu
>
>Any ideas?
>
>Thanks,
>Tao Liu
>
--
Best regards,
Coiby