[PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Alejandro Hernandez]

When using wic to create an image from a certain build, wic is expecting
to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
which is wrong, since the initrd file has not been copied to the final
directory yet, so instead of trying to use an initrd file from 
DEPLOY_DIR_IMAGE we get it from IMGDEPLOYDIR, which is the directory 
where the resulting images are placed before their final destination, 
and its where we can find the correct initrd file for our image.

Signed-off-by: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
---
 scripts/lib/wic/plugins/source/bootimg-efi.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
index 9879cb9fceb..d1d445ecfe1 100644
--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
@@ -105,9 +105,9 @@ class BootimgEFIPlugin(SourcePlugin):
 
         if initrd:
             # obviously we need to have a common common deploy var
-            bootimg_dir = get_bitbake_var("DEPLOY_DIR_IMAGE")
+            bootimg_dir = get_bitbake_var("IMGDEPLOYDIR")
             if not bootimg_dir:
-                raise WicError("Couldn't find DEPLOY_DIR_IMAGE, exiting")
+                raise WicError("Couldn't find IMGDEPLOYDIR, exiting")
 
             cp_cmd = "cp %s/%s %s" % (bootimg_dir, initrd, hdddir)
             exec_cmd(cp_cmd, True)
-- 
2.12.0

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Patrick Ohly]

On Fri, 2017-06-30 at 10:53 -0700, Alejandro Hernandez wrote:
> When using wic to create an image from a certain build, wic is expecting
> to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
> which is wrong, since the initrd file has not been copied to the final
> directory yet, so instead of trying to use an initrd file from 
> DEPLOY_DIR_IMAGE we get it from IMGDEPLOYDIR, which is the directory 
> where the resulting images are placed before their final destination, 
> and its where we can find the correct initrd file for our image.

Can you give an example with some real image and initramfs recipe where
this works? In other words, provide an example where the old behavior
fails and the new one works?

When using core-image-minimal and core-image-minimal-initramfs, then
both have different IMGDEPLOYDIRs, therefore code in core-image-minimal
can only find the .cpio.gz in the DEPLOY_DIR_IMAGE. So to me, the
current approach seems correct.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Alejandro Hernandez]

Hey Patrick,


On 06/30/2017 01:43 PM, Patrick Ohly wrote:
> On Fri, 2017-06-30 at 10:53 -0700, Alejandro Hernandez wrote:
>> When using wic to create an image from a certain build, wic is expecting
>> to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
>> which is wrong, since the initrd file has not been copied to the final
>> directory yet, so instead of trying to use an initrd file from
>> DEPLOY_DIR_IMAGE we get it from IMGDEPLOYDIR, which is the directory
>> where the resulting images are placed before their final destination,
>> and its where we can find the correct initrd file for our image.
> Can you give an example with some real image and initramfs recipe where
> this works? In other words, provide an example where the old behavior
> fails and the new one works?
Sure, so the problem comes when you add "wic" to IMAGE_FSTYPES (which is 
not default for core-image-minimal nor core-image-minimal-initramfs).
If you do this, do_image_wic will be executed automatically, and if it 
tries to use foo.cpio.gz file as initrd, it will fail to find it on
DEPLOY_DIR_IMAGE (unless a previous build was completed without 
executing do_image_wic) since this file is still on IMGDEPLOYDIR and hasnt
been copied to DEPLOY_DIR_IMAGE yet.

So a real example on real hardware:

if you build:
DISTRO="poky-tiny"
MACHINE="intel-corei7-64"
bitbake core-image-tiny-initramfs

You'll get an error stating that the foo.cpio.gz file cant be found.

I did a build for both core-image-minimal and 
core-image-minimal-initramfs and I dont see how theyre affected, the 
necessary files are
on IMGDEPLOYDIR in both cases as well.

>
> When using core-image-minimal and core-image-minimal-initramfs, then
> both have different IMGDEPLOYDIRs, therefore code in core-image-minimal
> can only find the .cpio.gz in the DEPLOY_DIR_IMAGE. So to me, the
> current approach seems correct.
>

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Patrick Ohly]

On Fri, 2017-06-30 at 15:38 -0500, Alejandro Hernandez wrote:
> Hey Patrick,
> 
> 
> On 06/30/2017 01:43 PM, Patrick Ohly wrote:
> > On Fri, 2017-06-30 at 10:53 -0700, Alejandro Hernandez wrote:
> >> When using wic to create an image from a certain build, wic is expecting
> >> to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
> >> which is wrong, since the initrd file has not been copied to the final
> >> directory yet, so instead of trying to use an initrd file from
> >> DEPLOY_DIR_IMAGE we get it from IMGDEPLOYDIR, which is the directory
> >> where the resulting images are placed before their final destination,
> >> and its where we can find the correct initrd file for our image.
> > Can you give an example with some real image and initramfs recipe where
> > this works? In other words, provide an example where the old behavior
> > fails and the new one works?
> Sure, so the problem comes when you add "wic" to IMAGE_FSTYPES (which is 
> not default for core-image-minimal nor core-image-minimal-initramfs).
> If you do this, do_image_wic will be executed automatically, and if it 
> tries to use foo.cpio.gz file as initrd, it will fail to find it on
> DEPLOY_DIR_IMAGE (unless a previous build was completed without 
> executing do_image_wic) since this file is still on IMGDEPLOYDIR and hasnt
> been copied to DEPLOY_DIR_IMAGE yet.
> 
> So a real example on real hardware:
> 
> if you build:
> DISTRO="poky-tiny"
> MACHINE="intel-corei7-64"
> bitbake core-image-tiny-initramfs
>
> You'll get an error stating that the foo.cpio.gz file cant be found.

Just for completeness, foo.cpio.gz =
core-image-tiny-initramfs-intel-corei7-64.cpio.gz, which is the
initramfs image produced by the recipe itself.

Is it really intended that do_image_wic runs for
core-image-tiny-initramfs? How and where is the output of
core-image-tiny-initramfs used?

From the DESCRIPTION:

        core-image-tiny-initramfs doesn't actually generate an image but
        rather generates boot and rootfs artifacts into a common
        location that can subsequently be picked up by external image
        generation tools such as wic.

If core-image-tiny-initramfs is meant to produce an initramfs, similar
to core-image-minimal-initramfs, then it shouldn't use "wic" in its
IMAGE_FSTYPES. Currently "wic" is listed:

# don't actually generate an image, just the artifacts needed for one
IMAGE_FSTYPES = "${INITRAMFS_FSTYPES} wic"

Does the recipe still work as intended without your patch when you
remove wic here?

> I did a build for both core-image-minimal and 
> core-image-minimal-initramfs and I dont see how theyre affected, the 
> necessary files are
> on IMGDEPLOYDIR in both cases as well.

Are you sure that core-image-minimal copies an initrd in bootimg-efi.py?
It checks source_params, but systemd-bootdisk.wks does not specify an
initrd in its sourceparams:

part /boot --source bootimg-efi --sourceparams="loader=systemd-boot"
--ondisk sda --label msdos --active --align 1024

That's different for the systemd-bootdisk-tiny64.wks that is used for
core-image-tiny-initramfs, which has:

part /boot --source bootimg-efi
--sourceparams="loader=systemd-boot,initrd=core-image-tiny-initramfs-intel-corei7-64.cpio.gz" --ondisk sda --label msdos --active --align 1024

I still think your patch breaks images which use the initrd parameter to
reference an initrd produced by some other recipe.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Alejandro Hernandez]

On 07/01/2017 10:09 AM, Patrick Ohly wrote:
> On Fri, 2017-06-30 at 15:38 -0500, Alejandro Hernandez wrote:
>> Hey Patrick,
>>
>>
>> On 06/30/2017 01:43 PM, Patrick Ohly wrote:
>>> On Fri, 2017-06-30 at 10:53 -0700, Alejandro Hernandez wrote:
>>>> When using wic to create an image from a certain build, wic is expecting
>>>> to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
>>>> which is wrong, since the initrd file has not been copied to the final
>>>> directory yet, so instead of trying to use an initrd file from
>>>> DEPLOY_DIR_IMAGE we get it from IMGDEPLOYDIR, which is the directory
>>>> where the resulting images are placed before their final destination,
>>>> and its where we can find the correct initrd file for our image.
>>> Can you give an example with some real image and initramfs recipe where
>>> this works? In other words, provide an example where the old behavior
>>> fails and the new one works?
>> Sure, so the problem comes when you add "wic" to IMAGE_FSTYPES (which is
>> not default for core-image-minimal nor core-image-minimal-initramfs).
>> If you do this, do_image_wic will be executed automatically, and if it
>> tries to use foo.cpio.gz file as initrd, it will fail to find it on
>> DEPLOY_DIR_IMAGE (unless a previous build was completed without
>> executing do_image_wic) since this file is still on IMGDEPLOYDIR and hasnt
>> been copied to DEPLOY_DIR_IMAGE yet.
>>
>> So a real example on real hardware:
>>
>> if you build:
>> DISTRO="poky-tiny"
>> MACHINE="intel-corei7-64"
>> bitbake core-image-tiny-initramfs
>>
>> You'll get an error stating that the foo.cpio.gz file cant be found.
> Just for completeness, foo.cpio.gz =
> core-image-tiny-initramfs-intel-corei7-64.cpio.gz, which is the
> initramfs image produced by the recipe itself.
Correct
>
> Is it really intended that do_image_wic runs for
> core-image-tiny-initramfs? How and where is the output of
> core-image-tiny-initramfs used?
Yes, do_image_wic is intended to run automatically, hence why wic was 
added to IMAGE_FSTYPES
I'm not sure I understand what you mean by the second question, but the 
output is just as in any other image
a .wic file, cpio.gz file, kernel, etc., where? at the DEPLOY_DIR_IMAGE 
directory, again just like in any other image
>
>  From the DESCRIPTION:
>
>          core-image-tiny-initramfs doesn't actually generate an image but
>          rather generates boot and rootfs artifacts into a common
>          location that can subsequently be picked up by external image
>          generation tools such as wic.
This is the old description, when it was though that we would have a common
artifacts directory, which never happened due to some changes in wic 
last year
after which it was decided that core-image-tiny-initramfs would generate 
the artifacts
AND a wic image.
>
> If core-image-tiny-initramfs is meant to produce an initramfs, similar
> to core-image-minimal-initramfs, then it shouldn't use "wic" in its
> IMAGE_FSTYPES. Currently "wic" is listed:
>
> # don't actually generate an image, just the artifacts needed for one
> IMAGE_FSTYPES = "${INITRAMFS_FSTYPES} wic"
Again, old description, wic should be there.
>
> Does the recipe still work as intended without your patch when you
> remove wic here?
The answer would be yes and no; "yes" it does generate the boot 
artifacts if wic isnt listed,
but since wic should be in IMAGE_FSTYPES, then "no", because it fails to 
generate an image, which
is what the recipe intends to do.
>
>> I did a build for both core-image-minimal and
>> core-image-minimal-initramfs and I dont see how theyre affected, the
>> necessary files are
>> on IMGDEPLOYDIR in both cases as well.
> Are you sure that core-image-minimal copies an initrd in bootimg-efi.py?
> It checks source_params, but systemd-bootdisk.wks does not specify an
> initrd in its sourceparams:
>
> part /boot --source bootimg-efi --sourceparams="loader=systemd-boot"
> --ondisk sda --label msdos --active --align 1024
Exactly, they dont specify an initrd so for starters they dont need to 
copy an initrd if its not specified
>
> That's different for the systemd-bootdisk-tiny64.wks that is used for
> core-image-tiny-initramfs, which has:
>
> part /boot --source bootimg-efi
> --sourceparams="loader=systemd-boot,initrd=core-image-tiny-initramfs-intel-corei7-64.cpio.gz" --ondisk sda --label msdos --active --align 1024
>
> I still think your patch breaks images which use the initrd parameter to
> reference an initrd produced by some other recipe.
I see your point, if an initrd is produced by some other recipe it'd be 
somewhere else, but at the same time, what if an initrd is produced by 
the image recipe itself?, like in this case
>

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Patrick Ohly]

On Sat, 2017-07-01 at 16:48 -0500, Alejandro Hernandez wrote:
> 
> On 07/01/2017 10:09 AM, Patrick Ohly wrote:
> > Is it really intended that do_image_wic runs for
> > core-image-tiny-initramfs? How and where is the output of
> > core-image-tiny-initramfs used?
> Yes, do_image_wic is intended to run automatically, hence why wic was 
> added to IMAGE_FSTYPES
> I'm not sure I understand what you mean by the second question, but the 
> output is just as in any other image

My point was that core-image-tiny-initramfs isn't like other *-initramfs
image recipes at all, and having an outdated DESCRIPTION in it doesn't
help much either.

> >  From the DESCRIPTION:
> >
> >          core-image-tiny-initramfs doesn't actually generate an image but
> >          rather generates boot and rootfs artifacts into a common
> >          location that can subsequently be picked up by external image
> >          generation tools such as wic.
> This is the old description, when it was though that we would have a common
> artifacts directory, which never happened due to some changes in wic 
> last year
> after which it was decided that core-image-tiny-initramfs would generate 
> the artifacts
> AND a wic image.

If that remains the solution (see below), will the DESCRIPTION and
perhaps even the file name be changed?

> > I still think your patch breaks images which use the initrd parameter to
> > reference an initrd produced by some other recipe.
> I see your point, if an initrd is produced by some other recipe it'd be 
> somewhere else, but at the same time, what if an initrd is produced by 
> the image recipe itself?, like in this case

I see two solutions:
1) keep using the recipe as currently intended and 
   add another sourceparam to bootimg-efi.py which changes the default
   source for initrd from DEPLOY_DIR_IMAGE (the current behavior) to
   IMGDEPLOYDIR
2) use two recipes, one for the initramfs (the current
   core-image-tiny-initramfs.bb, but without wic in IMAGE_FSTYPES) and
   a separate core-image-tiny.bb where do_rootfs gets replaced with
   an empty stub

I personally find the second solution cleaner. It's the standard way of
doing image + initramfs and thus will be easier to understand and have
less unexpected pitfalls.

One such pitfall in solution one exists even with your current patch:
do_image_wic does not depend on do_image_cpio (run "bitbake -g
core-image-tiny-initramfs:do_image_wic", "grep
'^.core-image-tiny-initramfs.do_image_wic' task-depends.dot") and thus
there's a race condition between the two tasks of the same recipe. That
can be fixed, of course, but it is making the solution even more
unusual.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Ed Bartosh]

On Sat, Jul 01, 2017 at 04:48:41PM -0500, Alejandro Hernandez wrote:
> 
> 
> > From the DESCRIPTION:
> >
> >         core-image-tiny-initramfs doesn't actually generate an image but
> >         rather generates boot and rootfs artifacts into a common
> >         location that can subsequently be picked up by external image
> >         generation tools such as wic.
> This is the old description, when it was though that we would have a common
> artifacts directory, which never happened due to some changes in wic
> last year

Can you elaborate on this please?


I'm still thinking that having a common artifacts directory is what we
should aim for. And DEPLOY_DIR_IMAGE is a good candidate for it as most
of artifacts are already there.

--
Regards,
Ed

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Ed Bartosh]

On Fri, Jun 30, 2017 at 10:53:30AM -0700, Alejandro Hernandez wrote:
> When using wic to create an image from a certain build, wic is expecting
> to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
> which is wrong, since the initrd file has not been copied to the final
> directory yet,

Is it possible to ensure that initrd is deployed before wic is run by
making do_image_wic depend on initrd deploy task?

