Bug 6290

Whenever I see a 'high' bug assigned to me, I have to ask the knee-jerk questions:

- Why is it high?
- Why is it M2?

As far as the specific requirements, I also need to ask for more details about what is meant by 'efi-plugin' and especially the 'other abstractions' mentioned.

Also, what specifically do we need to test in order to consider this fully implemented i.e. specific features to test and specific targets to test them on.

Assigned to person we are requesting info from per RP.

Tom,

Marking it high was basically a technique to ensure we finally got the grub to gummiboot transition to happen. It's been > medium(+?) for something like 3 releases and we wanted to get it done. Of course, lots of other things came through the flood gates since then. There's always risk in using the high moniker.

M2 was selected to ensure we had time to test the tooling throughout the release rather than toward the end.

I just sent patches for Bug 6295 to update mkefidisk.sh to support gummiboot. The gummiboot specific update was very straight forward. I believe you used mkefidisk.sh as a guide for the grub-efi support. You should now be able to use it for gummiboot support as well - hopefully that will reduce the time burden for this particular bug.

Reset with more info

OK, understood, and I appreciate the mkefidisk.sh code - it at least avoids the guesswork about what needs to be done.

On the other hand, I have to go on record again saying that I don't agree with general philosophy of marking things high so they 'get done'.  That wasn't meant to be the purpose of the category and didn't work out too well last time.

Fair enough, and I think in retrospect, I agree with you.

Note that the mkefidisk.sh for gummiboot is missing the removal of the initrd line, the same as we do for the grub block. I'll be updating that.

Discussed with Tom, ETA is most likely next week. It won't make M2. Leaving as M2 so Bug Triage can decide how to handle moving to M3.

commit 6e328c94ae402bdf6f664f2bbfd79fdfe7661d03
Author: Tom Zanussi <tom.zanussi@linux.intel.com>
Date:   Mon Aug 11 20:35:39 2014 -0500

    wic: Add gummiboot support to bootimg-efi

Hello,

I tried to test this wic feature, but I can't seem to pass over a step (tried both nuc BSP for nuc and intel-i7_64 BSP for minnowboard max).

The config I did in local.conf (aside from setting the MACHINE):
...
EFI_PROVIDER = "gummiboot"
...
then i do a core-image-minimal.
 
The thing is that when I try to do "wic create mkgummidisk -e core-image-minimal"
i get this error message:

Checking basic build environment...
Done.

Creating image(s)...

