| 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 |
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 >
Thanks for the suggestion. I’ll make sure to include a short changelog below the diffstat for future respins.
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")