On 03/28/2017 at 01:37 PM, Pratyush Anand wrote:

Hi Xunlei,

On Monday 27 March 2017 08:12 AM, Xunlei Pang wrote:

...

Consider block device in the special "--dracut-args --mount ..." in get_user_configured_dump_disk().

Above and below changes are not related. So, it would be good if they are in different patches. IMHO, it should be divided in 3 patches for easier review.

1. Move stuff from kdumpctl to kdump-lib.sh (no functional change)
2. modification in get_block_dump_target()
3. consider block device in the special "--dracut-args --mount ..."

Yes, I will do it if sending next version. Thanks!

Regards, Xunlei

~Pratyush

...

Consider save path instead of only root fs in get_block_dump_target(), and move it into kdump-lib.sh, becuase we will use it in kdumpctl. We did not add it, because get_block_dump_target() is only used by check_resettable/check_crypt and users are expected to make to work for the special "--dracut-args --mount ...", anyway there is nothing wrong we add it in. We will have another user in the next patch.

Moreover, for nfs/ssh dumping, we do not need check the root device.

Move get_save_path into kdump-lib.sh.

After this patch, get_block_dump_target() can always return our correct block dump target specified.

Signed-off-by: Xunlei Pang xlpang@redhat.com

kdump-lib.sh | 44 ++++++++++++++++++++++++++++++++++++-------- kdumpctl | 10 ---------- mkdumprd | 13 ------------- 3 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh index 8ebad70..ae54b7f 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,6 +82,12 @@ 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 @@ -97,9 +97,10 @@ get_user_configured_dump_disk() 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 +112,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
• _target=$(get_user_configured_dump_disk)
• [ -n "$_target" ] && echo $(to_dev_name $_target) && return
• if is_ssh_dump_target || is_nfs_dump_target; then

•    return
  

• fi
• # 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 4d68be0..d8f049f 100755 --- a/kdumpctl +++ b/kdumpctl @@ -930,16 +930,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 f30d9c2..426149b 100644 --- a/mkdumprd +++ b/mkdumprd @@ -256,19 +256,6 @@ add_mount() { fi }

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

Hi Xunlei,

On Monday 27 March 2017 08:12 AM, Xunlei Pang wrote:

We add a generic hook named kdump_target_hook() for the dump target, this is useful for us to do some extra work for the dump targets.

I have not seen the next patches so far, however I tried to read this patch again and again but I could not understand why we are doing all these, what problem we are trying to address. Moreover, patch is not just adding a generic hook, it is modifying existing functionality of check_files_modified(). IMO, it would again be good to divide it in multiple patches, with clear explanation that why do we do that. Even cover letter also does not talk about overall what we try to achieve with the series.

~Pratyush

kdump_targets_tell() is exported as a generic helper that can be used as some specific purpose.

To implement a new checklist: Add your new checklist(job) "xxx" separated by whitespace, and implement its logic to update its tag value in the tag inventory. Then "kdump_targets_tell xxx" will be available for you to use.

We will use this mechanism in the following patches.

Signed-off-by: Xunlei Pang xlpang@redhat.com

kdump-lib.sh | 28 +++++++++++++++ kdumpctl | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+)

diff --git a/kdump-lib.sh b/kdump-lib.sh index ae54b7f..d3c0d7b 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -434,3 +434,31 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }

+# $1: item +# Return the item tag value in the inventory. +kdump_target_get_inventory() +{

• local item=$1 tag
• tag=$(eval echo '${KDUMP_TARGET_TAG_'${item}'}')
• echo $tag

+}

+# $1: item +kdump_targets_tell() +{

• local item=$1 tagvalue
• tagvalue=$(kdump_target_get_inventory $item)
• if [ "$tagvalue" = "1" ]; then

•    return 0
  

• elif [ "$tagvalue" = "0" ]; then

•    return 1
  

• else

•    echo "Warning: wrong tag value for \"kdump_target_is $item\""
  

• fi
• return 1

