Re: [PATCH] kexec-tools: do not copy 1st kernel root= param in fs2dt.c

From: Simon Horman <horms@verge.net.au>
To: Dave Young <dyoung@redhat.com>
Cc: jstodola@redhat.com, kexec@lists.infradead.org
Subject: Re: [PATCH] kexec-tools: do not copy 1st kernel root= param in fs2dt.c
Date: Mon, 26 Oct 2015 13:31:19 +0900	[thread overview]
Message-ID: <20151026043119.GB2411@verge.net.au> (raw)
In-Reply-To: <20151019082246.GC31886@dhcp-129-115.nay.redhat.com>

On Mon, Oct 19, 2015 at 04:22:46PM +0800, Dave Young wrote:
> Hi, Simon
> 
> On 10/16/15 at 10:33am, Simon Horman wrote:
> > Hi Dave,
> > 
> > On Fri, Oct 09, 2015 at 01:57:22PM +0800, Dave Young wrote:
> > > Jan Stodola <jstodola@redhat.com> reported ppc64 root= is always added in kexec
> > > kernel cmdline. But sometimes we need boot without root= for example we use
> > > kexec to boot into installation initramfs image like below:
> > > kexec --load vmlinuz --initrd=initrd.img --command-line=\
> > > "inst.repo=http://<server>/<path>/Server/ppc64le/os/"
> > > 
> > > While creating dtb, in case there's no root= in user provided cmdline params 
> > > kexec-tools will find the original root= param used in 1st kernel and pass it
> > > to 2nd kernel. This caused that user have no way to remove root= cmdline.
> > > 
> > > Dropping that part of code so that one can get chance to kexec into 2nd kernel
> > > without root= param. One can still provide root= in --command-line=""
> > 
> > I'm a little concerned about the backwards-compatibility implications of
> > this change. Though I agree the new behaviour is entirely sane perhaps
> > it should be activated via a new command line option.
> 
> I did not notice this problem until Jan reported it so I thouhgt that it
> is just a hidden assumption, people might not notice it and use it that
> way.
> 
> For Fedora/RHEL kdump we will copy 1st kernel cmdline by default so one
> do not need to use it. For kexec and other distributions I'm not sure..
> 
> Since it is more of a fix to original logic, personally I tend to not
> introducing a new option. But if you think we need to add a new option I
> can do it.

> > Also, won't this affect other architectures that use DT.
> > I'm thinking about ARM. If so it might be good to tweak the changelog.
> 
> arm also uses the function, but I need verify the behavior. Will tune the
> changelog after I confirming it.

Thanks

I'm still concerned that people may be relying on this behaviour.
But I agree it seems painful to add an option to get what
should have been the default.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec