diff mbox series

[wic,v2] wic/plugins: gate root= with rootdev check for efi, pcbios and partition

Message ID 20260526093132.1956645-1-gouravsingh@siemens.com
State New
Headers show
Series [wic,v2] wic/plugins: gate root= with rootdev check for efi, pcbios and partition | expand

Commit Message

Gourav Singh May 26, 2026, 9:31 a.m. UTC
Checks for creator.rootdev (or cr.rootdev) being None were missing
and would cause the kernel command line to contain "root=None". When
using the Discoverable Partitions Specification, no root= parameter
should appear on the kernel command line, as root=None is not valid.

Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
Signed-off-by: Gourav Singh <gouravsingh@siemens.com>
---
 src/wic/plugins/source/bootimg_efi.py       |  5 +++--
 src/wic/plugins/source/bootimg_partition.py | 11 +++++++++--
 src/wic/plugins/source/bootimg_pcbios.py    | 10 ++++++++--
 3 files changed, 20 insertions(+), 6 deletions(-)

--
2.39.5

Comments

Trevor Woerner May 26, 2026, 6:32 p.m. UTC | #1
This update addresses everything from the v1 review, thanks!
Applied to wic, master branch.

On Tue 2026-05-26 @ 03:01:32 PM, Gourav Singh wrote:
> Checks for creator.rootdev (or cr.rootdev) being None were missing
> and would cause the kernel command line to contain "root=None". When
> using the Discoverable Partitions Specification, no root= parameter
> should appear on the kernel command line, as root=None is not valid.
> 
> Signed-off-by: Cedric Hombourger <cedric.hombourger@siemens.com>
> Signed-off-by: Gourav Singh <gouravsingh@siemens.com>
> ---

Process nit for future spins: a short `---` changelog (below the `---`
diffstat) listing what changed between v1 and v2 makes it easier to
see at a glance what moved. Not a blocker this time, but a habit worth
building for any series that respins.

See: https://docs.yoctoproject.org/contributor-guide/submit-changes.html#taking-patch-review-into-account


>  src/wic/plugins/source/bootimg_efi.py       |  5 +++--
>  src/wic/plugins/source/bootimg_partition.py | 11 +++++++++--
>  src/wic/plugins/source/bootimg_pcbios.py    | 10 ++++++++--
>  3 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/src/wic/plugins/source/bootimg_efi.py b/src/wic/plugins/source/bootimg_efi.py
> index 69aa38b..fe6c222 100644
> --- a/src/wic/plugins/source/bootimg_efi.py
> +++ b/src/wic/plugins/source/bootimg_efi.py
> @@ -97,7 +97,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 = "root=%s" % creator.rootdev if creator.rootdev else ""
>              if label:
>                  label_conf = "LABEL=%s" % label
> 
> @@ -186,7 +186,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 += (" root=%s" % creator.rootdev) if creator.rootdev else ""
>              if label:
>                  label_conf = "LABEL=%s" % label
> 
> diff --git a/src/wic/plugins/source/bootimg_partition.py b/src/wic/plugins/source/bootimg_partition.py
> index 96f5e14..2e89b6c 100644
> --- a/src/wic/plugins/source/bootimg_partition.py
> +++ b/src/wic/plugins/source/bootimg_partition.py
> @@ -118,8 +118,15 @@ class BootimgPartitionPlugin(SourcePlugin):
>              if has_dtb:
>                  extlinux_conf += "   fdtdir %s\n" % fdt_dir
>              bootloader = cr.ks.bootloader
> -            extlinux_conf += "append root=%s rootwait %s\n" \
> -                             % (cr.rootdev, bootloader.append if bootloader.append else '')
> +
> +            # Check if rootdev exists
> +            parts = ["rootwait"]
> +            if cr.rootdev:
> +                parts.insert(0, "root=%s" % cr.rootdev)
> +            if bootloader.append:
> +                parts.append(bootloader.append)
> +
> +            extlinux_conf += "append %s\n" % " ".join(parts)
> 
>          install_cmd = "install -d %s/extlinux/" % hdddir
>          exec_cmd(install_cmd)
> diff --git a/src/wic/plugins/source/bootimg_pcbios.py b/src/wic/plugins/source/bootimg_pcbios.py
> index 1e5ec3a..2ae4eb1 100644
> --- a/src/wic/plugins/source/bootimg_pcbios.py
> +++ b/src/wic/plugins/source/bootimg_pcbios.py
> @@ -231,8 +231,14 @@ class BootimgPcbiosPlugin(SourcePlugin):
>              kernel = "/" + get_bitbake_var("KERNEL_IMAGETYPE")
>              syslinux_conf += "KERNEL " + kernel + "\n"
> 
> -            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
> -                             (creator.rootdev, bootloader.append)
> +            # Check if rootdev exists
> +            parts = ["label=boot"]
> +            if creator.rootdev:
> +                parts.append("root=%s" % creator.rootdev)
> +            if bootloader.append:
> +                parts.append(bootloader.append)
> +
> +            syslinux_conf += "APPEND %s\n" % " ".join(parts)
> 
>          logger.debug("Writing syslinux config %s/syslinux.cfg", hdddir)
>          cfg = open("%s/hdd/boot/syslinux.cfg" % cr_workdir, "w")
> --
> 2.39.5
>
Gourav Singh May 27, 2026, 3:03 a.m. UTC | #2
Thanks for the suggestion. I’ll make sure to include a short changelog below the diffstat for future respins.
diff mbox series

Patch

diff --git a/src/wic/plugins/source/bootimg_efi.py b/src/wic/plugins/source/bootimg_efi.py
index 69aa38b..fe6c222 100644
--- a/src/wic/plugins/source/bootimg_efi.py
+++ b/src/wic/plugins/source/bootimg_efi.py
@@ -97,7 +97,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 = "root=%s" % creator.rootdev if creator.rootdev else ""
             if label:
                 label_conf = "LABEL=%s" % label

@@ -186,7 +186,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 += (" root=%s" % creator.rootdev) if creator.rootdev else ""
             if label:
                 label_conf = "LABEL=%s" % label

diff --git a/src/wic/plugins/source/bootimg_partition.py b/src/wic/plugins/source/bootimg_partition.py
index 96f5e14..2e89b6c 100644
--- a/src/wic/plugins/source/bootimg_partition.py
+++ b/src/wic/plugins/source/bootimg_partition.py
@@ -118,8 +118,15 @@  class BootimgPartitionPlugin(SourcePlugin):
             if has_dtb:
                 extlinux_conf += "   fdtdir %s\n" % fdt_dir
             bootloader = cr.ks.bootloader
-            extlinux_conf += "append root=%s rootwait %s\n" \
-                             % (cr.rootdev, bootloader.append if bootloader.append else '')
+
+            # Check if rootdev exists
+            parts = ["rootwait"]
+            if cr.rootdev:
+                parts.insert(0, "root=%s" % cr.rootdev)
+            if bootloader.append:
+                parts.append(bootloader.append)
+
+            extlinux_conf += "append %s\n" % " ".join(parts)

         install_cmd = "install -d %s/extlinux/" % hdddir
         exec_cmd(install_cmd)
diff --git a/src/wic/plugins/source/bootimg_pcbios.py b/src/wic/plugins/source/bootimg_pcbios.py
index 1e5ec3a..2ae4eb1 100644
--- a/src/wic/plugins/source/bootimg_pcbios.py
+++ b/src/wic/plugins/source/bootimg_pcbios.py
@@ -231,8 +231,14 @@  class BootimgPcbiosPlugin(SourcePlugin):
             kernel = "/" + get_bitbake_var("KERNEL_IMAGETYPE")
             syslinux_conf += "KERNEL " + kernel + "\n"

-            syslinux_conf += "APPEND label=boot root=%s %s\n" % \
-                             (creator.rootdev, bootloader.append)
+            # Check if rootdev exists
+            parts = ["label=boot"]
+            if creator.rootdev:
+                parts.append("root=%s" % creator.rootdev)
+            if bootloader.append:
+                parts.append(bootloader.append)
+
+            syslinux_conf += "APPEND %s\n" % " ".join(parts)

         logger.debug("Writing syslinux config %s/syslinux.cfg", hdddir)
         cfg = open("%s/hdd/boot/syslinux.cfg" % cr_workdir, "w")