Re: [PATCH] Don't attach needless options when launch pygrub

[Yu Zhiguo <yuzg@cn.fujitsu.com>]

Hi Ian,

Ian Campbell wrote:
> On Wed, 2010-07-14 at 07:15 +0100, Yu Zhiguo wrote:
>> We should always run grub if bootloader is specified,
>> options 'kernel' and 'ramdisk' are needless.
> 
> Not quite. If you specify both bootloader and kernel then this instructs
> pygrub to extract the specific named file from the guest file system,
> similarly for the ramdisk.
> 

 Do you mean in this case, pygrub will use specified kernel
that lie in the filesystem of the DomU?

 I think this is good. But now pygrub's action is using the
specified kernel in Dom0, but not run grub.

> With your patch the functionality is still available using the
> bootloader_args field to add the "--kernel=FOO" yourself but I'm not
> sure it is a big enough issue to be worth changing. Is there some
> specific reason you don't like the current behaviour?

 Yes, all the bootloader_args will be attach to pygrub option......
 I think if bootloader is specified, omit kernel is simple.

> 
> In any case you seem to have forgotten to update the caller in
> tools/python/xen/xend/XendDomainInfo.py.
> 
 Oh, sorry for this mistake.

> I also have patches pending on the list from Monday which add bootloader
> functionality, including this behaviour, to libxl so if we decide to
> change it here it will need to change there as well.
> 

 OK, I'll try after patchs are applied.

Yu

> Ian.
> 
> 
>> Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>
>>
>> diff -r d867eb643fe4 -r 38d9dac1de56 tools/python/xen/xend/XendBootloader.py
>> --- a/tools/python/xen/xend/XendBootloader.py	Tue Jul 13 18:17:28 2010 +0100
>> +++ b/tools/python/xen/xend/XendBootloader.py	Wed Jul 14 22:18:58 2010 +0800
>> @@ -24,8 +24,7 @@
>>  import pty, termios, fcntl
>>  from xen.lowlevel import ptsname
>>  
>> -def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel = '',
>> -               ramdisk = '', kernel_args = ''):
>> +def bootloader(blexec, disk, dom, quiet = False, blargs = '', kernel_args = ''):
>>      """Run the boot loader executable on the given disk and return a
>>      config image.
>>      @param blexec  Binary to use as the boot loader
>> @@ -96,10 +95,6 @@
>>      (child, m2) = pty.fork()
>>      if (not child):
>>          args = [ blexec ]
>> -        if kernel:
>> -            args.append("--kernel=%s" % kernel)
>> -        if ramdisk:
>> -            args.append("--ramdisk=%s" % ramdisk)
>>          if kernel_args:
>>              args.append("--args=%s" % kernel_args)
>>          if quiet:
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 
> 
> 
> 
>