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".
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.
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.
If we know ..., we can omit ...
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 99kdumpbase network dependency
dracut-module-setup.sh | 2 +- kdump-lib.sh | 111 +++++++++++++++++++++++++++++++++++++++++++------ kdumpctl | 69 ++++++++++++++++++------------ mkdumprd | 103 +++++++++++++++++++++++++-------------------- 4 files changed, 200 insertions(+), 85 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 --- v3->v4: -No change.
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() {
On 07/07/17 at 03:48pm, Xunlei Pang wrote:
-For nfs/ssh dumping, there is no need to check the root device. -Move get_save_path into kdump-lib.sh.
Does it mean that default dump_to_rootfs action is unavailable if nfs/ssh target is specified?
After this patch, get_block_dump_target() can always return the correct block dump target specified.
Signed-off-by: Xunlei Pang xlpang@redhat.com
v3->v4: -No change.
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() { -- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
On 07/10/2017 at 03:14 PM, Baoquan He wrote:
On 07/07/17 at 03:48pm, Xunlei Pang wrote:
-For nfs/ssh dumping, there is no need to check the root device. -Move get_save_path into kdump-lib.sh.
Does it mean that default dump_to_rootfs action is unavailable if nfs/ssh target is specified?
It's only for get_block_dump_target(), as we already removed "root=X", there is no need to get the root device if the dump target is not at root.
For dump_to_rootfs cases, it's handled after get_block_dump_target().
Regards, Xunlei
After this patch, get_block_dump_target() can always return the correct block dump target specified.
Signed-off-by: Xunlei Pang xlpang@redhat.com
v3->v4: -No change.
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() { -- 1.8.3.1 _______________________________________________ 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
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 --- v3->v4: -No change.
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 --- v3->v4: -Improved message in is_unresettable(). -"return 1" instead of keeping an index in for_each_block_target().
mkdumprd | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/mkdumprd b/mkdumprd index 337ae87..062b033 100644 --- a/mkdumprd +++ b/mkdumprd @@ -297,20 +297,11 @@ for_each_block_target() { local dev majmin
- #check dump target - dev=$(get_block_dump_target) - - if [ -n "$dev" ]; then + for dev in $(get_kdump_targets); do + [ -b "$dev" ] || continue 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 - majmin=$(get_maj_min $dev) - check_block_and_slaves $1 $majmin && return 2 - fi + done
return 0 } @@ -329,7 +320,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 because device $device is unresettable" return 0 } fi @@ -350,14 +341,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 }
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 --- v3->v4: -Made "GENERATED_LVM_CMDLINES" a local variable "lvm_cmdlines".
kdumpctl | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8c40564..3405530 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,6 +171,46 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
+# 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 + lvm_cmdlines="rd.lvm.lv=$vg/$lv $lvm_cmdlines" + fi + + return 0 +} + +generate_lvm_cmdlines() +{ + local lvm_cmdlines="" + + for_each_block_target_all generate_lvm_cmdline + + echo "$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 +230,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 03:48pm, 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
v3->v4: -Made "GENERATED_LVM_CMDLINES" a local variable "lvm_cmdlines".
kdumpctl | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8c40564..3405530 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,6 +171,46 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
+# 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
lvm_cmdlines="rd.lvm.lv=$vg/$lv $lvm_cmdlines"- fi
- return 0
+}
+generate_lvm_cmdlines() +{
- local lvm_cmdlines=""
- for_each_block_target_all generate_lvm_cmdline
Since lvm_cmdlines becomes a local variable, it is better to move function generate_lvm_cmdline code into getnerate_lvm_cmdlines directly
It is less likely we need generate lvm_cmdline for a specific block target.
Sorry for observed this late, I think this is the only comment for me to this series, other than this, the whole series looks good to me.
Can you resend this patch only if agree with above?
After waiting for several days I can merge the series if no other comments from other people.
- echo "$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 +230,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`
-- 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/10/2017 at 10:53 AM, Dave Young wrote:
On 07/07/17 at 03:48pm, 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
v3->v4: -Made "GENERATED_LVM_CMDLINES" a local variable "lvm_cmdlines".
kdumpctl | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8c40564..3405530 100755 --- a/kdumpctl +++ b/kdumpctl @@ -171,6 +171,46 @@ check_kdump_cpus() echo " try nr_cpus=$nr_min or larger instead" }
+# 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
lvm_cmdlines="rd.lvm.lv=$vg/$lv $lvm_cmdlines"- fi
- return 0
+}
+generate_lvm_cmdlines() +{
- local lvm_cmdlines=""
- for_each_block_target_all generate_lvm_cmdline
Since lvm_cmdlines becomes a local variable, it is better to move function generate_lvm_cmdline code into getnerate_lvm_cmdlines directly
It is less likely we need generate lvm_cmdline for a specific block target.
Sorry for observed this late, I think this is the only comment for me to this series, other than this, the whole series looks good to me.
Can you resend this patch only if agree with above?
Completely agree, thanks for noticing this, will resend this patch alone soon.
After waiting for several days I can merge the series if no other comments from other people.
Yes, thanks for all your valuable suggestions!
Regards, Xunlei
- echo "$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 +230,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`
-- 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 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.
We don't want to omit any module for fadump, thus we move is_fadump_capable() into kdump-lib.sh as a helper to use.
Signed-off-by: Xunlei Pang xlpang@redhat.com --- v3->v4: -Introduced global variable "crypt_exists".
kdump-lib.sh | 12 ++++++++++++ kdumpctl | 12 ------------ mkdumprd | 15 +++++++++++++++ 3 files changed, 27 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 3405530..07b7125 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" @@ -933,17 +932,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 062b033..644b5ce 100644 --- a/mkdumprd +++ b/mkdumprd @@ -372,11 +372,24 @@ check_crypt() return 1 }
+crypt_exists=0 +omit_dracut_modules() +{ + # Skip fadump case + is_fadump_capable && return + + # Omit "crypt", BZ1451717 + if [ "$crypt_exists" == "0" ]; then + add_dracut_arg "--omit" "crypt" + fi +} + if ! check_resettable; then exit 1 fi
if ! check_crypt; then + crypt_exists=1 echo "Warning: Encrypted device is in dump path. User will prompted for password during second kernel boot." fi
@@ -461,6 +474,8 @@ then add_dracut_arg "--add-drivers" "$extra_modules" fi
+omit_dracut_modules + dracut "${dracut_args[@]}" "$@" _rc=$? sync
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 --- v3->v4: -Rebased due to previos change.
mkdumprd | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/mkdumprd b/mkdumprd index 644b5ce..9c5339c 100644 --- a/mkdumprd +++ b/mkdumprd @@ -372,16 +372,44 @@ check_crypt() return 1 }
+is_dm() +{ + if [ -d "/sys/dev/block/$1/dm" ]; then + return 0 + fi + + return 1 +} + crypt_exists=0 omit_dracut_modules() { + local target majmin + local dm_exists + # Skip fadump case is_fadump_capable && return
+ dm_exists=0 + + for target in $(get_kdump_targets); do + if [ -b "$target" ]; then + majmin=$(get_maj_min $target) + # Check "dm" + check_block_and_slaves is_dm $majmin && dm_exists=1 + fi + done + # Omit "crypt", BZ1451717 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 --- v3->v4: -Rebased due to previous change.
mkdumprd | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/mkdumprd b/mkdumprd index 9c5339c..8efea77 100644 --- a/mkdumprd +++ b/mkdumprd @@ -385,12 +385,13 @@ crypt_exists=0 omit_dracut_modules() { local target majmin - local dm_exists + local dm_exists network_only
# Skip fadump case is_fadump_capable && return
dm_exists=0 + network_only=1
for target in $(get_kdump_targets); do if [ -b "$target" ]; then @@ -398,6 +399,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 @@ -410,6 +414,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 dumpings require network 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 --- v3->v4: -No change.
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
On 07/07/17 at 03:48pm, Xunlei Pang wrote:
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".
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.
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.
If we know ..., we can omit ...
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 99kdumpbase network dependency
dracut-module-setup.sh | 2 +- kdump-lib.sh | 111 +++++++++++++++++++++++++++++++++++++++++++------ kdumpctl | 69 ++++++++++++++++++------------ mkdumprd | 103 +++++++++++++++++++++++++-------------------- 4 files changed, 200 insertions(+), 85 deletions(-)
-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
For the series (with one patch updated) Acked-by: Dave Young dyoung@redhat.com
Thanks Dave
-
Baoquan He -
Dave Young -
Xunlei Pang -
Xunlei Pang