[PATCH v2 1/1] copy dtbs into a subdirectory based on image name

[Nicusor Liviu Huhulea]

From: Nicusor Huhulea <nicusor...@siemens.com>

There are cases when multiple images are build and because the same dtbs
are copied in deploydir we have a build failure. The build error is related
to files that are installed into a shared area where those files already exists.

To solve this situation we will copy each dtb in a subdirectory based on
image name thus avoiding the conflicts. Therefore the other areas needs to be
updated on the dtbs paths.


Signed-off-by: Nicusor Huhulea <nicusor...@siemens.com>
---
meta/classes/image.bbclass | 21 +++++++++++--------
meta/classes/imagetypes_wic.bbclass | 2 +-
.../wic/plugins/source/bootimg-efi-isar.py | 8 +++++--
scripts/lib/wic/plugins/source/bootimg-efi.py | 8 +++++--
.../wic/plugins/source/bootimg-partition.py | 10 ++++++++-
5 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 98741da0..e90f8fde 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -364,16 +364,19 @@ do_copy_boot_files() {
fi
fi

- for file in ${DTB_FILES}; do
- dtb="$(find '${IMAGE_ROOTFS}/usr/lib' -type f \
- -iwholename '*linux-image-*/'${file} | head -1)"
-
- if [ -z "$dtb" -o ! -e "$dtb" ]; then
- die "${file} not found"
- fi
+ if [ -n "${DTB_FILES}" ]; then
+ mkdir -p "${DEPLOYDIR}/${IMAGE_FULLNAME}.dtbs"
+ for file in ${DTB_FILES}; do
+ dtb="$(find '${IMAGE_ROOTFS}/usr/lib' -type f \
+ -iwholename '*linux-image-*/'${file} | head -1)"
+
+ if [ -z "$dtb" -o ! -e "$dtb" ]; then
+ die "${file} not found"
+ fi

- cp -f "$dtb" "${DEPLOYDIR}/"
- done
+ cp -f "$dtb" "${DEPLOYDIR}/${IMAGE_FULLNAME}.dtbs"
+ done
+ fi
}
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 bce881ed..adbde400 100644
--- a/meta/classes/imagetypes_wic.bbclass
+++ b/meta/classes/imagetypes_wic.bbclass
@@ -107,7 +107,7 @@ WICVARS += "\
ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR TARGET_SYS TRANSLATED_TARGET_ARCH"

# Isar specific vars used in our plugins
-WICVARS += "DISTRO DISTRO_ARCH"
+WICVARS += "DISTRO DISTRO_ARCH IMAGE_FULLNAME"