+} diff --git a/kdumpctl b/kdumpctl index d8f049f..ca97580 100755 --- a/kdumpctl +++ b/kdumpctl @@ -23,6 +23,12 @@ image_time=0 . $dracutbasedir/dracut-functions.sh . /lib/kdump/kdump-lib.sh

+# Add your new checklist(job) "xxx" separated by whitespace, and +# implement its logic to update its tag value in the tag inventory. +# Then "kdump_targets_tell xxx" will be available for you to use. +# See kdump_target_handle_item() for examples. +KDUMP_TARGET_CHECKLIST=""

standard_kexec_args="-p"

# Some default values in case /etc/sysconfig/kdump doesn't include @@ -435,6 +441,114 @@ setup_initrd() fi }

+kdump_target_inventory_init() +{

• kdump_target_checklist kdump_target_init_item

+}

+# Three values(0, 1, 2) are allowed in the tag inventory, +# basically each value has the following meaning: +# 0 - "surely no", 1 - "surely yes", 2 - "not sure" +# Each item maintains its own way to update the value. +# +# Update the item tag value to 2 +# $1: item +kdump_target_init_item() +{

• kdump_target_update_inventory $1 2

+}

+# $1: ssh, nfs, or block dev name +kdump_target_hook() +{

• local dev=$1
• if [ -b "$dev" ]; then

• dev=$(get_persistent_dev $dev)
  

• fi
• kdump_target_checklist kdump_target_handle_item $dev

+}

+# $1: function to be executed +# $2,..: arguments of $1 +kdump_target_checklist() +{

• local item func
• func=$1
• shift
• for item in $KDUMP_TARGET_CHECKLIST; do

• "$func" $item $@
  

• done

+}

+# $1: the item to be checked +# $2: persistent block dev name, nfs, or ssh +# +# You can add an item(job) in KDUMP_TARGET_CHECKLIST, implement its +# own logic on how to update its tag value in the tag inventory. +kdump_target_handle_item() +{

• local item=$1 dev=$2
• local value
• value=$(kdump_target_get_inventory $item)
• # The item handling begins
• case "$item" in
• *)

• echo "unhandled item in kdump target checklist"
  

• return
  

• esac
• kdump_target_update_inventory $item $value

+}

+# Update the item in the tag inventory. +# $1: item +# $2: value +kdump_target_update_inventory() +{

• local tag
• tag=KDUMP_TARGET_TAG_$1
• eval export $tag=$2

+}

+# Currently, we add no more than two target hooks +# 1) cases that only one root target +# 2) cases that non-root target and the root target +check_kdump_targets() +{

• local _target _root
• kdump_target_inventory_init
• _target=$(get_block_dump_target)
• if [ -n "$_target" ]; then

• kdump_target_hook $_target
  

• elif is_ssh_dump_target; then

• kdump_target_hook "ssh"
  

• else

• kdump_target_hook "nfs"
  

• fi
• # Add the root device if needed, currently kdump always mounts the root
• _root=$(get_root_fs_device)
• if [[ -b "$_root" && "$_root" != "$_target" ]]; then

• kdump_target_hook $_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".
• #
• # Fedora doesn't have "--hostonly-cmdline" specified, so there's no problem.

+}

check_files_modified() { local modified_files="" @@ -1144,6 +1258,8 @@ start() fi fi

• check_kdump_targets
• check_rebuild if [ $? != 0 ]; then echo "Starting kdump: [FAILED]"

On 03/28/2017 at 03:10 PM, Pratyush Anand wrote:

On Tuesday 28 March 2017 11:43 AM, Pratyush Anand wrote:

...

Hi Xunlei,

On Monday 27 March 2017 08:12 AM, Xunlei Pang wrote:

...

We add a generic hook named kdump_target_hook() for the dump target, this is useful for us to do some extra work for the dump targets.

I have not seen the next patches so far, however I tried to read this patch again and again but I could not understand why we are doing all these, what problem we are trying to address. Moreover, patch is not just adding a generic hook, it is modifying existing functionality of check_files_modified(). IMO, it would again be good to divide it in multiple patches, with clear explanation that why do we do that. Even cover letter also does not talk about overall what we try to achieve with the series.

OK..I read last patch 6/6 first and then backward, and its a bit clear now.

...

...

kdump_targets_tell() is exported as a generic helper that can be used as some specific purpose.

To implement a new checklist: Add your new checklist(job) "xxx" separated by whitespace, and implement its logic to update its tag value in the tag inventory. Then "kdump_targets_tell xxx" will be available for you to use.

We will use this mechanism in the following patches.

Signed-off-by: Xunlei Pang xlpang@redhat.com

kdump-lib.sh | 28 +++++++++++++++ kdumpctl | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+)

