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

[Gourav Singh]

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).

Signed-off-by: Cedric Hombourger <cedric.h...@siemens.com>
Signed-off-by: Gourav Singh <goura...@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 ""
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 ""
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} ")
+ 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 Kiszka]

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.h...@siemens.com>
> Signed-off-by: Gourav Singh <goura...@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

[Singh, Gourav]

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.

Thanks,
Gourav

[Jan Kiszka]

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

[Gourav Singh]

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).

A patch has been sent to upstream OE wic plugins to apply these changes:
https://lists.yoctoproject.org/g/yocto-patches/topic/wic_patch_wic_plugins/118740415

Signed-off-by: Cedric Hombourger <cedric.h...@siemens.com>
Signed-off-by: Gourav Singh <goura...@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..d68a8ff3 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 ""

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 ""

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..e12cba83 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} ")

[Jan Kiszka]

On 10.04.26 11:08, '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).
>
> A patch has been sent to upstream OE wic plugins to apply these changes:
> https://lists.yoctoproject.org/g/yocto-patches/topic/wic_patch_wic_plugins/118740415
>

Unless an issue is critical or an update causes regressions, we normally
sync by pulling in an upstream version of wic and then aligning our
derivatives to it.

Jan