Re: [OE-core] [PATCH] wic/plugins/source/bootimg-efi: Skip installing kernel-image into boot.

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hello Richard, Kareem,

On Tue, 07 Feb 2023 12:32:31 +0000
"Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:

> On Tue, 2023-02-07 at 11:49 +0100, Luca Ceresoli via
> lists.openembedded.org wrote:
> > Hello Kareem,
> > 
> > thanks for your patch.
> > 
> > I have a few suggestions to improve it, see below.
> > 
> > On Mon,  6 Feb 2023 20:16:14 +0100
> > "Kareem Zarka" <zarkakareem@gmail.com> wrote:
> >   
> > > The issue with installing the kernel-image to both rootfs
> > > and boot partition is that some systems rely on the kernel-image in
> > > rootfs and not in the boot partition.
> > > This leads to duplication of the kernel-image, which can cause
> > > unnecessary storage usage and potential compatibility issues.  
> > 
> > Except for the use of unnecessary storage, I don't understand exactly
> > what problems can be created by duplication.
> >   
> > > This patch provides a solution to this problem by adding a new
> > > parameter "skip-kernel-install" to the wic kickstart file, which can
> > > be passed to the plugin.
> > > If the parameter is provided, the plugin will skip installing the
> > > kernel-image to the boot partition, avoiding duplication and potential
> > > issues.
> > > 
> > > By adding this new parameter, we give the users the option to install
> > > the kernel-image only in rootfs, or to install it in both rootfs and
> > > boot partition, depending on their needs and preferences.
> > > This will help to improve the system's storage usage and compatibility.
> > > 
> > > Tests for this functionality will be added in the next patch.
> > > 
> > > Signed-off-by: Kareem Zarka <kareem.zarka@huawei.com>
> > > ---
> > >  scripts/lib/wic/plugins/source/bootimg-efi.py | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
> > > index 4b00913a70..363b9f5242 100644
> > > --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
> > > +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
> > > @@ -363,9 +363,13 @@ class BootimgEFIPlugin(SourcePlugin):
> > >                  objcopy_cmd += " %s %s/EFI/Linux/linux.efi" % (efi_stub, hdddir)
> > >                  exec_native_cmd(objcopy_cmd, native_sysroot)
> > >          else:
> > > -            install_cmd = "install -m 0644 %s/%s %s/%s" % \
> > > -                (staging_kernel_dir, kernel, hdddir, kernel)
> > > -            exec_cmd(install_cmd)
> > > +            # skip-kernal-install was added to source_params to conifgure installing the kernel-image.
> > > +            # set skip_kernal_install in the kickstart file to skip installing it into hdddir.
> > > +            # if not set then the kernel-image will be installed.  
> > 
> > s/conifgure/configure/
> > Also check underscores vs dashes.
> > 
> > A comment in the code is welcome, but it should not include the history
> > of why this got added. When someone will read this three years from now
> > they don't care. So just remove the first line.
> >   
> > > +            if not  source_params.get('skip-kernal-install'):  
> > 
> > s/kernal/kernel/, also on other lines.
> > Also remove the unneeded double space.
> > 
> > Out of personal taste, I would prefer a positive logic rather than a
> > negative one, e.g.:
> > 
> >     if source_params.get('install-kernel-into-boot-dir') != "false":  
> 
> Whilst I know what you mean, that isn't valid python and the original
> code is probably more pythonic in that "XXX != False" is a bit
> different to "not XXX" in python.

Aa, sure, consider the above line just quickyl written pseudocode! :-)

Regardless of the implementation, my idea is this:
 * install-kernel-into-boot-dir is True  -> kernel is installed
 * install-kernel-into-boot-dir is False -> kernel is not installed
 * install-kernel-into-boot-dir not set -> kernel is installed (for backward compatibility)

But as I said, this is out of personal taste and I have a limited
perception of the whole problem.

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com