diff --git a/kdump-lib.sh b/kdump-lib.sh index ae54b7f..d3c0d7b 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -434,3 +434,31 @@ get_dracut_args_target() { echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1 }

+# $1: item +# Return the item tag value in the inventory. +kdump_target_get_inventory() +{

• local item=$1 tag
• tag=$(eval echo '${KDUMP_TARGET_TAG_'${item}'}')
• echo $tag

+}

+# $1: item +kdump_targets_tell() +{

• local item=$1 tagvalue
• tagvalue=$(kdump_target_get_inventory $item)
• if [ "$tagvalue" = "1" ]; then

•    return 0
  

• elif [ "$tagvalue" = "0" ]; then

•    return 1
  

• else

•    echo "Warning: wrong tag value for \"kdump_target_is $item\""
  

• fi
• return 1

+} diff --git a/kdumpctl b/kdumpctl index d8f049f..ca97580 100755 --- a/kdumpctl +++ b/kdumpctl @@ -23,6 +23,12 @@ image_time=0 . $dracutbasedir/dracut-functions.sh . /lib/kdump/kdump-lib.sh

+# Add your new checklist(job) "xxx" separated by whitespace, and +# implement its logic to update its tag value in the tag inventory. +# Then "kdump_targets_tell xxx" will be available for you to use. +# See kdump_target_handle_item() for examples. +KDUMP_TARGET_CHECKLIST=""

standard_kexec_args="-p"

# Some default values in case /etc/sysconfig/kdump doesn't include @@ -435,6 +441,114 @@ setup_initrd() fi }

+kdump_target_inventory_init() +{

• kdump_target_checklist kdump_target_init_item

+}

+# Three values(0, 1, 2) are allowed in the tag inventory, +# basically each value has the following meaning: +# 0 - "surely no", 1 - "surely yes", 2 - "not sure" +# Each item maintains its own way to update the value. +# +# Update the item tag value to 2 +# $1: item +kdump_target_init_item() +{

• kdump_target_update_inventory $1 2

+}

+# $1: ssh, nfs, or block dev name +kdump_target_hook()

This hook seems to update inventory. It would be good to mention that in comment *atleast*.

...

...

+{

• local dev=$1
• if [ -b "$dev" ]; then

•    dev=$(get_persistent_dev $dev)
  

• fi
• kdump_target_checklist kdump_target_handle_item $dev

+}

+# $1: function to be executed +# $2,..: arguments of $1 +kdump_target_checklist() +{

• local item func
• func=$1
• shift
• for item in $KDUMP_TARGET_CHECKLIST; do

•    "$func" $item $@
  

• done

+}

+# $1: the item to be checked +# $2: persistent block dev name, nfs, or ssh +# +# You can add an item(job) in KDUMP_TARGET_CHECKLIST, implement its +# own logic on how to update its tag value in the tag inventory. +kdump_target_handle_item() +{

• local item=$1 dev=$2
• local value
• value=$(kdump_target_get_inventory $item)
• # The item handling begins
• case "$item" in
• *)

•    echo "unhandled item in kdump target checklist"
  

•    return
  

• esac
• kdump_target_update_inventory $item $value

+}

+# Update the item in the tag inventory. +# $1: item +# $2: value +kdump_target_update_inventory() +{

• local tag
• tag=KDUMP_TARGET_TAG_$1
• eval export $tag=$2

Humm..not sure if sharing info between functions by exporting global names is a good idea...

