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(a)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:
1) 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"
2) check_resettable(): perror "Rootfs device $_target is not resettable, can not be
used as the default target, please specify a default action"
3) 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 $index
>> + done
>>
>> return 0
>> }
>> --
>> 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
Thanks
Dave