Hello Richard, Kareem, On Tue, 07 Feb 2023 12:32:31 +0000 "Richard Purdie" <[email protected]> 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" <[email protected]> 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 <[email protected]> > > > --- > > > 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
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#176896): https://lists.openembedded.org/g/openembedded-core/message/176896 Mute This Topic: https://lists.openembedded.org/mt/96791012/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
