Re: [PATCH v3] wic/plugins: gate root= with creator.rootdev for efi-isar and pcbios-isar

["'Jan Kiszka' via isar-users" <isar-users@googlegroups.com>]

On 09.04.26 12:24, Singh, Gourav (FT FDS CES LX PBU 2) wrote:
> Hi Jan,
> 
> Thanks for the review. These changes are currently specific to ISAR. A patch will be submitted to upstream OE to incorporate these changes there as well.
> 

Then let's wait for upstream first.

My whitespace comments likely apply to the upstream version as well, so
have a look at them in that context, please.

Jan

> Thanks,
> Gourav
> -----Original Message-----
> From: Kiszka, Jan (FT RPD CED) <jan.kiszka@siemens.com> 
> Sent: 09 April 2026 15:36
> To: Singh, Gourav (FT FDS CES LX PBU 2) <gouravsingh@siemens.com>; isar-users@googlegroups.com
> Cc: Hombourger, Cedric (FT FDS CES LX) <cedric.hombourger@siemens.com>
> Subject: Re: [PATCH v3] wic/plugins: gate root= with creator.rootdev for efi-isar and pcbios-isar
> 
> On 09.04.26 11:33, 'Gourav Singh' via isar-users wrote:
>> Checks for creator.rootdev not being None were missing and would cause 
>> the kernel command line to read "root=None". When using the 
>> Discoverable Partitions Specification, we really want no root= 
>> parameter on the kernel command line (and root=None is anyhow not a valid option).
> 
> Is this a backport from upstream wic? Or are we touching code that does not exist in the original version, is Isar-specific?
> 
>>
>> Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
>> Signed-off-by: Gourav Singh <gouravsingh@siemens.com>
>> ---
>>  .../lib/wic/plugins/source/bootimg-efi-isar.py  |  7 ++++---
>>  .../wic/plugins/source/bootimg-pcbios-isar.py   | 17 ++++++++++++-----
>>  2 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py 
>> b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
>> index 6bc78d42..6c6698d6 100644
>> --- a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
>> +++ b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
>> @@ -104,7 +104,7 @@ class BootimgEFIPlugin(SourcePlugin):
>>                          (get_bitbake_var("KERNEL_IMAGETYPE"), 
>> get_bitbake_var("INITRAMFS_LINK_NAME"))
>>
>>              label = source_params.get('label')
>> -            label_conf = "root=%s" % creator.rootdev
>> +            label_conf = f" root={creator.rootdev}" if creator.rootdev else ""
>                              ^^^
> Extra space, not needed.
> 
>>              if label:
>>                  label_conf = "LABEL=%s" % label
>>
>> @@ -201,7 +201,8 @@ class BootimgEFIPlugin(SourcePlugin):
>>              boot_conf += "linux /%s\n" % kernel
>>
>>              label = source_params.get('label')
>> -            label_conf = "LABEL=Boot root=%s" % creator.rootdev
>> +            label_conf = "LABEL=Boot"
>> +            label_conf += f" root={creator.rootdev}" if creator.rootdev else ""
>>              if label:
>>                  label_conf = "LABEL=%s" % label
>>
>> @@ -366,7 +367,7 @@ class BootimgEFIPlugin(SourcePlugin):
>>
>>              with tempfile.TemporaryDirectory() as tmp_dir:
>>                  label = source_params.get('label')
>> -                label_conf = "root=%s" % creator.rootdev
>> +                label_conf = f" root={creator.rootdev}" if creator.rootdev else ""
> 
> Also here: leading whitespace is not needed.
> 
>>                  if label:
>>                      label_conf = "LABEL=%s" % label
>>
>> diff --git 
>> a/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py 
>> b/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py
>> index d5040b72..5dd76761 100644
>> --- a/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py
>> +++ b/meta/scripts/lib/wic/plugins/source/bootimg-pcbios-isar.py
>> @@ -140,14 +140,21 @@ class BootimgPcbiosIsarPlugin(SourcePlugin):
>>
>>              syslinux_conf += "KERNEL " + kernel + "\n"
>>
>> -            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
>> -                             (creator.rootdev, bootloader.append)
>> +            # Check if rootdev exists
>> +            root_param = f"root={creator.rootdev}" if creator.rootdev else ""
>> +
>> +            syslinux_conf += f"APPEND label=boot {root_param} {bootloader.append}\n"
>>
>>              # if we are using an initrd, smuggle it in
>>              if initrd:
>> -                syslinux_conf = syslinux_conf.replace(
>> -                    " root=%s " % (creator.rootdev),
>> -                    " root=%s initrd=%s " % (creator.rootdev, initrd))
>> +                if root_param:
>> +                    syslinux_conf = syslinux_conf.replace(
>> +                        root_param,
>> +                        f" {root_param} initrd={initrd} ")
> 
> And more extra whitespaces, leading and trainling.
> 
>> +                else:
>> +                    syslinux_conf = syslinux_conf.replace(
>> +                        " label=boot ",
>> +                        f" label=boot initrd={initrd} ")
>>
>>          logger.debug("Writing syslinux config %s/hdd/boot/syslinux.cfg",
>>                       cr_workdir)
>> --
>> 2.39.5
>>
> 
> Jan
> 
> --
> Siemens AG, Foundational Technologies
> Linux Expert Center


-- 
Siemens AG, Foundational Technologies
Linux Expert Center

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/f5ec7ff0-6526-48ce-91c9-df0a8c675023%40siemens.com.