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