[PATCHv2] wic: fix images build in parallel

[PATCHv2] wic: fix images build in parallel

[Maxim Uvarov]

OE wic plugins create temporary file with the index of the line
tmp file name. This causes race in case several builds run in time.
Add more entropy as timestamp to remove this race.

Suggested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 v2: shortlog change to match template.

 scripts/lib/wic/plugins/source/bootimg-partition.py | 5 +++--
 scripts/lib/wic/plugins/source/bootimg-pcbios.py    | 3 ++-
 scripts/lib/wic/plugins/source/rawcopy.py           | 3 ++-
 scripts/lib/wic/plugins/source/rootfs.py            | 3 ++-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/wic/plugins/source/bootimg-partition.py b/scripts/lib/wic/plugins/source/bootimg-partition.py
index 138986a71e..b6cea1096a 100644
--- a/scripts/lib/wic/plugins/source/bootimg-partition.py
+++ b/scripts/lib/wic/plugins/source/bootimg-partition.py
@@ -13,6 +13,7 @@
 import logging
 import os
 import re
+import time
 
 from glob import glob
 
@@ -38,7 +39,7 @@ class BootimgPartitionPlugin(SourcePlugin):
         """
         Called before do_prepare_partition(), create u-boot specific boot config
         """
-        hdddir = "%s/boot.%d" % (cr_workdir, part.lineno)
+        hdddir = "%s/boot.%d" % (cr_workdir, part.lineno, time.time())
         install_cmd = "install -d %s" % hdddir
         exec_cmd(install_cmd)
 
@@ -171,7 +172,7 @@ class BootimgPartitionPlugin(SourcePlugin):
         - sets up a vfat partition
         - copies all files listed in IMAGE_BOOT_FILES variable
         """
-        hdddir = "%s/boot.%d" % (cr_workdir, part.lineno)
+        hdddir = "%s/boot.%d_%s" % (cr_workdir, part.lineno, time.time())
 
         if not kernel_dir:
             kernel_dir = get_bitbake_var("DEPLOY_DIR_IMAGE")
diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
index f2639e7004..8ac73c2067 100644
--- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
+++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
@@ -13,6 +13,7 @@
 import logging
 import os
 import re
+import time
 
 from wic import WicError
 from wic.engine import get_custom_config
@@ -184,7 +185,7 @@ class BootimgPcbiosPlugin(SourcePlugin):
                      extra_blocks, part.mountpoint, blocks)
 
         # dosfs image, created by mkdosfs
-        bootimg = "%s/boot%s.img" % (cr_workdir, part.lineno)
+        bootimg = "%s/boot%s_%s.img" % (cr_workdir, part.lineno, time.time())
 
         dosfs_cmd = "mkdosfs -n boot -i %s -S 512 -C %s %d" % \
                     (part.fsuuid, bootimg, blocks)
diff --git a/scripts/lib/wic/plugins/source/rawcopy.py b/scripts/lib/wic/plugins/source/rawcopy.py
index 82970ce51b..9ada3d39c9 100644
--- a/scripts/lib/wic/plugins/source/rawcopy.py
+++ b/scripts/lib/wic/plugins/source/rawcopy.py
@@ -4,6 +4,7 @@
 
 import logging
 import os
+import time
 
 from wic import WicError
 from wic.pluginbase import SourcePlugin
@@ -57,7 +58,7 @@ class RawCopyPlugin(SourcePlugin):
             raise WicError("No file specified")
 
         src = os.path.join(kernel_dir, source_params['file'])
-        dst = os.path.join(cr_workdir, "%s.%s" % (source_params['file'], part.lineno))
+        dst = os.path.join(cr_workdir, "%s.%s_%s" % (source_params['file'], part.lineno, time.time()))
 
         if not os.path.exists(os.path.dirname(dst)):
             os.makedirs(os.path.dirname(dst))
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index 705aeb5563..37ebee89ea 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -15,6 +15,7 @@ import logging
 import os
 import shutil
 import sys
+import time
 
 from oe.path import copyhardlinktree
 
@@ -74,7 +75,7 @@ class RootfsPlugin(SourcePlugin):
         if part.exclude_path or part.include_path:
             # We need a new rootfs directory we can delete files from. Copy to
             # workdir.
-            new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d" % part.lineno))
+            new_rootfs = os.path.realpath(os.path.join(cr_workdir, "rootfs%d_%s" % (part.lineno, time.time())))
 
             if os.path.lexists(new_rootfs):
                 shutil.rmtree(os.path.join(new_rootfs))
-- 
2.17.1

Re: [PATCHv2] wic: fix images build in parallel

[Paul Barker]

On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> OE wic plugins create temporary file with the index of the line
> tmp file name. This causes race in case several builds run in time.
> Add more entropy as timestamp to remove this race.

How would two wic images to be built in parallel with the same work
directory? To my understanding an image recipe only supports building
a single wic image.

Thanks,
Paul

Re: [PATCHv2] wic: fix images build in parallel

[Maxim Uvarov]

On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
>
> On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > OE wic plugins create temporary file with the index of the line
> > tmp file name. This causes race in case several builds run in time.
> > Add more entropy as timestamp to remove this race.
>
> How would two wic images to be built in parallel with the same work
> directory? To my understanding an image recipe only supports building
> a single wic image.
>
> Thanks,
> Paul

bitbake image1 image2 image3
all images build .wics and use about the same files, like firmware.
Issue is similar to that:
https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html

Maxim.

Re: [PATCHv2] wic: fix images build in parallel

[Paul Barker]

On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > OE wic plugins create temporary file with the index of the line
> > > tmp file name. This causes race in case several builds run in time.
> > > Add more entropy as timestamp to remove this race.
> >
> > How would two wic images to be built in parallel with the same work
> > directory? To my understanding an image recipe only supports building
> > a single wic image.
> >
> > Thanks,
> > Paul
>
> bitbake image1 image2 image3
> all images build .wics and use about the same files, like firmware.
> Issue is similar to that:
> https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html

Each image has its own work directory though.

I'll take a look in more detail later today or tomorrow, if wic is
writing temporary files outside of the work directory then that's a
bug and should be fixed.

Re: [PATCHv2] wic: fix images build in parallel

[Maxim Uvarov]

On Mon, 13 Jan 2020 at 17:00, Paul Barker <pbarker@konsulko.com> wrote:
>
> On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> > >
> > > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > >
> > > > OE wic plugins create temporary file with the index of the line
> > > > tmp file name. This causes race in case several builds run in time.
> > > > Add more entropy as timestamp to remove this race.
> > >
> > > How would two wic images to be built in parallel with the same work
> > > directory? To my understanding an image recipe only supports building
> > > a single wic image.
> > >
> > > Thanks,
> > > Paul
> >
> > bitbake image1 image2 image3
> > all images build .wics and use about the same files, like firmware.
> > Issue is similar to that:
> > https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html
>
> Each image has its own work directory though.
>
> I'll take a look in more detail later today or tomorrow, if wic is
> writing temporary files outside of the work directory then that's a
> bug and should be fixed.

Thanks. I saw bug in rawcopy plugin. All other places were patched due
to the same code.  I guess then for rawcopy temp files are in
DEST_IMAGE_DIR (which is common) instead of WORKDIR.

Maxim.

Re: [PATCHv2] wic: fix images build in parallel

[Paul Barker]

On Mon, 13 Jan 2020 at 14:12, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> On Mon, 13 Jan 2020 at 17:00, Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> > > >
> > > > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > >
> > > > > OE wic plugins create temporary file with the index of the line
> > > > > tmp file name. This causes race in case several builds run in time.
> > > > > Add more entropy as timestamp to remove this race.
> > > >
> > > > How would two wic images to be built in parallel with the same work
> > > > directory? To my understanding an image recipe only supports building
> > > > a single wic image.
> > > >
> > > > Thanks,
> > > > Paul
> > >
> > > bitbake image1 image2 image3
> > > all images build .wics and use about the same files, like firmware.
> > > Issue is similar to that:
> > > https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html
> >
> > Each image has its own work directory though.
> >
> > I'll take a look in more detail later today or tomorrow, if wic is
> > writing temporary files outside of the work directory then that's a
> > bug and should be fixed.
>
> Thanks. I saw bug in rawcopy plugin. All other places were patched due
> to the same code.  I guess then for rawcopy temp files are in
> DEST_IMAGE_DIR (which is common) instead of WORKDIR.

I can't see how these files can possibly collide across parallel image
builds. In the lines you changed in your patch, cr_workdir passed in
as an argument set to imager.workdir (in
scripts/lib/wic/plugins/imager/direct.py) which is a temporary
directory created under options.outdir. image_types_wic.bbclass passes
the -o option to wic to set the outdir to something under ${WORKDIR}.
And different images have different WORKDIR paths. So things should be
safe.

Which branch & version of poky or oe-core are you using?

DEST_IMAGE_DIR is not defined in openembedded-core. Are you sure
that's the right variable name?

Could you provide full bitbake output showing the exact error message
when this collision happens?

And lastly if you want to look into this in more detail maybe add some
print statements just after each of those lines you patched to print
the full paths used so we can see if they step outside WORKDIR. I
think doing this on your side with your exact wks files that are
causing errors will give the most useful output.

Thanks,
Paul

Re: [PATCHv2] wic: fix images build in parallel

[Maxim Uvarov]

On Fri, 17 Jan 2020 at 13:18, Paul Barker <pbarker@konsulko.com> wrote:
>
> On Mon, 13 Jan 2020 at 14:12, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > On Mon, 13 Jan 2020 at 17:00, Paul Barker <pbarker@konsulko.com> wrote:
> > >
> > > On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > >
> > > > On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> > > > >
> > > > > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > >
> > > > > > OE wic plugins create temporary file with the index of the line
> > > > > > tmp file name. This causes race in case several builds run in time.
> > > > > > Add more entropy as timestamp to remove this race.
> > > > >
> > > > > How would two wic images to be built in parallel with the same work
> > > > > directory? To my understanding an image recipe only supports building
> > > > > a single wic image.
> > > > >
> > > > > Thanks,
> > > > > Paul
> > > >
> > > > bitbake image1 image2 image3
> > > > all images build .wics and use about the same files, like firmware.
> > > > Issue is similar to that:
> > > > https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html
> > >
> > > Each image has its own work directory though.
> > >
> > > I'll take a look in more detail later today or tomorrow, if wic is
> > > writing temporary files outside of the work directory then that's a
> > > bug and should be fixed.
> >
> > Thanks. I saw bug in rawcopy plugin. All other places were patched due
> > to the same code.  I guess then for rawcopy temp files are in
> > DEST_IMAGE_DIR (which is common) instead of WORKDIR.
>
> I can't see how these files can possibly collide across parallel image
> builds. In the lines you changed in your patch, cr_workdir passed in
> as an argument set to imager.workdir (in
> scripts/lib/wic/plugins/imager/direct.py) which is a temporary
> directory created under options.outdir. image_types_wic.bbclass passes
> the -o option to wic to set the outdir to something under ${WORKDIR}.
> And different images have different WORKDIR paths. So things should be
> safe.
>
> Which branch & version of poky or oe-core are you using?
>

zeus

> DEST_IMAGE_DIR is not defined in openembedded-core. Are you sure
> that's the right variable name?
>

DEPLOY_DIR_IMAGE

> Could you provide full bitbake output showing the exact error message
> when this collision happens?

See traceback here:
https://ci.linaro.org/job/ledge-oe/357/DISTRO=rpb,MACHINE=ledge-stm32mp157c-dk2,label=docker-stretch-amd64/console

>
> And lastly if you want to look into this in more detail maybe add some
> print statements just after each of those lines you patched to print
> the full paths used so we can see if they step outside WORKDIR. I
> think doing this on your side with your exact wks files that are
> causing errors will give the most useful output.
>

Is there a good way to add debug messages and see then on console,
other then rise WicError() ?

Maxim.

> Thanks,
> Paul

Re: [PATCHv2] wic: fix images build in parallel

[Paul Barker]

On Fri, 17 Jan 2020 at 11:59, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> On Fri, 17 Jan 2020 at 13:18, Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Mon, 13 Jan 2020 at 14:12, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > On Mon, 13 Jan 2020 at 17:00, Paul Barker <pbarker@konsulko.com> wrote:
> > > >
> > > > On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > >
> > > > > On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > > >
> > > > > > > OE wic plugins create temporary file with the index of the line
> > > > > > > tmp file name. This causes race in case several builds run in time.
> > > > > > > Add more entropy as timestamp to remove this race.
> > > > > >
> > > > > > How would two wic images to be built in parallel with the same work
> > > > > > directory? To my understanding an image recipe only supports building
> > > > > > a single wic image.
> > > > > >
> > > > > > Thanks,
> > > > > > Paul
> > > > >
> > > > > bitbake image1 image2 image3
> > > > > all images build .wics and use about the same files, like firmware.
> > > > > Issue is similar to that:
> > > > > https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html
> > > >
> > > > Each image has its own work directory though.
> > > >
> > > > I'll take a look in more detail later today or tomorrow, if wic is
> > > > writing temporary files outside of the work directory then that's a
> > > > bug and should be fixed.
> > >
> > > Thanks. I saw bug in rawcopy plugin. All other places were patched due
> > > to the same code.  I guess then for rawcopy temp files are in
> > > DEST_IMAGE_DIR (which is common) instead of WORKDIR.
> >
> > I can't see how these files can possibly collide across parallel image
> > builds. In the lines you changed in your patch, cr_workdir passed in
> > as an argument set to imager.workdir (in
> > scripts/lib/wic/plugins/imager/direct.py) which is a temporary
> > directory created under options.outdir. image_types_wic.bbclass passes
> > the -o option to wic to set the outdir to something under ${WORKDIR}.
> > And different images have different WORKDIR paths. So things should be
> > safe.
> >
> > Which branch & version of poky or oe-core are you using?
> >
>
> zeus
>
> > DEST_IMAGE_DIR is not defined in openembedded-core. Are you sure
> > that's the right variable name?
> >
>
> DEPLOY_DIR_IMAGE
>
> > Could you provide full bitbake output showing the exact error message
> > when this collision happens?
>
> See traceback here:
> https://ci.linaro.org/job/ledge-oe/357/DISTRO=rpb,MACHINE=ledge-stm32mp157c-dk2,label=docker-stretch-amd64/console

I think I'm starting to understand now. Is this the wks file?
https://github.com/Linaro/meta-ledge/blob/zeus/meta-ledge-bsp/wic/ledge-stm32mp157c-dk2-optee.wks.in

>
> >
> > And lastly if you want to look into this in more detail maybe add some
> > print statements just after each of those lines you patched to print
> > the full paths used so we can see if they step outside WORKDIR. I
> > think doing this on your side with your exact wks files that are
> > causing errors will give the most useful output.
> >
>
> Is there a good way to add debug messages and see then on console,
> other then rise WicError() ?

In wic you should just be able to use print() and look in the
do_image_wic log file IIRC.

Thanks,
Paul

Re: [PATCHv2] wic: fix images build in parallel

[Paul Barker]

On Fri, 17 Jan 2020 at 12:17, Paul Barker <pbarker@konsulko.com> wrote:
>
> On Fri, 17 Jan 2020 at 11:59, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > On Fri, 17 Jan 2020 at 13:18, Paul Barker <pbarker@konsulko.com> wrote:
> > >
> > > On Mon, 13 Jan 2020 at 14:12, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > >
> > > > On Mon, 13 Jan 2020 at 17:00, Paul Barker <pbarker@konsulko.com> wrote:
> > > > >
> > > > > On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > >
> > > > > > On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > > > >
> > > > > > > > OE wic plugins create temporary file with the index of the line
> > > > > > > > tmp file name. This causes race in case several builds run in time.
> > > > > > > > Add more entropy as timestamp to remove this race.
> > > > > > >
> > > > > > > How would two wic images to be built in parallel with the same work
> > > > > > > directory? To my understanding an image recipe only supports building
> > > > > > > a single wic image.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Paul
> > > > > >
> > > > > > bitbake image1 image2 image3
> > > > > > all images build .wics and use about the same files, like firmware.
> > > > > > Issue is similar to that:
> > > > > > https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html
> > > > >
> > > > > Each image has its own work directory though.
> > > > >
> > > > > I'll take a look in more detail later today or tomorrow, if wic is
> > > > > writing temporary files outside of the work directory then that's a
> > > > > bug and should be fixed.
> > > >
> > > > Thanks. I saw bug in rawcopy plugin. All other places were patched due
> > > > to the same code.  I guess then for rawcopy temp files are in
> > > > DEST_IMAGE_DIR (which is common) instead of WORKDIR.
> > >
> > > I can't see how these files can possibly collide across parallel image
> > > builds. In the lines you changed in your patch, cr_workdir passed in
> > > as an argument set to imager.workdir (in
> > > scripts/lib/wic/plugins/imager/direct.py) which is a temporary
> > > directory created under options.outdir. image_types_wic.bbclass passes
> > > the -o option to wic to set the outdir to something under ${WORKDIR}.
> > > And different images have different WORKDIR paths. So things should be
> > > safe.
> > >
> > > Which branch & version of poky or oe-core are you using?
> > >
> >
> > zeus
> >
> > > DEST_IMAGE_DIR is not defined in openembedded-core. Are you sure
> > > that's the right variable name?
> > >
> >
> > DEPLOY_DIR_IMAGE
> >
> > > Could you provide full bitbake output showing the exact error message
> > > when this collision happens?
> >
> > See traceback here:
> > https://ci.linaro.org/job/ledge-oe/357/DISTRO=rpb,MACHINE=ledge-stm32mp157c-dk2,label=docker-stretch-amd64/console
>
> I think I'm starting to understand now. Is this the wks file?
> https://github.com/Linaro/meta-ledge/blob/zeus/meta-ledge-bsp/wic/ledge-stm32mp157c-dk2-optee.wks.in

Got it. Nasty little bug there!

In the rawcopy source plugin, dst is set as follows:

    dst = os.path.join(cr_workdir, "%s.%s" % (source_params['file'],
part.lineno))

If source_params['file'] is an absolute path (as it is in the wks file
above), the cr_workdir prefix is not applied by os.path.join(). So
instead it writes to a ".1" file next to the original image - this is
outside the WORKDIR and at risk of collision.

The solution is to fix dst above, maybe use
os.path.basename(source_params['file']). Could you give that a try or
do you want me to propose a patch?

Thanks,
Paul

Re: [PATCHv2] wic: fix images build in parallel

[Maxim Uvarov]

On Fri, 17 Jan 2020 at 15:27, Paul Barker <pbarker@konsulko.com> wrote:
>
> On Fri, 17 Jan 2020 at 12:17, Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Fri, 17 Jan 2020 at 11:59, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > >
> > > On Fri, 17 Jan 2020 at 13:18, Paul Barker <pbarker@konsulko.com> wrote:
> > > >
> > > > On Mon, 13 Jan 2020 at 14:12, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > >
> > > > > On Mon, 13 Jan 2020 at 17:00, Paul Barker <pbarker@konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > > >
> > > > > > > On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > OE wic plugins create temporary file with the index of the line
> > > > > > > > > tmp file name. This causes race in case several builds run in time.
> > > > > > > > > Add more entropy as timestamp to remove this race.
> > > > > > > >
> > > > > > > > How would two wic images to be built in parallel with the same work
> > > > > > > > directory? To my understanding an image recipe only supports building
> > > > > > > > a single wic image.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > Paul
> > > > > > >
> > > > > > > bitbake image1 image2 image3
> > > > > > > all images build .wics and use about the same files, like firmware.
> > > > > > > Issue is similar to that:
> > > > > > > https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html
> > > > > >
> > > > > > Each image has its own work directory though.
> > > > > >
> > > > > > I'll take a look in more detail later today or tomorrow, if wic is
> > > > > > writing temporary files outside of the work directory then that's a
> > > > > > bug and should be fixed.
> > > > >
> > > > > Thanks. I saw bug in rawcopy plugin. All other places were patched due
> > > > > to the same code.  I guess then for rawcopy temp files are in
> > > > > DEST_IMAGE_DIR (which is common) instead of WORKDIR.
> > > >
> > > > I can't see how these files can possibly collide across parallel image
> > > > builds. In the lines you changed in your patch, cr_workdir passed in
> > > > as an argument set to imager.workdir (in
> > > > scripts/lib/wic/plugins/imager/direct.py) which is a temporary
> > > > directory created under options.outdir. image_types_wic.bbclass passes
> > > > the -o option to wic to set the outdir to something under ${WORKDIR}.
> > > > And different images have different WORKDIR paths. So things should be
> > > > safe.
> > > >
> > > > Which branch & version of poky or oe-core are you using?
> > > >
> > >
> > > zeus
> > >
> > > > DEST_IMAGE_DIR is not defined in openembedded-core. Are you sure
> > > > that's the right variable name?
> > > >
> > >
> > > DEPLOY_DIR_IMAGE
> > >
> > > > Could you provide full bitbake output showing the exact error message
> > > > when this collision happens?
> > >
> > > See traceback here:
> > > https://ci.linaro.org/job/ledge-oe/357/DISTRO=rpb,MACHINE=ledge-stm32mp157c-dk2,label=docker-stretch-amd64/console
> >
> > I think I'm starting to understand now. Is this the wks file?
> > https://github.com/Linaro/meta-ledge/blob/zeus/meta-ledge-bsp/wic/ledge-stm32mp157c-dk2-optee.wks.in
>
> Got it. Nasty little bug there!
>
> In the rawcopy source plugin, dst is set as follows:
>
>     dst = os.path.join(cr_workdir, "%s.%s" % (source_params['file'],
> part.lineno))
>
> If source_params['file'] is an absolute path (as it is in the wks file
> above), the cr_workdir prefix is not applied by os.path.join(). So
> instead it writes to a ".1" file next to the original image - this is
> outside the WORKDIR and at risk of collision.
>
> The solution is to fix dst above, maybe use
> os.path.basename(source_params['file']). Could you give that a try or
> do you want me to propose a patch?
>
Nice!
I can test it today. Compilation will take some time...

Maxim.

> Thanks,
> Paul

Re: [PATCHv2] wic: fix images build in parallel

[Maxim Uvarov]

works.  v3 is on the way..

On Fri, 17 Jan 2020 at 15:39, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
>
> On Fri, 17 Jan 2020 at 15:27, Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Fri, 17 Jan 2020 at 12:17, Paul Barker <pbarker@konsulko.com> wrote:
> > >
> > > On Fri, 17 Jan 2020 at 11:59, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > >
> > > > On Fri, 17 Jan 2020 at 13:18, Paul Barker <pbarker@konsulko.com> wrote:
> > > > >
> > > > > On Mon, 13 Jan 2020 at 14:12, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > >
> > > > > > On Mon, 13 Jan 2020 at 17:00, Paul Barker <pbarker@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Mon, 13 Jan 2020 at 13:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > > > >
> > > > > > > > On Mon, 13 Jan 2020 at 16:31, Paul Barker <pbarker@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Mon, 13 Jan 2020 at 13:08, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> > > > > > > > > >
> > > > > > > > > > OE wic plugins create temporary file with the index of the line
> > > > > > > > > > tmp file name. This causes race in case several builds run in time.
> > > > > > > > > > Add more entropy as timestamp to remove this race.
> > > > > > > > >
> > > > > > > > > How would two wic images to be built in parallel with the same work
> > > > > > > > > directory? To my understanding an image recipe only supports building
> > > > > > > > > a single wic image.
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > Paul
> > > > > > > >
> > > > > > > > bitbake image1 image2 image3
> > > > > > > > all images build .wics and use about the same files, like firmware.
> > > > > > > > Issue is similar to that:
> > > > > > > > https://www.yoctoproject.org/pipermail/yocto/2018-June/041373.html
> > > > > > >
> > > > > > > Each image has its own work directory though.
> > > > > > >
> > > > > > > I'll take a look in more detail later today or tomorrow, if wic is
> > > > > > > writing temporary files outside of the work directory then that's a
> > > > > > > bug and should be fixed.
> > > > > >
> > > > > > Thanks. I saw bug in rawcopy plugin. All other places were patched due
> > > > > > to the same code.  I guess then for rawcopy temp files are in
> > > > > > DEST_IMAGE_DIR (which is common) instead of WORKDIR.
> > > > >
> > > > > I can't see how these files can possibly collide across parallel image
> > > > > builds. In the lines you changed in your patch, cr_workdir passed in
> > > > > as an argument set to imager.workdir (in
> > > > > scripts/lib/wic/plugins/imager/direct.py) which is a temporary
> > > > > directory created under options.outdir. image_types_wic.bbclass passes
> > > > > the -o option to wic to set the outdir to something under ${WORKDIR}.
> > > > > And different images have different WORKDIR paths. So things should be
> > > > > safe.
> > > > >
> > > > > Which branch & version of poky or oe-core are you using?
> > > > >
> > > >
> > > > zeus
> > > >
> > > > > DEST_IMAGE_DIR is not defined in openembedded-core. Are you sure
> > > > > that's the right variable name?
> > > > >
> > > >
> > > > DEPLOY_DIR_IMAGE
> > > >
> > > > > Could you provide full bitbake output showing the exact error message
> > > > > when this collision happens?
> > > >
> > > > See traceback here:
> > > > https://ci.linaro.org/job/ledge-oe/357/DISTRO=rpb,MACHINE=ledge-stm32mp157c-dk2,label=docker-stretch-amd64/console
> > >
> > > I think I'm starting to understand now. Is this the wks file?
> > > https://github.com/Linaro/meta-ledge/blob/zeus/meta-ledge-bsp/wic/ledge-stm32mp157c-dk2-optee.wks.in
> >
> > Got it. Nasty little bug there!
> >
> > In the rawcopy source plugin, dst is set as follows:
> >
> >     dst = os.path.join(cr_workdir, "%s.%s" % (source_params['file'],
> > part.lineno))
> >
> > If source_params['file'] is an absolute path (as it is in the wks file
> > above), the cr_workdir prefix is not applied by os.path.join(). So
> > instead it writes to a ".1" file next to the original image - this is
> > outside the WORKDIR and at risk of collision.
> >
> > The solution is to fix dst above, maybe use
> > os.path.basename(source_params['file']). Could you give that a try or
> > do you want me to propose a patch?
> >
> Nice!
> I can test it today. Compilation will take some time...
>
> Maxim.
>
> > Thanks,
> > Paul