[PATCH v3] Seperate dracut and dracut-squash compressor for zstd

[Tao Liu]

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.

Since dracut(>= 057) has decoupled the compressor for dracut and
dracut-squash, So in this patch, we will pass the compressor seperately.

Note:

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 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".

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.

Reviewed-by: Philipp Rudo 
Signed-off-by: Tao Liu 
---
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.

v2 -> v3: updated the commit log.
---
 kdump-lib.sh     | 8 +++++++-
 kexec-tools.spec | 1 -
 mkdumprd         | 7 +++----
 mkfadumprd       | 4 +++-
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh
index 4b88908..f93e7d6 100755
--- a/kdump-lib.sh
+++ b/kdump-lib.sh
@@ -37,6 +37,12 @@ is_zstd_command_available()
    [[ -x "$(command -v zstd)" ]]
 }

+dracut_have_option()
+{
+	local _option=$1
+	! $(dracut "$_option" 2>&1 | grep -q "unrecognized option")
+}
+
 perror_exit()
 {
    derror "$@"
@@ -458,7 +464,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 ee43b13..c829bbe 100644
--- a/kexec-tools.spec
+++ b/kexec-tools.spec
@@ -64,7 +64,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 c0f131d..f553b45 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_option "--squash-compressor"; then
+		add_dracut_arg "--squash-compressor" "zstd"
+	elif is_zstd_command_available; then
    	add_dracut_arg "--compress" "zstd"
    fi
 fi
diff --git a/mkfadumprd b/mkfadumprd
index 86dfcee..f353f15 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_option "--squash-compressor"; then
+		_dracut_isolate_args+=(--squash-compressor zstd)
+	elif is_zstd_command_available; then
    	_dracut_isolate_args+=(--compress zstd)
    fi
 fi

[Coiby Xu]

Hi Tao,

On Fri, Sep 23, 2022 at 07:36:18PM +0800, Tao Liu 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.
>
> Since dracut(>= 057) has decoupled the compressor for dracut and
> dracut-squash, So in this patch, we will pass the compressor seperately.
>
> Note:
>
> 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:
> 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 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".
>
> 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.
>
> Reviewed-by: Philipp Rudo 
> Signed-off-by: Tao Liu
>
> v1 -> v2:
> 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.
> v2 -> v3: updated the commit log.
>
> kdump-lib.sh     | 8 +++++++-
> kexec-tools.spec | 1 -
> mkdumprd         | 7 +++----
> mkfadumprd       | 4 +++-
> 4 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 4b88908..f93e7d6 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -37,6 +37,12 @@ is_zstd_command_available()
>    [[ -x "$(command -v zstd)" ]]
> }
>
> +dracut_have_option()
> +{
> local _option=$1
> ! $(dracut "$_option" 2>&1 | grep -q "unrecognized option")
>
> +}

shellcheck catches an issue [1],

In kdump-lib.sh line 45:
    ! $(dracut "$_option" 2>&1 | grep -q "unrecognized option")
           ^-- SC2091 (warning): Remove surrounding $() to avoid executing output (or use eval if intentional).

[1] https://github.com/coiby/kexec-tools/actions/runs/3148610706/jobs/5119328225
> perror_exit()
> {
>    derror "$@"
> @@ -458,7 +464,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 ee43b13..c829bbe 100644
> --- a/kexec-tools.spec
> +++ b/kexec-tools.spec
> @@ -64,7 +64,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 c0f131d..f553b45 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_option "--squash-compressor"; then
> add_dracut_arg "--squash-compressor" "zstd"
> elif is_zstd_command_available; then
> add_dracut_arg "--compress" "zstd"
> fi
>
> fi
> diff --git a/mkfadumprd b/mkfadumprd
> index 86dfcee..f353f15 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_option "--squash-compressor"; then
> _dracut_isolate_args+=(--squash-compressor zstd)
> elif is_zstd_command_available; then
> _dracut_isolate_args+=(--compress zstd)
> fi
> fi
>
> 2.33.1

[Tao Liu]

Hi Coiby,

On Thu, Sep 29, 2022 at 12:25 PM Coiby Xu  wrote:
> Hi Tao,
>
> On Fri, Sep 23, 2022 at 07:36:18PM +0800, Tao Liu 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.
> >
> > Since dracut(>= 057) has decoupled the compressor for dracut and
> > dracut-squash, So in this patch, we will pass the compressor seperately.
> >
> > Note:
> >
> > 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:
> > 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 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".
> >
> > 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.
> >
> > Reviewed-by: Philipp Rudo 
> > Signed-off-by: Tao Liu
> >
> > v1 -> v2:
> > 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.
> > v2 -> v3: updated the commit log.
> >
> > kdump-lib.sh     | 8 +++++++-
> > kexec-tools.spec | 1 -
> > mkdumprd         | 7 +++----
> > mkfadumprd       | 4 +++-
> > 4 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/kdump-lib.sh b/kdump-lib.sh
> > index 4b88908..f93e7d6 100755
> > --- a/kdump-lib.sh
> > +++ b/kdump-lib.sh
> > @@ -37,6 +37,12 @@ is_zstd_command_available()
> >       [[ -x "$(command -v zstd)" ]]
> > }
> >
> > +dracut_have_option()
> > +{
> >  local _option=$1
> >  ! $(dracut "$_option" 2>&1 | grep -q "unrecognized option")
> >
> > +}
>
> shellcheck catches an issue [1],
>
> In kdump-lib.sh line 45:
>         ! $(dracut "$_option" 2>&1 | grep -q "unrecognized option")
>            ^-- SC2091 (warning): Remove surrounding $() to avoid executing output (or use eval if intentional).

Thanks for pointing it out, I will fix it in v4.

Thanks,
Tao Liu
> [1] https://github.com/coiby/kexec-tools/actions/runs/3148610706/jobs/5119328225
> > perror_exit()
> > {
> >       derror "$@"
> > @@ -458,7 +464,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 ee43b13..c829bbe 100644
> > --- a/kexec-tools.spec
> > +++ b/kexec-tools.spec
> > @@ -64,7 +64,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 c0f131d..f553b45 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_option "--squash-compressor"; then
> >          add_dracut_arg "--squash-compressor" "zstd"
> >  elif is_zstd_command_available; then
> >         add_dracut_arg "--compress" "zstd"
> > fi
> >
> > fi
> > diff --git a/mkfadumprd b/mkfadumprd
> > index 86dfcee..f353f15 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_option "--squash-compressor"; then
> >          _dracut_isolate_args+=(--squash-compressor zstd)
> >  elif is_zstd_command_available; then
> >         _dracut_isolate_args+=(--compress zstd)
> > fi
> > fi
> >
> > 2.33.1
>
> --
> Best regards,
> Coiby