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" <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":
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
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#176813):
https://lists.openembedded.org/g/openembedded-core/message/176813
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]
-=-=-=-=-=-=-=-=-=-=-=-
