Simon Horman
2007-Aug-28 09:34 UTC
[Xen-devel] [patch] Only skip the image name if it is the image name
When kexecing the BOOT_IMAGE isn''t provided on the second kernel''s command line and thus cmdline_parse() was just skipping over the first parameter. Observed on ia64 Signed-off-by: Simon Horman <horms@verge.net.au> Index: xen-unstable.hg/xen/common/kernel.c ==================================================================--- xen-unstable.hg.orig/xen/common/kernel.c 2007-08-28 18:24:54.000000000 +0900 +++ xen-unstable.hg/xen/common/kernel.c 2007-08-28 18:25:01.000000000 +0900 @@ -37,8 +37,9 @@ void cmdline_parse(char *cmdline) /* Skip whitespace and the image name. */ while ( *p == '' '' ) p++; - if ( (p = strchr(p, '' '')) == NULL ) - return; + if (! strncmp(p, "BOOT_IMAGE=", strlen("BOOT_IMAGE=")) ) + if ( (p = strchr(p, '' '')) == NULL ) + return; for ( ; ; ) { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Aug-28 10:05 UTC
Re: [Xen-devel] [patch] Only skip the image name if it is the image name
Isn''t that a kexec bug? Multiboot command lines include the image name in all other cases. If there''s no suitable name, perhaps a dummy name should be included? -- Keir On 28/8/07 10:34, "Simon Horman" <horms@verge.net.au> wrote:> When kexecing the BOOT_IMAGE isn''t provided on the second kernel''s > command line and thus cmdline_parse() was just skipping over > the first parameter. > > Observed on ia64 > > Signed-off-by: Simon Horman <horms@verge.net.au> > > Index: xen-unstable.hg/xen/common/kernel.c > ==================================================================> --- xen-unstable.hg.orig/xen/common/kernel.c 2007-08-28 18:24:54.000000000 > +0900 > +++ xen-unstable.hg/xen/common/kernel.c 2007-08-28 18:25:01.000000000 +0900 > @@ -37,8 +37,9 @@ void cmdline_parse(char *cmdline) > /* Skip whitespace and the image name. */ > while ( *p == '' '' ) > p++; > - if ( (p = strchr(p, '' '')) == NULL ) > - return; > + if (! strncmp(p, "BOOT_IMAGE=", strlen("BOOT_IMAGE=")) ) > + if ( (p = strchr(p, '' '')) == NULL ) > + return; > > for ( ; ; ) > { > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Horms
2007-Aug-29 00:43 UTC
Re: [Xen-devel] [patch] Only skip the image name if it is the image name
On Tue, Aug 28, 2007 at 03:05:05AM -0700, Keir Fraser wrote:> Isn''t that a kexec bug? Multiboot command lines include the image name in > all other cases. If there''s no suitable name, perhaps a dummy name should be > included?I was wondering where it should be fixed. Do you know if supplying BOOT_IMAGE is part of the multiboot specification? -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Aug-29 14:20 UTC
Re: [Xen-devel] [patch] Only skip the image name if it is the image name
On 29/8/07 01:43, "Horms" <horms@verge.net.au> wrote:> On Tue, Aug 28, 2007 at 03:05:05AM -0700, Keir Fraser wrote: >> Isn''t that a kexec bug? Multiboot command lines include the image name in >> all other cases. If there''s no suitable name, perhaps a dummy name should be >> included? > > I was wondering where it should be fixed. Do you know if > supplying BOOT_IMAGE is part of the multiboot specification?Ah, sorry, I forgot that you repro''ed this on IA64 which does not use multiboot at all. The problem here is, I believe, that the cooking of the command line to strip the image name is actually an arch-specific operation. I notice that e.g., Linux on IA64 does nto bother to do this at all. The fact that the name is prefixed with BOOT_IMAGE= absolutely avoids any possible name clash with cmdline parameters. Look at /proc/cmdline on a native IA64 Linux box for example -- the image name is not stripped off. What I will do is make arch/x86/setup.c strip the image name before calling the generic parser. That should make IA64 much happier. Hopefully it will be okay for PPC too. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Horms
2007-Aug-30 01:37 UTC
Re: [Xen-devel] [patch] Only skip the image name if it is the image name
On Wed, Aug 29, 2007 at 03:20:06PM +0100, Keir Fraser wrote:> On 29/8/07 01:43, "Horms" <horms@verge.net.au> wrote: > > > On Tue, Aug 28, 2007 at 03:05:05AM -0700, Keir Fraser wrote: > >> Isn''t that a kexec bug? Multiboot command lines include the image name in > >> all other cases. If there''s no suitable name, perhaps a dummy name should be > >> included? > > > > I was wondering where it should be fixed. Do you know if > > supplying BOOT_IMAGE is part of the multiboot specification? > > Ah, sorry, I forgot that you repro''ed this on IA64 which does not use > multiboot at all. > > The problem here is, I believe, that the cooking of the command line to > strip the image name is actually an arch-specific operation. I notice that > e.g., Linux on IA64 does nto bother to do this at all. The fact that the > name is prefixed with BOOT_IMAGE= absolutely avoids any possible name clash > with cmdline parameters. Look at /proc/cmdline on a native IA64 Linux box > for example -- the image name is not stripped off. > > What I will do is make arch/x86/setup.c strip the image name before calling > the generic parser. That should make IA64 much happier. Hopefully it will be > okay for PPC too.Great, I think that solution will work quite well. -- Horms H: http://www.vergenet.net/~horms/ W: http://www.valinux.co.jp/en/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel