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

[Maxim Uvarov <maxim.uvarov@linaro.org>]

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