We collect all the kdump targets i.e. devices recognized under kdump, then improve kdump according to the type of the target. Currently we have two: dump target and root target in case of "dump_to_rootfs".
E.g. If we know there is no crypt target, we can remove dracut "crypt" module, we only add rd.lvm.lv=X regarding the lvm target to kdump.
E.g. If we know there is only network dumping(ssh/nfs), we can omit some non-network modules like dm/lvm/multipath/crypt/iscsi/fcoe/etc.
This series does two major things: 1)Patch 1~5 solve https://bugzilla.redhat.com/1451717 2)Patch 6~8 reduce the initramfs size by omitting needless dracut modules.
Xunlei Pang (8): kdump-lib.sh: fix improper get_block_dump_target() kdump-lib.sh: introduce get_kdump_targets() mkdumprd: change for_each_block_target() to use get_kdump_targets() kdumpctl: use generated rd.lvm.lv=X mkdumprd: omit crypt when there is no crypt kdump target mkdumprd: omit dracut modules in case of no dm target mkdumprd: omit dracut modules in case of network dumping module-setup: fix kdump network dependency
dracut-module-setup.sh | 2 +- kdump-lib.sh | 111 +++++++++++++++++++++++++++++++++++++++++++------ kdumpctl | 66 ++++++++++++++++++----------- mkdumprd | 111 ++++++++++++++++++++++++++++--------------------- 4 files changed, 204 insertions(+), 86 deletions(-)
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
This patch improves get_block_dump_target as follows: -Consider block device in the special "--dracut-args --mount ..." in get_user_configured_dump_disk(). -Consider save path instead of root fs in get_block_dump_target(), and move it into kdump-lib.sh because we will have another user in the following patch. -For nfs/ssh dumping, there is no need to check the root device. -Move get_save_path into kdump-lib.sh.
After this patch, get_block_dump_target() can always return the correct block dump target specified.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- kdump-lib.sh | 48 ++++++++++++++++++++++++++++++++++++------------ kdumpctl | 10 ---------- mkdumprd | 13 ------------- 3 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 8ebad70..3cf7af5 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -44,12 +44,6 @@ is_fs_dump_target() egrep -q "^ext[234]|^xfs|^btrfs|^minix" /etc/kdump.conf }
-is_user_configured_dump_target() -{ - return $(is_mount_in_dracut_args || is_ssh_dump_target || is_nfs_dump_target || \ - is_raw_dump_target || is_fs_dump_target) -} - strip_comments() { echo $@ | sed -e 's/(.*)#.*/\1/' @@ -88,18 +82,21 @@ to_dev_name() { echo $dev }
+is_user_configured_dump_target() +{ + return $(is_mount_in_dracut_args || is_ssh_dump_target || is_nfs_dump_target || \ + is_raw_dump_target || is_fs_dump_target) +} + get_user_configured_dump_disk() { local _target
- if is_ssh_dump_target || is_nfs_dump_target; then - return - fi - _target=$(egrep "^ext[234]|^xfs|^btrfs|^minix|^raw" /etc/kdump.conf 2>/dev/null |awk '{print $2}') - [ -n "$_target" ] && echo $_target + [ -n "$_target" ] && echo $_target && return
- return + _target=$(get_dracut_args_target "$(grep "^dracut_args .*--mount" /etc/kdump.conf)") + [ -b "$_target" ] && echo $_target }
get_root_fs_device() @@ -111,6 +108,33 @@ get_root_fs_device() return }
+get_save_path() +{ + local _save_path=$(grep "^path" /etc/kdump.conf|awk '{print $2}') + if [ -z "$_save_path" ]; then + _save_path=$DEFAULT_PATH + fi + + echo $_save_path +} + +get_block_dump_target() +{ + local _target _path + + if is_ssh_dump_target || is_nfs_dump_target; then + return + fi + + _target=$(get_user_configured_dump_disk) + [ -n "$_target" ] && echo $(to_dev_name $_target) && return + + # Get block device name from local save path + _path=$(get_save_path) + _target=$(get_target_from_path $_path) + [ -b "$_target" ] && echo $(to_dev_name $_target) +} + # findmnt uses the option "-v, --nofsroot" to exclusive the [/dir] # in the SOURCE column for bind-mounts, then if $_mntpoint equals to # $_mntpoint_nofsroot, the mountpoint is not bind mounted directory. diff --git a/kdumpctl b/kdumpctl index e440bbb..0e53793 100755 --- a/kdumpctl +++ b/kdumpctl @@ -974,16 +974,6 @@ save_raw() return 0 }
-get_save_path() -{ - local _save_path=$(grep "^path" /etc/kdump.conf|awk '{print $2}') - if [ -z "$_save_path" ]; then - _save_path="/var/crash" - fi - - echo $_save_path -} - is_dump_target_configured() { local _target diff --git a/mkdumprd b/mkdumprd index 5a25853..45b185a 100644 --- a/mkdumprd +++ b/mkdumprd @@ -250,19 +250,6 @@ add_mount() { add_dracut_mount "$_mnt" }
-get_block_dump_target() -{ - local _target - - - _target=$(get_user_configured_dump_disk) - [ -n "$_target" ] && echo $(to_dev_name $_target) && return - - #get rootfs device name - _target=$(get_root_fs_device) - [ -b "$_target" ] && echo $(to_dev_name $_target) -} - #handle the case user does not specify the dump target explicitly handle_default_dump_target() {
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
We need to know all the kdump targets including the dump target and root in case of "dump_to_rootfs".
This is useful for us to do some extra work related to the type of different targets.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- kdump-lib.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ kdumpctl | 5 ----- mkdumprd | 12 ------------ 3 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 3cf7af5..a33f172 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -135,6 +135,57 @@ get_block_dump_target() [ -b "$_target" ] && echo $(to_dev_name $_target) }
+is_dump_to_rootfs() +{ + grep "^default[[:space:]]dump_to_rootfs" /etc/kdump.conf >/dev/null +} + +get_default_action_target() +{ + local _target + + if is_dump_to_rootfs; then + # Get rootfs device name + _target=$(get_root_fs_device) + [ -b "$_target" ] && echo $(to_dev_name $_target) && return + # Then, must be nfs root + echo "nfs" + fi +} + +# Get kdump targets(including root in case of dump_to_rootfs). +get_kdump_targets() +{ + local _target _root + local kdump_targets + + _target=$(get_block_dump_target) + if [ -n "$_target" ]; then + kdump_targets=$_target + elif is_ssh_dump_target; then + kdump_targets="ssh" + else + kdump_targets="nfs" + fi + + # Add the root device if dump_to_rootfs is specified. + _root=$(get_default_action_target) + if [ -n "$_root" -a "$kdump_targets" != "$_root" ]; then + kdump_targets="$kdump_targets $_root" + fi + + # NOTE: + # dracut parses devices from "/etc/fstab" with the "x-initrd.mount" option, + # which will be added as host_devs, it also includes usually simple devices + # (say mounted to /boot, /boot/efi/, etc) plus the root device. Then kdump + # must wait for these devices if initramfs is built with "--hostonly-cmdline". + # + # We don't pass "--hostonly-cmdline" to dracut, so there's no problem. + + echo "$kdump_targets" +} + + # findmnt uses the option "-v, --nofsroot" to exclusive the [/dir] # in the SOURCE column for bind-mounts, then if $_mntpoint equals to # $_mntpoint_nofsroot, the mountpoint is not bind mounted directory. diff --git a/kdumpctl b/kdumpctl index 0e53793..8c40564 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,11 +171,6 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
-is_dump_to_rootfs() -{ - grep "^default[[:space:]]dump_to_rootfs" /etc/kdump.conf >/dev/null -} - # This function performs a series of edits on the command line. # Store the final result in global $KDUMP_COMMANDLINE. prepare_cmdline() diff --git a/mkdumprd b/mkdumprd index 45b185a..337ae87 100644 --- a/mkdumprd +++ b/mkdumprd @@ -278,18 +278,6 @@ handle_default_dump_target() check_size fs $_target }
-get_default_action_target() -{ - local _target - local _action=$(grep "^default" /etc/kdump.conf 2>/dev/null | awk '{print $2}') - if [ -n "$_action" ] && [ "$_action" = "dump_to_rootfs" ]; then - #get rootfs device name - _target=$(findmnt -k -f -n -o SOURCE /) - [ -b "$_target" ] && echo $(to_dev_name $_target) - fi - return -} - get_override_resettable() { local override_resettable
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
Now that we have get_kdump_targets(), use it to simplify the code.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- mkdumprd | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/mkdumprd b/mkdumprd index 337ae87..6956d8e 100644 --- a/mkdumprd +++ b/mkdumprd @@ -295,22 +295,15 @@ get_override_resettable() # $1: function name for_each_block_target() { - local dev majmin + local index dev majmin
- #check dump target - dev=$(get_block_dump_target) - - if [ -n "$dev" ]; then - majmin=$(get_maj_min $dev) - check_block_and_slaves $1 $majmin && return 1 - fi - - #check rootfs when default action dump_to_rootfs is set - dev=$(get_default_action_target) - if [ -n "$dev" ]; then + index=0 + for dev in $(get_kdump_targets); do + index=$((index+1)) + [ -b "$dev" ] || continue majmin=$(get_maj_min $dev) - check_block_and_slaves $1 $majmin && return 2 - fi + check_block_and_slaves $1 $majmin && return $index + done
return 0 } @@ -329,7 +322,7 @@ is_unresettable() resettable="$(cat $path)" [ $resettable -eq 0 -a "$OVERRIDE_RESETTABLE" -eq 0 ] && { local device=$(udevadm info --query=all --path=/sys/dev/block/$1 | awk -F= '/DEVNAME/{print $2}') - echo "Device $device is unresettable" + echo "Error: Can not save vmcore to unresettable target $device" return 0 } fi @@ -350,14 +343,6 @@ check_resettable()
[ $_ret -eq 0 ] && return
- if [ $_ret -eq 1 ]; then - _target=$(get_block_dump_target) - perror "Can not save vmcore to target device $_target . This device can not be initialized in kdump kernel as it is not resettable" - elif [ $_ret -eq 2 ]; then - _target=$(get_default_action_target) - perror "Rootfs device $_target is not resettable, can not be used as the default target, please specify a default action" - fi - return 1 }
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
Now that we have get_kdump_targets(), use it to simplify the code.
Signed-off-by: Xunlei Pang xlpang@redhat.com
mkdumprd | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/mkdumprd b/mkdumprd index 337ae87..6956d8e 100644 --- a/mkdumprd +++ b/mkdumprd @@ -295,22 +295,15 @@ get_override_resettable() # $1: function name for_each_block_target() {
- local dev majmin
- local index dev majmin
- #check dump target
- dev=$(get_block_dump_target)
- if [ -n "$dev" ]; then
majmin=$(get_maj_min $dev)check_block_and_slaves $1 $majmin && return 1- fi
- #check rootfs when default action dump_to_rootfs is set
- dev=$(get_default_action_target)
- if [ -n "$dev" ]; then
- index=0
- for dev in $(get_kdump_targets); do
index=$((index+1))
Since we do not differentiate the return value, the we can just return 1 below, we can add think to add it later if we really need it..
[ -b "$dev" ] || continue majmin=$(get_maj_min $dev)
check_block_and_slaves $1 $majmin && return 2- fi
check_block_and_slaves $1 $majmin && return $indexdone
return 0
} @@ -329,7 +322,7 @@ is_unresettable() resettable="$(cat $path)" [ $resettable -eq 0 -a "$OVERRIDE_RESETTABLE" -eq 0 ] && { local device=$(udevadm info --query=all --path=/sys/dev/block/$1 | awk -F= '/DEVNAME/{print $2}')
echo "Device $device is unresettable"
echo "Error: Can not save vmcore to unresettable target $device"
The $device is probably a stacked device which is not obvious as an dump target, how about: echo "Error: Can not save vmcore because device $device is unresettable"
return 0 } fi@@ -350,14 +343,6 @@ check_resettable()
[ $_ret -eq 0 ] && return
- if [ $_ret -eq 1 ]; then
_target=$(get_block_dump_target)perror "Can not save vmcore to target device $_target . This device can not be initialized in kdump kernel as it is not resettable"- elif [ $_ret -eq 2 ]; then
_target=$(get_default_action_target)perror "Rootfs device $_target is not resettable, can not be used as the default target, please specify a default action"- fi
- return 1
}
-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Thanks Dave
On 07/07/2017 at 11:21 AM, Dave Young wrote:
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
Now that we have get_kdump_targets(), use it to simplify the code.
Signed-off-by: Xunlei Pang xlpang@redhat.com
mkdumprd | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/mkdumprd b/mkdumprd index 337ae87..6956d8e 100644 --- a/mkdumprd +++ b/mkdumprd @@ -295,22 +295,15 @@ get_override_resettable() # $1: function name for_each_block_target() {
- local dev majmin
- local index dev majmin
- #check dump target
- dev=$(get_block_dump_target)
- if [ -n "$dev" ]; then
majmin=$(get_maj_min $dev)check_block_and_slaves $1 $majmin && return 1- fi
- #check rootfs when default action dump_to_rootfs is set
- dev=$(get_default_action_target)
- if [ -n "$dev" ]; then
- index=0
- for dev in $(get_kdump_targets); do
index=$((index+1))Since we do not differentiate the return value, the we can just return 1 below, we can add think to add it later if we really need it..
ok
[ -b "$dev" ] || continue majmin=$(get_maj_min $dev)
check_block_and_slaves $1 $majmin && return 2- fi
check_block_and_slaves $1 $majmin && return $indexdone
return 0
} @@ -329,7 +322,7 @@ is_unresettable() resettable="$(cat $path)" [ $resettable -eq 0 -a "$OVERRIDE_RESETTABLE" -eq 0 ] && { local device=$(udevadm info --query=all --path=/sys/dev/block/$1 | awk -F= '/DEVNAME/{print $2}')
echo "Device $device is unresettable"
echo "Error: Can not save vmcore to unresettable target $device"The $device is probably a stacked device which is not obvious as an dump target, how about: echo "Error: Can not save vmcore because device $device is unresettable"
Ah, you are right, will improve, thanks!
Regards, Xunlei
return 0 } fi@@ -350,14 +343,6 @@ check_resettable()
[ $_ret -eq 0 ] && return
- if [ $_ret -eq 1 ]; then
_target=$(get_block_dump_target)perror "Can not save vmcore to target device $_target . This device can not be initialized in kdump kernel as it is not resettable"- elif [ $_ret -eq 2 ]; then
_target=$(get_default_action_target)perror "Rootfs device $_target is not resettable, can not be used as the default target, please specify a default action"- fi
- return 1
}
-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Thanks Dave
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is any "rd.lvm.lv=X", "lvm" dracut module will try to recognize all the lvm volumes which is unnecessary and probably cause trouble for us.
See https://bugzilla.redhat.com/show_bug.cgi?id=1451717#c2
Remove all the rd.lvm.lv=X inherited from the kernel cmdline, and generate the corresponding cmdline as needed for kdump. Because prepare_cmdline() is only used by kdump, we don't need to add any fadump judgement(also remove the existing judgement in passing).
Currently, we don't handle "rd.lvm.vg=X", we can add it in when there is some bug reported in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- kdumpctl | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8c40564..2783e23 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,6 +171,45 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
+GENERATED_LVM_CMDLINES="" +# Generate rd.lvm.lv=X for the kdump targets if any. +generate_lvm_cmdline() +{ + local majmin=$1 dev + + [ -d "/sys/dev/block/$majmin/dm" ] || return 0 + dev=/dev/mapper/$(< "/sys/dev/block/$majmin/dm/name") + + vg=$(lvm lvs --rows $dev -o vg_name --separator=* 2>/dev/null | cut -d "*" -f 2) + lv=$(lvm lvs --rows $dev -o lv_name --separator=* 2>/dev/null | cut -d "*" -f 2) + if [ -n "$vg" -a -n "$lv" ]; then + GENERATED_LVM_CMDLINES="rd.lvm.lv=$vg/$lv $GENERATED_LVM_CMDLINES" + fi + + return 0 +} + +generate_lvm_cmdlines() +{ + for_each_block_target_all generate_lvm_cmdline + + echo "$GENERATED_LVM_CMDLINES" +} + +# $1: function name +for_each_block_target_all() +{ + local dev majmin + + for dev in $(get_kdump_targets); do + [ -b "$dev" ] || continue + majmin=$(get_maj_min $dev) + check_block_and_slaves_all $1 $majmin + done + + return 0 +} + # This function performs a series of edits on the command line. # Store the final result in global $KDUMP_COMMANDLINE. prepare_cmdline() @@ -190,15 +229,19 @@ prepare_cmdline()
# Always remove "root=X", as we now explicitly generate all kinds # of dump target mount information including root fs. But we can - # not remove it in case of fadump or "default dump_to_rootfs". + # not remove it in case of "default dump_to_rootfs". # # We do this before KDUMP_COMMANDLINE_APPEND, if one really cares # about it(e.g. for debug purpose), then can pass "root=X" using # KDUMP_COMMANDLINE_APPEND. - if [ $DEFAULT_DUMP_MODE != "fadump" ] && ! is_dump_to_rootfs; then + if ! is_dump_to_rootfs; then cmdline=`remove_cmdline_param "$cmdline" root` fi
+ # Remove all the inherited rd.lvm.lv=X and generate those as needed. + cmdline=`remove_cmdline_param "$cmdline" rd.lvm.lv` + cmdline="${cmdline} $(generate_lvm_cmdlines)" + cmdline="${cmdline} ${KDUMP_COMMANDLINE_APPEND}"
id=`get_bootcpu_initial_apicid`
On 07/06/2017 at 02:37 PM, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is any "rd.lvm.lv=X", "lvm" dracut module will try to recognize all the lvm volumes which is unnecessary and probably cause trouble for us.
See https://bugzilla.redhat.com/show_bug.cgi?id=1451717#c2
Remove all the rd.lvm.lv=X inherited from the kernel cmdline, and generate the corresponding cmdline as needed for kdump. Because prepare_cmdline() is only used by kdump, we don't need to add any fadump judgement(also remove the existing judgement in passing).
Currently, we don't handle "rd.lvm.vg=X", we can add it in when there is some bug reported in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdumpctl | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8c40564..2783e23 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,6 +171,45 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
+GENERATED_LVM_CMDLINES=""
Defined as a local variable in generate_lvm_cmdlines() also work, I will change it to be local next version.
Regards, Xunlei
+# Generate rd.lvm.lv=X for the kdump targets if any. +generate_lvm_cmdline() +{
- local majmin=$1 dev
- [ -d "/sys/dev/block/$majmin/dm" ] || return 0
- dev=/dev/mapper/$(< "/sys/dev/block/$majmin/dm/name")
- vg=$(lvm lvs --rows $dev -o vg_name --separator=* 2>/dev/null | cut -d "*" -f 2)
- lv=$(lvm lvs --rows $dev -o lv_name --separator=* 2>/dev/null | cut -d "*" -f 2)
- if [ -n "$vg" -a -n "$lv" ]; then
GENERATED_LVM_CMDLINES="rd.lvm.lv=$vg/$lv $GENERATED_LVM_CMDLINES"- fi
- return 0
+}
+generate_lvm_cmdlines() +{
- for_each_block_target_all generate_lvm_cmdline
- echo "$GENERATED_LVM_CMDLINES"
+}
+# $1: function name +for_each_block_target_all() +{
- local dev majmin
- for dev in $(get_kdump_targets); do
[ -b "$dev" ] || continuemajmin=$(get_maj_min $dev)check_block_and_slaves_all $1 $majmin- done
- return 0
+}
# This function performs a series of edits on the command line. # Store the final result in global $KDUMP_COMMANDLINE. prepare_cmdline() @@ -190,15 +229,19 @@ prepare_cmdline()
# Always remove "root=X", as we now explicitly generate all kinds # of dump target mount information including root fs. But we can
- # not remove it in case of fadump or "default dump_to_rootfs".
- # not remove it in case of "default dump_to_rootfs". # # We do this before KDUMP_COMMANDLINE_APPEND, if one really cares # about it(e.g. for debug purpose), then can pass "root=X" using # KDUMP_COMMANDLINE_APPEND.
- if [ $DEFAULT_DUMP_MODE != "fadump" ] && ! is_dump_to_rootfs; then
if ! is_dump_to_rootfs; then cmdline=`remove_cmdline_param "$cmdline" root` fi
# Remove all the inherited rd.lvm.lv=X and generate those as needed.
cmdline=`remove_cmdline_param "$cmdline" rd.lvm.lv`
cmdline="${cmdline} $(generate_lvm_cmdlines)"
cmdline="${cmdline} ${KDUMP_COMMANDLINE_APPEND}"
id=`get_bootcpu_initial_apicid`
On 07/07/17 at 12:14pm, Xunlei Pang wrote:
On 07/06/2017 at 02:37 PM, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is any "rd.lvm.lv=X", "lvm" dracut module will try to recognize all the lvm volumes which is unnecessary and probably cause trouble for us.
See https://bugzilla.redhat.com/show_bug.cgi?id=1451717#c2
Remove all the rd.lvm.lv=X inherited from the kernel cmdline, and generate the corresponding cmdline as needed for kdump. Because prepare_cmdline() is only used by kdump, we don't need to add any fadump judgement(also remove the existing judgement in passing).
Currently, we don't handle "rd.lvm.vg=X", we can add it in when there is some bug reported in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdumpctl | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8c40564..2783e23 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,6 +171,45 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
+GENERATED_LVM_CMDLINES=""
Defined as a local variable in generate_lvm_cmdlines() also work, I will change it to be local next version.
That would be better, then we can use a shorter lowercase name if be a local variable..
Regards, Xunlei
+# Generate rd.lvm.lv=X for the kdump targets if any. +generate_lvm_cmdline() +{
- local majmin=$1 dev
- [ -d "/sys/dev/block/$majmin/dm" ] || return 0
- dev=/dev/mapper/$(< "/sys/dev/block/$majmin/dm/name")
- vg=$(lvm lvs --rows $dev -o vg_name --separator=* 2>/dev/null | cut -d "*" -f 2)
- lv=$(lvm lvs --rows $dev -o lv_name --separator=* 2>/dev/null | cut -d "*" -f 2)
- if [ -n "$vg" -a -n "$lv" ]; then
GENERATED_LVM_CMDLINES="rd.lvm.lv=$vg/$lv $GENERATED_LVM_CMDLINES"- fi
- return 0
+}
+generate_lvm_cmdlines() +{
- for_each_block_target_all generate_lvm_cmdline
- echo "$GENERATED_LVM_CMDLINES"
+}
+# $1: function name +for_each_block_target_all() +{
- local dev majmin
- for dev in $(get_kdump_targets); do
[ -b "$dev" ] || continuemajmin=$(get_maj_min $dev)check_block_and_slaves_all $1 $majmin- done
- return 0
+}
# This function performs a series of edits on the command line. # Store the final result in global $KDUMP_COMMANDLINE. prepare_cmdline() @@ -190,15 +229,19 @@ prepare_cmdline()
# Always remove "root=X", as we now explicitly generate all kinds # of dump target mount information including root fs. But we can
- # not remove it in case of fadump or "default dump_to_rootfs".
- # not remove it in case of "default dump_to_rootfs". # # We do this before KDUMP_COMMANDLINE_APPEND, if one really cares # about it(e.g. for debug purpose), then can pass "root=X" using # KDUMP_COMMANDLINE_APPEND.
- if [ $DEFAULT_DUMP_MODE != "fadump" ] && ! is_dump_to_rootfs; then
if ! is_dump_to_rootfs; then cmdline=`remove_cmdline_param "$cmdline" root` fi
# Remove all the inherited rd.lvm.lv=X and generate those as needed.
cmdline=`remove_cmdline_param "$cmdline" rd.lvm.lv`
cmdline="${cmdline} $(generate_lvm_cmdlines)"
cmdline="${cmdline} ${KDUMP_COMMANDLINE_APPEND}"
id=`get_bootcpu_initial_apicid`
kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
On 07/07/2017 at 01:24 PM, Dave Young wrote:
On 07/07/17 at 12:14pm, Xunlei Pang wrote:
On 07/06/2017 at 02:37 PM, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is any "rd.lvm.lv=X", "lvm" dracut module will try to recognize all the lvm volumes which is unnecessary and probably cause trouble for us.
See https://bugzilla.redhat.com/show_bug.cgi?id=1451717#c2
Remove all the rd.lvm.lv=X inherited from the kernel cmdline, and generate the corresponding cmdline as needed for kdump. Because prepare_cmdline() is only used by kdump, we don't need to add any fadump judgement(also remove the existing judgement in passing).
Currently, we don't handle "rd.lvm.vg=X", we can add it in when there is some bug reported in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdumpctl | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8c40564..2783e23 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,6 +171,45 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
+GENERATED_LVM_CMDLINES=""
Defined as a local variable in generate_lvm_cmdlines() also work, I will change it to be local next version.
That would be better, then we can use a shorter lowercase name if be a local variable..
Yes, done.
Regards, Xunlei
+# Generate rd.lvm.lv=X for the kdump targets if any. +generate_lvm_cmdline() +{
- local majmin=$1 dev
- [ -d "/sys/dev/block/$majmin/dm" ] || return 0
- dev=/dev/mapper/$(< "/sys/dev/block/$majmin/dm/name")
- vg=$(lvm lvs --rows $dev -o vg_name --separator=* 2>/dev/null | cut -d "*" -f 2)
- lv=$(lvm lvs --rows $dev -o lv_name --separator=* 2>/dev/null | cut -d "*" -f 2)
- if [ -n "$vg" -a -n "$lv" ]; then
GENERATED_LVM_CMDLINES="rd.lvm.lv=$vg/$lv $GENERATED_LVM_CMDLINES"- fi
- return 0
+}
+generate_lvm_cmdlines() +{
- for_each_block_target_all generate_lvm_cmdline
- echo "$GENERATED_LVM_CMDLINES"
+}
+# $1: function name +for_each_block_target_all() +{
- local dev majmin
- for dev in $(get_kdump_targets); do
[ -b "$dev" ] || continuemajmin=$(get_maj_min $dev)check_block_and_slaves_all $1 $majmin- done
- return 0
+}
# This function performs a series of edits on the command line. # Store the final result in global $KDUMP_COMMANDLINE. prepare_cmdline() @@ -190,15 +229,19 @@ prepare_cmdline()
# Always remove "root=X", as we now explicitly generate all kinds # of dump target mount information including root fs. But we can
- # not remove it in case of fadump or "default dump_to_rootfs".
- # not remove it in case of "default dump_to_rootfs". # # We do this before KDUMP_COMMANDLINE_APPEND, if one really cares # about it(e.g. for debug purpose), then can pass "root=X" using # KDUMP_COMMANDLINE_APPEND.
- if [ $DEFAULT_DUMP_MODE != "fadump" ] && ! is_dump_to_rootfs; then
if ! is_dump_to_rootfs; then cmdline=`remove_cmdline_param "$cmdline" root` fi
# Remove all the inherited rd.lvm.lv=X and generate those as needed.
cmdline=`remove_cmdline_param "$cmdline" rd.lvm.lv`
cmdline="${cmdline} $(generate_lvm_cmdlines)"
cmdline="${cmdline} ${KDUMP_COMMANDLINE_APPEND}"
id=`get_bootcpu_initial_apicid`
kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is no crypt related kdump target, we can safely omit "crypt" dracut module, this can avoid the pop asking disk password during kdump boot in some cases.
This patch introduces omit_dracut_modules() before calling dracut, we can omit more modules to reduce initrd size in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- kdump-lib.sh | 12 ++++++++++++ kdumpctl | 12 ------------ mkdumprd | 26 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index a33f172..3f0af91 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -6,6 +6,7 @@ DEFAULT_PATH="/var/crash/" FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send" +FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
perror_exit() { echo $@ >&2 @@ -481,3 +482,14 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 } + +is_fadump_capable() +{ + # Check if firmware-assisted dump is enabled + # if no, fallback to kdump check + if [ -f $FADUMP_ENABLED_SYS_NODE ]; then + rc=`cat $FADUMP_ENABLED_SYS_NODE` + [ $rc -eq 1 ] && return 0 + fi + return 1 +} diff --git a/kdumpctl b/kdumpctl index 2783e23..2f158fc 100755 --- a/kdumpctl +++ b/kdumpctl @@ -13,7 +13,6 @@ DUMP_TARGET="" DEFAULT_INITRD="" DEFAULT_INITRD_BAK="" TARGET_INITRD="" -FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled" FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered" #kdump shall be the default dump mode DEFAULT_DUMP_MODE="kdump" @@ -932,17 +931,6 @@ handle_mode_switch() fi }
-is_fadump_capable() -{ - # Check if firmware-assisted dump is enabled - # if no, fallback to kdump check - if [ -f $FADUMP_ENABLED_SYS_NODE ]; then - rc=`cat $FADUMP_ENABLED_SYS_NODE` - [ $rc -eq 1 ] && return 0 - fi - return 1 -} - check_current_fadump_status() { # Check if firmware-assisted dump has been registered. diff --git a/mkdumprd b/mkdumprd index 6956d8e..501a689 100644 --- a/mkdumprd +++ b/mkdumprd @@ -374,6 +374,30 @@ check_crypt() return 1 }
+omit_dracut_modules() +{ + local target majmin + local crypt_exists + + # Skip fadump case + is_fadump_capable && return + + crypt_exists=0 + + for target in $(get_kdump_targets); do + if [ -b "$target" ]; then + # Check "crypt" + majmin=$(get_maj_min $target) + check_block_and_slaves is_crypt $majmin && crypt_exists=1 + fi + done + + # Omit "crypt", BZ1451717 + if [ "$crypt_exists" == "0" ]; then + add_dracut_arg "--omit" "crypt" + fi +} + if ! check_resettable; then exit 1 fi @@ -463,6 +487,8 @@ then add_dracut_arg "--add-drivers" "$extra_modules" fi
+omit_dracut_modules + dracut "${dracut_args[@]}" "$@" _rc=$? sync
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is no crypt related kdump target, we can safely omit "crypt" dracut module, this can avoid the pop asking disk password during kdump boot in some cases.
This patch introduces omit_dracut_modules() before calling dracut, we can omit more modules to reduce initrd size in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdump-lib.sh | 12 ++++++++++++ kdumpctl | 12 ------------ mkdumprd | 26 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index a33f172..3f0af91 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -6,6 +6,7 @@ DEFAULT_PATH="/var/crash/" FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send" +FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
perror_exit() { echo $@ >&2 @@ -481,3 +482,14 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }
+is_fadump_capable() +{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
+} diff --git a/kdumpctl b/kdumpctl index 2783e23..2f158fc 100755 --- a/kdumpctl +++ b/kdumpctl @@ -13,7 +13,6 @@ DUMP_TARGET="" DEFAULT_INITRD="" DEFAULT_INITRD_BAK="" TARGET_INITRD="" -FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled" FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered" #kdump shall be the default dump mode DEFAULT_DUMP_MODE="kdump" @@ -932,17 +931,6 @@ handle_mode_switch() fi }
-is_fadump_capable() -{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
-}
check_current_fadump_status() { # Check if firmware-assisted dump has been registered. diff --git a/mkdumprd b/mkdumprd index 6956d8e..501a689 100644 --- a/mkdumprd +++ b/mkdumprd @@ -374,6 +374,30 @@ check_crypt() return 1 }
+omit_dracut_modules() +{
- local target majmin
- local crypt_exists
- # Skip fadump case
- is_fadump_capable && return
- crypt_exists=0
- for target in $(get_kdump_targets); do
if [ -b "$target" ]; then# Check "crypt"majmin=$(get_maj_min $target)check_block_and_slaves is_crypt $majmin && crypt_exists=1fi- done
We have for_each_block_target adapted with your get_dump_targets so Can we use check_crypt function instead?
Something like
omit_dracut_module() { if is_fadump then return
add_dracut_arg "--omit" "crypt" }
Then we can call it in mkdumprd, like below then we do not iternate the devices again, can save some boot time:
if ! check_crypt; then echo "Warning: Encrypted device is in dump path. User will prompted for password during second kernel boot." else omit_dracut_module crypt fi
- # Omit "crypt", BZ1451717
- if [ "$crypt_exists" == "0" ]; then
add_dracut_arg "--omit" "crypt"- fi
+}
if ! check_resettable; then exit 1 fi @@ -463,6 +487,8 @@ then add_dracut_arg "--add-drivers" "$extra_modules" fi
+omit_dracut_modules
dracut "${dracut_args[@]}" "$@" _rc=$? sync -- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Thanks Dave
On 07/07/17 at 10:43am, Dave Young wrote:
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is no crypt related kdump target, we can safely omit "crypt" dracut module, this can avoid the pop asking disk password during kdump boot in some cases.
This patch introduces omit_dracut_modules() before calling dracut, we can omit more modules to reduce initrd size in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdump-lib.sh | 12 ++++++++++++ kdumpctl | 12 ------------ mkdumprd | 26 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index a33f172..3f0af91 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -6,6 +6,7 @@ DEFAULT_PATH="/var/crash/" FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send" +FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
perror_exit() { echo $@ >&2 @@ -481,3 +482,14 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }
+is_fadump_capable() +{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
+} diff --git a/kdumpctl b/kdumpctl index 2783e23..2f158fc 100755 --- a/kdumpctl +++ b/kdumpctl @@ -13,7 +13,6 @@ DUMP_TARGET="" DEFAULT_INITRD="" DEFAULT_INITRD_BAK="" TARGET_INITRD="" -FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled" FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered" #kdump shall be the default dump mode DEFAULT_DUMP_MODE="kdump" @@ -932,17 +931,6 @@ handle_mode_switch() fi }
-is_fadump_capable() -{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
-}
check_current_fadump_status() { # Check if firmware-assisted dump has been registered. diff --git a/mkdumprd b/mkdumprd index 6956d8e..501a689 100644 --- a/mkdumprd +++ b/mkdumprd @@ -374,6 +374,30 @@ check_crypt() return 1 }
+omit_dracut_modules() +{
- local target majmin
- local crypt_exists
- # Skip fadump case
- is_fadump_capable && return
- crypt_exists=0
- for target in $(get_kdump_targets); do
if [ -b "$target" ]; then# Check "crypt"majmin=$(get_maj_min $target)check_block_and_slaves is_crypt $majmin && crypt_exists=1fi- done
We have for_each_block_target adapted with your get_dump_targets so Can we use check_crypt function instead?
Something like
omit_dracut_module() { if is_fadump then return
add_dracut_arg "--omit" "crypt"}
Then we can call it in mkdumprd, like below then we do not iternate the devices again, can save some boot time:
if ! check_crypt; then echo "Warning: Encrypted device is in dump path. User will prompted for password during second kernel boot." else omit_dracut_module crypt fi
Or maybe save the status of crypt as a global variable and check it in your current omit_dracut_modules() version..
- # Omit "crypt", BZ1451717
- if [ "$crypt_exists" == "0" ]; then
add_dracut_arg "--omit" "crypt"- fi
+}
if ! check_resettable; then exit 1 fi @@ -463,6 +487,8 @@ then add_dracut_arg "--add-drivers" "$extra_modules" fi
+omit_dracut_modules
dracut "${dracut_args[@]}" "$@" _rc=$? sync -- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Thanks Dave
On 07/07/2017 at 10:52 AM, Dave Young wrote:
On 07/07/17 at 10:43am, Dave Young wrote:
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is no crypt related kdump target, we can safely omit "crypt" dracut module, this can avoid the pop asking disk password during kdump boot in some cases.
This patch introduces omit_dracut_modules() before calling dracut, we can omit more modules to reduce initrd size in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdump-lib.sh | 12 ++++++++++++ kdumpctl | 12 ------------ mkdumprd | 26 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index a33f172..3f0af91 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -6,6 +6,7 @@ DEFAULT_PATH="/var/crash/" FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send" +FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
perror_exit() { echo $@ >&2 @@ -481,3 +482,14 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }
+is_fadump_capable() +{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
+} diff --git a/kdumpctl b/kdumpctl index 2783e23..2f158fc 100755 --- a/kdumpctl +++ b/kdumpctl @@ -13,7 +13,6 @@ DUMP_TARGET="" DEFAULT_INITRD="" DEFAULT_INITRD_BAK="" TARGET_INITRD="" -FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled" FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered" #kdump shall be the default dump mode DEFAULT_DUMP_MODE="kdump" @@ -932,17 +931,6 @@ handle_mode_switch() fi }
-is_fadump_capable() -{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
-}
check_current_fadump_status() { # Check if firmware-assisted dump has been registered. diff --git a/mkdumprd b/mkdumprd index 6956d8e..501a689 100644 --- a/mkdumprd +++ b/mkdumprd @@ -374,6 +374,30 @@ check_crypt() return 1 }
+omit_dracut_modules() +{
- local target majmin
- local crypt_exists
- # Skip fadump case
- is_fadump_capable && return
- crypt_exists=0
- for target in $(get_kdump_targets); do
if [ -b "$target" ]; then# Check "crypt"majmin=$(get_maj_min $target)check_block_and_slaves is_crypt $majmin && crypt_exists=1fi- done
We have for_each_block_target adapted with your get_dump_targets so Can we use check_crypt function instead?
Something like
omit_dracut_module() { if is_fadump then return
add_dracut_arg "--omit" "crypt"}
Then we can call it in mkdumprd, like below then we do not iternate the devices again, can save some boot time:
if ! check_crypt; then echo "Warning: Encrypted device is in dump path. User will prompted for password during second kernel boot." else omit_dracut_module crypt fi
Or maybe save the status of crypt as a global variable and check it in your current omit_dracut_modules() version..
Yes, we can, my thought was to handle them uniformly in omit_dracut_modules(), that's why I appended Patch 6~7 which actually should be in a separate series.
When we reach here, it does initramfs rebuild, the time it consumes should be negligible compared to the total rebuild time.
But I am also fine with this global crypt status variable approach, it looks harmless.
Regards, Xunlei
- # Omit "crypt", BZ1451717
- if [ "$crypt_exists" == "0" ]; then
add_dracut_arg "--omit" "crypt"- fi
+}
if ! check_resettable; then exit 1 fi @@ -463,6 +487,8 @@ then add_dracut_arg "--add-drivers" "$extra_modules" fi
+omit_dracut_modules
dracut "${dracut_args[@]}" "$@" _rc=$? sync -- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Thanks Dave
On 07/07/17 at 11:24am, Xunlei Pang wrote:
On 07/07/2017 at 10:52 AM, Dave Young wrote:
On 07/07/17 at 10:43am, Dave Young wrote:
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is no crypt related kdump target, we can safely omit "crypt" dracut module, this can avoid the pop asking disk password during kdump boot in some cases.
This patch introduces omit_dracut_modules() before calling dracut, we can omit more modules to reduce initrd size in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdump-lib.sh | 12 ++++++++++++ kdumpctl | 12 ------------ mkdumprd | 26 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index a33f172..3f0af91 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -6,6 +6,7 @@ DEFAULT_PATH="/var/crash/" FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send" +FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
perror_exit() { echo $@ >&2 @@ -481,3 +482,14 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }
+is_fadump_capable() +{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
+} diff --git a/kdumpctl b/kdumpctl index 2783e23..2f158fc 100755 --- a/kdumpctl +++ b/kdumpctl @@ -13,7 +13,6 @@ DUMP_TARGET="" DEFAULT_INITRD="" DEFAULT_INITRD_BAK="" TARGET_INITRD="" -FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled" FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered" #kdump shall be the default dump mode DEFAULT_DUMP_MODE="kdump" @@ -932,17 +931,6 @@ handle_mode_switch() fi }
-is_fadump_capable() -{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
-}
check_current_fadump_status() { # Check if firmware-assisted dump has been registered. diff --git a/mkdumprd b/mkdumprd index 6956d8e..501a689 100644 --- a/mkdumprd +++ b/mkdumprd @@ -374,6 +374,30 @@ check_crypt() return 1 }
+omit_dracut_modules() +{
- local target majmin
- local crypt_exists
- # Skip fadump case
- is_fadump_capable && return
- crypt_exists=0
- for target in $(get_kdump_targets); do
if [ -b "$target" ]; then# Check "crypt"majmin=$(get_maj_min $target)check_block_and_slaves is_crypt $majmin && crypt_exists=1fi- done
We have for_each_block_target adapted with your get_dump_targets so Can we use check_crypt function instead?
Something like
omit_dracut_module() { if is_fadump then return
add_dracut_arg "--omit" "crypt"}
Then we can call it in mkdumprd, like below then we do not iternate the devices again, can save some boot time:
if ! check_crypt; then echo "Warning: Encrypted device is in dump path. User will prompted for password during second kernel boot." else omit_dracut_module crypt fi
Or maybe save the status of crypt as a global variable and check it in your current omit_dracut_modules() version..
Yes, we can, my thought was to handle them uniformly in omit_dracut_modules(), that's why I appended Patch 6~7 which actually should be in a separate series.
When we reach here, it does initramfs rebuild, the time it consumes should be negligible compared to the total rebuild time.
But I am also fine with this global crypt status variable approach, it looks harmless.
Yes, a global status var is better then my reply to omit module multiple times..
Thanks Dave
On 07/07/2017 at 11:26 AM, Dave Young wrote:
On 07/07/17 at 11:24am, Xunlei Pang wrote:
On 07/07/2017 at 10:52 AM, Dave Young wrote:
On 07/07/17 at 10:43am, Dave Young wrote:
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
When there is no crypt related kdump target, we can safely omit "crypt" dracut module, this can avoid the pop asking disk password during kdump boot in some cases.
This patch introduces omit_dracut_modules() before calling dracut, we can omit more modules to reduce initrd size in the future.
Signed-off-by: Xunlei Pang xlpang@redhat.com
kdump-lib.sh | 12 ++++++++++++ kdumpctl | 12 ------------ mkdumprd | 26 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index a33f172..3f0af91 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -6,6 +6,7 @@ DEFAULT_PATH="/var/crash/" FENCE_KDUMP_CONFIG_FILE="/etc/sysconfig/fence_kdump" FENCE_KDUMP_SEND="/usr/libexec/fence_kdump_send" +FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
perror_exit() { echo $@ >&2 @@ -481,3 +482,14 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }
+is_fadump_capable() +{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
+} diff --git a/kdumpctl b/kdumpctl index 2783e23..2f158fc 100755 --- a/kdumpctl +++ b/kdumpctl @@ -13,7 +13,6 @@ DUMP_TARGET="" DEFAULT_INITRD="" DEFAULT_INITRD_BAK="" TARGET_INITRD="" -FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled" FADUMP_REGISTER_SYS_NODE="/sys/kernel/fadump_registered" #kdump shall be the default dump mode DEFAULT_DUMP_MODE="kdump" @@ -932,17 +931,6 @@ handle_mode_switch() fi }
-is_fadump_capable() -{
- # Check if firmware-assisted dump is enabled
- # if no, fallback to kdump check
- if [ -f $FADUMP_ENABLED_SYS_NODE ]; then
rc=`cat $FADUMP_ENABLED_SYS_NODE`[ $rc -eq 1 ] && return 0- fi
- return 1
-}
check_current_fadump_status() { # Check if firmware-assisted dump has been registered. diff --git a/mkdumprd b/mkdumprd index 6956d8e..501a689 100644 --- a/mkdumprd +++ b/mkdumprd @@ -374,6 +374,30 @@ check_crypt() return 1 }
+omit_dracut_modules() +{
- local target majmin
- local crypt_exists
- # Skip fadump case
- is_fadump_capable && return
- crypt_exists=0
- for target in $(get_kdump_targets); do
if [ -b "$target" ]; then# Check "crypt"majmin=$(get_maj_min $target)check_block_and_slaves is_crypt $majmin && crypt_exists=1fi- done
We have for_each_block_target adapted with your get_dump_targets so Can we use check_crypt function instead?
Something like
omit_dracut_module() { if is_fadump then return
add_dracut_arg "--omit" "crypt"}
Then we can call it in mkdumprd, like below then we do not iternate the devices again, can save some boot time:
if ! check_crypt; then echo "Warning: Encrypted device is in dump path. User will prompted for password during second kernel boot." else omit_dracut_module crypt fi
Or maybe save the status of crypt as a global variable and check it in your current omit_dracut_modules() version..
Yes, we can, my thought was to handle them uniformly in omit_dracut_modules(), that's why I appended Patch 6~7 which actually should be in a separate series.
When we reach here, it does initramfs rebuild, the time it consumes should be negligible compared to the total rebuild time.
But I am also fine with this global crypt status variable approach, it looks harmless.
Yes, a global status var is better then my reply to omit module multiple times..
yes, done
Regards, Xunlei
In case of on dm related target, we can clearly and safely remove many unnecessary modules to reduce initramfs size, and to enhance stability.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- mkdumprd | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/mkdumprd b/mkdumprd index 501a689..d108811 100644 --- a/mkdumprd +++ b/mkdumprd @@ -374,21 +374,34 @@ check_crypt() return 1 }
+is_dm() +{ + if [ -d "/sys/dev/block/$1/dm" ]; then + return 0 + fi + + return 1 +} + omit_dracut_modules() { local target majmin - local crypt_exists + local crypt_exists dm_exists
# Skip fadump case is_fadump_capable && return
crypt_exists=0 + dm_exists=0
for target in $(get_kdump_targets); do if [ -b "$target" ]; then # Check "crypt" majmin=$(get_maj_min $target) check_block_and_slaves is_crypt $majmin && crypt_exists=1 + + # Check "dm" + check_block_and_slaves is_dm $majmin && dm_exists=1 fi done
@@ -396,6 +409,12 @@ omit_dracut_modules() if [ "$crypt_exists" == "0" ]; then add_dracut_arg "--omit" "crypt" fi + + # Further omit more modules in case of no dm related target + if [ "$dm_exists" == "0" ]; then + # "dm_exists=0" implies "crypt_exists=0" + add_dracut_arg "--omit" "lvm dm multipath dmraid" + fi }
if ! check_resettable; then
In case of only network target, we can clearly and safely remove more unnecessary modules to reduce initramfs size, and to enhance stability.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- mkdumprd | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/mkdumprd b/mkdumprd index d108811..28f4fd7 100644 --- a/mkdumprd +++ b/mkdumprd @@ -386,13 +386,14 @@ is_dm() omit_dracut_modules() { local target majmin - local crypt_exists dm_exists + local crypt_exists dm_exists network_only
# Skip fadump case is_fadump_capable && return
crypt_exists=0 dm_exists=0 + network_only=1
for target in $(get_kdump_targets); do if [ -b "$target" ]; then @@ -403,6 +404,9 @@ omit_dracut_modules() # Check "dm" check_block_and_slaves is_dm $majmin && dm_exists=1 fi + + # Check nfs/ssh dumping + [[ "$target" != "nfs" && "$target" != "ssh" ]] && network_only=0 done
# Omit "crypt", BZ1451717 @@ -415,6 +419,12 @@ omit_dracut_modules() # "dm_exists=0" implies "crypt_exists=0" add_dracut_arg "--omit" "lvm dm multipath dmraid" fi + + # Further omit more modules in case of nfs/ssh dumping + if [ "$network_only" == "1" ]; then + # "network_only=1" implies "dm_exists=0" + add_dracut_arg "--omit" "iscsi fcoe fcoe-uefi" + fi }
if ! check_resettable; then
I noticed that network is still enabled for local dumping, like the following kdump boot message on my test machine using local disk as the dump target: tg3.c:v3.137 (May 11, 2014) tg3 0000:02:00.0 eth2: Tigon3 [partno(BCM95720) rev (PCI Express) MAC address c8:1f:66:c9:35:0d tg3 0000:02:00.0 eth2: attached PHY is 5720C
After some debugging, found it due to a misuse in code below: if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then _dep="$_dep network" fi The "if" condition always results in "true", and should be changed as follows: if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
After this, network won't be involved in non-network dumping, as for network related dumping such as nfs/ssh/iscsi/fcoe/etc, dracut will add network accordingly. And kdump initramfs size can be reduced from 24MB to 17MB tested on some real hardware, and from 19MB to 14MB on my kvm. Moreover, it could avoid the network (driver) initialization thereby saving us more memory.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- dracut-module-setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 8495fd9..59b4f04 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -24,7 +24,7 @@ depends() { _dep="$_dep drm" fi
- if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then + if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
I noticed that network is still enabled for local dumping, like the following kdump boot message on my test machine using local disk as the dump target: tg3.c:v3.137 (May 11, 2014) tg3 0000:02:00.0 eth2: Tigon3 [partno(BCM95720) rev (PCI Express) MAC address c8:1f:66:c9:35:0d tg3 0000:02:00.0 eth2: attached PHY is 5720C
After some debugging, found it due to a misuse in code below: if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then _dep="$_dep network" fi The "if" condition always results in "true", and should be changed as follows: if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
After this, network won't be involved in non-network dumping, as for network related dumping such as nfs/ssh/iscsi/fcoe/etc, dracut will add network accordingly. And kdump initramfs size can be reduced from 24MB to 17MB tested on some real hardware, and from 19MB to 14MB on my kvm. Moreover, it could avoid the network (driver) initialization thereby saving us more memory.
Signed-off-by: Xunlei Pang xlpang@redhat.com
dracut-module-setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 8495fd9..59b4f04 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -24,7 +24,7 @@ depends() { _dep="$_dep drm" fi
- if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then
- if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Good catch and looks good.
Do you see anywhere about the documentation of the behavior in bash guide?
Thanks Dave
On 07/07/2017 at 11:16 AM, Dave Young wrote:
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
I noticed that network is still enabled for local dumping, like the following kdump boot message on my test machine using local disk as the dump target: tg3.c:v3.137 (May 11, 2014) tg3 0000:02:00.0 eth2: Tigon3 [partno(BCM95720) rev (PCI Express) MAC address c8:1f:66:c9:35:0d tg3 0000:02:00.0 eth2: attached PHY is 5720C
After some debugging, found it due to a misuse in code below: if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then _dep="$_dep network" fi The "if" condition always results in "true", and should be changed as follows: if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
After this, network won't be involved in non-network dumping, as for network related dumping such as nfs/ssh/iscsi/fcoe/etc, dracut will add network accordingly. And kdump initramfs size can be reduced from 24MB to 17MB tested on some real hardware, and from 19MB to 14MB on my kvm. Moreover, it could avoid the network (driver) initialization thereby saving us more memory.
Signed-off-by: Xunlei Pang xlpang@redhat.com
dracut-module-setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 8495fd9..59b4f04 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -24,7 +24,7 @@ depends() { _dep="$_dep drm" fi
- if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then
- if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Good catch and looks good.
Do you see anywhere about the documentation of the behavior in bash guide?
No, just found it not my way of using function as conditions, and did some tests.
Regards, Xunlei
On 07/07/17 at 11:37am, Xunlei Pang wrote:
On 07/07/2017 at 11:16 AM, Dave Young wrote:
Hi Xunlei, On 07/06/17 at 02:37pm, Xunlei Pang wrote:
I noticed that network is still enabled for local dumping, like the following kdump boot message on my test machine using local disk as the dump target: tg3.c:v3.137 (May 11, 2014) tg3 0000:02:00.0 eth2: Tigon3 [partno(BCM95720) rev (PCI Express) MAC address c8:1f:66:c9:35:0d tg3 0000:02:00.0 eth2: attached PHY is 5720C
After some debugging, found it due to a misuse in code below: if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then _dep="$_dep network" fi The "if" condition always results in "true", and should be changed as follows: if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
After this, network won't be involved in non-network dumping, as for network related dumping such as nfs/ssh/iscsi/fcoe/etc, dracut will add network accordingly. And kdump initramfs size can be reduced from 24MB to 17MB tested on some real hardware, and from 19MB to 14MB on my kvm. Moreover, it could avoid the network (driver) initialization thereby saving us more memory.
Signed-off-by: Xunlei Pang xlpang@redhat.com
dracut-module-setup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 8495fd9..59b4f04 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -24,7 +24,7 @@ depends() { _dep="$_dep drm" fi
- if [ is_generic_fence_kdump -o is_pcs_fence_kdump ]; then
- if is_generic_fence_kdump -o is_pcs_fence_kdump; then _dep="$_dep network" fi
-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Good catch and looks good.
Do you see anywhere about the documentation of the behavior in bash guide?
No, just found it not my way of using function as conditions, and did some tests.
Seems there are two problems: 1) in [ function_a -o function_b ], function_a does not get called at all, it is just a string
2) for [ ], it is equal to a test command, but test command with a single argument will be same as below: function_a == -n function_a
So it always be true..
Get some details from below document: http://wiki.bash-hackers.org/commands/classictest
Regards, Xunlei
-
Dave Young -
Xunlei Pang -
Xunlei Pang