Re: [PATCH V2] earlykdump: provide a prompt message during the rebuild of kdump initramfs.

[lijiang]

在 2019年05月16日 00:20, Kazuhito Hagio 写道:
>  Hi Lianbo,
>  
>  On 5/15/2019 6:56 AM, Lianbo Jiang wrote:
> > Early kdump inherits the settings of normal kdump, so any changes that
> > caused normal kdump rebuilding also require rebuilding the system initramfs
> > to make sure that the changes take effect for early kdump.> 
> > Therefore, when the early kdump is enabled, provide a prompt message during
> > the rebuild of kdump initramfs.
> >
> > Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com&gt;
> > ---
> >  kdumpctl | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/kdumpctl b/kdumpctl
> > index 75eebacef92b..09ede5857fb2 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -549,6 +549,16 @@ check_system_modified()
> >  	return 0
> >  }
> >  
> > +check_earlykdump_is_enabled()
> > +{
> > +	rc=`grep -w "rd.earlykdump" /proc/cmdline`
> > +	if [ -n "$rc" ]; then
> > +		return 1
> > +	fi
> > +
> > +	return 0
> > +}
> > +
>  
>  I think generally a bash function like this returns 0 when the target
>  is enabled, and returns 1 when disabled. Please see other functions.
>  So we can write like this:
>   
Thanks for your comment.

I will improve it according to your suggestions.

>  check_earlykdump_is_enabled()
>  {
>  	grep -q -w "rd.earlykdump" /proc/cmdline
>  	return $?	# this also can be omitted in fact
>  }
>  
> >  check_rebuild()
> >  {
> >  	local extra_modules
> > @@ -628,6 +638,10 @@ check_rebuild()
> >  	fi
> >  
> >  	echo "Rebuilding $TARGET_INITRD"
> > +	check_earlykdump_is_enabled
> > +	if [ $? -eq 1 ]; then
> > +		echo "Tips: If early kdump is enabled, also require rebuilding the system
> initramfs to ensure that the changes take effect for early kdump."
> > +	fi
>  
>  Then, we can use this style:
>  
>  if check_earlykdump_is_enabled ; then
>  	...
>  fi
>  
>  And, I think this should be printed after the rebuilding is completed
>  because most users use "systemctl restart kdump" to rebuild initramfs,
>  where there is no prompt output, and they can easily miss the message
>  in many dracut messages in journal. So it would be better to be seen
>  in "systemctl status kdump".
>   
OK, this will be improved and i will post again.

>  Also, it might be good to have more detailed explanation about this
>  in early-kdump-howto.txt, in order for users to realize the need to
>  rebuild the system initramfs to make changes applied, when they start
>  to enable early kdump.
>   
Indeed, adding a detailed explanation for early-kdump-howto.txt would be
nice. Thank you, Kazu.

Lianbo

> Thanks,
> Kazu
> 
>>  	rebuild_initrd
>>  	return $?
>>  }
>> @@ -1140,6 +1154,10 @@ rebuild() {
>>  	fi
>>  
>>  	echo "Rebuilding $TARGET_INITRD"
>> +	check_earlykdump_is_enabled
>> +	if [ $? -eq 1 ]; then
>> +		echo "Tips: If early kdump is enabled, also require rebuilding the system
initramfs to ensure that the changes take effect for early kdump."
>> +	fi
>>  	rebuild_initrd
>>  	return $?
>>  }
>>