> so instead of trying to use an initrd file from 
> DEPLOY_DIR_IMAGE we get it from IMGDEPLOYDIR, which is the directory 
> where the resulting images are placed before their final destination, 
> and its where we can find the correct initrd file for our image.
> 
> Signed-off-by: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
> ---
>  scripts/lib/wic/plugins/source/bootimg-efi.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
> index 9879cb9fceb..d1d445ecfe1 100644
> --- a/scripts/lib/wic/plugins/source/bootimg-efi.py
> +++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
> @@ -105,9 +105,9 @@ class BootimgEFIPlugin(SourcePlugin):
>  
>          if initrd:
>              # obviously we need to have a common common deploy var
> -            bootimg_dir = get_bitbake_var("DEPLOY_DIR_IMAGE")
> +            bootimg_dir = get_bitbake_var("IMGDEPLOYDIR")
>              if not bootimg_dir:
> -                raise WicError("Couldn't find DEPLOY_DIR_IMAGE, exiting")
> +                raise WicError("Couldn't find IMGDEPLOYDIR, exiting")
>  
>              cp_cmd = "cp %s/%s %s" % (bootimg_dir, initrd, hdddir)
>              exec_cmd(cp_cmd, True)
> -- 
> 2.12.0
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
--
Regards,
Ed

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Patrick Ohly]

On Mon, 2017-07-03 at 11:36 +0300, Ed Bartosh wrote:
> On Fri, Jun 30, 2017 at 10:53:30AM -0700, Alejandro Hernandez wrote:
> > When using wic to create an image from a certain build, wic is expecting
> > to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
> > which is wrong, since the initrd file has not been copied to the final
> > directory yet,
> 
> Is it possible to ensure that initrd is deployed before wic is run by
> making do_image_wic depend on initrd deploy task?

Not in this case, because both do_image_wic and do_image_cpio share the
same deploy task.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Ed Bartosh]

On Mon, Jul 03, 2017 at 11:27:53AM +0200, Patrick Ohly wrote:
> On Mon, 2017-07-03 at 11:36 +0300, Ed Bartosh wrote:
> > On Fri, Jun 30, 2017 at 10:53:30AM -0700, Alejandro Hernandez wrote:
> > > When using wic to create an image from a certain build, wic is expecting
> > > to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
> > > which is wrong, since the initrd file has not been copied to the final
> > > directory yet,
> > 
> > Is it possible to ensure that initrd is deployed before wic is run by
> > making do_image_wic depend on initrd deploy task?
> 
> Not in this case, because both do_image_wic and do_image_cpio share the
> same deploy task.

Now I understand what's going on here. Basically Alejandro is trying to
use one format of the same image in another. It's like trying to put
.wic image inside ext2 image or something similar.

I believe it doesn't fit current design of oe image creation system.
I'd suggest to use different image recipes to achieve this.

--
Regards,
Ed

Re: [PATCH] bootimg-efi.py: Use IMGDEPLOYDIR instead of DEPLOY_DIR_IMAGE for initrd

[Patrick Ohly]

On Mon, 2017-07-03 at 15:44 +0300, Ed Bartosh wrote:
> On Mon, Jul 03, 2017 at 11:27:53AM +0200, Patrick Ohly wrote:
> > On Mon, 2017-07-03 at 11:36 +0300, Ed Bartosh wrote:
> > > On Fri, Jun 30, 2017 at 10:53:30AM -0700, Alejandro Hernandez wrote:
> > > > When using wic to create an image from a certain build, wic is expecting
> > > > to find initrd at the final destination of our images (DEPLOY_DIR_IMAGE),
> > > > which is wrong, since the initrd file has not been copied to the final
> > > > directory yet,
> > > 
> > > Is it possible to ensure that initrd is deployed before wic is run by
> > > making do_image_wic depend on initrd deploy task?
> > 
> > Not in this case, because both do_image_wic and do_image_cpio share the
> > same deploy task.
> 
> Now I understand what's going on here. Basically Alejandro is trying to
> use one format of the same image in another. It's like trying to put
> .wic image inside ext2 image or something similar.
> 
> I believe it doesn't fit current design of oe image creation system.
> I'd suggest to use different image recipes to achieve this.

Yes, that is also my preferred solution (see other email from this
morning).

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.