At the beginning, I used a tmp file to store all the results. Then I found in mkdumprd there has "export IN_KDUMP=1" and then use it in 99kdump module-setup.sh, that's why I changed to use this way. At least It has one advantage that we don't need to worry about the problem of informatin gets removed during kdump starting when using tmp file.

Regards, Xunlei

...

...

+}

+# Currently, we add no more than two target hooks +# 1) cases that only one root target +# 2) cases that non-root target and the root target +check_kdump_targets() +{

• local _target _root
• kdump_target_inventory_init
• _target=$(get_block_dump_target)
• if [ -n "$_target" ]; then

•    kdump_target_hook $_target
  

• elif is_ssh_dump_target; then

•    kdump_target_hook "ssh"
  

• else

•    kdump_target_hook "nfs"
  

• fi
• # Add the root device if needed, currently kdump always mounts

the root

• _root=$(get_root_fs_device)
• if [[ -b "$_root" && "$_root" != "$_target" ]]; then

•    kdump_target_hook $_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".

• #
• # Fedora doesn't have "--hostonly-cmdline" specified, so there's

no problem. +}

check_files_modified() { local modified_files="" @@ -1144,6 +1258,8 @@ start() fi fi

• check_kdump_targets
• check_rebuild if [ $? != 0 ]; then echo "Starting kdump: [FAILED]"

On 03/27/17 at 10:42am, Xunlei Pang wrote:

For the following scripts, cmdline="root=/dev/mapper/rhel-root rd.lvm.lv=rhel/root quiet" remove_cmdline_param $cmdline "root"

we will get the result "rd.lvm.lv=rhel/ quiet", after this patch we can get the correct "rd.lvm.lv=rhel/root quiet".

Signed-off-by: Xunlei Pang xlpang@redhat.com

kdumpctl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl index ca97580..e9dc8e5 100755 --- a/kdumpctl +++ b/kdumpctl @@ -75,7 +75,8 @@ remove_cmdline_param() for arg in $@; do cmdline=`echo $cmdline | \ sed -e "s/\b$arg=[^ ]*\b//g" \

• 	     -e "s/\b$arg\b//g" \
  

• 	     -e "s/^$arg\b//g" \
  

• 	     -e "s/[[:space:]]$arg\b//g" \
       -e "s/\s\+/ /g"`
  

  done echo $cmdline

-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org

Acked-by: Dave Young dyoung@redhat.com

Xunlei, I can merge this one first so that it does not being included in the series again.

Thanks Dave

On 03/28/2017 at 04:01 PM, Dave Young wrote:

On 03/27/17 at 10:42am, Xunlei Pang wrote:

...

For the following scripts, cmdline="root=/dev/mapper/rhel-root rd.lvm.lv=rhel/root quiet" remove_cmdline_param $cmdline "root"

we will get the result "rd.lvm.lv=rhel/ quiet", after this patch we can get the correct "rd.lvm.lv=rhel/root quiet".

Signed-off-by: Xunlei Pang xlpang@redhat.com

kdumpctl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl index ca97580..e9dc8e5 100755 --- a/kdumpctl +++ b/kdumpctl @@ -75,7 +75,8 @@ remove_cmdline_param() for arg in $@; do cmdline=`echo $cmdline | \ sed -e "s/\b$arg=[^ ]*\b//g" \

• 	     -e "s/\b$arg\b//g" \
  

• 	     -e "s/^$arg\b//g" \
  

• 	     -e "s/[[:space:]]$arg\b//g" \
       -e "s/\s\+/ /g"`
  

  done echo $cmdline

-- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org

Acked-by: Dave Young dyoung@redhat.com

Xunlei, I can merge this one first so that it does not being included in the series again.

Hi Dave,

I found another case needs to be fixed as well, so please hold this one, I will send the updated one out with the "remove root=X" patchset. Thanks!

Regards, Xunlei

On 03/27/2017 at 10:42 AM, Xunlei Pang wrote:

Since the current dracut of Fedora already supports not always mount the root device, we can remove "root=X" from the cmdline. If the dump target is not located at root filesystem, will remove "root=X". It works for us as Feodra has no "--hostonly-cmdline", otherwise "95rootfs-block" dracut module will generate "root=X" information in "${initdir}/etc/cmdline.d/95root-dev.conf".

We can easily achieve this by adding a "all_root" job in the KDUMP_TARGET_CHECKLIST.

We will have only one target mounted in kdump after this patch.

For example, in case of nfs/ssh/usb/etc(non-root) dumping, kdump will not mount the unnecessary root filesystem after this change, unless "default dump_to_rootfs" is specified in "/etc/kdump.conf".

Signed-off-by: Xunlei Pang xlpang@redhat.com

kdump-lib.sh | 6 ++++++ kdumpctl | 30 +++++++++++++++++++++++++----- mkdumprd | 6 ------ 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/kdump-lib.sh b/kdump-lib.sh index d3c0d7b..a003376 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -103,6 +103,12 @@ get_user_configured_dump_disk() [ -b "$_target" ] && echo $_target }

