diff mbox series

image: copy DTB_FILES to DEPLOYDIR/IMAGE_FULLNAME/KERNEL_NAME/

Message ID 20250403170701.807745-1-badrikesh.prusty@siemens.com
State New
Headers show
Series image: copy DTB_FILES to DEPLOYDIR/IMAGE_FULLNAME/KERNEL_NAME/ | expand

Commit Message

Badrikesh Prusty April 3, 2025, 5:07 p.m. UTC 
From: badrikesh prusty <badrikesh.prusty@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.

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 <badrikesh.prusty@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(-)

Comments

Jan Kiszka April 4, 2025, 9:55 a.m. UTC | #1 
On 03.04.25 19:07, 'Badrikesh Prusty' via isar-users wrote:
> From: badrikesh prusty <badrikesh.prusty@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?

>
> 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 <badrikesh.prusty@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_FILES').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
Uladzimir Bely April 4, 2025, 2:31 p.m. UTC | #2 
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 <badrikesh.prusty@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?
> 

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

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 <badrikesh.prusty@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
diff mbox series

Patch

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