Re: [PATCH] wic: Generate startup.nsh for EFI cases if none found

[Ed Bartosh <ed.bartosh@linux.intel.com>]

On Fri, Sep 29, 2017 at 08:35:42AM -0400, Tom Rini wrote:
> On Fri, Sep 29, 2017 at 02:27:57PM +0300, Ed Bartosh wrote:
> > On Thu, Sep 28, 2017 at 01:44:29PM -0400, Tom Rini wrote:
> > > On Thu, Sep 28, 2017 at 06:47:07PM +0300, Ed Bartosh wrote:
> > > > On Wed, Sep 20, 2017 at 12:03:27PM -0400, Tom Rini wrote:
> > > > > In the case of non-wic images there is logic today to generate a
> > > > > startup.nsh file that will be executed by EFI to run the loader that the
> > > > > image contains.  In the WIC case is currently depends on that file being
> > > > > generated elsewhere and placed in DEPLOY_DIR_IMAGE and only used if
> > > > > present there.
> > > > 
> > > > What's wrong with this approach?
> > > 
> > > No one ever provides a startup.nsh and everyone that wants one creates
> > > the same one line trivial example.  The end result is that no WIC images
> > > are Just Bootable on UEFI systems unless you first go and spell that out
> > > as the desired booting device.  This isn't an awesome workflow which is
> > > why the non-WIC cases make the required startup.nsh :)
> > 
> > Would it be better if EFI providers create this file?
> > 
> > I still believe that wic should't hack the filesystem content unless
> > it's really unavoidable. So far I know only one exception: updating
> > /etc/fstab. And even that is not always needed (see --no-update-fstab
> > patchset for further details.)
> 
> Well, it depends on your view of who is supposed to do what.  Today, in
> wic BootimgEFIPlugin mirrors the efi_populate() function of
> systemd-boot/grub-efi.bbclass.
I'd call this unnecessary duplication. This content should be prepared
by EFI provider class or recipe and taken by wic as is, without
modification.

I did some work on this: http://lists.openembedded.org/pipermail/openembedded-core/2017-May/136656.html
If this or similar approach is accepted wic wouldn't need most of boot
plugings. Boot partition can be prepared out of bootfs directory using
rootfs plugin.

>  That's where startup.nsh is made because
> it needs to know the name of the EFI application (also technically the
> path, but EFI\BOOT\ is spec mandatated I believe).  So we can't
> easily make the deploy functions create startup.nsh without duplicating
> logic from the bbclasss.
> 
> > > > I'd be happy to make wic to do only partitioning and assembling the
> > > > image and avoiding to modify image content as much as possible.
> > > > That would make wic design much more clear and let us to remove
> > > > a lot of duplication between wic and meta/classes code.
> > > > 
> > > > Regarding boot partition content, I think preparing it from bootfs
> > > > directory the same way as it's done for root partition is the way to go.
> > > > You can find more details about it here:
> > > > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10073
> > > 
> > > I don't conceptually see a problem with going that route.  But today WIC
> > > images aren't nearly as useful as they could be, with a rather tiny
> > > change.
> > 
> > If we agree that wic should avoid modifying content then the obvious way
> > to solve this is to provide required content (startup.nsh in this case)
> > either by EFI related recipes or core classes.
> 
> Maybe I have to change my mind after thinking harder :)  Where's the
> logic that creates the boot partition now?
>
Now it's in several places: in meta/classes, efi recipes and wic. I
think wic should be removed from this list. It's not a wic job to prepare boot content.

> > > My patch is also a regression-fix, I believe, in that at some point in
> > > the past, when Christopher's patch went in, things were laid out such
> > > that startup.nsh was often/always generated by another class and placed
> > > where WIC would find it and copy it in.  At some point that was
> > > broken/changed, and no one noticed / was interested enough to fix it.
> > 
> > If this functionality is covered by wic test suite this wouldn't
> > happen.
> 
> Once we agree on what the fix looks like, I'll see if I can figure out
> how to add in another test. :)


--
Regards,
Ed