Error: exec_cmd: cp /home/autobuilder/poky/build/tmp/deploy/images/nuc/EFI/BOOT/* /var/tmp/wic/build/hdd/boot/EFI/BOOT returned '1' instead of 0


... and indeed there is nothing in that directory.

tested with:

meta-intel : a369fa8f2d76528cb296ef9314e613e26585d54d
poky : 8ac8eca2e3bd8c78e2b31ea974930ed0243258a3

Please let me know if I'm missing something.

Costin

*** Bug 6131 has been marked as a duplicate of this bug. ***

I also see the problem, but I retried the original git tree and it works fine, so something changed after it was pulled in:

http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=tzanussi/wic-gummiboot-v1


[trz@medianoche build]$ wic create mkgummidisk -e core-image-minimal
Checking basic build environment...
Done.

Creating image(s)...

Info: The new image(s) can be found here:
  /var/tmp/wic/build/mkgummidisk-201410201343-sda.direct

The following build artifacts were used to create the image(s):
  ROOTFS_DIR:                   /usr/local/dev/yocto/wic-test-daisy/build/tmp/work/nuc-poky-linux/core-image-minimal/1.0-r0/rootfs
  BOOTIMG_DIR:                  /usr/local/dev/yocto/wic-test-daisy/build/tmp/work/nuc-poky-linux/core-image-minimal/1.0-r0/core-image-minimal-1.0/hddimg
  KERNEL_DIR:                   /usr/local/dev/yocto/wic-test-daisy/build/tmp/sysroots/nuc/usr/src/kernel
  NATIVE_SYSROOT:               /usr/local/dev/yocto/wic-test-daisy/build/tmp/sysroots/x86_64-linux


The image(s) were created using OE kickstart file:
  /usr/local/dev/yocto/wic-test-daisy/scripts/lib/image/canned-wks/mkgummidisk.wks

And this is the commit that causes the problem.  The wrong value for boot_img_dir is used instead of the expected HDDDIR.

commit 7ce1dc13f91df70e8a2f420e7c3eba51cbc4bd48
Author: Maciej Borzecki <maciej.borzecki@open-rnd.pl>
Date:   Mon Sep 22 13:35:16 2014 +0200

    wic: set bootimg_dir when using image-name artifacts

Hello,

I retested the bug and this time the situation changed a litle because the error message is much scarse than before:

"
cosscat@ubnt:/media/poky/poky/build$ wic create mkgummidisk -e core-image-minimal
Checking basic build environment...
Done.

Creating image(s)...

Error: exec_cmd: cp /media/poky/poky/build/tmp/deploy/images/nuc/EFI/BOOT/* /var/tmp/wic/build/hdd/boot/EFI/BOOT returned '1' instead of 0
cosscat@ubnt:/media/poky/poky/build$ 
"


This is my build environment over a fresly cloned poky:


cosscat@ubnt:/media/poky/poky/build$ bitbake core-image-minimal
Parsing recipes: 100% |##################################################################################################################################################################################################| Time: 00:00:18
Parsing of 910 .bb files complete (0 cached, 910 parsed). 1307 targets, 44 skipped, 0 masked, 0 errors.
NOTE: Resolving any missing task queue dependencies

Build Configuration:
BB_VERSION        = "1.24.0"
BUILD_SYS         = "x86_64-linux"
NATIVELSBSTRING   = "Ubuntu-14.04"
TARGET_SYS        = "x86_64-poky-linux"
MACHINE           = "nuc"
DISTRO            = "poky"
DISTRO_VERSION    = "1.7"
TUNE_FEATURES     = "m64 corei7"
TARGET_FPU        = ""
meta              
meta-yocto        
meta-yocto-bsp    
meta-selftest     = "(detachedfrom924da20):924da20c6f2f183fa7e930e4c3c6b14c090acbe5"
meta-intel        
meta-nuc          = "(detachedfrom9f815ab):9f815ab761e6b524625e4180397233eeb728f84e"

NOTE: Preparing runqueue
NOTE: Executing SetScene Tasks
NOTE: Executing RunQueue Tasks
WARNING: Failed to fetch URL ftp://ftp.uni-erlangen.de/pub/Linux/LOCAL/dosfstools/dosfstools-2.11.src.tar.gz, attempting MIRRORS if available
WARNING: Failed to fetch URL ftp://ftp.berlios.de/pub/cdrecord/alpha/cdrtools-3.01a20.tar.bz2, attempting MIRRORS if available
NOTE: validating kernel config, see log.do_kernel_configcheck for details
NOTE: Tasks Summary: Attempted 2301 tasks of which 780 didn't need to be rerun and all succeeded.

Summary: There were 2 WARNING messages shown.
cosscat@ubnt:/media/poky/poky/build$ 
"

The content of the indicated directory contains no EFI dir: 

cosscat@ubnt:/media/poky/poky/build/tmp/deploy/images/nuc$ ls
bzImage                                                                 core-image-minimal-initramfs-nuc.manifest              core-image-minimal-nuc.hddimg    modules--3.17-rc4+git0+7df849fc7e_1a3939eb74-r0-nuc-20141022125726.tgz
bzImage--3.17-rc4+git0+7df849fc7e_1a3939eb74-r0-nuc-20141022125726.bin  core-image-minimal-nuc-20141022125726.hddimg           core-image-minimal-nuc.iso       modules-nuc.tgz
bzImage-nuc.bin                                                         core-image-minimal-nuc-20141022125726.iso              core-image-minimal-nuc.manifest  README_-_DO_NOT_DELETE_FILES_IN_THIS_DIRECTORY.txt
core-image-minimal-initramfs-nuc-20141022125726.rootfs.cpio.gz          core-image-minimal-nuc-20141022125726.rootfs.ext3      gummibootx64.efi
core-image-minimal-initramfs-nuc-20141022125726.rootfs.manifest         core-image-minimal-nuc-20141022125726.rootfs.manifest  microcode_20140624.cpio
core-image-minimal-initramfs-nuc.cpio.gz                                core-image-minimal-nuc.ext3                            microcode.cpio
cosscat@ubnt:/media/poky/poky/build/tmp/deploy/images/nuc$ 
cosscat@ubnt:/media/poky/poky/build/tmp/deploy/images/nuc$ 


Reopening bug.


Regards,

Costin

(In reply to comment #13)
> Hello,
> 
> I retested the bug and this time the situation changed a litle because the
> error message is much scarse than before:
> 
> "
> cosscat@ubnt:/media/poky/poky/build$ wic create mkgummidisk -e
> core-image-minimal
> Checking basic build environment...
> Done.
> 
> Creating image(s)...
> 
> Error: exec_cmd: cp /media/poky/poky/build/tmp/deploy/images/nuc/EFI/BOOT/*
> /var/tmp/wic/build/hdd/boot/EFI/BOOT returned '1' instead of 0
> cosscat@ubnt:/media/poky/poky/build$ 
> "

Indeed, this looks like my doing. I'll try to look into this. Can you post output of: wic create -D ..

Hello,

As of last comment, I've attached a file with wic debug's output.

Costin

Created attachment 2207 [details]
debug output

Shouldn't the patch:

commit 7ce1dc13f91df70e8a2f420e7c3eba51cbc4bd48
Author: Maciej Borzecki <maciej.borzecki@open-rnd.pl>
Date:   Mon Sep 22 13:35:16 2014 +0200

    wic: set bootimg_dir when using image-name artifacts

just be reverted?  AFAICT, it was added so that bootimg-partition would have DEPLOY_DIR_IMAGE, but bootimg-partition doesn't even use the bootimg_dir value except to print somthing out, and then does a get_bitbake_var("DEPLOY_DIR_IMAGE") anyway.

(In reply to comment #17)
> Shouldn't the patch:
> 
> commit 7ce1dc13f91df70e8a2f420e7c3eba51cbc4bd48
> Author: Maciej Borzecki <maciej.borzecki@open-rnd.pl>
> Date:   Mon Sep 22 13:35:16 2014 +0200
> 
>     wic: set bootimg_dir when using image-name artifacts
> 
> just be reverted?  AFAICT, it was added so that bootimg-partition would have
> DEPLOY_DIR_IMAGE, but bootimg-partition doesn't even use the bootimg_dir
> value except to print somthing out, and then does a
> get_bitbake_var("DEPLOY_DIR_IMAGE") anyway.

Before the patch, bootimg_dir was always be empty in this call in wic_create_subcommand():

    wic_create(args, wks_file, rootfs_dir, bootimg_dir, kernel_dir,
               native_sysroot, hdddir, staging_data_dir, scripts_path,
               image_output_dir, options.debug, options.properties_file)

That's because it was initialized to "" but never assigned when calling find_artifacts(). Then, down in the source plugin, in do_prepare_partition() call, bootimg_dir would also be empty.

My intention was to have this variable hold a path to the location where boot related files might be located. Now, as a sane default, bootimg_dir ends up being set to $DEPLOY_DIR_IMAGE (as for instance u-boot files end up in this location, so I felt that it'd be natural to use that location). Then in source plugin, in  do_prepare_partition() instead of calling get_bitbake_var("DEPLOY_DIR_IMAGE") like bootimg-partition does now, you could directly use bootimg_dir. 

Hopefully, the explanation is convincing :)

(In reply to comment #18)
> (In reply to comment #17)
> > Shouldn't the patch:
> > 
> > commit 7ce1dc13f91df70e8a2f420e7c3eba51cbc4bd48
> > Author: Maciej Borzecki <maciej.borzecki@open-rnd.pl>
> > Date:   Mon Sep 22 13:35:16 2014 +0200
> > 
> >     wic: set bootimg_dir when using image-name artifacts
> > 
> > just be reverted?  AFAICT, it was added so that bootimg-partition would have
> > DEPLOY_DIR_IMAGE, but bootimg-partition doesn't even use the bootimg_dir
> > value except to print somthing out, and then does a
> > get_bitbake_var("DEPLOY_DIR_IMAGE") anyway.
> 
> Before the patch, bootimg_dir was always be empty in this call in
> wic_create_subcommand():
> 
>     wic_create(args, wks_file, rootfs_dir, bootimg_dir, kernel_dir,
>                native_sysroot, hdddir, staging_data_dir, scripts_path,
>                image_output_dir, options.debug, options.properties_file)
> 
> That's because it was initialized to "" but never assigned when calling
> find_artifacts(). Then, down in the source plugin, in do_prepare_partition()
> call, bootimg_dir would also be empty.
> 
> My intention was to have this variable hold a path to the location where
> boot related files might be located. Now, as a sane default, bootimg_dir
> ends up being set to $DEPLOY_DIR_IMAGE (as for instance u-boot files end up
> in this location, so I felt that it'd be natural to use that location). Then
> in source plugin, in  do_prepare_partition() instead of calling
> get_bitbake_var("DEPLOY_DIR_IMAGE") like bootimg-partition does now, you
> could directly use bootimg_dir. 
> 
> Hopefully, the explanation is convincing :)

I assume that bootimg_dir and hdddir we expected to be different at some point, so perhaps passing hdddir explicitly do do_prepare_partition() would be a good solution. Tom, what do you think?

Well, it should probably be explicitly stated somewhere, but the intention of bootimg_dir is that it allows the plugin to distinguish between a value explicitly specified by the user (using -b) or if not set, it's up to the plugin to find.  Which was why the get_bitbake_var() function was added, so that plugins could find those variables as needed, and the existing plugins do that - e.g. the basic pattern is 'if !bootimg_dir bootimg_dir = get_bitbake_var()'.

So with the value now being hardcoded to the value of get_bitbake_var("DEPLOY_DIR_IMAGE") every plugin sees that value and assumes it was the value passed in by the user and just takes it.  Which is fine for u-boot but breaks everything else.

So I think I'll have to fix this up so that things work for everyone, but so far I haven't been successful creating a u-boot build to to test this with - could you post details on how I can test what I fix so that it works for u-boot?

In 1.8, we really need to have a testsuite that we run wic through to verify that a change doesn't break any use case we care about - I already have that in the plan for 1.8.

(In reply to comment #20)
> Well, it should probably be explicitly stated somewhere, but the intention
> of bootimg_dir is that it allows the plugin to distinguish between a value
> explicitly specified by the user (using -b) or if not set, it's up to the
> plugin to find.  Which was why the get_bitbake_var() function was added, so
> that plugins could find those variables as needed, and the existing plugins
> do that - e.g. the basic pattern is 'if !bootimg_dir bootimg_dir =
> get_bitbake_var()'.
> 
> So with the value now being hardcoded to the value of
> get_bitbake_var("DEPLOY_DIR_IMAGE") every plugin sees that value and assumes
> it was the value passed in by the user and just takes it.  Which is fine for
> u-boot but breaks everything else.
> 
> So I think I'll have to fix this up so that things work for everyone, but so
> far I haven't been successful creating a u-boot build to to test this with -
> could you post details on how I can test what I fix so that it works for
> u-boot?
> 
> In 1.8, we really need to have a testsuite that we run wic through to verify
> that a change doesn't break any use case we care about - I already have that
> in the plan for 1.8.

It should be fine to revert 7ce1dc13f91df70e8a2f420e7c3eba51cbc4bd48 then. I somehow missed that fact that empty var means that it had not been set explicitly.

As for testing, that's definitely a good idea. I've started using pylint with current code, but so far it's a bit problematic. Pylint does not handle live patching of objects or calling methods from named keys in dict too well.

This should now be fixed in master:

http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=38400617febd61a1594969c86a6f1712a9d80516

Hello,

Tested on Minnow Max with an core-i7_64 image, and it works. No more errors with wic. Marking as verified.

Costin