All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] wic: Allow to user defined files as config for bootloaders
@ 2015-11-18  8:25 mariano.lopez
  2015-11-18  8:25 ` [PATCH 1/2] wic: Prepare wicboot to allow custom bootloader config mariano.lopez
  2015-11-18  8:25 ` [PATCH 2/2] wic: Allow to use a custom config for bootloaders mariano.lopez
  0 siblings, 2 replies; 9+ messages in thread
From: mariano.lopez @ 2015-11-18  8:25 UTC (permalink / raw)
  To: openembedded-core

From: Mariano Lopez <mariano.lopez@linux.intel.com>

These two patches add a new option for the bootloaders. This new option is
to have a configuration file as the bootloader config and it can be used with
"configfile" variable in the bootloader line. This is very useful when there
is need to create a multiboot image or when there is need to use scripting in
the bootloader.

This change doesn't include the changes in the documentation but once the
patches are commited to master, I will work on the documentation of this new
feature.

The following changes since commit d9aabf9639510fdb3e2ccc21ba5ae4aa9f6e4a57:

  gcc: Drop 4.8 (2015-11-16 14:59:18 +0000)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib mariano/bug8003
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=mariano/bug8003

Mariano Lopez (2):
  wic: Prepare wicboot to allow custom bootloader config
  wic: Allow to use a custom config for bootloaders

 scripts/lib/wic/kickstart/__init__.py              |  7 +++
 .../lib/wic/kickstart/custom_commands/wicboot.py   |  5 ++
 scripts/lib/wic/plugins/source/bootimg-efi.py      | 66 ++++++++++++++--------
 scripts/lib/wic/plugins/source/bootimg-pcbios.py   | 66 +++++++++++++---------
 4 files changed, 95 insertions(+), 49 deletions(-)

-- 
1.8.4.5



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] wic: Prepare wicboot to allow custom bootloader config
  2015-11-18  8:25 [PATCH 0/2] wic: Allow to user defined files as config for bootloaders mariano.lopez
@ 2015-11-18  8:25 ` mariano.lopez
  2015-11-18  8:25 ` [PATCH 2/2] wic: Allow to use a custom config for bootloaders mariano.lopez
  1 sibling, 0 replies; 9+ messages in thread
From: mariano.lopez @ 2015-11-18  8:25 UTC (permalink / raw)
  To: openembedded-core

From: Mariano Lopez <mariano.lopez@linux.intel.com>

Currently wic does the bootloader configuration file on the fly.
This change introduce a configfile variable for the bootloader;
this is to have a user defined configuration file for the
bootloaders (grub, syslinux, and gummiboot). This is particular
useful when having a multiboot system or scripts embedded in the
configuration file.

[YOCTO #8003]

Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
---
 scripts/lib/wic/kickstart/__init__.py                | 7 +++++++
 scripts/lib/wic/kickstart/custom_commands/wicboot.py | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/scripts/lib/wic/kickstart/__init__.py b/scripts/lib/wic/kickstart/__init__.py
index c9b0e51..79b39fb 100644
--- a/scripts/lib/wic/kickstart/__init__.py
+++ b/scripts/lib/wic/kickstart/__init__.py
@@ -97,6 +97,13 @@ def get_timeout(kickstart, default=None):
         return default
     return int(kickstart.handler.bootloader.timeout)
 
+def get_bootloader_file(kickstart, default=None):
+    if not hasattr(kickstart.handler.bootloader, "configfile"):
+        return default
+    if kickstart.handler.bootloader.configfile is None:
+        return default
+    return kickstart.handler.bootloader.configfile
+
 def get_kernel_args(kickstart, default="ro rd.live.image"):
     if not hasattr(kickstart.handler.bootloader, "appendLine"):
         return default
diff --git a/scripts/lib/wic/kickstart/custom_commands/wicboot.py b/scripts/lib/wic/kickstart/custom_commands/wicboot.py
index a3e1852..eb17dab 100644
--- a/scripts/lib/wic/kickstart/custom_commands/wicboot.py
+++ b/scripts/lib/wic/kickstart/custom_commands/wicboot.py
@@ -35,6 +35,7 @@ class Wic_Bootloader(F8_Bootloader):
         self.menus = ""
         self.ptable = "msdos"
         self.source = ""
+        self.configfile = ""
 
     def _getArgsAsStr(self):
         retval = F8_Bootloader._getArgsAsStr(self)
@@ -45,6 +46,8 @@ class Wic_Bootloader(F8_Bootloader):
             retval += " --ptable=\"%s\"" %(self.ptable,)
         if self.source:
             retval += " --source=%s" % self.source
+        if self.configfile:
+            retval += " --configfile=%s" % self.configfile
 
         return retval
 
@@ -56,5 +59,7 @@ class Wic_Bootloader(F8_Bootloader):
         # use specified source plugin to implement bootloader-specific methods
         parser.add_option("--source", type="string", action="store",
                       dest="source", default=None)
+        parser.add_option("--configfile", type="string", action="store",
+                      dest="configfile", default=None)
         return parser
 
-- 
1.8.4.5



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] wic: Allow to use a custom config for bootloaders
  2015-11-18  8:25 [PATCH 0/2] wic: Allow to user defined files as config for bootloaders mariano.lopez
  2015-11-18  8:25 ` [PATCH 1/2] wic: Prepare wicboot to allow custom bootloader config mariano.lopez
@ 2015-11-18  8:25 ` mariano.lopez
  2015-11-23 17:37   ` Ed Bartosh
  1 sibling, 1 reply; 9+ messages in thread
From: mariano.lopez @ 2015-11-18  8:25 UTC (permalink / raw)
  To: openembedded-core

From: Mariano Lopez <mariano.lopez@linux.intel.com>

This change will allow to use a user defined file as the
configuration for the bootloaders (grub, gummiboot, syslinux).

The config file is defined in the wks file with the "configfile"
option in the bootloader line.

[YOCTO #8003]

Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
---
 scripts/lib/wic/plugins/source/bootimg-efi.py    | 66 ++++++++++++++++--------
 scripts/lib/wic/plugins/source/bootimg-pcbios.py | 66 ++++++++++++++----------
 2 files changed, 83 insertions(+), 49 deletions(-)

diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
index fa63c6a..8fc879e 100644
--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
@@ -45,22 +45,33 @@ class BootimgEFIPlugin(SourcePlugin):
         """
         Create loader-specific (grub-efi) config
         """
-        options = creator.ks.handler.bootloader.appendLine
-
-        grubefi_conf = ""
-        grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
-        grubefi_conf += "default=boot\n"
-        timeout = kickstart.get_timeout(creator.ks)
-        if not timeout:
-            timeout = 0
-        grubefi_conf += "timeout=%s\n" % timeout
-        grubefi_conf += "menuentry 'boot'{\n"
-
-        kernel = "/bzImage"
-
-        grubefi_conf += "linux %s root=%s rootwait %s\n" \
-            % (kernel, creator.rootdev, options)
-        grubefi_conf += "}\n"
+        configfile = kickstart.get_bootloader_file(creator.ks)
+
+        if configfile and os.path.exists(configfile):
+            # Use a custom configuration file for grub
+            msger.info("Using custom configuration file "
+                    "%s for grub.cfg" % configfile)
+            user_conf = open(configfile, "r")
+            grubefi_conf = user_conf.read()
+            user_conf.close()
+        else:
+            # Create grub configuration using parameters from wks file
+            options = creator.ks.handler.bootloader.appendLine
+
+            grubefi_conf = ""
+            grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
+            grubefi_conf += "default=boot\n"
+            timeout = kickstart.get_timeout(creator.ks)
+            if not timeout:
+                timeout = 0
+            grubefi_conf += "timeout=%s\n" % timeout
+            grubefi_conf += "menuentry 'boot'{\n"
+
+            kernel = "/bzImage"
+
+            grubefi_conf += "linux %s root=%s rootwait %s\n" \
+                % (kernel, creator.rootdev, options)
+            grubefi_conf += "}\n"
 
         msger.debug("Writing grubefi config %s/hdd/boot/EFI/BOOT/grub.cfg" \
                         % cr_workdir)
@@ -95,12 +106,23 @@ class BootimgEFIPlugin(SourcePlugin):
         cfg.write(loader_conf)
         cfg.close()
 
-        kernel = "/bzImage"
-
-        boot_conf = ""
-        boot_conf += "title boot\n"
-        boot_conf += "linux %s\n" % kernel
-        boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
+        configfile = kickstart.get_bootloader_file(creator.ks)
+
+        if configfile and os.path.exists(configfile):
+            # Use a custom configuration file for gummiboot
+            msger.info("Using custom configuration file "
+                    "%s for gummiboot's boot.conf" % configfile)
+            user_conf = open(configfile, "r")
+            boot_conf = user_conf.read()
+            user_conf.close()
+        else:
+            # Create gummiboot configuration using parameters from wks file
+            kernel = "/bzImage"
+
+            boot_conf = ""
+            boot_conf += "title boot\n"
+            boot_conf += "linux %s\n" % kernel
+            boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
 
         msger.debug("Writing gummiboot config %s/hdd/boot/loader/entries/boot.conf" \
                         % cr_workdir)
diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
index 96ed54d..9e21572 100644
--- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
+++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
@@ -83,34 +83,46 @@ class BootimgPcbiosPlugin(SourcePlugin):
         install_cmd = "install -d %s" % hdddir
         exec_cmd(install_cmd)
 
-        splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
-        if os.path.exists(splash):
-            splashline = "menu background splash.jpg"
+        configfile = kickstart.get_bootloader_file(creator.ks)
+
+        if configfile and os.path.exists(configfile):
+            # Use a custom configuration file for syslinux
+            msger.info("Using custom configuration file "
+                    "%s for syslinux.cfg" % configfile)
+            user_conf = open(configfile, "r")
+            syslinux_conf = user_conf.read()
+            user_conf.close()
+
         else:
-            splashline = ""
-
-        options = creator.ks.handler.bootloader.appendLine
-
-        syslinux_conf = ""
-        syslinux_conf += "PROMPT 0\n"
-        timeout = kickstart.get_timeout(creator.ks)
-        if not timeout:
-            timeout = 0
-        syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
-        syslinux_conf += "\n"
-        syslinux_conf += "ALLOWOPTIONS 1\n"
-        syslinux_conf += "SERIAL 0 115200\n"
-        syslinux_conf += "\n"
-        if splashline:
-            syslinux_conf += "%s\n" % splashline
-        syslinux_conf += "DEFAULT boot\n"
-        syslinux_conf += "LABEL boot\n"
-
-        kernel = "/vmlinuz"
-        syslinux_conf += "KERNEL " + kernel + "\n"
-
-        syslinux_conf += "APPEND label=boot root=%s %s\n" % \
-                             (creator.rootdev, options)
+            # Create syslinux configuration using parameters from wks file
+            splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
+            if os.path.exists(splash):
+                splashline = "menu background splash.jpg"
+            else:
+                splashline = ""
+
+            options = creator.ks.handler.bootloader.appendLine
+
+            syslinux_conf = ""
+            syslinux_conf += "PROMPT 0\n"
+            timeout = kickstart.get_timeout(creator.ks)
+            if not timeout:
+                timeout = 0
+            syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
+            syslinux_conf += "\n"
+            syslinux_conf += "ALLOWOPTIONS 1\n"
+            syslinux_conf += "SERIAL 0 115200\n"
+            syslinux_conf += "\n"
+            if splashline:
+                syslinux_conf += "%s\n" % splashline
+            syslinux_conf += "DEFAULT boot\n"
+            syslinux_conf += "LABEL boot\n"
+
+            kernel = "/vmlinuz"
+            syslinux_conf += "KERNEL " + kernel + "\n"
+
+            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
+                                 (creator.rootdev, options)
 
         msger.debug("Writing syslinux config %s/hdd/boot/syslinux.cfg" \
                     % cr_workdir)
-- 
1.8.4.5



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] wic: Allow to use a custom config for bootloaders
  2015-11-18  8:25 ` [PATCH 2/2] wic: Allow to use a custom config for bootloaders mariano.lopez
