On Tue, Jan 06, 2015 at 08:46:44PM +0800, Baoquan He wrote: > On 01/06/15 at 04:05pm, Dave Young wrote: > > On 01/05/15 at 08:54pm, Vivek Goyal wrote: > > > On Tue, Jan 06, 2015 at 09:44:05AM +0800, Dave Young wrote: > > > > On 01/02/15 at 08:17am, Vivek Goyal wrote: > > > > > On Fri, Jan 02, 2015 at 08:07:20AM -0500, Prarit Bhargava wrote: > > > > > > > > > > > > > > > > > > On 01/02/2015 07:54 AM, Vivek Goyal wrote: > > > > > > > On Tue, Dec 30, 2014 at 09:57:51AM -0500, Prarit Bhargava wrote: > > > > > > >> panic_on_warn kernel parameter will cause the kernel to panic when a > > > > > > >> WARN() is hit in the kernel. This is not a good situation for the kdump > > > > > > >> kernel because then it would be possible for the kdump kernel to panic in > > > > > > >> a non-fatal WARN(). > > > > > > >> > > > > > > >> This patch removes panic_on_warn as a kernel parameter for the kdump > > > > > > >> kernel. > > > > > > >> > > > > > > > > > > > > > > I think modifying kexec-tools is not best place for this. It probably is better to take care of this in distribution specific scripts. > > > > > > > > > > > > > > In the past we have learnt that it is best that kexec-tools does least > > > > > > > amount of manipulation with command line. > > > > > > > > > > > > Well .. here's the question to think about: what does adding panic_on_warn to > > > > > > the kdump kernel get you? AFAICT, nothing. > > > > > > > > > > Let us consider a hypothetical situation. What if we have some buggy code > > > > > which will corrupt file system in certain situation and we detect that > > > > > situation and throw a warning. > > > > > > > > > > In that case as a work around specifying panic_on_warn in kdump kernel > > > > > will make sense as we don't want to make further progress if we hit > > > > > the warning as it has potential to corrupt fs. > > > > > > > > > > Again this is hypothetical but it can happen. So panic_on_warn might > > > > > still be useful in kdump kernel for some corner debugging cases. > > > > > > > > > > That's why I think we should do it in distribution specific scripts > > > > > and that too only if user did not specify panic_on_warn for second > > > > > kernel explicitly. > > > > > > > > Thinking of user who use upstream kexec-tools instead of distribution toolset, > > > > In case kexec --reuse-cmdline, it will copy /proc/cmdline, but user will have > > > > no way to remove part of them. > > > > > > > > I do want to insist on removing 'panic_on_warn' in upstream kexec-tools, but > > > > we should give user an option to remove it. Something like: > > > > > > > > kexec --reuse-cmdline --remove-params="panic_on_warn" will be good. > > > > > > If user is using --reuse-commandline at the same time does not want some > > > of the parameters from command line, then don't use --reuse-commandline. > > > > > > This is overenginnering. First provide an option to reuse the commandline > > > and provide another option to selectively remove some parameters from that > > > commandline. > > > > > > What's wrong with existing parameters of --command-line. This just allows > > > user to specify whatever command line is suitable. > > > > > > So, no, we should not provide --remove-params. If existing command line > > > does not work for new kenrel, then user should not use > > > --reuse-commandline option. > > > > Hmm, ok. So hope one who is use panic_on_warn in 1st kernel know what he is doing > > and do not simply copy the 1st kernel cmdline for 2nd kernel. > > I am fine with which postion it should be cared in some extent. > This is truly a problem we need consider. If one distribution used > doesn't handle it, and user using latest upstream kernel will be > surprised by this. This is true for all kernel parameters. One distribution might decide to use some kenrel parameter by default during installation and other might not. Is kexec-tools supposed to keep track of all kernel parameters? It is just not possible. That's why it is up to distributions to figure out what parameters work for them and modify their scripts accordingly. Thanks Vivek
