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

Hi,

Thanks for the patch and the ping. The motivation is clear and the
change is in the right direction, but I would like to see an update
before merging:

1. extlinux has the same pattern and needs the same fix
   src/wic/plugins/source/bootimg_partition.py:121 currently has

   	extlinux_conf += "append root=%s rootwait %s\n" \
			% (cr.rootdev, bootloader.append if bootloader.append else '')

   With cr.rootdev == None on a DPS layout this produces
   "append root=None rootwait ..."

2. All the surrounding code uses %-formatted strings, please use the
   same style

On Fri 2026-04-24 @ 03:57:42 PM, Gourav Singh 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).
> 
> Signed-off-by: Cedric Hombourger <[email protected]>
> Signed-off-by: Gourav Singh <[email protected]>
> ---
>  src/wic/plugins/source/bootimg_efi.py    | 5 +++--
>  src/wic/plugins/source/bootimg_pcbios.py | 6 ++++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/wic/plugins/source/bootimg_efi.py b/src/wic/plugins/source/bootimg_efi.py
> index 69aa38b..7d2175d 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 = f"root={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 += f" root={creator.rootdev}" if creator.rootdev else ""
>              if label:
>                  label_conf = "LABEL=%s" % label
> 
> diff --git a/src/wic/plugins/source/bootimg_pcbios.py b/src/wic/plugins/source/bootimg_pcbios.py
> index 1e5ec3a..b49d48d 100644
> --- a/src/wic/plugins/source/bootimg_pcbios.py
> +++ b/src/wic/plugins/source/bootimg_pcbios.py
> @@ -231,8 +231,10 @@ 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
> +            root_param = f"root={creator.rootdev}" if creator.rootdev else ""
> +
> +            syslinux_conf += f"APPEND label=boot {root_param} {bootloader.append}\n"

3. This change leaves a double space when rootdev is None. Build the
   line from only the populated tokens (e.g. " ".join(parts)) so the
   line looks the same whether or no root= is present

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