python do_rootfs_wicenv () {
wicvars = d.getVar('WICVARS')
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 4bfb70a0..218a7fe7 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")
+ dtbs_dir = os.path.join(bootimg_dir + "/" + image_fullname + ".dtbs")
+ cp_cmd = "cp %s/%s %s" % (dtbs_dir, dtb, hdddir)
exec_cmd(cp_cmd, True)

@classmethod
@@ -353,8 +355,10 @@ class BootimgEFIPlugin(SourcePlugin):
if dtb:
if ';' in dtb:
raise WicError("Only one DTB supported, exiting")
+ image_fullname = get_bitbake_var("IMAGE_FULLNAME")
+ dtbs_dir = os.path.join(deploy_dir + "/" + image_fullname + ".dtbs")
dtb_params = '--add-section .dtb=%s/%s --change-section-vma .dtb=0x40000' % \
- (deploy_dir, dtb)
+ (dtbs_dir, dtb)
else:
dtb_params = ''

diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
index 634a808d..07b177df 100644
--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
@@ -51,7 +51,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")
+ dtbs_dir = os.path.join(bootimg_dir + "/" + image_fullname + ".dtbs")
+ cp_cmd = "cp %s/%s %s" % (dtbs_dir, dtb, hdddir)
exec_cmd(cp_cmd, True)

@classmethod
@@ -334,8 +336,10 @@ class BootimgEFIPlugin(SourcePlugin):
if dtb:
if ';' in dtb:
raise WicError("Only one DTB supported, exiting")
+ image_fullname = get_bitbake_var("IMAGE_FULLNAME")
+ dtbs_dir = os.path.join(deploy_dir + "/" + image_fullname + ".dtbs")
dtb_params = '--add-section .dtb=%s/%s --change-section-vma .dtb=0x40000' % \
- (deploy_dir, dtb)
+ (dtbs_dir, dtb)
else:
dtb_params = ''

diff --git a/scripts/lib/wic/plugins/source/bootimg-partition.py b/scripts/lib/wic/plugins/source/bootimg-partition.py
index 5dbe2558..1ae6216f 100644
--- a/scripts/lib/wic/plugins/source/bootimg-partition.py
+++ b/scripts/lib/wic/plugins/source/bootimg-partition.py
@@ -180,10 +180,18 @@ class BootimgPartitionPlugin(SourcePlugin):

logger.debug('Kernel dir: %s', bootimg_dir)

+ image_fullname = get_bitbake_var("IMAGE_FULLNAME")
+ dtbs_dir = os.path.join(kernel_dir + "/" + image_fullname + ".dtbs/")

for task in cls.install_task:
src_path, dst_path = task
- logger.debug('Install %s as %s', src_path, dst_path)
+
+ dtb_file = os.path.join(dtbs_dir + os.path.basename(src_path))
+
+ if os.path.exists(dtb_file):
+ src_path = os.path.join(dtbs_dir + src_path)
+
+ logger.debug('Install %s as %s', (src_path, dst_path))
install_cmd = "install -m 0644 -D %s %s" \
% (os.path.join(kernel_dir, src_path),
os.path.join(hdddir, dst_path))
--
2.39.2

[Gylstorff Quirin]

On 4/16/24 12:07 PM, Nicusor Liviu Huhulea wrote:
> From: Nicusor Huhulea <nicusor...@siemens.com>
>
> There are cases when multiple images are build and because the same dtbs
> are copied in deploydir we have a build failure. The build error is related
> to files that are installed into a shared area where those files already exists.
>
> To solve this situation we will copy each dtb in a subdirectory based on
> image name thus avoiding the conflicts. Therefore the other areas needs to be
> updated on the dtbs paths.
>

This is a special use case, which breaks all downstream layers which
currently use devicetrees. Coping to the directory dtbs should be
disabled by default as for most uses it is unnecessary.

Quirin

[Uladzimir Bely]

On Tue, 2024-04-16 at 17:36 +0200, 'Gylstorff Quirin' via isar-users
wrote:

>
>
> On 4/16/24 12:07 PM, Nicusor Liviu Huhulea wrote:
> > From: Nicusor Huhulea <nicusor...@siemens.com>
> >
> > There are cases when multiple images are build and because the same
> > dtbs
> > are copied in deploydir we have a build failure. The build error is
> > related
> > to files that are installed into a shared area where those files
> > already exists.
> >
> > To solve this situation we will copy each dtb in a subdirectory
> > based on
> > image name thus avoiding the conflicts. Therefore the other areas
> > needs to be
> > updated on the dtbs paths.
> >
>
> This is a special use case, which breaks all downstream layers which
> currently use devicetrees. Coping to the directory dtbs should be
> disabled by default as for most uses it is unnecessary.
>

As an idea - could it be possible to move dtb file deployment to the
kernel recipe?

We have the conflict in deployment when, e.g., "debug" and "base"
images are built at the same time. But both them use same DTB files
that kernel recipe built and put into /usr/lib/linux-image-${krel}. Why
not make the kernel recipe responcible for deployment of DTB files
then?

On the other hand, this case won't work if we use prebuilt kernel/dtb
packages.

[Baurzhan Ismagulov]

On 2024-04-16 17:36, 'Gylstorff Quirin' via isar-users wrote:
> On 4/16/24 12:07 PM, Nicusor Liviu Huhulea wrote:
> > There are cases when multiple images are build and because the same dtbs
> > are copied in deploydir we have a build failure. The build error is related
> > to files that are installed into a shared area where those files already exists.
> >
> > To solve this situation we will copy each dtb in a subdirectory based on
> > image name thus avoiding the conflicts. Therefore the other areas needs to be
> > updated on the dtbs paths.
>
> This is a special use case, which breaks all downstream layers which
> currently use devicetrees. Coping to the directory dtbs should be disabled
> by default as for most uses it is unnecessary.

The origin of the problem is bitbake QA with multiple images. The kernel recipe
builds the dtb which is deployed by the image. In case of multiple images, the
dtbs are considered as different from bitbake PoV and it complains about it --
this is the problem we are trying to address. In our experience, multiple
images are common -- and we would still need a solution for a supported feature
even if they weren't.

From the user PoV, multiple directories with the same dtbs are not necessary at
all. So, I'm not a fan of the per-image dtb approach although we also have a
similar patch to address the immediate failures that we are experiencing. If we
go with this approach, I wouldn't welcome switching per-image dtb on and off --
we already have many variables and are missing coverage for untested
combinations; maybe some configurable deployment path at most.

That said, I think we should really try to bringe the bitbake model closer to
the reality and look at moving this to the kernel recipe as Uladzimir has
suggested. Maybe there are cases that need to be verified (like different
kernels building the same dtbs) and might still need further adjustments.

With kind regards,
Baurzhan

[nicusor...@siemens.com]

The idea of the kernel's recipe being responsible for the deployment of the dtbs was explored but it just did not seem something straight forward.

Since this problem appeared, several possible solutions have been exposed by different users in different threads, and I just chose one that I believe is the best and has the least interference:
1. changing the bitbake variable DEPLOY_DIR_IMAGE
2. no copy of the dtbs of they are the same
3. change the dtb filename based on the image name
4. move the dtbs into a subdirectory based on image name
5. kernel's recipe should handle this
...(there can be more)

So, each has advantages and disadvantages. What I have implemented just keeps in line with what is currently present. The disadvantages are those dtbs path changes. A case that Jan mentioned about a dirty deploy dir with a different config for the same machine/distro and a need for the down layers to modify the dtb paths as Quirin mentioned.
Having the same dtbs in different folder I don't consider it a disadvantage as there can be cases where the same dtb name implement different functionality.

Kernel's recipe solution seems much feasible but I don't know the implications on it. We can consider this as a transitory solution until a better one is found

Nicu













________________________________________
From: Uladzimir Bely <ub...@ilbers.de>
Sent: Wednesday, April 17, 2024 10:59 AM
To: Gylstorff, Quirin (T CED OES-DE); Nicusor Liviu Huhulea; isar-...@googlegroups.com
Cc: Huhulea, Nicusor Liviu (DI CTO FDS CES LX SVCS)
Subject: Re: [PATCH v2 1/1] copy dtbs into a subdirectory based on image name

[Gylstorff Quirin]

For me the kernel approach would be fine - from the looks the kernel
provides the DTB in yocto:
https://git.yoctoproject.org/poky/tree/meta/classes-recipe/devicetree.bbclass

Best regards
Quirin

> With kind regards,
> Baurzhan
>