isar-installer: Added option to add additional kernel cmdline arguments for isar-image-installer.

Message ID 20241211093522.43743-1-alexander.heinisch@siemens.com
State Superseded, archived
Headers show
Series isar-installer: Added option to add additional kernel cmdline arguments for isar-image-installer. | expand

Commit Message

alexander.heinisch Dec. 11, 2024, 9:35 a.m. UTC
From: Alexander Heinisch <alexander.heinisch@siemens.com>

In combination with unattended mode this allows to specify
several settings like target device, target image path, aso.
to be specified during buildtime.

Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>
---
 meta-isar/recipes-core/images/isar-image-installer.bb     | 1 +
 meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Florian Bezdeka Dec. 11, 2024, 9:49 a.m. UTC | #1
On Wed, 2024-12-11 at 10:35 +0100, alexander.heinisch via isar-users
wrote:
> From: Alexander Heinisch <alexander.heinisch@siemens.com>
> 
> In combination with unattended mode this allows to specify
> several settings like target device, target image path, aso.
> to be specified during buildtime.
> 
> Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>
> ---
>  meta-isar/recipes-core/images/isar-image-installer.bb     | 1 +
>  meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meta-isar/recipes-core/images/isar-image-installer.bb b/meta-isar/recipes-core/images/isar-image-installer.bb
> index dfce311a..a325ab9f 100644
> --- a/meta-isar/recipes-core/images/isar-image-installer.bb
> +++ b/meta-isar/recipes-core/images/isar-image-installer.bb
> @@ -10,6 +10,7 @@ DESCRIPTION = "Example of a ISAR based Installer Image"
>  # Use variable to switch easily to another wks
>  INSTALLER_WKS_FILE ??= "installer-efi.wks.in"
>  WKS_FILE = "${INSTALLER_WKS_FILE}"
> +ADDITIONAL_KERNEL_CMDLINE ??= ""

As this is specific for the installer image:

Should we prefix the variable with INSTALLER_ (or something similar?)


>  IMAGER_INSTALL:wic:append = " ${SYSTEMD_BOOTLOADER_INSTALL}"
>  
>  IMAGE_INSTALL += "deploy-image-service"
> diff --git a/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in b/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
> index be8127cb..ecc6adbb 100644
> --- a/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
> +++ b/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
> @@ -3,7 +3,7 @@
>  #
>  # SPDX-License-Identifier: MIT
>  
> -bootloader --ptable gpt --timeout 0 --append "rootwait console=ttyS0,115200 console=tty0 earlyprintk"
> +bootloader --ptable gpt --timeout 0 --append "rootwait console=ttyS0,115200 console=tty0 earlyprintk ${ADDITIONAL_KERNEL_CMDLINE}"
>  part /boot --source bootimg-efi-isar --sourceparams "loader=systemd-boot" --label efi --part-type EF00 --align 1024 --use-uuid
>  part / --source rootfs --fstype ext4 --exclude-path=install --label installroot --align 1024 --use-uuid
>  part /install --source rootfs --change-directory=install --label INSTALLDATA --size 4G --fstype=vfat --use-uuid --align 1024
> -- 
> 2.43.0
Anton Mikanovich Dec. 11, 2024, 9:53 a.m. UTC | #2
11/12/2024 11:35, alexander.heinisch via isar-users wrote:
> From: Alexander Heinisch <alexander.heinisch@siemens.com>
>
> In combination with unattended mode this allows to specify
> several settings like target device, target image path, aso.
> to be specified during buildtime.
>
> Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>

Hello Alexander,

Can you also add any kind of usage example?
alexander.heinisch Dec. 11, 2024, 11:52 a.m. UTC | #3
>On Wed, 2024-12-11 at 10:35 +0100, alexander.heinisch via isar-users
>wrote:
>> From: Alexander Heinisch <alexander.heinisch@siemens.com>
>> 
>> In combination with unattended mode this allows to specify several 
>> settings like target device, target image path, aso.
>> to be specified during buildtime.
>> 
>> Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>
>> ---
>>  meta-isar/recipes-core/images/isar-image-installer.bb     | 1 +
>>  meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in | 2 +-
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/meta-isar/recipes-core/images/isar-image-installer.bb 
>> b/meta-isar/recipes-core/images/isar-image-installer.bb
>> index dfce311a..a325ab9f 100644
>> --- a/meta-isar/recipes-core/images/isar-image-installer.bb
>> +++ b/meta-isar/recipes-core/images/isar-image-installer.bb
>> @@ -10,6 +10,7 @@ DESCRIPTION = "Example of a ISAR based Installer Image"
>>  # Use variable to switch easily to another wks  INSTALLER_WKS_FILE 
>> ??= "installer-efi.wks.in"
>>  WKS_FILE = "${INSTALLER_WKS_FILE}"
>> +ADDITIONAL_KERNEL_CMDLINE ??= ""
>
>As this is specific for the installer image:
>
>Should we prefix the variable with INSTALLER_ (or something similar?)

While having such a prefix makes it more clear to the user, 
most of the user's setup installer image in a multiconfig env.
Thus, there shouldn't be too much conflict/confusion.

On the other hand, once we have unique `ADDITIONAL_KERNEL_CMDLINE`
it allows for easier swapping of wks files once adoption of 
`--append="...${ADDITIONAL_KERNEL_CMDLINE}"` is available for multiple wks files.

e.g. a signed installer image, could reuse parts of a generic wks for signed ukis
only adding the additional `/install`.

Maybe that's YAGNI, so I am completely open to adding the prefix as well, 
those were just the thoughts/concerns I had in mind when coming up with the unified approach.

