Re: [PATCH] image: copy DTB_FILES to DEPLOYDIR/IMAGE_FULLNAME/KERNEL_NAME/

["'Badrikesh Prusty' via isar-users" <isar-users@googlegroups.com>]

[-- Attachment #1.1: Type: text/plain, Size: 11648 bytes --]

Hello all,

Apologies for the delayed response. I somehow missed the message.

> This sounds familiar, and I think we discussed that multiple times
> already. Did you study previous threads/patches, e.g.
> https://patchwork.isar-build.org/project/isar/list/?series=1241?
>
> Yes, there were few patches that tried to solve the issue.
>
> Using subdirectory (variant similar to TS's)
> - https://patchwork.isar-build.org/project/isar/list/?series=1149
> Allows to overlap files
> - https://patchwork.isar-build.org/project/isar/list/?series=1235
> Make kernel deploy the files, instead of image recipes
> - https://patchwork.isar-build.org/project/isar/list/?series=1241
>

I reviewed the previously shared patch, but not in full — thanks for 
sharing it here.

The patch I sent is very similar to @nicusor 
patch: https://patchwork.isar-build.org/project/isar/list/?series=1149

I just reviewed the other patches and believe that many may encounter the 
same issue. For instance, when attempting to build multiple kernels for a 
single image—such as `linux-image-amd64` and `linux-image-amd64-rt` 
kernels—this overwrite problem is likely to occur.

This patch also resolves the issue with image builds that include multiple 
kernels by storing the DTB files in the directory 
DEPLOYDIR/IMAGE_FULLNAME/KERNEL_NAME. This will also fix issues when 
building with different DISTROs as IMAGE_FULLNAME includes target DISTRO. 
Additionally, it fixes problems that arise when building with different 
DISTROs, since IMAGE_FULLNAME includes the target DISTRO.

> This would affect downstream as well. You are missing a
> RECIPE-API-CHANGELOG.md entry.
>

Yes, it will affect downstream layers. E.g. I checked with the 
isar-cip-core repo, changes will be required in `efibootguard-boot` wic 
plugin. I will work on fixing it, once this is approved.

Thanks and Regards,
Badrikesh
On Thursday, September 4, 2025 at 7:42:08 PM UTC+5:30 Jan Kiszka wrote:

> On 04.09.25 11:36, Arulpandiyan Vadivel wrote:
> > @Uladzimir, @Jan Kiszka
> > 
> > As 0.11 release was out  already, Can we consider merging this change?
> > 
>
> Not in its current form as we explained. Plus you didn't answer the
> question I gave you.
>
> Jan
>
> > 
> > Regards,
> > Arulpandiyan V
> > 
> > On Friday, 4 April 2025 at 20:01:24 UTC+5:30 Uladzimir Bely wrote:
> > 
> > On Fri, 2025-04-04 at 11:55 +0200, Jan Kiszka wrote:
> > > On 03.04.25 19:07, 'Badrikesh Prusty' via isar-users wrote:
> > > > From: badrikesh prusty <badrikes...@siemens.com>
> > > >
> > > > Copy DTB_FILES to DEPLOYDIR/IMAGE_FULLNAME/KERNEL_NAME instead of
> > > > DEPLOYDIR/.
> > > >
> > > > An issue is observed when we attempt to build a second image for a
> > > > machine
> > > > utilizing DTB_FILES. During the do_copy_boot_files task of image
> > > > creation, the
> > > > DTB_FILES are copied to the shared location DEPLOYDIR/<dtb files>.
> > > > When the
> > > > build of a second image is triggered, it detects that the DTB_FILES
> > > > are already
> > > > present and avoids overwriting them.
> > >
> > > This sounds familiar, and I think we discussed that multiple times
> > > already. Did you study previous threads/patches, e.g.
> > > https://patchwork.isar-build.org/project/isar/list/?series=1241
> > <https://patchwork.isar-build.org/project/isar/list/?series=1241>?
> > >
> > 
> > Yes, there were few patches that tried to solve the issue.
> > 
> > Using subdirectory (variant similar to TS's)
> > - https://patchwork.isar-build.org/project/isar/list/?series=1149
> > <https://patchwork.isar-build.org/project/isar/list/?series=1149>
> > Allows to overlap files
> > - https://patchwork.isar-build.org/project/isar/list/?series=1235
> > <https://patchwork.isar-build.org/project/isar/list/?series=1235>
> > Make kernel deploy the files, instead of image recipes
> > - https://patchwork.isar-build.org/project/isar/list/?series=1241
> > <https://patchwork.isar-build.org/project/isar/list/?series=1241>
> > 
> > Finally, there is now perfect solution that would solve all possible
> > issues and wouldn't break compatibility with downstreams...
> > 
> > Anyway, we are planning to prepare Isar release 0.11 without the issue
> > fix (no downstream breaking changes) and apply some (or mixed) solution
> > after that, even at the cost of downstreams compatibility.
> > 
> > 
> > > >
> > > > Reproducer:
> > > > bitbake mc:phyboard-mira-bookworm:isar-image-base
> > > > bitbake mc:phyboard-mira-bookworm:isar-image-debug
> > > >
> > > > Copy the DTB_FILES to the directory:
> > > > DEPLOYDIR/IMAGE_FULLNAME/KERNEL_NAME/.
> > > > * This will allow building multiple images.
> > > > * As each kernel recipe ships its own DTB_FILES, if a user tries to
> > > > rebuild
> > > > the same image with a new kernel, the DTB_FILES associated with the
> > > > older
> > > > kernel will not be overwritten.
> > > >
> > > > Update the DTB_IMG variable to check for DTB_FILES in their new
> > > > location.
> > > > Update the WIC plugin scripts to use the DTB_FILES from their
> > > > updated location
> > > > within the DEPLOYDIR.
> > > > Add IMAGE_FULLNAME and KERNEL_NAME to WICVARS to allow the scripts
> > > > to retrieve
> > > > these variable values during WIC image generation.
> > > >
> > > > Signed-off-by: badrikesh prusty <badrikes...@siemens.com>
> > > > ---
> > > >  meta/classes/image.bbclass                              | 5 +++--
> > > >  meta/classes/imagetypes_wic.bbclass                     | 6 +++---
> > > >  meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py | 4 +++-
> > > >  scripts/lib/wic/plugins/source/bootimg-efi.py           | 4 +++-
> > > >  4 files changed, 12 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/meta/classes/image.bbclass
> > > > b/meta/classes/image.bbclass
> > > > index ff3cd737..218e7c35 100644
> > > > --- a/meta/classes/image.bbclass
> > > > +++ b/meta/classes/image.bbclass
> > > > @@ -326,7 +326,7 @@ EOF
> > > >  KERNEL_IMG = "${PP_DEPLOY}/${KERNEL_IMAGE}"
> > > >  INITRD_IMG = "${PP_DEPLOY}/${INITRD_DEPLOY_FILE}"
> > > >  # only one dtb file supported, pick the first
> > > > -DTB_IMG = "${PP_DEPLOY}/${@(d.getVar('DTB_FILES').split() or
> > > > [''])[0]}"
> > > > +DTB_IMG =
> > > > "${PP_DEPLOY}/${IMAGE_FULLNAME}/${KERNEL_NAME}/${@(d.getVar('DTB_FI
> > > > LES').split() or [''])[0]}"
> > > >
> > > >  do_copy_boot_files[cleandirs] += "${DEPLOYDIR}"
> > > >  do_copy_boot_files[sstate-inputdirs] = "${DEPLOYDIR}"
> > > > @@ -360,7 +360,8 @@ do_copy_boot_files() {
> > > >              die "${file} not found"
> > > >          fi
> > > >
> > > > -        cp -f "$dtb" "${DEPLOYDIR}/"
> > > > +        mkdir -p "${DEPLOYDIR}/${IMAGE_FULLNAME}/${KERNEL_NAME}"
> > > > +        cp -f "$dtb"
> > > > "${DEPLOYDIR}/${IMAGE_FULLNAME}/${KERNEL_NAME}/"
> > > >      done
> > > >  }
> > > >  addtask copy_boot_files before do_rootfs_postprocess after
> > > > do_rootfs_install
> > > > diff --git a/meta/classes/imagetypes_wic.bbclass
> > > > b/meta/classes/imagetypes_wic.bbclass
> > > > index 7a050e73..6b7b3727 100644
> > > > --- a/meta/classes/imagetypes_wic.bbclass
> > > > +++ b/meta/classes/imagetypes_wic.bbclass
> > > > @@ -102,9 +102,9 @@ WIC_DEPLOY_PARTITIONS ?= "0"
> > > >
> > > >  # taken from OE, do not touch directly
> > > >  WICVARS += "\
> > > > -           BBLAYERS IMGDEPLOYDIR DEPLOY_DIR_IMAGE FAKEROOTCMD
> > > > IMAGE_BASENAME IMAGE_BOOT_FILES IMAGE_EFI_BOOT_FILES \
> > > > -           IMAGE_LINK_NAME IMAGE_ROOTFS INITRAMFS_FSTYPES INITRD
> > > > INITRD_LIVE ISODIR RECIPE_SYSROOT_NATIVE \
> > > > -           ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR
> > > > TARGET_SYS TRANSLATED_TARGET_ARCH"
> > > > +           BBLAYERS IMGDEPLOYDIR DEPLOY_DIR_IMAGE FAKEROOTCMD
> > > > IMAGE_BASENAME IMAGE_FULLNAME IMAGE_BOOT_FILES \
> > > > +           IMAGE_EFI_BOOT_FILES IMAGE_LINK_NAME IMAGE_ROOTFS
> > > > INITRAMFS_FSTYPES INITRD INITRD_LIVE ISODIR KERNEL_NAME \
> > > > +           RECIPE_SYSROOT_NATIVE ROOTFS_SIZE STAGING_DATADIR
> > > > STAGING_DIR STAGING_LIBDIR TARGET_SYS TRANSLATED_TARGET_ARCH"
> > > >
> > > >  # Isar specific vars used in our plugins
> > > >  WICVARS += "DISTRO DISTRO_ARCH KERNEL_FILE"
> > > > 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 78ae4fb2..5b23671f 100644
> > > > --- a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> > > > +++ b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> > > > @@ -57,7 +57,9 @@ class BootimgEFIPlugin(SourcePlugin):
> > > >          if dtb:
> > > >              if ';' in dtb:
> > > >                  raise WicError("Only one DTB supported, exiting")
> > > > -            cp_cmd = "cp %s/%s %s" % (bootimg_dir, dtb, hdddir)
> > > > +            image_fullname = get_bitbake_var("IMAGE_FULLNAME")
> > > > +            kernel_name = get_bitbake_var("KERNEL_NAME")
> > > > +            cp_cmd = "cp %s/%s/%s/%s %s" % (bootimg_dir,
> > > > image_fullname, kernel_name, dtb, hdddir)
> > > >              exec_cmd(cp_cmd, True)
> > > >
> > > >      @classmethod
> > > > diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py
> > > > b/scripts/lib/wic/plugins/source/bootimg-efi.py
> > > > index 13a9cddf..cf83a6f3 100644
> > > > --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
> > > > +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
> > > > @@ -368,7 +368,9 @@ class BootimgEFIPlugin(SourcePlugin):
> > > >                  if dtb:
> > > >                      if ';' in dtb:
> > > >                          raise WicError("Only one DTB supported,
> > > > exiting")
> > > > -                    dtb_path = "%s/%s" % (deploy_dir, dtb)
> > > > +                    image_fullname =
> > > > get_bitbake_var("IMAGE_FULLNAME")
> > > > +                    kernel_name = get_bitbake_var("KERNEL_NAME")
> > > > +                    dtb_path = "%s/%s/%s/%s" % (deploy_dir,
> > > > image_fullname, kernel_name, dtb)
> > > >                      dtb_params = '--add-section .dtb=%s --change-
> > > > section-vma .dtb=0x%x' % \
> > > >                              (dtb_path, dtb_off)
> > > >                      linux_off = dtb_off +
> > > > os.stat(dtb_path).st_size
> > >
> > > This would affect downstream as well. You are missing a
> > > RECIPE-API-CHANGELOG.md entry.
> > >
> > > Jan
> > 
> > -- 
> > Best regards,
> > Uladzimir.
> > 
> > 
> > -- 
> > 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+...@googlegroups.com <mailto:isar-
> > users+un...@googlegroups.com>.
> > To view this discussion visit https://groups.google.com/d/msgid/isar-
> > users/95421431-bec5-4297-8aba-2bd9207f1785n%40googlegroups.com <https://
> > groups.google.com/d/msgid/isar-users/95421431-
> > bec5-4297-8aba-2bd9207f1785n%40googlegroups.com?
> > utm_medium=email&utm_source=footer>.
>
>
> -- 
> 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/1a2a38b0-49aa-4167-a95e-16f4c5ba385dn%40googlegroups.com.

[-- Attachment #1.2: Type: text/html, Size: 20213 bytes --]