[PATCH] pygrub: always append --args

[PATCH] pygrub: always append --args

[Olaf Hering]

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1346945306 -7200
# Node ID 8a2eef481d3ab3ca5692dd0083c95cf314fe1da3
# Parent  19d367bf07b7687b831c212a57a70e73ea14d3b7
pygrub: always append --args

If a bootloader entry in menu.lst has no additional kernel command line
options listed and the domU.cfg has 'bootargs="--args=something"' the
additional arguments from the config file are not passed to the kernel.
The reason for that incorrect behaviour is that run_grub appends arg
only if the parsed config file has arguments listed.

Fix this by appending args from image section and the config file separatly.
To avoid adding to a NoneType initialize grubcfg['args'] to an empty string.
This does not change behaviour but simplifies the code which appends the
string.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 19d367bf07b7 -r 8a2eef481d3a tools/pygrub/src/pygrub
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -615,13 +615,15 @@ def run_grub(file, entry, fs, arg):
     except IndexError:
         img = g.cf.images[0]
 
-    grubcfg = { "kernel": None, "ramdisk": None, "args": None }
+    grubcfg = { "kernel": None, "ramdisk": None, "args": "" }
 
     grubcfg["kernel"] = img.kernel[1]
     if img.initrd:
         grubcfg["ramdisk"] = img.initrd[1]
     if img.args:
-        grubcfg["args"] = img.args + " " + arg
+        grubcfg["args"] += img.args
+    if arg:
+        grubcfg["args"] += " " + args
 
     return grubcfg

Re: [PATCH] pygrub: always append --args

[Ian Campbell]

On Thu, 2012-09-06 at 16:29 +0100, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1346945306 -7200
> # Node ID 8a2eef481d3ab3ca5692dd0083c95cf314fe1da3
> # Parent  19d367bf07b7687b831c212a57a70e73ea14d3b7
> pygrub: always append --args
> 
> If a bootloader entry in menu.lst has no additional kernel command line
> options listed and the domU.cfg has 'bootargs="--args=something"' the
> additional arguments from the config file are not passed to the kernel.
> The reason for that incorrect behaviour is that run_grub appends arg
> only if the parsed config file has arguments listed.
> 
> Fix this by appending args from image section and the config file separatly.
> To avoid adding to a NoneType initialize grubcfg['args'] to an empty string.
> This does not change behaviour but simplifies the code which appends the
> string.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

This has to be 4.3 at this point.

> 
> diff -r 19d367bf07b7 -r 8a2eef481d3a tools/pygrub/src/pygrub
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -615,13 +615,15 @@ def run_grub(file, entry, fs, arg):
>      except IndexError:
>          img = g.cf.images[0]
>  
> -    grubcfg = { "kernel": None, "ramdisk": None, "args": None }
> +    grubcfg = { "kernel": None, "ramdisk": None, "args": "" }
>  
>      grubcfg["kernel"] = img.kernel[1]
>      if img.initrd:
>          grubcfg["ramdisk"] = img.initrd[1]
>      if img.args:

With the above change isn't this always true?

It probably should have read
	if img.args is not None
in the first place.

> -        grubcfg["args"] = img.args + " " + arg
> +        grubcfg["args"] += img.args
> +    if arg:
> +        grubcfg["args"] += " " + args

>  
>      return grubcfg
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] pygrub: always append --args

[Olaf Hering]

On Fri, Sep 07, Ian Campbell wrote:

> >      except IndexError:
> >          img = g.cf.images[0]
> >  
> > -    grubcfg = { "kernel": None, "ramdisk": None, "args": None }
> > +    grubcfg = { "kernel": None, "ramdisk": None, "args": "" }
> >  
> >      grubcfg["kernel"] = img.kernel[1]
> >      if img.initrd:
> >          grubcfg["ramdisk"] = img.initrd[1]
> >      if img.args:
> 
> With the above change isn't this always true?

img.args comes from img = g.cf.images[sel]?

Olaf

Re: [PATCH] pygrub: always append --args

[Ian Campbell]

On Fri, 2012-09-07 at 10:14 +0100, Olaf Hering wrote:
> On Fri, Sep 07, Ian Campbell wrote:
> 
> > >      except IndexError:
> > >          img = g.cf.images[0]
> > >  
> > > -    grubcfg = { "kernel": None, "ramdisk": None, "args": None }
> > > +    grubcfg = { "kernel": None, "ramdisk": None, "args": "" }
> > >  
> > >      grubcfg["kernel"] = img.kernel[1]
> > >      if img.initrd:
> > >          grubcfg["ramdisk"] = img.initrd[1]
> > >      if img.args:
> > 
> > With the above change isn't this always true?
> 
> img.args comes from img = g.cf.images[sel]?

Oh sorry, for some reason I did s/grubcfg/img/ in my head in that
assignment.

Ian.

Re: [PATCH] pygrub: always append --args

[Ian Campbell]

On Thu, 2012-09-06 at 16:29 +0100, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1346945306 -7200
> # Node ID 8a2eef481d3ab3ca5692dd0083c95cf314fe1da3
> # Parent  19d367bf07b7687b831c212a57a70e73ea14d3b7
> pygrub: always append --args
> 
> If a bootloader entry in menu.lst has no additional kernel command line
> options listed and the domU.cfg has 'bootargs="--args=something"' the
> additional arguments from the config file are not passed to the kernel.
> The reason for that incorrect behaviour is that run_grub appends arg
> only if the parsed config file has arguments listed.
> 
> Fix this by appending args from image section and the config file separatly.
> To avoid adding to a NoneType initialize grubcfg['args'] to an empty string.
> This does not change behaviour but simplifies the code which appends the
> string.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r 19d367bf07b7 -r 8a2eef481d3a tools/pygrub/src/pygrub
> --- a/tools/pygrub/src/pygrub
> +++ b/tools/pygrub/src/pygrub
> @@ -615,13 +615,15 @@ def run_grub(file, entry, fs, arg):
>      except IndexError:
>          img = g.cf.images[0]
>  
> -    grubcfg = { "kernel": None, "ramdisk": None, "args": None }
> +    grubcfg = { "kernel": None, "ramdisk": None, "args": "" }
>  
>      grubcfg["kernel"] = img.kernel[1]
>      if img.initrd:
>          grubcfg["ramdisk"] = img.initrd[1]
>      if img.args:
> -        grubcfg["args"] = img.args + " " + arg
> +        grubcfg["args"] += img.args
> +    if arg:
> +        grubcfg["args"] += " " + args

I don't imagine any kernel would care about the potential leading space
here so:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

and applied, thanks.

>  
>      return grubcfg
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel