6:17 a.m.
This set tries to do a few improvements to fadump. The 1st patch makes
'zstd' as the default compression method for fadump. The next patch
fixes the default initrd backup/restore logic. The 3rd patch ensures
fadump initramfs is cleaned up on kernel uninstall.
Changes in v2:
* Improve changelog on why 'zstd' is used for fadump in patch#1.
* Patch#2: A new patch to fix default initrd backup/restore logic.
* Patch#3: Cleaning up kernel version based checksum file.
Hari Bathini (3):
fadump: use 'zstd' as the default compression method
fadump: fix default initrd backup and restore logic
fadump: add a kernel install hook to clean up fadump initramfs
60-fadump.install | 32 ++++++++++++++++++++++++++++++++
kdumpctl | 8 ++++----
kexec-tools.spec | 3 +++
mkfadumprd | 6 ++----
4 files changed, 41 insertions(+), 8 deletions(-)
create mode 100755 60-fadump.install
--
2.38.1
6:18 a.m.
New subject: [PATCH v2 1/3] fadump: use 'zstd' as the default compression method
If available, use 'zstd' compression method to optimize the size of
the initrd built with fadump support. Also, 'squash+zstd' is not
preferred because more disk space is consumed with 'squash+zstd' due
to the additional binaries needed for fadump with squash case.
Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
---
Changes in v2:
* Updated changelog with why 'zstd' is used for fadump.
mkfadumprd | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/mkfadumprd b/mkfadumprd
index 36ad645..2fc396c 100644
--- a/mkfadumprd
+++ b/mkfadumprd
@@ -62,11 +62,9 @@ _dracut_isolate_args=(
/usr/lib/dracut/fadump-kernel-modules.txt
)
-# Same as setting zstd in mkdumprd
+# Use zstd compression method, if available
if ! have_compression_in_dracut_args; then
- if is_squash_available && dracut_have_option "--squash-compressor";
then
- _dracut_isolate_args+=(--squash-compressor zstd)
- elif is_zstd_command_available; then
+ if is_zstd_command_available; then
_dracut_isolate_args+=(--compress zstd)
fi
fi
--
2.38.1
7:24 p.m.
New subject: [PATCH v2 1/3] fadump: use 'zstd' as the default compression method
Hi Hari,
On Mon, Nov 21, 2022 at 8:24 PM Hari Bathini <hbathini(a)linux.ibm.com> wrote:
If available, use 'zstd' compression method to optimize the size of
the initrd built with fadump support. Also, 'squash+zstd' is not
preferred because more disk space is consumed with 'squash+zstd' due
to the additional binaries needed for fadump with squash case.
Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
---
Changes in v2:
* Updated changelog with why 'zstd' is used for fadump.
Thanks for the update, the patch looks good to me.
Acked-by: Tao Liu <ltao(a)redhat.com>
mkfadumprd | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/mkfadumprd b/mkfadumprd
index 36ad645..2fc396c 100644
--- a/mkfadumprd
+++ b/mkfadumprd
@@ -62,11 +62,9 @@ _dracut_isolate_args=(
/usr/lib/dracut/fadump-kernel-modules.txt
)
-# Same as setting zstd in mkdumprd
+# Use zstd compression method, if available
if ! have_compression_in_dracut_args; then
- if is_squash_available && dracut_have_option
"--squash-compressor"; then
- _dracut_isolate_args+=(--squash-compressor zstd)
- elif is_zstd_command_available; then
+ if is_zstd_command_available; then
_dracut_isolate_args+=(--compress zstd)
fi
fi
--
2.38.1
_______________________________________________
kexec mailing list -- kexec(a)lists.fedoraproject.org
To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
8:12 p.m.
New subject: [PATCH v2 1/3] fadump: use 'zstd' as the default compression
method
On Tue, Nov 22, 2022 at 09:24:24AM +0800, Tao Liu wrote:
Hi Hari,
On Mon, Nov 21, 2022 at 8:24 PM Hari Bathini <hbathini(a)linux.ibm.com> wrote:
>
> If available, use 'zstd' compression method to optimize the size of
> the initrd built with fadump support. Also, 'squash+zstd' is not
> preferred because more disk space is consumed with 'squash+zstd' due
> to the additional binaries needed for fadump with squash case.
>
> Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
> ---
>
> Changes in v2:
> * Updated changelog with why 'zstd' is used for fadump.
>
Thanks for the update, the patch looks good to me.
Acked-by: Tao Liu <ltao(a)redhat.com>
Hi Tao,
When trying to learn how squash affects the memory requirement of kdump,
I notice currently for both F36 and F37, the initramfs is no longer
compressed,
[root@ci-vm-10-0-137-168 t]# file /boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img
/boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img: ASCII cpio archive (SVR4 with no
CRC)
[root@ci-vm-10-0-139-47 ~]# file /boot/initramfs-6.0.9-200.fc36.x86_64kdump.img
/boot/initramfs-6.0.9-200.fc36.x86_64kdump.img: ASCII cpio archive (SVR4 with no CRC)
This is fc1c79ff ("Seperate dracut and dracut-squash compressor for
zstd") disabls passing "--compress zstd" to dracut when the squashfs
dracut module is going to be used. Is the change intentional? If not,
maybe we should drop this patch and do it in mkdumprd instead.
Btw, it seems "--compress zstd" or "--zstd" no longer compresses the
initramfs in rawhide (2022-11-24) whereas in f36 it works as expected. Another
difference between rawhide and f36 is in 36 with "--zstd" not specified,
dracut will gzip-compress the initramfs while in rawhide, it doesn't
compress the initramfs.
>
> mkfadumprd | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/mkfadumprd b/mkfadumprd
> index 36ad645..2fc396c 100644
> --- a/mkfadumprd
> +++ b/mkfadumprd
> @@ -62,11 +62,9 @@ _dracut_isolate_args=(
> /usr/lib/dracut/fadump-kernel-modules.txt
> )
>
> -# Same as setting zstd in mkdumprd
> +# Use zstd compression method, if available
> if ! have_compression_in_dracut_args; then
> - if is_squash_available && dracut_have_option
"--squash-compressor"; then
> - _dracut_isolate_args+=(--squash-compressor zstd)
> - elif is_zstd_command_available; then
> + if is_zstd_command_available; then
> _dracut_isolate_args+=(--compress zstd)
> fi
> fi
> --
> 2.38.1
> _______________________________________________
> kexec mailing list -- kexec(a)lists.fedoraproject.org
> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
> Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
> Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
--
Best regards,
Coiby
8:26 p.m.
New subject: [PATCH v2 1/3] fadump: use 'zstd' as the default compression
method
On Thu, Nov 24, 2022 at 10:12:14AM +0800, Coiby Xu wrote:
On Tue, Nov 22, 2022 at 09:24:24AM +0800, Tao Liu wrote:
>Hi Hari,
>
>On Mon, Nov 21, 2022 at 8:24 PM Hari Bathini <hbathini(a)linux.ibm.com> wrote:
>>
>>If available, use 'zstd' compression method to optimize the size of
>>the initrd built with fadump support. Also, 'squash+zstd' is not
>>preferred because more disk space is consumed with 'squash+zstd' due
>>to the additional binaries needed for fadump with squash case.
>>
>>Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
>>---
>>
>>Changes in v2:
>>* Updated changelog with why 'zstd' is used for fadump.
>>
>
>Thanks for the update, the patch looks good to me.
>
>Acked-by: Tao Liu <ltao(a)redhat.com>
Hi Tao,
When trying to learn how squash affects the memory requirement of kdump,
I notice currently for both F36 and F37, the initramfs is no longer
compressed,
[root@ci-vm-10-0-137-168 t]# file /boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img
/boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img: ASCII cpio archive (SVR4 with no
CRC)
[root@ci-vm-10-0-139-47 ~]# file /boot/initramfs-6.0.9-200.fc36.x86_64kdump.img
/boot/initramfs-6.0.9-200.fc36.x86_64kdump.img: ASCII cpio archive (SVR4 with no CRC)
This is fc1c79ff ("Seperate dracut and dracut-squash compressor for
zstd") disabls passing "--compress zstd" to dracut when the squashfs
dracut module is going to be used. Is the change intentional? If not,
maybe we should drop this patch and do it in mkdumprd instead.
Btw, it seems "--compress zstd" or "--zstd" no longer compresses the
initramfs in rawhide (2022-11-24) whereas in f36 it works as expected. Another
difference between rawhide and f36 is in 36 with "--zstd" not specified,
dracut will gzip-compress the initramfs while in rawhide, it doesn't
compress the initramfs.
After having a further look, I realize the latter issue has nothing to
do with the difference between rawhide and f36. Once the dracut squash
module is enabled, "--compress zstd" will no longer work.
--
Best regards,
Coiby
11:55 p.m.
New subject: [PATCH v2 1/3] fadump: use 'zstd' as the default compression
method
On Thu, Nov 24, 2022 at 10:26:22AM +0800, Coiby Xu wrote:
On Thu, Nov 24, 2022 at 10:12:14AM +0800, Coiby Xu wrote:
>On Tue, Nov 22, 2022 at 09:24:24AM +0800, Tao Liu wrote:
>>Hi Hari,
>>
>>On Mon, Nov 21, 2022 at 8:24 PM Hari Bathini <hbathini(a)linux.ibm.com>
wrote:
>>>
>>>If available, use 'zstd' compression method to optimize the size of
>>>the initrd built with fadump support. Also, 'squash+zstd' is not
>>>preferred because more disk space is consumed with 'squash+zstd' due
>>>to the additional binaries needed for fadump with squash case.
>>>
>>>Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
>>>---
>>>
>>>Changes in v2:
>>>* Updated changelog with why 'zstd' is used for fadump.
>>>
>>
>>Thanks for the update, the patch looks good to me.
>>
>>Acked-by: Tao Liu <ltao(a)redhat.com>
>
>Hi Tao,
>
>When trying to learn how squash affects the memory requirement of kdump,
>I notice currently for both F36 and F37, the initramfs is no longer
>compressed,
>
> [root@ci-vm-10-0-137-168 t]# file
/boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img
> /boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img: ASCII cpio archive (SVR4 with no
CRC)
> [root@ci-vm-10-0-139-47 ~]# file /boot/initramfs-6.0.9-200.fc36.x86_64kdump.img
>/boot/initramfs-6.0.9-200.fc36.x86_64kdump.img: ASCII cpio archive (SVR4 with no CRC)
>
>This is fc1c79ff ("Seperate dracut and dracut-squash compressor for
>zstd") disabls passing "--compress zstd" to dracut when the squashfs
>dracut module is going to be used. Is the change intentional? If not,
>maybe we should drop this patch and do it in mkdumprd instead.
>
>Btw, it seems "--compress zstd" or "--zstd" no longer compresses
the
>initramfs in rawhide (2022-11-24) whereas in f36 it works as expected. Another
>difference between rawhide and f36 is in 36 with "--zstd" not specified,
>dracut will gzip-compress the initramfs while in rawhide, it doesn't
>compress the initramfs.
After having a further look, I realize the latter issue has nothing to
do with the difference between rawhide and f36. Once the dracut squash
module is enabled, "--compress zstd" will no longer work.
After having discussion with Tao, I realize my confusion is due to
to my misconception. In fact, dracut explicitly prevents compressing
the initramfs when the squash module is used because there is very
little benefit when compressing it for a second time so it's pointless
to pass "--compress zstd" to dracut in this case. Thanks to Tao's
clarification!
--
Best regards,
Coiby
midnight
New subject: [PATCH v2 1/3] fadump: use 'zstd' as the default compression method
Hi,
Currently the --zstd and --compressor zstd only effect on initramfs
compression, --squash-compressor only effect on squash-root.img
compression. We don't want to compress initramfs and squash-root.img
both, because it is unnecessary, if doing so the squash-root.img will
be compressed 2 times, the 2nd compression ratio will be low and waste
the cpu time for decompression in vain.
So
if squashfs avaliable, --squash-compressor zstd will be passed.
squashfs-root.img will be compressed into zstd, and initramfs will not
be compressed.
if squashfs unavaliable --compress zstd will be passed, and initramfs
will be zstd compressed.
if squashfs avaliable, but --squash-compressor option not supported by
dracut(due to dracut version < 057), squashfs-root.img will be
compressed into default format(zlib).
Thanks,
Tao Liu
On Thu, Nov 24, 2022 at 10:30 AM Coiby Xu <coxu(a)redhat.com> wrote:
On Thu, Nov 24, 2022 at 10:12:14AM +0800, Coiby Xu wrote:
>On Tue, Nov 22, 2022 at 09:24:24AM +0800, Tao Liu wrote:
>>Hi Hari,
>>
>>On Mon, Nov 21, 2022 at 8:24 PM Hari Bathini <hbathini(a)linux.ibm.com>
wrote:
>>>
>>>If available, use 'zstd' compression method to optimize the size of
>>>the initrd built with fadump support. Also, 'squash+zstd' is not
>>>preferred because more disk space is consumed with 'squash+zstd' due
>>>to the additional binaries needed for fadump with squash case.
>>>
>>>Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
>>>---
>>>
>>>Changes in v2:
>>>* Updated changelog with why 'zstd' is used for fadump.
>>>
>>
>>Thanks for the update, the patch looks good to me.
>>
>>Acked-by: Tao Liu <ltao(a)redhat.com>
>
>Hi Tao,
>
>When trying to learn how squash affects the memory requirement of kdump,
>I notice currently for both F36 and F37, the initramfs is no longer
>compressed,
>
> [root@ci-vm-10-0-137-168 t]# file
/boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img
> /boot/initramfs-6.1.0-0.rc6.46.fc38.x86_64kdump.img: ASCII cpio archive (SVR4 with
no CRC)
> [root@ci-vm-10-0-139-47 ~]# file /boot/initramfs-6.0.9-200.fc36.x86_64kdump.img
>/boot/initramfs-6.0.9-200.fc36.x86_64kdump.img: ASCII cpio archive (SVR4 with no
CRC)
>
>This is fc1c79ff ("Seperate dracut and dracut-squash compressor for
>zstd") disabls passing "--compress zstd" to dracut when the squashfs
>dracut module is going to be used. Is the change intentional? If not,
>maybe we should drop this patch and do it in mkdumprd instead.
>
>Btw, it seems "--compress zstd" or "--zstd" no longer compresses
the
>initramfs in rawhide (2022-11-24) whereas in f36 it works as expected. Another
>difference between rawhide and f36 is in 36 with "--zstd" not specified,
>dracut will gzip-compress the initramfs while in rawhide, it doesn't
>compress the initramfs.
After having a further look, I realize the latter issue has nothing to
do with the difference between rawhide and f36. Once the dracut squash
module is enabled, "--compress zstd" will no longer work.
--
Best regards,
Coiby
6:18 a.m.
New subject: [PATCH v2 2/3] fadump: fix default initrd backup and restore logic
In case of fadump, default initrd is rebuilt with dump capturing
capability, as the same initrd is used for booting production kernel
as well as capture kernel.
The original initrd file is backed up with a checksum, to restore
it as the default initrd when fadump is disabled. As the checksum
file is not kernel version specific, switching between different
kernel versions and kdump/fadump dump mode breaks the default initrd
backup/restore logic. Fix this by having a kernel version specific
checksum file.
Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
---
* New patch to fix default initrd backup/restore logic.
kdumpctl | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kdumpctl b/kdumpctl
index 7caaae5..03c91f3 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -163,7 +163,7 @@ backup_default_initrd()
if [[ ! -e $DEFAULT_INITRD_BAK ]]; then
dinfo "Backing up $DEFAULT_INITRD before rebuild."
# save checksum to verify before restoring
- sha1sum "$DEFAULT_INITRD" > "$INITRD_CHECKSUM_LOCATION"
+ sha1sum "$DEFAULT_INITRD" >
"$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER"
if ! cp "$DEFAULT_INITRD" "$DEFAULT_INITRD_BAK"; then
dwarn "WARNING: failed to backup $DEFAULT_INITRD."
rm -f "$DEFAULT_INITRD_BAK"
@@ -181,14 +181,14 @@ restore_default_initrd()
# If a backup initrd exists, we must be switching back from
# fadump to kdump. Restore the original default initrd.
- if [[ -f $DEFAULT_INITRD_BAK ]] && [[ -f $INITRD_CHECKSUM_LOCATION ]]; then
+ if [[ -f $DEFAULT_INITRD_BAK ]] && [[ -f
$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER ]]; then
# verify checksum before restoring
backup_checksum=$(sha1sum "$DEFAULT_INITRD_BAK" | awk '{ print $1
}')
- default_checksum=$(awk '{ print $1 }' "$INITRD_CHECKSUM_LOCATION")
+ default_checksum=$(awk '{ print $1 }'
"$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER")
if [[ $default_checksum != "$backup_checksum" ]]; then
dwarn "WARNING: checksum mismatch! Can't restore original initrd.."
else
- rm -f $INITRD_CHECKSUM_LOCATION
+ rm -f $INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER
if mv "$DEFAULT_INITRD_BAK" "$DEFAULT_INITRD"; then
derror "Restoring original initrd as fadump mode is disabled."
sync -f "$DEFAULT_INITRD"
--
2.38.1
6:08 a.m.
New subject: [PATCH v2 2/3] fadump: fix default initrd backup and restore
logic
Hi Hari,
On Mon, 21 Nov 2022 17:48:01 +0530
Hari Bathini <hbathini(a)linux.ibm.com> wrote:
In case of fadump, default initrd is rebuilt with dump capturing
capability, as the same initrd is used for booting production kernel
as well as capture kernel.
The original initrd file is backed up with a checksum, to restore
it as the default initrd when fadump is disabled. As the checksum
file is not kernel version specific, switching between different
kernel versions and kdump/fadump dump mode breaks the default initrd
backup/restore logic. Fix this by having a kernel version specific
checksum file.
Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
Thanks for taking care!
Two small comments:
1) in backup_default_initrd you should remove the checksum file in the
error case, when the initrd cannot be backed up (this should
probably be done today already...)
2) why don't you simply update INITRD_CHECKSUM_OCATION one instead of
editing every line where it is used? For example consider the patch
below. I see multiple benefits (1) it's shorter, (2) the name shows
clearer to which initrd the checksum belongs to, (3) it reuses
KDUMP_BOOTDIR instead of hard coding /boot and (4)
INITRD_CHECKSUM_LOCATION now is basically local to setup_initrd now
making it easier for a clean up to remove the global variable later
on.
What do you think?
Thanks
Philipp
diff --git a/kdumpctl b/kdumpctl
index 7caaae5..94af3e2 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -9,7 +9,7 @@ KDUMP_LOG_PATH="/var/log"
MKDUMPRD="/sbin/mkdumprd -f"
MKFADUMPRD="/sbin/mkfadumprd"
DRACUT_MODULES_FILE="/usr/lib/dracut/modules.txt"
-INITRD_CHECKSUM_LOCATION="/boot/.fadump_initrd_checksum"
+INITRD_CHECKSUM_LOCATION=""
DEFAULT_INITRD=""
DEFAULT_INITRD_BAK=""
KDUMP_INITRD=""
@@ -166,7 +166,8 @@ backup_default_initrd()
sha1sum "$DEFAULT_INITRD" >
"$INITRD_CHECKSUM_LOCATION"
if ! cp "$DEFAULT_INITRD" "$DEFAULT_INITRD_BAK"; then
dwarn "WARNING: failed to backup $DEFAULT_INITRD."
- rm -f "$DEFAULT_INITRD_BAK"
+ rm -f -- "$INITRD_CHECKSUM_LOCATION"
+ rm -f -- "$DEFAULT_INITRD_BAK"
fi
fi
}
@@ -317,6 +318,7 @@ setup_initrd()
fi
DEFAULT_INITRD_BAK="$KDUMP_BOOTDIR/.$(basename
"$DEFAULT_INITRD").default"
+ INITRD_CHECKSUM_LOCATION="$DEFAULT_INITRD_BAK.checksum"
if [[ $DEFAULT_DUMP_MODE == "fadump" ]]; then
TARGET_INITRD="$DEFAULT_INITRD"
---
* New patch to fix default initrd backup/restore logic.
kdumpctl | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kdumpctl b/kdumpctl
index 7caaae5..03c91f3 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -163,7 +163,7 @@ backup_default_initrd()
if [[ ! -e $DEFAULT_INITRD_BAK ]]; then
dinfo "Backing up $DEFAULT_INITRD before rebuild."
# save checksum to verify before restoring
- sha1sum "$DEFAULT_INITRD" > "$INITRD_CHECKSUM_LOCATION"
+ sha1sum "$DEFAULT_INITRD" >
"$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER"
if ! cp "$DEFAULT_INITRD" "$DEFAULT_INITRD_BAK"; then
dwarn "WARNING: failed to backup $DEFAULT_INITRD."
rm -f "$DEFAULT_INITRD_BAK"
@@ -181,14 +181,14 @@ restore_default_initrd()
# If a backup initrd exists, we must be switching back from
# fadump to kdump. Restore the original default initrd.
- if [[ -f $DEFAULT_INITRD_BAK ]] && [[ -f $INITRD_CHECKSUM_LOCATION ]]; then
+ if [[ -f $DEFAULT_INITRD_BAK ]] && [[ -f
$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER ]]; then
# verify checksum before restoring
backup_checksum=$(sha1sum "$DEFAULT_INITRD_BAK" | awk '{ print $1
}')
- default_checksum=$(awk '{ print $1 }' "$INITRD_CHECKSUM_LOCATION")
+ default_checksum=$(awk '{ print $1 }'
"$INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER")
if [[ $default_checksum != "$backup_checksum" ]]; then
dwarn "WARNING: checksum mismatch! Can't restore original initrd.."
else
- rm -f $INITRD_CHECKSUM_LOCATION
+ rm -f $INITRD_CHECKSUM_LOCATION.$KDUMP_KERNELVER
if mv "$DEFAULT_INITRD_BAK" "$DEFAULT_INITRD"; then
derror "Restoring original initrd as fadump mode is disabled."
sync -f "$DEFAULT_INITRD"
4:28 a.m.
New subject: [PATCH v2 2/3] fadump: fix default initrd backup and restore
logic
Hi Philipp,
Thanks for the review.
On 23/11/22 5:38 pm, Philipp Rudo wrote:
Hi Hari,
On Mon, 21 Nov 2022 17:48:01 +0530
Hari Bathini <hbathini(a)linux.ibm.com> wrote:
> In case of fadump, default initrd is rebuilt with dump capturing
> capability, as the same initrd is used for booting production kernel
> as well as capture kernel.
>
> The original initrd file is backed up with a checksum, to restore
> it as the default initrd when fadump is disabled. As the checksum
> file is not kernel version specific, switching between different
> kernel versions and kdump/fadump dump mode breaks the default initrd
> backup/restore logic. Fix this by having a kernel version specific
> checksum file.
>
> Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
Thanks for taking care!
Two small comments:
1) in backup_default_initrd you should remove the checksum file in the
error case, when the initrd cannot be backed up (this should
probably be done today already...)
2) why don't you simply update INITRD_CHECKSUM_OCATION one instead of
editing every line where it is used? For example consider the patch
below. I see multiple benefits (1) it's shorter, (2) the name shows
clearer to which initrd the checksum belongs to, (3) it reuses
KDUMP_BOOTDIR instead of hard coding /boot and (4)
INITRD_CHECKSUM_LOCATION now is basically local to setup_initrd now
making it easier for a clean up to remove the global variable later
on.
What do you think?
Yeah. Makes it much better and simpler. Will respin with the change.
Thanks
Hari
6:18 a.m.
New subject: [PATCH v2 3/3] fadump: add a kernel install hook to clean up fadump initramfs
Kdump service will create fadump initramfs when needed, but it won't
clean up the fadump initramfs on kernel uninstall. So create a kernel
install hook to do the clean up job.
Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
---
* Could have had 60-kdump.install.ppc64 that takes care of both kdump
& fadump cleanup for ppc64 but that needs kdump cleanup code to be
maintained in two files. So, preferred to go with a separate install
file (60-fadump.install) instead.
Changes in v2:
* Added cleanup of kernel version based checksum file.
60-fadump.install | 32 ++++++++++++++++++++++++++++++++
kexec-tools.spec | 3 +++
2 files changed, 35 insertions(+)
create mode 100755 60-fadump.install
diff --git a/60-fadump.install b/60-fadump.install
new file mode 100755
index 0000000..c317d9c
--- /dev/null
+++ b/60-fadump.install
@@ -0,0 +1,32 @@
+#!/usr/bin/bash
+
+COMMAND="$1"
+KERNEL_VERSION="$2"
+
+if ! [[ ${KERNEL_INSTALL_MACHINE_ID-x} ]]; then
+ exit 0
+fi
+
+# Currently, fadump is supported only in environments with
+# writable /boot directory.
+if [[ ! -w "/boot" ]]; then
+ exit 0
+fi
+
+FADUMP_INITRD_DIR_ABS="/boot"
+FADUMP_INITRD=".initramfs-${KERNEL_VERSION}.img.default"
+FADUMP_INITRD_CHECKSUM=".fadump_initrd_checksum.${KERNEL_VERSION}"
+
+ret=0
+case "$COMMAND" in
+ add)
+ # Do nothing, fadump initramfs is strictly host only
+ # and managed by kdump service
+ ;;
+ remove)
+ rm -f -- "$FADUMP_INITRD_DIR_ABS/$FADUMP_INITRD"
+ rm -f -- "$FADUMP_INITRD_DIR_ABS/$FADUMP_INITRD_CHECKSUM"
+ ret=$?
+ ;;
+esac
+exit $ret
diff --git a/kexec-tools.spec b/kexec-tools.spec
index bfa7a5b..d4c3590 100644
--- a/kexec-tools.spec
+++ b/kexec-tools.spec
@@ -38,6 +38,7 @@ Source33: 92-crashkernel.install
Source34: crashkernel-howto.txt
Source35: kdump-migrate-action.sh
Source36: kdump-restart.sh
+Source37: 60-fadump.install
#######################################
# These are sources for mkdumpramfs
@@ -203,6 +204,7 @@ install -m 644 %{SOURCE13}
$RPM_BUILD_ROOT%{_udevrulesdir}/98-kexec.rules
%endif
%ifarch ppc64 ppc64le
install -m 644 %{SOURCE14} $RPM_BUILD_ROOT%{_udevrulesdir}/98-kexec.rules
+install -m 755 -D %{SOURCE37}
$RPM_BUILD_ROOT%{_prefix}/lib/kernel/install.d/60-fadump.install
%endif
install -m 644 %{SOURCE15} $RPM_BUILD_ROOT%{_mandir}/man5/kdump.conf.5
install -m 644 %{SOURCE16} $RPM_BUILD_ROOT%{_unitdir}/kdump.service
@@ -365,6 +367,7 @@ fi
%endif
%ifarch ppc64 ppc64le
/usr/sbin/mkfadumprd
+%{_prefix}/lib/kernel/install.d/60-fadump.install
%endif
/usr/sbin/mkdumprd
/usr/sbin/vmcore-dmesg
--
2.38.1
6:07 a.m.
New subject: [PATCH v2 3/3] fadump: add a kernel install hook to clean up
fadump initramfs
Hi Hari,
On Mon, 21 Nov 2022 17:48:02 +0530
Hari Bathini <hbathini(a)linux.ibm.com> wrote:
Kdump service will create fadump initramfs when needed, but it
won't
clean up the fadump initramfs on kernel uninstall. So create a kernel
install hook to do the clean up job.
Signed-off-by: Hari Bathini <hbathini(a)linux.ibm.com>
---
* Could have had 60-kdump.install.ppc64 that takes care of both kdump
& fadump cleanup for ppc64 but that needs kdump cleanup code to be
maintained in two files. So, preferred to go with a separate install
file (60-fadump.install) instead.
Besides the potential name change for the checksum.
Reviewed-by: Philipp Rudo <prudo(a)redhat.com>
Changes in v2:
* Added cleanup of kernel version based checksum file.
60-fadump.install | 32 ++++++++++++++++++++++++++++++++
kexec-tools.spec | 3 +++
2 files changed, 35 insertions(+)
create mode 100755 60-fadump.install
diff --git a/60-fadump.install b/60-fadump.install
new file mode 100755
index 0000000..c317d9c
--- /dev/null
+++ b/60-fadump.install
@@ -0,0 +1,32 @@
+#!/usr/bin/bash
+
+COMMAND="$1"
+KERNEL_VERSION="$2"
+
+if ! [[ ${KERNEL_INSTALL_MACHINE_ID-x} ]]; then
+ exit 0
+fi
+
+# Currently, fadump is supported only in environments with
+# writable /boot directory.
+if [[ ! -w "/boot" ]]; then
+ exit 0
+fi
+
+FADUMP_INITRD_DIR_ABS="/boot"
+FADUMP_INITRD=".initramfs-${KERNEL_VERSION}.img.default"
+FADUMP_INITRD_CHECKSUM=".fadump_initrd_checksum.${KERNEL_VERSION}"
+
+ret=0
+case "$COMMAND" in
+ add)
+ # Do nothing, fadump initramfs is strictly host only
+ # and managed by kdump service
+ ;;
+ remove)
+ rm -f -- "$FADUMP_INITRD_DIR_ABS/$FADUMP_INITRD"
+ rm -f -- "$FADUMP_INITRD_DIR_ABS/$FADUMP_INITRD_CHECKSUM"
+ ret=$?
+ ;;
+esac
+exit $ret
diff --git a/kexec-tools.spec b/kexec-tools.spec
index bfa7a5b..d4c3590 100644
--- a/kexec-tools.spec
+++ b/kexec-tools.spec
@@ -38,6 +38,7 @@ Source33: 92-crashkernel.install
Source34: crashkernel-howto.txt
Source35: kdump-migrate-action.sh
Source36: kdump-restart.sh
+Source37: 60-fadump.install
#######################################
# These are sources for mkdumpramfs
@@ -203,6 +204,7 @@ install -m 644 %{SOURCE13}
$RPM_BUILD_ROOT%{_udevrulesdir}/98-kexec.rules
%endif
%ifarch ppc64 ppc64le
install -m 644 %{SOURCE14} $RPM_BUILD_ROOT%{_udevrulesdir}/98-kexec.rules
+install -m 755 -D %{SOURCE37}
$RPM_BUILD_ROOT%{_prefix}/lib/kernel/install.d/60-fadump.install
%endif
install -m 644 %{SOURCE15} $RPM_BUILD_ROOT%{_mandir}/man5/kdump.conf.5
install -m 644 %{SOURCE16} $RPM_BUILD_ROOT%{_unitdir}/kdump.service
@@ -365,6 +367,7 @@ fi
%endif
%ifarch ppc64 ppc64le
/usr/sbin/mkfadumprd
+%{_prefix}/lib/kernel/install.d/60-fadump.install
%endif
/usr/sbin/mkdumprd
/usr/sbin/vmcore-dmesg
108
days inactive
119
days old
11 comments
4 participants
participants (4)
-
Coiby Xu -
Hari Bathini -
Philipp Rudo -
Tao Liu