@ 2015-11-23 17:37   ` Ed Bartosh
  2015-11-23 22:13     ` Mariano Lopez
  0 siblings, 1 reply; 9+ messages in thread
From: Ed Bartosh @ 2015-11-23 17:37 UTC (permalink / raw)
  To: mariano.lopez; +Cc: openembedded-core

Hi Mariano,

Thank you for the patchset!

Would it be better to put content of configuration file into .wks
instead of just referring to it?

It would be also nice to have this code covered by oe-selftest.

Regards,
Ed

On Wed, Nov 18, 2015 at 08:25:54AM +0000, mariano.lopez@linux.intel.com wrote:
> From: Mariano Lopez <mariano.lopez@linux.intel.com>
> 
> This change will allow to use a user defined file as the
> configuration for the bootloaders (grub, gummiboot, syslinux).
> 
> The config file is defined in the wks file with the "configfile"
> option in the bootloader line.
> 
> [YOCTO #8003]
> 
> Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
> ---
>  scripts/lib/wic/plugins/source/bootimg-efi.py    | 66 ++++++++++++++++--------
>  scripts/lib/wic/plugins/source/bootimg-pcbios.py | 66 ++++++++++++++----------
>  2 files changed, 83 insertions(+), 49 deletions(-)
> 
> diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
> index fa63c6a..8fc879e 100644
> --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
> +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
> @@ -45,22 +45,33 @@ class BootimgEFIPlugin(SourcePlugin):
>          """
>          Create loader-specific (grub-efi) config
>          """
> -        options = creator.ks.handler.bootloader.appendLine
> -
> -        grubefi_conf = ""
> -        grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
> -        grubefi_conf += "default=boot\n"
> -        timeout = kickstart.get_timeout(creator.ks)
> -        if not timeout:
> -            timeout = 0
> -        grubefi_conf += "timeout=%s\n" % timeout
> -        grubefi_conf += "menuentry 'boot'{\n"
> -
> -        kernel = "/bzImage"
> -
> -        grubefi_conf += "linux %s root=%s rootwait %s\n" \
> -            % (kernel, creator.rootdev, options)
> -        grubefi_conf += "}\n"
> +        configfile = kickstart.get_bootloader_file(creator.ks)
> +
> +        if configfile and os.path.exists(configfile):
> +            # Use a custom configuration file for grub
> +            msger.info("Using custom configuration file "
> +                    "%s for grub.cfg" % configfile)
> +            user_conf = open(configfile, "r")
> +            grubefi_conf = user_conf.read()
> +            user_conf.close()
> +        else:
> +            # Create grub configuration using parameters from wks file
> +            options = creator.ks.handler.bootloader.appendLine
> +
> +            grubefi_conf = ""
> +            grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
> +            grubefi_conf += "default=boot\n"
> +            timeout = kickstart.get_timeout(creator.ks)
> +            if not timeout:
> +                timeout = 0
> +            grubefi_conf += "timeout=%s\n" % timeout
> +            grubefi_conf += "menuentry 'boot'{\n"
> +
> +            kernel = "/bzImage"
> +
> +            grubefi_conf += "linux %s root=%s rootwait %s\n" \
> +                % (kernel, creator.rootdev, options)
> +            grubefi_conf += "}\n"
>  
>          msger.debug("Writing grubefi config %s/hdd/boot/EFI/BOOT/grub.cfg" \
>                          % cr_workdir)
> @@ -95,12 +106,23 @@ class BootimgEFIPlugin(SourcePlugin):
>          cfg.write(loader_conf)
>          cfg.close()
>  
> -        kernel = "/bzImage"
> -
> -        boot_conf = ""
> -        boot_conf += "title boot\n"
> -        boot_conf += "linux %s\n" % kernel
> -        boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
> +        configfile = kickstart.get_bootloader_file(creator.ks)
> +
> +        if configfile and os.path.exists(configfile):
> +            # Use a custom configuration file for gummiboot
> +            msger.info("Using custom configuration file "
> +                    "%s for gummiboot's boot.conf" % configfile)
> +            user_conf = open(configfile, "r")
> +            boot_conf = user_conf.read()
> +            user_conf.close()
> +        else:
> +            # Create gummiboot configuration using parameters from wks file
> +            kernel = "/bzImage"
> +
> +            boot_conf = ""
> +            boot_conf += "title boot\n"
> +            boot_conf += "linux %s\n" % kernel
> +            boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
>  
>          msger.debug("Writing gummiboot config %s/hdd/boot/loader/entries/boot.conf" \
>                          % cr_workdir)
> diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
> index 96ed54d..9e21572 100644
> --- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
> +++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
> @@ -83,34 +83,46 @@ class BootimgPcbiosPlugin(SourcePlugin):
>          install_cmd = "install -d %s" % hdddir
>          exec_cmd(install_cmd)
>  
> -        splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
> -        if os.path.exists(splash):
> -            splashline = "menu background splash.jpg"
> +        configfile = kickstart.get_bootloader_file(creator.ks)
> +
> +        if configfile and os.path.exists(configfile):
> +            # Use a custom configuration file for syslinux
> +            msger.info("Using custom configuration file "
> +                    "%s for syslinux.cfg" % configfile)
> +            user_conf = open(configfile, "r")
> +            syslinux_conf = user_conf.read()
> +            user_conf.close()
> +
>          else:
> -            splashline = ""
> -
> -        options = creator.ks.handler.bootloader.appendLine
> -
> -        syslinux_conf = ""
> -        syslinux_conf += "PROMPT 0\n"
> -        timeout = kickstart.get_timeout(creator.ks)
> -        if not timeout:
> -            timeout = 0
> -        syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
> -        syslinux_conf += "\n"
> -        syslinux_conf += "ALLOWOPTIONS 1\n"
> -        syslinux_conf += "SERIAL 0 115200\n"
> -        syslinux_conf += "\n"
> -        if splashline:
> -            syslinux_conf += "%s\n" % splashline
> -        syslinux_conf += "DEFAULT boot\n"
> -        syslinux_conf += "LABEL boot\n"
> -
> -        kernel = "/vmlinuz"
> -        syslinux_conf += "KERNEL " + kernel + "\n"
> -
> -        syslinux_conf += "APPEND label=boot root=%s %s\n" % \
> -                             (creator.rootdev, options)
> +            # Create syslinux configuration using parameters from wks file
> +            splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
> +            if os.path.exists(splash):
> +                splashline = "menu background splash.jpg"
> +            else:
> +                splashline = ""
> +
> +            options = creator.ks.handler.bootloader.appendLine
> +
> +            syslinux_conf = ""
> +            syslinux_conf += "PROMPT 0\n"
> +            timeout = kickstart.get_timeout(creator.ks)
> +            if not timeout:
> +                timeout = 0
> +            syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
> +            syslinux_conf += "\n"
> +            syslinux_conf += "ALLOWOPTIONS 1\n"
> +            syslinux_conf += "SERIAL 0 115200\n"
> +            syslinux_conf += "\n"
> +            if splashline:
> +                syslinux_conf += "%s\n" % splashline
> +            syslinux_conf += "DEFAULT boot\n"
> +            syslinux_conf += "LABEL boot\n"
> +
> +            kernel = "/vmlinuz"
> +            syslinux_conf += "KERNEL " + kernel + "\n"
> +
> +            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
> +                                 (creator.rootdev, options)
>  
>          msger.debug("Writing syslinux config %s/hdd/boot/syslinux.cfg" \
>                      % cr_workdir)
> -- 
> 1.8.4.5
> 

-- 
--
Regards,
Ed


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] wic: Allow to use a custom config for bootloaders
  2015-11-23 17:37   ` Ed Bartosh
@ 2015-11-23 22:13     ` Mariano Lopez
  2015-11-24 15:43       ` Ed Bartosh
  0 siblings, 1 reply; 9+ messages in thread
From: Mariano Lopez @ 2015-11-23 22:13 UTC (permalink / raw)
  To: ed.bartosh; +Cc: openembedded-core



On 11/23/2015 11:37 AM, Ed Bartosh wrote:
> Hi Mariano,
>
> Thank you for the patchset!
>
> Would it be better to put content of configuration file into .wks
> instead of just referring to it?

If the configuration is simple I agree with you; however if the 
configuration have scripts I think it's better to have separated file. 
The file can growth and would be a real mess inside a wks file.

>
> It would be also nice to have this code covered by oe-selftest.

Yes, I plan do  create a test and update the documentation once it is 
integrated in master.

>
> Regards,
> Ed
>
> On Wed, Nov 18, 2015 at 08:25:54AM +0000, mariano.lopez@linux.intel.com wrote:
>> From: Mariano Lopez <mariano.lopez@linux.intel.com>
>>
>> This change will allow to use a user defined file as the
>> configuration for the bootloaders (grub, gummiboot, syslinux).
>>
>> The config file is defined in the wks file with the "configfile"
>> option in the bootloader line.
>>
>> [YOCTO #8003]
>>
>> Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
>> ---
>>   scripts/lib/wic/plugins/source/bootimg-efi.py    | 66 ++++++++++++++++--------
>>   scripts/lib/wic/plugins/source/bootimg-pcbios.py | 66 ++++++++++++++----------
>>   2 files changed, 83 insertions(+), 49 deletions(-)
>>
>> diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
>> index fa63c6a..8fc879e 100644
>> --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
>> +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
>> @@ -45,22 +45,33 @@ class BootimgEFIPlugin(SourcePlugin):
>>           """
>>           Create loader-specific (grub-efi) config
>>           """
>> -        options = creator.ks.handler.bootloader.appendLine
>> -
>> -        grubefi_conf = ""
>> -        grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
>> -        grubefi_conf += "default=boot\n"
>> -        timeout = kickstart.get_timeout(creator.ks)
>> -        if not timeout:
>> -            timeout = 0
>> -        grubefi_conf += "timeout=%s\n" % timeout
>> -        grubefi_conf += "menuentry 'boot'{\n"
>> -
>> -        kernel = "/bzImage"
>> -
>> -        grubefi_conf += "linux %s root=%s rootwait %s\n" \
>> -            % (kernel, creator.rootdev, options)
>> -        grubefi_conf += "}\n"
>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>> +
>> +        if configfile and os.path.exists(configfile):
>> +            # Use a custom configuration file for grub
>> +            msger.info("Using custom configuration file "
>> +                    "%s for grub.cfg" % configfile)
>> +            user_conf = open(configfile, "r")
>> +            grubefi_conf = user_conf.read()
>> +            user_conf.close()
>> +        else:
>> +            # Create grub configuration using parameters from wks file
>> +            options = creator.ks.handler.bootloader.appendLine
>> +
>> +            grubefi_conf = ""
>> +            grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
>> +            grubefi_conf += "default=boot\n"
>> +            timeout = kickstart.get_timeout(creator.ks)
>> +            if not timeout:
>> +                timeout = 0
>> +            grubefi_conf += "timeout=%s\n" % timeout
>> +            grubefi_conf += "menuentry 'boot'{\n"
>> +
>> +            kernel = "/bzImage"
>> +
>> +            grubefi_conf += "linux %s root=%s rootwait %s\n" \
>> +                % (kernel, creator.rootdev, options)
>> +            grubefi_conf += "}\n"
>>   
>>           msger.debug("Writing grubefi config %s/hdd/boot/EFI/BOOT/grub.cfg" \
>>                           % cr_workdir)
>> @@ -95,12 +106,23 @@ class BootimgEFIPlugin(SourcePlugin):
>>           cfg.write(loader_conf)
>>           cfg.close()
>>   
>> -        kernel = "/bzImage"
>> -
>> -        boot_conf = ""
>> -        boot_conf += "title boot\n"
>> -        boot_conf += "linux %s\n" % kernel
>> -        boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>> +
>> +        if configfile and os.path.exists(configfile):
>> +            # Use a custom configuration file for gummiboot
>> +            msger.info("Using custom configuration file "
>> +                    "%s for gummiboot's boot.conf" % configfile)
>> +            user_conf = open(configfile, "r")
>> +            boot_conf = user_conf.read()
>> +            user_conf.close()
>> +        else:
>> +            # Create gummiboot configuration using parameters from wks file
>> +            kernel = "/bzImage"
>> +
>> +            boot_conf = ""
>> +            boot_conf += "title boot\n"
>> +            boot_conf += "linux %s\n" % kernel
>> +            boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
>>   
>>           msger.debug("Writing gummiboot config %s/hdd/boot/loader/entries/boot.conf" \
>>                           % cr_workdir)
>> diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>> index 96ed54d..9e21572 100644
>> --- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>> +++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>> @@ -83,34 +83,46 @@ class BootimgPcbiosPlugin(SourcePlugin):
>>           install_cmd = "install -d %s" % hdddir
>>           exec_cmd(install_cmd)
>>   
>> -        splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
>> -        if os.path.exists(splash):
>> -            splashline = "menu background splash.jpg"
>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>> +
>> +        if configfile and os.path.exists(configfile):
>> +            # Use a custom configuration file for syslinux
>> +            msger.info("Using custom configuration file "
>> +                    "%s for syslinux.cfg" % configfile)
>> +            user_conf = open(configfile, "r")
>> +            syslinux_conf = user_conf.read()
>> +            user_conf.close()
>> +
>>           else:
>> -            splashline = ""
>> -
>> -        options = creator.ks.handler.bootloader.appendLine
>> -
>> -        syslinux_conf = ""
>> -        syslinux_conf += "PROMPT 0\n"
>> -        timeout = kickstart.get_timeout(creator.ks)
>> -        if not timeout:
>> -            timeout = 0
>> -        syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
>> -        syslinux_conf += "\n"
>> -        syslinux_conf += "ALLOWOPTIONS 1\n"
>> -        syslinux_conf += "SERIAL 0 115200\n"
>> -        syslinux_conf += "\n"
>> -        if splashline:
>> -            syslinux_conf += "%s\n" % splashline
>> -        syslinux_conf += "DEFAULT boot\n"
>> -        syslinux_conf += "LABEL boot\n"
>> -
>> -        kernel = "/vmlinuz"
>> -        syslinux_conf += "KERNEL " + kernel + "\n"
>> -
>> -        syslinux_conf += "APPEND label=boot root=%s %s\n" % \
>> -                             (creator.rootdev, options)
>> +            # Create syslinux configuration using parameters from wks file
>> +            splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
>> +            if os.path.exists(splash):
>> +                splashline = "menu background splash.jpg"
>> +            else:
>> +                splashline = ""
>> +
>> +            options = creator.ks.handler.bootloader.appendLine
>> +
>> +            syslinux_conf = ""
>> +            syslinux_conf += "PROMPT 0\n"
>> +            timeout = kickstart.get_timeout(creator.ks)
>> +            if not timeout:
>> +                timeout = 0
>> +            syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
>> +            syslinux_conf += "\n"
>> +            syslinux_conf += "ALLOWOPTIONS 1\n"
>> +            syslinux_conf += "SERIAL 0 115200\n"
>> +            syslinux_conf += "\n"
>> +            if splashline:
>> +                syslinux_conf += "%s\n" % splashline
>> +            syslinux_conf += "DEFAULT boot\n"
>> +            syslinux_conf += "LABEL boot\n"
>> +
>> +            kernel = "/vmlinuz"
>> +            syslinux_conf += "KERNEL " + kernel + "\n"
>> +
>> +            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
>> +                                 (creator.rootdev, options)
>>   
>>           msger.debug("Writing syslinux config %s/hdd/boot/syslinux.cfg" \
>>                       % cr_workdir)
>> -- 
>> 1.8.4.5
>>

-- 
Mariano Lopez


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] wic: Allow to use a custom config for bootloaders
  2015-11-23 22:13     ` Mariano Lopez
