[PATCH v3 0/8] collect kdump targets and improve kdump per those information

[Xunlei Pang]

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(-)

-- 
1.8.3.1

[PATCH v3 1/8] kdump-lib.sh: fix improper get_block_dump_target()

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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

[PATCH v3 2/8] kdump-lib.sh: introduce get_kdump_targets()

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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
-- 
1.8.3.1

[PATCH v3 3/8] mkdumprd: change for_each_block_target() to use get_kdump_targets()

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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
 }
 
-- 
1.8.3.1

[PATCH v3 3/8] mkdumprd: change for_each_block_target() to use get_kdump_targets()

[Dave Young]

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(a)redhat.com&gt;
>  ---
>   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 $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"
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(a)lists.fedoraproject.org
>  To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org 
Thanks
Dave

[PATCH v3 3/8] mkdumprd: change for_each_block_target() to use get_kdump_targets()

[Xunlei Pang]

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(a)redhat.com&gt;
> > ---
> >  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 $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"
>  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(a)lists.fedoraproject.org
> > To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>  Thanks
>  Dave

[PATCH v3 4/8] kdumpctl: use generated rd.lvm.lv=X

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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`
-- 
1.8.3.1

[PATCH v3 4/8] kdumpctl: use generated rd.lvm.lv=X

[Xunlei Pang]

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(a)redhat.com&gt;
>  ---
>   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" ] || 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`

[PATCH v3 4/8] kdumpctl: use generated rd.lvm.lv=X

[Dave Young]

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(a)redhat.com&gt;
>  > ---
>  >  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" ] || 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`
> _______________________________________________
> kexec mailing list -- kexec(a)lists.fedoraproject.org
> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org

[PATCH v3 4/8] kdumpctl: use generated rd.lvm.lv=X

[Xunlei Pang]

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(a)redhat.com&gt;
> >> ---
> >>  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" ] || 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`
> > _______________________________________________
> > kexec mailing list -- kexec(a)lists.fedoraproject.org
> > To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org

[PATCH v3 5/8] mkdumprd: omit crypt when there is no crypt kdump target

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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
-- 
1.8.3.1

[PATCH v3 5/8] mkdumprd: omit crypt when there is no crypt kdump target

[Dave Young]

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(a)redhat.com&gt;
>  ---
>   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 
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(a)lists.fedoraproject.org
>  To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org 
Thanks
Dave

[PATCH v3 5/8] mkdumprd: omit crypt when there is no crypt kdump target

[Dave Young]

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(a)redhat.com&gt;
>  > ---
>  >  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
>  
>  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(a)lists.fedoraproject.org
> > To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
> 
> Thanks
> Dave

[PATCH v3 5/8] mkdumprd: omit crypt when there is no crypt kdump target

[Xunlei Pang]

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(a)redhat.com&gt;
> >> ---
> >>  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
> > 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(a)lists.fedoraproject.org
> >> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
> > Thanks
> > Dave

[PATCH v3 5/8] mkdumprd: omit crypt when there is no crypt kdump target

[Dave Young]

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(a)redhat.com&gt;
>  >>> ---
>  >>>  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
>  >> 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

[PATCH v3 5/8] mkdumprd: omit crypt when there is no crypt kdump target

[Xunlei Pang]

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(a)redhat.com&gt;
> >>>> ---
> >>>>  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
> >>> 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

[PATCH v3 6/8] mkdumprd: omit dracut modules in case of no dm target

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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
-- 
1.8.3.1

[PATCH v3 7/8] mkdumprd: omit dracut modules in case of network dumping

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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
-- 
1.8.3.1

[PATCH v3 8/8] module-setup: fix kdump network dependency

[Xunlei Pang]

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(a)redhat.com&gt;
---
 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

[PATCH v3 8/8] module-setup: fix kdump network dependency

[Dave Young]

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(a)redhat.com&gt;
>  ---
>   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(a)lists.fedoraproject.org
>  To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org 

Good catch and looks good.

Do you see anywhere about the documentation of the behavior in bash
guide?

Thanks
Dave

[PATCH v3 8/8] module-setup: fix kdump network dependency

[Xunlei Pang]

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(a)redhat.com&gt;
> > ---
> >  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(a)lists.fedoraproject.org
> > To unsubscribe send an email to kexec-leave(a)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

[PATCH v3 8/8] module-setup: fix kdump network dependency

[Dave Young]

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(a)redhat.com&gt;
>  >> ---
>  >>  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(a)lists.fedoraproject.org
>  >> To unsubscribe send an email to kexec-leave(a)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