On 07/05/17 at 04:36pm, Xunlei Pang wrote:
On 07/05/2017 at 11:30 AM, Dave Young wrote:
On 07/04/17 at 01:45pm, Xunlei Pang wrote:
Resolves: bz1451717 https://bugzilla.redhat.com/1451717
Now that we have get_kdump_targets(), use it to simplify the code.
Signed-off-by: Xunlei Pang xlpang@redhat.com
mkdumprd | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/mkdumprd b/mkdumprd index 337ae87..0fe1b30 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
seems here "index" is for the return value, originally 1 means dump target, 2 means dump_to_rootfs, but the index value is hiding this logic behind, if we later change get_kdump_targets, it is likely to be missed..
Maybe we can add the error message in check_resettable to is_resettable and only return same error value.
Yes, it stopped me too when I reached here, there are three messages:
- check_resettable(): perror "Can not save vmcore to target device $_target . This device can not be initialized in kdump kernel as it is not resettable"
- check_resettable(): perror "Rootfs device $_target is not resettable, can not be used as the default target, please specify a default action"
- is_unresettable(): echo "Device $device is unresettable"
Which one do you think we can keep or drop in is_unresettable()? Or simply drop both 1) and 2) and keep 3) just like check_crypt() does?
Drop 1) and 2) should be fine, we inherit these from old RHEL versions and maybe some old cards can not reset correctly, remove them make the code simpler.
If we can get the dump target in is_resettable it would be better we can print the target name as well. And print like: "Device $device is unresettable, can not save vmcore to $_target", but if can not it is also acceptable..
Regards, Xunlei
- 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 $indexdone
return 0
}
1.8.3.1 _______________________________________________ kexec mailing list -- kexec@lists.fedoraproject.org To unsubscribe send an email to kexec-leave@lists.fedoraproject.org
Thanks Dave
Thanks Dave