@ 2015-11-24 15:43       ` Ed Bartosh
  2015-11-26 14:48         ` Mariano Lopez
  0 siblings, 1 reply; 9+ messages in thread
From: Ed Bartosh @ 2015-11-24 15:43 UTC (permalink / raw)
  To: Mariano Lopez; +Cc: openembedded-core

On Mon, Nov 23, 2015 at 04:13:15PM -0600, Mariano Lopez wrote:
> 
> 
> On 11/23/2015 11:37 AM, Ed Bartosh wrote:
> >Hi Mariano,
> >
> >Thank you for the patchset!
> >
> >Would it be better to put content of configuration file into .wks
> >instead of just referring to it?
> 
> If the configuration is simple I agree with you; however if the
> configuration have scripts I think it's better to have separated
> file. The file can growth and would be a real mess inside a wks
> file.
>
What bothers me here is that reference to the external entity (config file in this case) which
may or may not exist. This makes wic more fragile than it is now.

Can we put bootloader configs to some predefined place, e.g. to the same
directory where .wks is?

> >
> >It would be also nice to have this code covered by oe-selftest.
>
> Yes, I plan do  create a test and update the documentation once it
> is integrated in master.
>

I'd prefer to have tests in the same patchset with the code. It would
help to understand better how to handle external configs. Can you do
that?