+target_is_root() {

• local _t
• _t=$(findmnt -k -n -r -o TARGET $1|sort|head -1)
• [ "$_t" = "/" ]

+}

get_root_fs_device() { local _target diff --git a/kdumpctl b/kdumpctl index e9dc8e5..1cb6f36 100755 --- a/kdumpctl +++ b/kdumpctl @@ -27,7 +27,7 @@ image_time=0 # implement its logic to update its tag value in the tag inventory. # Then "kdump_targets_tell xxx" will be available for you to use. # See kdump_target_handle_item() for examples. -KDUMP_TARGET_CHECKLIST="" +KDUMP_TARGET_CHECKLIST="all_root"

standard_kexec_args="-p"

@@ -177,6 +177,11 @@ 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() @@ -205,6 +210,12 @@ prepare_cmdline() cmdline=`append_cmdline "${cmdline}" disable_cpu_apicid ${id}` fi

• # "false" means non-root dump target, then remove "root=X"

• # unless dump_to_rootfs is specified.

• if ! kdump_targets_tell all_root && ! is_dump_to_rootfs; then

• cmdline=`remove_cmdline_param "$cmdline" root`
  

• fi

• KDUMP_COMMANDLINE=$cmdline

  check_kdump_cpus

@@ -498,6 +509,13 @@ kdump_target_handle_item()

# The item handling begins case "$item" in

• all_root)

• # Sticky value is 0
  

• [[ $value = "0" ]] && return
  

• value=1
  

• target_is_root $dev || value=0
  

• ;;
  

  *) echo "unhandled item in kdump target checklist" return

@@ -535,10 +553,12 @@ check_kdump_targets() kdump_target_hook "nfs" fi

• # Add the root device if needed, currently kdump always mounts the root
• _root=$(get_root_fs_device)
• if [[ -b "$_root" && "$_root" != "$_target" ]]; then

• kdump_target_hook $_root
  

• # Add the root device if /etc/kdump.conf has "default dump_to_rootfs"
• if is_dump_to_rootfs; then

• _root=$(get_root_fs_device)
  

• if [[ -b "$_root" && "$_root" != "$_target" ]]; then
  

• 	kdump_target_hook $_root
  

• fi
  

Currently, I missed the diskless case, I need some discussion about the diskless environment. For diskless systems, will "findmnt /" always return the "nfs" mount information?

Regards, Xunlei

fi

# NOTE: diff --git a/mkdumprd b/mkdumprd index 426149b..d27bf94 100644 --- a/mkdumprd +++ b/mkdumprd @@ -80,12 +80,6 @@ add_dracut_sshkey() { add_dracut_arg "--sshkey" "$1" }

-target_is_root() {

• local _t
• _t=$(findmnt -k -n -r -o TARGET $1|sort|head -1)
• [ "$_t" = "/" ]

-}

# caller should ensure $1 is valid and mounted in 1st kernel to_mount() { local _dev=$1 _source _target _fstype _options _mntopts _pdev

On 03/27/17 at 10:42am, Xunlei Pang wrote:

We can easily achieve this by adding a "any_lvm" job in the KDUMP_TARGET_CHECKLIST.

