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

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

[Kareem Zarka]

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.

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.
+            if not  source_params.get('skip-kernal-install'):
+                install_cmd = "install -m 0644 %s/%s %s/%s" % \
+                    (staging_kernel_dir, kernel, hdddir, kernel)
+                exec_cmd(install_cmd)
 
         if get_bitbake_var("IMAGE_EFI_BOOT_FILES"):
             for src_path, dst_path in cls.install_task:
-- 
2.25.1

[PATCH] meta/lib/oeqa/selftest/cases/wic: Add tests for kernel installation and skip-kernel-install in wic plugin.

[Kareem Zarka]

This commit adds two tests to the wic plugin to verify that the kernel
is installed correctly when `skip-kernel-install` is not provided
and not installed when `skip-kernel-install=true`.
These tests ensure that the wic plugin is working correctly and will
help catch any future issues with kernel installation.

Signed-off-by: Kareem Zarka <kareem.zarka@huawei.com>
---
 meta/lib/oeqa/selftest/cases/wic.py | 71 +++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/wic.py b/meta/lib/oeqa/selftest/cases/wic.py
index ca1abb970a..40188a866a 100644
--- a/meta/lib/oeqa/selftest/cases/wic.py
+++ b/meta/lib/oeqa/selftest/cases/wic.py
@@ -16,6 +16,7 @@ import hashlib
 from glob import glob
 from shutil import rmtree, copy
 from tempfile import NamedTemporaryFile
+from tempfile import TemporaryDirectory
 
 from oeqa.selftest.case import OESelftestTestCase
 from oeqa.core.decorator import OETestTag
@@ -220,6 +221,76 @@ class Wic(WicTestCase):
         result = runCmd("wic ls %s:1/ -n %s" % (images[0], sysroot))       
         self.assertIn("kernel",result.output)
 
+    def test_skip_kernel_install(self):
+        """Test skip_kernel_install in wic plugin"""
+        # create a temporary file for the WKS content
+        with NamedTemporaryFile("w", suffix=".wks") as wks:
+            # write the WKS content to the temporary file
+            wks.writelines([
+                'part --source bootimg-efi \
+                --sourceparams="loader=grub-efi,skip-kernal-install=true"\
+                --label boot --active'
+                ])
+            wks.flush()
+            # create a temporary directory to extract the disk image to
+            with TemporaryDirectory() as tmpdir:
+                img = 'core-image-minimal'
+                # build the image using the WKS file
+                cmd = "wic create %s -e %s -o %s" % (wks.name, img, self.resultdir)
+                runCmd(cmd)
+                wksname = os.path.splitext(os.path.basename(wks.name))[0]
+                out = glob(os.path.join(self.resultdir, "%s-*.direct" % wksname))
+                self.assertEqual(1, len(out))
+
+                # extract the content of the disk image to the temporary directory                
+                cmd = "wic cp %s:1 %s" % (out[0], tmpdir)
+                runCmd(cmd)
+                
+                # check if the kernel is installed or not
+                kimgtype = get_bb_var('KERNEL_IMAGETYPE', 'core-image-minimal')
+                for file in os.listdir(tmpdir):
+                        if file == kimgtype :
+                            raise AssertionError(
+                                "The kernel image '{}' was found in\
+                                the partition".format(kimgtype)
+                                )
+
+    def test_kernel_installation(self):
+        """Test kernel installation in wic plugin"""
+        # create a temporary file for the WKS content
+        with NamedTemporaryFile("w", suffix=".wks") as wks:
+            # write the WKS content to the temporary file
+            wks.writelines([
+                'part --source bootimg-efi \
+                --sourceparams="loader=grub-efi"\
+                --label boot --active\n'
+                ])
+            wks.flush()
+            # create a temporary directory to extract the disk image to
+            with TemporaryDirectory() as tmpdir:
+                img = 'core-image-minimal'
+                # build the image using the WKS file
+                cmd = "wic create %s -e %s -o %s" % (wks.name, img, self.resultdir)
+                runCmd(cmd)
+                wksname = os.path.splitext(os.path.basename(wks.name))[0]
+                out = glob(os.path.join(self.resultdir, "%s-*.direct" % wksname))
+                self.assertEqual(1, len(out))
+
+                # extract the content of the disk image to the temporary directory                
+                cmd = "wic cp %s:1 %s" % (out[0], tmpdir)
+                runCmd(cmd)
+                
+                # check if the kernel is installed or not
+                kimgtype = get_bb_var('KERNEL_IMAGETYPE', 'core-image-minimal')
+                for file in os.listdir(tmpdir):
+                    if file == kimgtype :
+                        found = True
+                        break
+                self.assertTrue(
+                    found,"The kernel image was not found\
+                    in the boot prtition".format(kimgtype)
+                    )
+
     def test_sdimage_bootpart(self):
         """Test creation of sdimage-bootpart image"""
         cmd = "wic create sdimage-bootpart -e core-image-minimal -o %s" % self.resultdir
-- 
2.25.1

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

[Luca Ceresoli]

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":

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

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

[Richard Purdie]

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.

Cheers,

Richard

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

[Kareem Zarka]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

Dear Richard and Luca,

Thank you for your feedback. I will take it into consideration.

I understand that "XXX != False" and "not XXX" have different interpretations in Python.

The expression "XXX != False" only evaluates to True if XXX is not equal to False.

This means that if skip-kernel-install is None, the condition will be False.

On the other hand, "not XXX" evaluates to True if XXX is falsy, which includes None, False, 0, empty string, etc.

This is why I used it in my code, as I wanted to consider the case where skip-kernel-install is None.

Therefore, if skip-kernel-install is not provided, its value will be None, and the condition will be True, which means the kernel image will be installed.

However, if we want to enforce the requirement for the user to always provide the parameter skip-kernel-install with either the value true or false, we can make use (!= false , == true).

Let me know what suits us best in this case .

Kindest regards .

Kareem Zarka.

[-- Attachment #2: Type: text/html, Size: 1068 bytes --]

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

[Luca Ceresoli]

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

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

[Kareem Zarka]

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

Dear Luca

Thank you for the reply .

Please check https://lists.openembedded.org/g/openembedded-core/message/176829 when you have time.

Kindest regards .

Kareem Zarka

[-- Attachment #2: Type: text/html, Size: 348 bytes --]