Regards,
Ed

> >
> >On Wed, Nov 18, 2015 at 08:25:54AM +0000, mariano.lopez@linux.intel.com wrote:
> >>From: Mariano Lopez <mariano.lopez@linux.intel.com>
> >>
> >>This change will allow to use a user defined file as the
> >>configuration for the bootloaders (grub, gummiboot, syslinux).
> >>
> >>The config file is defined in the wks file with the "configfile"
> >>option in the bootloader line.
> >>
> >>[YOCTO #8003]
> >>
> >>Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
> >>---
> >>  scripts/lib/wic/plugins/source/bootimg-efi.py    | 66 ++++++++++++++++--------
> >>  scripts/lib/wic/plugins/source/bootimg-pcbios.py | 66 ++++++++++++++----------
> >>  2 files changed, 83 insertions(+), 49 deletions(-)
> >>
> >>diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
> >>index fa63c6a..8fc879e 100644
> >>--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
> >>+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
> >>@@ -45,22 +45,33 @@ class BootimgEFIPlugin(SourcePlugin):
> >>          """
> >>          Create loader-specific (grub-efi) config
> >>          """
> >>-        options = creator.ks.handler.bootloader.appendLine
> >>-
> >>-        grubefi_conf = ""
> >>-        grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
> >>-        grubefi_conf += "default=boot\n"
> >>-        timeout = kickstart.get_timeout(creator.ks)
> >>-        if not timeout:
> >>-            timeout = 0
> >>-        grubefi_conf += "timeout=%s\n" % timeout
> >>-        grubefi_conf += "menuentry 'boot'{\n"
> >>-
> >>-        kernel = "/bzImage"
> >>-
> >>-        grubefi_conf += "linux %s root=%s rootwait %s\n" \
> >>-            % (kernel, creator.rootdev, options)
> >>-        grubefi_conf += "}\n"
> >>+        configfile = kickstart.get_bootloader_file(creator.ks)
> >>+
> >>+        if configfile and os.path.exists(configfile):
> >>+            # Use a custom configuration file for grub
> >>+            msger.info("Using custom configuration file "
> >>+                    "%s for grub.cfg" % configfile)
> >>+            user_conf = open(configfile, "r")
> >>+            grubefi_conf = user_conf.read()
> >>+            user_conf.close()
> >>+        else:
> >>+            # Create grub configuration using parameters from wks file
> >>+            options = creator.ks.handler.bootloader.appendLine
> >>+
> >>+            grubefi_conf = ""
> >>+            grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
> >>+            grubefi_conf += "default=boot\n"
> >>+            timeout = kickstart.get_timeout(creator.ks)
> >>+            if not timeout:
> >>+                timeout = 0
> >>+            grubefi_conf += "timeout=%s\n" % timeout
> >>+            grubefi_conf += "menuentry 'boot'{\n"
> >>+
> >>+            kernel = "/bzImage"
> >>+
> >>+            grubefi_conf += "linux %s root=%s rootwait %s\n" \
> >>+                % (kernel, creator.rootdev, options)
> >>+            grubefi_conf += "}\n"
> >>          msger.debug("Writing grubefi config %s/hdd/boot/EFI/BOOT/grub.cfg" \
> >>                          % cr_workdir)
> >>@@ -95,12 +106,23 @@ class BootimgEFIPlugin(SourcePlugin):
> >>          cfg.write(loader_conf)
> >>          cfg.close()
> >>-        kernel = "/bzImage"
> >>-
> >>-        boot_conf = ""
> >>-        boot_conf += "title boot\n"
> >>-        boot_conf += "linux %s\n" % kernel
> >>-        boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
> >>+        configfile = kickstart.get_bootloader_file(creator.ks)
> >>+
> >>+        if configfile and os.path.exists(configfile):
> >>+            # Use a custom configuration file for gummiboot
> >>+            msger.info("Using custom configuration file "
> >>+                    "%s for gummiboot's boot.conf" % configfile)
> >>+            user_conf = open(configfile, "r")
> >>+            boot_conf = user_conf.read()
> >>+            user_conf.close()
> >>+        else:
> >>+            # Create gummiboot configuration using parameters from wks file
> >>+            kernel = "/bzImage"
> >>+
> >>+            boot_conf = ""
> >>+            boot_conf += "title boot\n"
> >>+            boot_conf += "linux %s\n" % kernel
> >>+            boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
> >>          msger.debug("Writing gummiboot config %s/hdd/boot/loader/entries/boot.conf" \
> >>                          % cr_workdir)
> >>diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
> >>index 96ed54d..9e21572 100644
> >>--- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
> >>+++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
> >>@@ -83,34 +83,46 @@ class BootimgPcbiosPlugin(SourcePlugin):
> >>          install_cmd = "install -d %s" % hdddir
> >>          exec_cmd(install_cmd)
> >>-        splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
> >>-        if os.path.exists(splash):
> >>-            splashline = "menu background splash.jpg"
> >>+        configfile = kickstart.get_bootloader_file(creator.ks)
> >>+
> >>+        if configfile and os.path.exists(configfile):
> >>+            # Use a custom configuration file for syslinux
> >>+            msger.info("Using custom configuration file "
> >>+                    "%s for syslinux.cfg" % configfile)
> >>+            user_conf = open(configfile, "r")
> >>+            syslinux_conf = user_conf.read()
> >>+            user_conf.close()
> >>+
> >>          else:
> >>-            splashline = ""
> >>-
> >>-        options = creator.ks.handler.bootloader.appendLine
> >>-
> >>-        syslinux_conf = ""
> >>-        syslinux_conf += "PROMPT 0\n"
> >>-        timeout = kickstart.get_timeout(creator.ks)
> >>-        if not timeout:
> >>-            timeout = 0
> >>-        syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
> >>-        syslinux_conf += "\n"
> >>-        syslinux_conf += "ALLOWOPTIONS 1\n"
> >>-        syslinux_conf += "SERIAL 0 115200\n"
> >>-        syslinux_conf += "\n"
> >>-        if splashline:
> >>-            syslinux_conf += "%s\n" % splashline
> >>-        syslinux_conf += "DEFAULT boot\n"
> >>-        syslinux_conf += "LABEL boot\n"
> >>-
> >>-        kernel = "/vmlinuz"
> >>-        syslinux_conf += "KERNEL " + kernel + "\n"
> >>-
> >>-        syslinux_conf += "APPEND label=boot root=%s %s\n" % \
> >>-                             (creator.rootdev, options)
> >>+            # Create syslinux configuration using parameters from wks file
> >>+            splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
> >>+            if os.path.exists(splash):
> >>+                splashline = "menu background splash.jpg"
> >>+            else:
> >>+                splashline = ""
> >>+
> >>+            options = creator.ks.handler.bootloader.appendLine
> >>+
> >>+            syslinux_conf = ""
> >>+            syslinux_conf += "PROMPT 0\n"
> >>+            timeout = kickstart.get_timeout(creator.ks)
> >>+            if not timeout:
> >>+                timeout = 0
> >>+            syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
> >>+            syslinux_conf += "\n"
> >>+            syslinux_conf += "ALLOWOPTIONS 1\n"
> >>+            syslinux_conf += "SERIAL 0 115200\n"
> >>+            syslinux_conf += "\n"
> >>+            if splashline:
> >>+                syslinux_conf += "%s\n" % splashline
> >>+            syslinux_conf += "DEFAULT boot\n"
> >>+            syslinux_conf += "LABEL boot\n"
> >>+
> >>+            kernel = "/vmlinuz"
> >>+            syslinux_conf += "KERNEL " + kernel + "\n"
> >>+
> >>+            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
> >>+                                 (creator.rootdev, options)
> >>          msger.debug("Writing syslinux config %s/hdd/boot/syslinux.cfg" \
> >>                      % cr_workdir)
> >>-- 
> >>1.8.4.5
> >>
> 
> -- 
> Mariano Lopez

-- 
--
Regards,
Ed


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] wic: Allow to use a custom config for bootloaders
  2015-11-24 15:43       ` Ed Bartosh
@ 2015-11-26 14:48         ` Mariano Lopez
  2015-11-26 19:23           ` Mariano Lopez
  0 siblings, 1 reply; 9+ messages in thread
From: Mariano Lopez @ 2015-11-26 14:48 UTC (permalink / raw)
  To: ed.bartosh; +Cc: openembedded-core



On 11/24/2015 09:43 AM, Ed Bartosh wrote:
> On Mon, Nov 23, 2015 at 04:13:15PM -0600, Mariano Lopez wrote:
>>
>> On 11/23/2015 11:37 AM, Ed Bartosh wrote:
>>> Hi Mariano,
>>>
>>> Thank you for the patchset!
>>>
>>> Would it be better to put content of configuration file into .wks
>>> instead of just referring to it?
>> If the configuration is simple I agree with you; however if the
>> configuration have scripts I think it's better to have separated
>> file. The file can growth and would be a real mess inside a wks
>> file.
>>
> What bothers me here is that reference to the external entity (config file in this case) which
> may or may not exist. This makes wic more fragile than it is now.
>
> Can we put bootloader configs to some predefined place, e.g. to the same
> directory where .wks is?

I see your point now. I'll change the code to have the configuration in 
the wks file.

>
>>> It would be also nice to have this code covered by oe-selftest.
>> Yes, I plan do  create a test and update the documentation once it
>> is integrated in master.
>>
> I'd prefer to have tests in the same patchset with the code. It would
> help to understand better how to handle external configs. Can you do
> that?

I'll work on that and send the series again.

>
> Regards,
> Ed
>
>>> On Wed, Nov 18, 2015 at 08:25:54AM +0000, mariano.lopez@linux.intel.com wrote:
>>>> From: Mariano Lopez <mariano.lopez@linux.intel.com>
>>>>
>>>> This change will allow to use a user defined file as the
>>>> configuration for the bootloaders (grub, gummiboot, syslinux).
>>>>
>>>> The config file is defined in the wks file with the "configfile"
>>>> option in the bootloader line.
>>>>
>>>> [YOCTO #8003]
>>>>
>>>> Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
>>>> ---
>>>>   scripts/lib/wic/plugins/source/bootimg-efi.py    | 66 ++++++++++++++++--------
>>>>   scripts/lib/wic/plugins/source/bootimg-pcbios.py | 66 ++++++++++++++----------
>>>>   2 files changed, 83 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
>>>> index fa63c6a..8fc879e 100644
>>>> --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
>>>> +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
>>>> @@ -45,22 +45,33 @@ class BootimgEFIPlugin(SourcePlugin):
>>>>           """
>>>>           Create loader-specific (grub-efi) config
>>>>           """
>>>> -        options = creator.ks.handler.bootloader.appendLine
>>>> -
>>>> -        grubefi_conf = ""
>>>> -        grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
>>>> -        grubefi_conf += "default=boot\n"
>>>> -        timeout = kickstart.get_timeout(creator.ks)
>>>> -        if not timeout:
>>>> -            timeout = 0
>>>> -        grubefi_conf += "timeout=%s\n" % timeout
>>>> -        grubefi_conf += "menuentry 'boot'{\n"
>>>> -
>>>> -        kernel = "/bzImage"
>>>> -
>>>> -        grubefi_conf += "linux %s root=%s rootwait %s\n" \
>>>> -            % (kernel, creator.rootdev, options)
>>>> -        grubefi_conf += "}\n"
>>>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>>>> +
>>>> +        if configfile and os.path.exists(configfile):
>>>> +            # Use a custom configuration file for grub
>>>> +            msger.info("Using custom configuration file "
>>>> +                    "%s for grub.cfg" % configfile)
>>>> +            user_conf = open(configfile, "r")
>>>> +            grubefi_conf = user_conf.read()
>>>> +            user_conf.close()
>>>> +        else:
>>>> +            # Create grub configuration using parameters from wks file
>>>> +            options = creator.ks.handler.bootloader.appendLine
>>>> +
>>>> +            grubefi_conf = ""
>>>> +            grubefi_conf += "serial --unit=0 --speed=115200 --word=8 --parity=no --stop=1\n"
>>>> +            grubefi_conf += "default=boot\n"
>>>> +            timeout = kickstart.get_timeout(creator.ks)
>>>> +            if not timeout:
>>>> +                timeout = 0
>>>> +            grubefi_conf += "timeout=%s\n" % timeout
>>>> +            grubefi_conf += "menuentry 'boot'{\n"
>>>> +
>>>> +            kernel = "/bzImage"
>>>> +
>>>> +            grubefi_conf += "linux %s root=%s rootwait %s\n" \
>>>> +                % (kernel, creator.rootdev, options)
>>>> +            grubefi_conf += "}\n"
>>>>           msger.debug("Writing grubefi config %s/hdd/boot/EFI/BOOT/grub.cfg" \
>>>>                           % cr_workdir)
>>>> @@ -95,12 +106,23 @@ class BootimgEFIPlugin(SourcePlugin):
>>>>           cfg.write(loader_conf)
>>>>           cfg.close()
>>>> -        kernel = "/bzImage"
>>>> -
>>>> -        boot_conf = ""
>>>> -        boot_conf += "title boot\n"
>>>> -        boot_conf += "linux %s\n" % kernel
>>>> -        boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
>>>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>>>> +
>>>> +        if configfile and os.path.exists(configfile):
>>>> +            # Use a custom configuration file for gummiboot
>>>> +            msger.info("Using custom configuration file "
>>>> +                    "%s for gummiboot's boot.conf" % configfile)
>>>> +            user_conf = open(configfile, "r")
>>>> +            boot_conf = user_conf.read()
>>>> +            user_conf.close()
>>>> +        else:
>>>> +            # Create gummiboot configuration using parameters from wks file
>>>> +            kernel = "/bzImage"
>>>> +
>>>> +            boot_conf = ""
>>>> +            boot_conf += "title boot\n"
>>>> +            boot_conf += "linux %s\n" % kernel
>>>> +            boot_conf += "options LABEL=Boot root=%s %s\n" % (creator.rootdev, options)
>>>>           msger.debug("Writing gummiboot config %s/hdd/boot/loader/entries/boot.conf" \
>>>>                           % cr_workdir)
>>>> diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>>>> index 96ed54d..9e21572 100644
>>>> --- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>>>> +++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>>>> @@ -83,34 +83,46 @@ class BootimgPcbiosPlugin(SourcePlugin):
>>>>           install_cmd = "install -d %s" % hdddir
>>>>           exec_cmd(install_cmd)
>>>> -        splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
>>>> -        if os.path.exists(splash):
>>>> -            splashline = "menu background splash.jpg"
>>>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>>>> +
>>>> +        if configfile and os.path.exists(configfile):
>>>> +            # Use a custom configuration file for syslinux
>>>> +            msger.info("Using custom configuration file "
>>>> +                    "%s for syslinux.cfg" % configfile)
>>>> +            user_conf = open(configfile, "r")
>>>> +            syslinux_conf = user_conf.read()
>>>> +            user_conf.close()
>>>> +
>>>>           else:
>>>> -            splashline = ""
>>>> -
>>>> -        options = creator.ks.handler.bootloader.appendLine
>>>> -
>>>> -        syslinux_conf = ""
>>>> -        syslinux_conf += "PROMPT 0\n"
>>>> -        timeout = kickstart.get_timeout(creator.ks)
>>>> -        if not timeout:
>>>> -            timeout = 0
>>>> -        syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
>>>> -        syslinux_conf += "\n"
>>>> -        syslinux_conf += "ALLOWOPTIONS 1\n"
>>>> -        syslinux_conf += "SERIAL 0 115200\n"
>>>> -        syslinux_conf += "\n"
>>>> -        if splashline:
>>>> -            syslinux_conf += "%s\n" % splashline
>>>> -        syslinux_conf += "DEFAULT boot\n"
>>>> -        syslinux_conf += "LABEL boot\n"
>>>> -
>>>> -        kernel = "/vmlinuz"
>>>> -        syslinux_conf += "KERNEL " + kernel + "\n"
>>>> -
>>>> -        syslinux_conf += "APPEND label=boot root=%s %s\n" % \
>>>> -                             (creator.rootdev, options)
>>>> +            # Create syslinux configuration using parameters from wks file
>>>> +            splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
>>>> +            if os.path.exists(splash):
>>>> +                splashline = "menu background splash.jpg"
>>>> +            else:
>>>> +                splashline = ""
>>>> +
>>>> +            options = creator.ks.handler.bootloader.appendLine
>>>> +
>>>> +            syslinux_conf = ""
>>>> +            syslinux_conf += "PROMPT 0\n"
>>>> +            timeout = kickstart.get_timeout(creator.ks)
>>>> +            if not timeout:
>>>> +                timeout = 0
>>>> +            syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
>>>> +            syslinux_conf += "\n"
>>>> +            syslinux_conf += "ALLOWOPTIONS 1\n"
>>>> +            syslinux_conf += "SERIAL 0 115200\n"
>>>> +            syslinux_conf += "\n"
>>>> +            if splashline:
>>>> +                syslinux_conf += "%s\n" % splashline
>>>> +            syslinux_conf += "DEFAULT boot\n"
>>>> +            syslinux_conf += "LABEL boot\n"
>>>> +
>>>> +            kernel = "/vmlinuz"
>>>> +            syslinux_conf += "KERNEL " + kernel + "\n"
>>>> +
>>>> +            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
>>>> +                                 (creator.rootdev, options)
>>>>           msger.debug("Writing syslinux config %s/hdd/boot/syslinux.cfg" \
>>>>                       % cr_workdir)
>>>> -- 
>>>> 1.8.4.5
>>>>
>> -- 
>> Mariano Lopez

-- 
Mariano Lopez


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] wic: Allow to use a custom config for bootloaders
  2015-11-26 14:48         ` Mariano Lopez