BR Alexander

>
>
>>  IMAGER_INSTALL:wic:append = " ${SYSTEMD_BOOTLOADER_INSTALL}"
>>  
>>  IMAGE_INSTALL += "deploy-image-service"
>> diff --git a/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in 
>> b/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
>> index be8127cb..ecc6adbb 100644
>> --- a/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
>> +++ b/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
>> @@ -3,7 +3,7 @@
>>  #
>>  # SPDX-License-Identifier: MIT
>>  
>> -bootloader --ptable gpt --timeout 0 --append "rootwait console=ttyS0,115200 console=tty0 earlyprintk"
>> +bootloader --ptable gpt --timeout 0 --append "rootwait console=ttyS0,115200 console=tty0 earlyprintk ${ADDITIONAL_KERNEL_CMDLINE}"
>>  part /boot --source bootimg-efi-isar --sourceparams 
>> "loader=systemd-boot" --label efi --part-type EF00 --align 1024 
>> --use-uuid  part / --source rootfs --fstype ext4 
>> --exclude-path=install --label installroot --align 1024 --use-uuid  
>> part /install --source rootfs --change-directory=install --label 
>> INSTALLDATA --size 4G --fstype=vfat --use-uuid --align 1024
>> -- 
>> 2.43.0
alexander.heinisch Dec. 11, 2024, 11:57 a.m. UTC | #4
>11/12/2024 11:35, alexander.heinisch via isar-users wrote:
>> From: Alexander Heinisch <alexander.heinisch@siemens.com>
>>
>> In combination with unattended mode this allows to specify several 
>> settings like target device, target image path, aso.
>> to be specified during buildtime.
>>
>> Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>
>
>Hello Alexander,
>
>Can you also add any kind of usage example?

You can find some examples here: https://github.com/ilbers/isar/commit/4c66faf1110ded164440df8108effef7e9a52d34
 
 - installer.unattended
 - installer.image.uri ...file name of the image to be installed (parameter name uri chosen since we plan to support download of images in upcoming patches)
 - installer.target.dev ...target device name (e.g. /dev/sda) for the image to be installed to
 - installer.target.overwrite ...strategy how to handle target devices not empty (possible values: OVERWRITE - overwrite data on target | ABORT - abort installation if target not empty)

I will put the link and a short description in the commit message!
Since, I plan to push further changes for the target bootstrapping, 
I'd prefer to update the documentation, once that's done.

BR Alexander

>
Anton Mikanovich Dec. 11, 2024, 12:18 p.m. UTC | #5
11/12/2024 13:57, Heinisch, Alexander wrote:
>> Hello Alexander,
>>
>> Can you also add any kind of usage example?
> You can find some examples here: https://github.com/ilbers/isar/commit/4c66faf1110ded164440df8108effef7e9a52d34
>   
>   - installer.unattended
>   - installer.image.uri ...file name of the image to be installed (parameter name uri chosen since we plan to support download of images in upcoming patches)
>   - installer.target.dev ...target device name (e.g. /dev/sda) for the image to be installed to
>   - installer.target.overwrite ...strategy how to handle target devices not empty (possible values: OVERWRITE - overwrite data on target | ABORT - abort installation if target not empty)
>
> I will put the link and a short description in the commit message!
> Since, I plan to push further changes for the target bootstrapping,
> I'd prefer to update the documentation, once that's done.
>
> BR Alexander

Thanks, but I mean the usage of ADDITIONAL_KERNEL_CMDLINE with any
sample target, so we will have all the way of that variables starting
from image generation to have them handled in handle-config.sh.
This will help to keep tracking this functionality for regressions.

Patch

diff --git a/meta-isar/recipes-core/images/isar-image-installer.bb b/meta-isar/recipes-core/images/isar-image-installer.bb
index dfce311a..a325ab9f 100644
--- a/meta-isar/recipes-core/images/isar-image-installer.bb
+++ b/meta-isar/recipes-core/images/isar-image-installer.bb
@@ -10,6 +10,7 @@  DESCRIPTION = "Example of a ISAR based Installer Image"
 # Use variable to switch easily to another wks
 INSTALLER_WKS_FILE ??= "installer-efi.wks.in"
 WKS_FILE = "${INSTALLER_WKS_FILE}"
+ADDITIONAL_KERNEL_CMDLINE ??= ""
 IMAGER_INSTALL:wic:append = " ${SYSTEMD_BOOTLOADER_INSTALL}"
 
 IMAGE_INSTALL += "deploy-image-service"
diff --git a/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in b/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
index be8127cb..ecc6adbb 100644
--- a/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
+++ b/meta-isar/scripts/lib/wic/canned-wks/installer-efi.wks.in
@@ -3,7 +3,7 @@ 
 #
 # SPDX-License-Identifier: MIT
 
-bootloader --ptable gpt --timeout 0 --append "rootwait console=ttyS0,115200 console=tty0 earlyprintk"
+bootloader --ptable gpt --timeout 0 --append "rootwait console=ttyS0,115200 console=tty0 earlyprintk ${ADDITIONAL_KERNEL_CMDLINE}"
 part /boot --source bootimg-efi-isar --sourceparams "loader=systemd-boot" --label efi --part-type EF00 --align 1024 --use-uuid
 part / --source rootfs --fstype ext4 --exclude-path=install --label installroot --align 1024 --use-uuid
 part /install --source rootfs --change-directory=install --label INSTALLDATA --size 4G --fstype=vfat --use-uuid --align 1024