We need this, because for example in case of "nfs"(and root device is not a lvm volmue) dumping, if "lvm" module is added, "lvm2" tool under kdump still operates the lvm volume due to the "rd.lvm.lv=X" inherited from the first kernel if any.

This patch removes "lvm" and all the related cmdline from the kdump/initramfs in case of no lvm target, it can reduce several megabytes from initramfs.

Signed-off-by: Xunlei Pang xlpang@redhat.com

kdumpctl | 15 ++++++++++++++- mkdumprd | 5 +++++ 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl index 1cb6f36..8a0c400 100755 --- a/kdumpctl +++ b/kdumpctl @@ -27,7 +27,7 @@ image_time=0 # implement its logic to update its tag value in the tag inventory. # Then "kdump_targets_tell xxx" will be available for you to use. # See kdump_target_handle_item() for examples. -KDUMP_TARGET_CHECKLIST="all_root" +KDUMP_TARGET_CHECKLIST="all_root any_lvm"

standard_kexec_args="-p"

@@ -216,6 +216,11 @@ prepare_cmdline() cmdline=`remove_cmdline_param "$cmdline" root` fi

• # Remove all the lvm related cmdlines to avoid confusion if no lvm targets

• if ! kdump_targets_tell any_lvm; then

• cmdline=`remove_cmdline_param "$cmdline" rd.lvm.lv rd_LVM_LV rd.lvm.vg rd_LVM_VG`
  

• fi

• KDUMP_COMMANDLINE=$cmdline

  check_kdump_cpus

@@ -515,6 +520,14 @@ kdump_target_handle_item()

value=1
target_is_root $dev || value=0

• any_lvm)

• # Sticky value is 1
  

• [[ $value = "1" ]] && return
  

• value=0
  

• if [[ $dev =~ "/dev/mapper/" ]]; then
  

/dev/mapper/* could be other devices which is not lvm related?

• 	value=1
  

• fi
  

  ;; *) echo "unhandled item in kdump target checklist"

diff --git a/mkdumprd b/mkdumprd index d27bf94..92d68a3 100644 --- a/mkdumprd +++ b/mkdumprd @@ -495,6 +495,11 @@ then add_dracut_arg "--add-drivers" "$extra_modules" fi

+# Omit "lvm" dracut module if we have no lvm targets. +if ! kdump_targets_tell any_lvm ; then

• add_dracut_arg "-o" "lvm"

+fi

dracut "${dracut_args[@]}" "$@" _rc=$? sync -- 1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org

Thanks Dave

On 03/28/2017 at 03:55 PM, Dave Young wrote:

On 03/27/17 at 10:42am, Xunlei Pang wrote:

...

v1->v2: Add the hook in kdumpctl, so we use this hook anywhere, this helps a loot for the "all_root" checklist(see PATCH4).

We add a generic dump target hook for kdump, supposed to monitor all the devices mounted by kdump.

PATCH2 implements the basic framework, add a hook kdump_target_hook(), and provide a generic helper kdump_targets_tell() for use.

Xunlei, I personally like the v1 more, since it is easier to understand.

Dropping root parameter is simple. For changing lvm parameter as we discussed we do not need to consider linear or not, so we can just change the lvm.conf if it exists in 2nd kernel /etc/ unconditionally.

Yes, thanks, PATCH6 can be independent now, I already updated it to be the following implementation since we don't need to judge the linear lvm, I did it unconditionally and ignored the error by using &>/dev/null:

diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh index 1f96bb8..674f3ee 100755 --- a/dracut-module-setup.sh +++ b/dracut-module-setup.sh @@ -743,4 +743,9 @@ install() { # target. Ideally all this should be pushed into dracut iscsi module # at some point of time. kdump_check_iscsi_targets + + # For the lvm type target under kdump, in /etc/lvm/lvm.conf we can safely replace + # "reserved_memory = 8192" with "reserved_memory = 1024" to lower memory pressure + sed -i -e 's/(^[[:space:]]*)reserved_memory[[:space:]]*=[[:space:]]*[[:digit:]]*/ \ + \1reserved_memory = 1024/' ${initdir}/etc/lvm/lvm.conf &>/dev/null }

Regards, Xunlei