@ 2015-11-26 19:23           ` Mariano Lopez
  2015-11-27 12:47             ` Ed Bartosh
  0 siblings, 1 reply; 9+ messages in thread
From: Mariano Lopez @ 2015-11-26 19:23 UTC (permalink / raw)
  To: ed.bartosh; +Cc: openembedded-core



On 11/26/2015 08:48 AM, Mariano Lopez wrote:
>
>
> On 11/24/2015 09:43 AM, Ed Bartosh wrote:
>> On Mon, Nov 23, 2015 at 04:13:15PM -0600, Mariano Lopez wrote:
>>>
>>> On 11/23/2015 11:37 AM, Ed Bartosh wrote:
>>>> Hi Mariano,
>>>>
>>>> Thank you for the patchset!
>>>>
>>>> Would it be better to put content of configuration file into .wks
>>>> instead of just referring to it?
>>> If the configuration is simple I agree with you; however if the
>>> configuration have scripts I think it's better to have separated
>>> file. The file can growth and would be a real mess inside a wks
>>> file.
>>>
>> What bothers me here is that reference to the external entity (config 
>> file in this case) which
>> may or may not exist. This makes wic more fragile than it is now.
>>
>> Can we put bootloader configs to some predefined place, e.g. to the same
>> directory where .wks is?
>
> I see your point now. I'll change the code to have the configuration 
> in the wks file.

