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" <zarkakar...@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.za...@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
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#176812): 
https://lists.openembedded.org/g/openembedded-core/message/176812
Mute This Topic: https://lists.openembedded.org/mt/96791012/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to