I was checking in the kickstart code and the parser is a state machine 
that reads line by line, so a multi line bootloader config file is not 
currently and option. I was about to modify the code when I saw the 
comment for this class:

"Methods that don't need to do anything may just pass.  However, 
_stateMachine should never be overridden."

So, in order to have the config file inside the wks file we would need 
to hack in kickstart code; I would like to avoid that. What do you think?

>
>>
>>>> It would be also nice to have this code covered by oe-selftest.
>>> Yes, I plan do  create a test and update the documentation once it
>>> is integrated in master.
>>>
>> I'd prefer to have tests in the same patchset with the code. It would
>> help to understand better how to handle external configs. Can you do
>> that?
>
> I'll work on that and send the series again.
>
>>
>> Regards,
>> Ed
>>
>>>> On Wed, Nov 18, 2015 at 08:25:54AM +0000, 
>>>> mariano.lopez@linux.intel.com wrote:
>>>>> From: Mariano Lopez <mariano.lopez@linux.intel.com>
>>>>>
>>>>> This change will allow to use a user defined file as the
>>>>> configuration for the bootloaders (grub, gummiboot, syslinux).
>>>>>
>>>>> The config file is defined in the wks file with the "configfile"
>>>>> option in the bootloader line.
>>>>>
>>>>> [YOCTO #8003]
>>>>>
>>>>> Signed-off-by: Mariano Lopez <mariano.lopez@linux.intel.com>
>>>>> ---
>>>>>   scripts/lib/wic/plugins/source/bootimg-efi.py    | 66 
>>>>> ++++++++++++++++--------
>>>>>   scripts/lib/wic/plugins/source/bootimg-pcbios.py | 66 
>>>>> ++++++++++++++----------
>>>>>   2 files changed, 83 insertions(+), 49 deletions(-)
>>>>>
>>>>> diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py 
>>>>> b/scripts/lib/wic/plugins/source/bootimg-efi.py
>>>>> index fa63c6a..8fc879e 100644
>>>>> --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
>>>>> +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
>>>>> @@ -45,22 +45,33 @@ class BootimgEFIPlugin(SourcePlugin):
>>>>>           """
>>>>>           Create loader-specific (grub-efi) config
>>>>>           """
>>>>> -        options = creator.ks.handler.bootloader.appendLine
>>>>> -
>>>>> -        grubefi_conf = ""
>>>>> -        grubefi_conf += "serial --unit=0 --speed=115200 --word=8 
>>>>> --parity=no --stop=1\n"
>>>>> -        grubefi_conf += "default=boot\n"
>>>>> -        timeout = kickstart.get_timeout(creator.ks)
>>>>> -        if not timeout:
>>>>> -            timeout = 0
>>>>> -        grubefi_conf += "timeout=%s\n" % timeout
>>>>> -        grubefi_conf += "menuentry 'boot'{\n"
>>>>> -
>>>>> -        kernel = "/bzImage"
>>>>> -
>>>>> -        grubefi_conf += "linux %s root=%s rootwait %s\n" \
>>>>> -            % (kernel, creator.rootdev, options)
>>>>> -        grubefi_conf += "}\n"
>>>>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>>>>> +
>>>>> +        if configfile and os.path.exists(configfile):
>>>>> +            # Use a custom configuration file for grub
>>>>> +            msger.info("Using custom configuration file "
>>>>> +                    "%s for grub.cfg" % configfile)
>>>>> +            user_conf = open(configfile, "r")
>>>>> +            grubefi_conf = user_conf.read()
>>>>> +            user_conf.close()
>>>>> +        else:
>>>>> +            # Create grub configuration using parameters from wks 
>>>>> file
>>>>> +            options = creator.ks.handler.bootloader.appendLine
>>>>> +
>>>>> +            grubefi_conf = ""
>>>>> +            grubefi_conf += "serial --unit=0 --speed=115200 
>>>>> --word=8 --parity=no --stop=1\n"
>>>>> +            grubefi_conf += "default=boot\n"
>>>>> +            timeout = kickstart.get_timeout(creator.ks)
>>>>> +            if not timeout:
>>>>> +                timeout = 0
>>>>> +            grubefi_conf += "timeout=%s\n" % timeout
>>>>> +            grubefi_conf += "menuentry 'boot'{\n"
>>>>> +
>>>>> +            kernel = "/bzImage"
>>>>> +
>>>>> +            grubefi_conf += "linux %s root=%s rootwait %s\n" \
>>>>> +                % (kernel, creator.rootdev, options)
>>>>> +            grubefi_conf += "}\n"
>>>>>           msger.debug("Writing grubefi config 
>>>>> %s/hdd/boot/EFI/BOOT/grub.cfg" \
>>>>>                           % cr_workdir)
>>>>> @@ -95,12 +106,23 @@ class BootimgEFIPlugin(SourcePlugin):
>>>>>           cfg.write(loader_conf)
>>>>>           cfg.close()
>>>>> -        kernel = "/bzImage"
>>>>> -
>>>>> -        boot_conf = ""
>>>>> -        boot_conf += "title boot\n"
>>>>> -        boot_conf += "linux %s\n" % kernel
>>>>> -        boot_conf += "options LABEL=Boot root=%s %s\n" % 
>>>>> (creator.rootdev, options)
>>>>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>>>>> +
>>>>> +        if configfile and os.path.exists(configfile):
>>>>> +            # Use a custom configuration file for gummiboot
>>>>> +            msger.info("Using custom configuration file "
>>>>> +                    "%s for gummiboot's boot.conf" % configfile)
>>>>> +            user_conf = open(configfile, "r")
>>>>> +            boot_conf = user_conf.read()
>>>>> +            user_conf.close()
>>>>> +        else:
>>>>> +            # Create gummiboot configuration using parameters 
>>>>> from wks file
>>>>> +            kernel = "/bzImage"
>>>>> +
>>>>> +            boot_conf = ""
>>>>> +            boot_conf += "title boot\n"
>>>>> +            boot_conf += "linux %s\n" % kernel
>>>>> +            boot_conf += "options LABEL=Boot root=%s %s\n" % 
>>>>> (creator.rootdev, options)
>>>>>           msger.debug("Writing gummiboot config 
>>>>> %s/hdd/boot/loader/entries/boot.conf" \
>>>>>                           % cr_workdir)
>>>>> diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py 
>>>>> b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>>>>> index 96ed54d..9e21572 100644
>>>>> --- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>>>>> +++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
>>>>> @@ -83,34 +83,46 @@ class BootimgPcbiosPlugin(SourcePlugin):
>>>>>           install_cmd = "install -d %s" % hdddir
>>>>>           exec_cmd(install_cmd)
>>>>> -        splash = os.path.join(cr_workdir, "/hdd/boot/splash.jpg")
>>>>> -        if os.path.exists(splash):
>>>>> -            splashline = "menu background splash.jpg"
>>>>> +        configfile = kickstart.get_bootloader_file(creator.ks)
>>>>> +
>>>>> +        if configfile and os.path.exists(configfile):
>>>>> +            # Use a custom configuration file for syslinux
>>>>> +            msger.info("Using custom configuration file "
>>>>> +                    "%s for syslinux.cfg" % configfile)
>>>>> +            user_conf = open(configfile, "r")
>>>>> +            syslinux_conf = user_conf.read()
>>>>> +            user_conf.close()
>>>>> +
>>>>>           else:
>>>>> -            splashline = ""
>>>>> -
>>>>> -        options = creator.ks.handler.bootloader.appendLine
>>>>> -
>>>>> -        syslinux_conf = ""
>>>>> -        syslinux_conf += "PROMPT 0\n"
>>>>> -        timeout = kickstart.get_timeout(creator.ks)
>>>>> -        if not timeout:
>>>>> -            timeout = 0
>>>>> -        syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
>>>>> -        syslinux_conf += "\n"
>>>>> -        syslinux_conf += "ALLOWOPTIONS 1\n"
>>>>> -        syslinux_conf += "SERIAL 0 115200\n"
>>>>> -        syslinux_conf += "\n"
>>>>> -        if splashline:
>>>>> -            syslinux_conf += "%s\n" % splashline
>>>>> -        syslinux_conf += "DEFAULT boot\n"
>>>>> -        syslinux_conf += "LABEL boot\n"
>>>>> -
>>>>> -        kernel = "/vmlinuz"
>>>>> -        syslinux_conf += "KERNEL " + kernel + "\n"
>>>>> -
>>>>> -        syslinux_conf += "APPEND label=boot root=%s %s\n" % \
>>>>> -                             (creator.rootdev, options)
>>>>> +            # Create syslinux configuration using parameters from 
>>>>> wks file
>>>>> +            splash = os.path.join(cr_workdir, 
>>>>> "/hdd/boot/splash.jpg")
>>>>> +            if os.path.exists(splash):
>>>>> +                splashline = "menu background splash.jpg"
>>>>> +            else:
>>>>> +                splashline = ""
>>>>> +
>>>>> +            options = creator.ks.handler.bootloader.appendLine
>>>>> +
>>>>> +            syslinux_conf = ""
>>>>> +            syslinux_conf += "PROMPT 0\n"
>>>>> +            timeout = kickstart.get_timeout(creator.ks)
>>>>> +            if not timeout:
>>>>> +                timeout = 0
>>>>> +            syslinux_conf += "TIMEOUT " + str(timeout) + "\n"
>>>>> +            syslinux_conf += "\n"
>>>>> +            syslinux_conf += "ALLOWOPTIONS 1\n"
>>>>> +            syslinux_conf += "SERIAL 0 115200\n"
>>>>> +            syslinux_conf += "\n"
>>>>> +            if splashline:
>>>>> +                syslinux_conf += "%s\n" % splashline
>>>>> +            syslinux_conf += "DEFAULT boot\n"
>>>>> +            syslinux_conf += "LABEL boot\n"
>>>>> +
>>>>> +            kernel = "/vmlinuz"
>>>>> +            syslinux_conf += "KERNEL " + kernel + "\n"
>>>>> +
>>>>> +            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
>>>>> +                                 (creator.rootdev, options)
>>>>>           msger.debug("Writing syslinux config 
>>>>> %s/hdd/boot/syslinux.cfg" \
>>>>>                       % cr_workdir)
>>>>> -- 
>>>>> 1.8.4.5
>>>>>
>>> -- 
>>> Mariano Lopez
>

-- 
Mariano Lopez


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] wic: Allow to use a custom config for bootloaders
  2015-11-26 19:23           ` Mariano Lopez
@ 2015-11-27 12:47             ` Ed Bartosh
  0 siblings, 0 replies; 9+ messages in thread
From: Ed Bartosh @ 2015-11-27 12:47 UTC (permalink / raw)
  To: Mariano Lopez; +Cc: openembedded-core

On Thu, Nov 26, 2015 at 01:23:50PM -0600, Mariano Lopez wrote:
> 
> 
> On 11/26/2015 08:48 AM, Mariano Lopez wrote:
> >
> >
> >On 11/24/2015 09:43 AM, Ed Bartosh wrote:
> >>On Mon, Nov 23, 2015 at 04:13:15PM -0600, Mariano Lopez wrote:
> >>>
> >>>On 11/23/2015 11:37 AM, Ed Bartosh wrote:
> >>>>Hi Mariano,
> >>>>
> >>>>Thank you for the patchset!
> >>>>
> >>>>Would it be better to put content of configuration file into .wks
> >>>>instead of just referring to it?
> >>>If the configuration is simple I agree with you; however if the
> >>>configuration have scripts I think it's better to have separated
> >>>file. The file can growth and would be a real mess inside a wks
> >>>file.
> >>>
> >>What bothers me here is that reference to the external entity
> >>(config file in this case) which
> >>may or may not exist. This makes wic more fragile than it is now.
> >>
> >>Can we put bootloader configs to some predefined place, e.g. to the same
> >>directory where .wks is?
> >
> >I see your point now. I'll change the code to have the
> >configuration in the wks file.
> 
> I was checking in the kickstart code and the parser is a state
> machine that reads line by line, so a multi line bootloader config
> file is not currently and option. I was about to modify the code
> when I saw the comment for this class:
> 
> "Methods that don't need to do anything may just pass.  However,
> _stateMachine should never be overridden."
> 
> So, in order to have the config file inside the wks file we would
> need to hack in kickstart code; I would like to avoid that. What do
> you think?
> 

I agree with you. Let's not touch kickstart code.

Let's put bootloader config at the same location where .wks is then.
It's also not the best solution though as it can be inconvenient for the
user, but it's better than pointing to the file which might not exist.

--
Regards,
Ed


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-11-27 13:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-18  8:25 [PATCH 0/2] wic: Allow to user defined files as config for bootloaders mariano.lopez
2015-11-18  8:25 ` [PATCH 1/2] wic: Prepare wicboot to allow custom bootloader config mariano.lopez
2015-11-18  8:25 ` [PATCH 2/2] wic: Allow to use a custom config for bootloaders mariano.lopez
2015-11-23 17:37   ` Ed Bartosh
2015-11-23 22:13     ` Mariano Lopez
2015-11-24 15:43       ` Ed Bartosh
2015-11-26 14:48         ` Mariano Lopez
2015-11-26 19:23           ` Mariano Lopez
2015-11-27 12:47             ` Ed Bartosh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.