public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
From: Henning Schild <henning.schild@siemens.com>
To: "Bezdeka, Florian (T RDA IOT SES-DE)" <florian.bezdeka@siemens.com>
Cc: "isar-users@googlegroups.com" <isar-users@googlegroups.com>,
	"Kiszka, Jan (T RDA IOT)" <jan.kiszka@siemens.com>,
	"Vijaikumar_Kanagarajan@mentor.com"
	<Vijaikumar_Kanagarajan@mentor.com>
Subject: Re: [PATCH v2 0/7] re-fork wic pcbios and efi plugins
Date: Tue, 7 Sep 2021 10:00:09 +0200	[thread overview]
Message-ID: <20210907100009.4c6c4ee1@md1za8fc.ad001.siemens.net> (raw)
In-Reply-To: <fdb52349e4243fe53c6719ed4ee66d1f8e573992.camel@siemens.com>

Am Tue, 7 Sep 2021 09:15:36 +0200
schrieb "Bezdeka, Florian (T RDA IOT SES-DE)"
<florian.bezdeka@siemens.com>:

> On Fri, 2021-09-03 at 14:53 +0200, Henning Schild wrote:
> > changes since v1:
> >   - efi plugin forked as well
> >   - systemd-boot support in efi plugin enabled
> >   - common functionality in utility library
> >   - test case for system-boot
> >   - "cp -a" moved to "find exec cp" because of ubuntu
> >   - changed wks files to exclude boot from root and mount it
> >
> > The forked plugins have gotten out of sync with the last wic version
> > bumps. And the original fork was not exactly minimal or made for
> > easy maintenance.
> >
> > This series does a re-fork of the two plugins with the aim to come
> > up with something readable, minimal and maintainable.
> >
> > There used to be a special case for grub-efi where the actual
> > kernel and initrd would remain in the root partition, which kind of
> > allowed kernel updates with apt-get. Now all three bootloaders
> > (systemd-boot now works as well) place bootloader, config and boot
> > artifacts in a boot-partition.
> >
> > Kernel updates with apt-get are now consistantly "broken". That
> > consistency very likely is not too bad. A generic solution for this
> > feature (if wanted) will need to be found. Covering not just these
> > three bootloaders but possibly also u-boot and efibootguard.  
> 
> I had a quick look and asked myself if it would be possible to
> implement the ISAR specific parts in sub-classes of such plugins. I
> assume that would require some upstream work first, followed by a re-
> sync and adding the ISAR stuff in plugins that inherit from the
> upstream ones.

My original goal was in fact to reduce it to only bits that can be
upstreamed. In general we need to hook into generation of the config,
because we use another name for that kernel binary and because we use
an initrd.

After that the installation of the binaries differs mainly in where
they come from. That looks really simple for systemd-boot, a little
more involved for syslinux and really very different for grub.

In addition the population of boot with kernel initrd and misc files
differs in isar.

I would not easily find a way to make that generic enough to try and
motivate OE to change those classes to be "more generic". It is in bits
just pretty different.

> As far as I can see all affected plugins are python classes. I'm not a
> python expert but I guess that should be doable and would completely
> isolate the ISAR parts from the upstream stuff.

Yes an no. Once forked they are only there to serve isar. Any kind of
"monkey patching" would likely require more code and still be
patching/forking. If even possible i would not know how.

As we can see in this series, a regular fork inherits useful bits from
the original. I was really surprised how easy it was to enable
systemd-boot. And i tend to prefer that over grub, because the
bootloader is installed from the rootfs and not buildchroot ... and is
therefore also part of the Bill Of Materials.

But yes, such a plugin just needs to implement the interface of such a
class. But it seems like deriving from the original results in way less
isar-specific code.

In fact we do have a fork of "bootimg-biosplusefi" in one of our
layers, that one is only about very little renaming. Basically calling
the efi and the pcbios plugins. That very likely only works so well
because our plugins are forked and so very similar.

I have nothing against further improvements in the future. But as it
stands this series remains with "forking" but doing so in a hopefully
easier to maintain way. And it aligns our forks with the originals.
IMHO that is worth merging and whether further steps will be needed in
the future remains to be seen.

The upstream classes do not change too often and not too dramatic. So
the biggest risk in isar is lack of disciple/care when bumping wic and
low test coverage. Both points that we might want to improve.
We have the suggestion from Jan to write a documents where we list
"forked" bits, and i have a bunch more patches to increase test
coverage. i.e. arm64 efi booting

https://github.com/henning-schild-work/isar/tree/henning/staging9

regards,
Henning

> >
> > Henning Schild (7):
> >   wic: reset our plugin forks to OE upstream for re-forking
> >   wic: add utility library for common bits of isar plugins
> >   wic: apply the actual fork changes to our pcbios plugin fork
> >   wic: clean up wic class in terms of isar variables
> >   wic: apply the actual fork changes to our efi plugin fork
> >   wic: mount /boot and exlude it from root for efi
> >   meta-isar: use "systemd-boot" for one test target
> >
> >  RECIPE-API-CHANGELOG.md                       |   9 +
> >  meta-isar/conf/machine/qemuamd64.conf         |   3 +
> >  .../scripts/lib/wic/canned-wks/hikey.wks      |   4 +-
> >  .../lib/wic/canned-wks/sdimage-efi-sd.wks     |   9 +
> >  .../lib/wic/canned-wks/sdimage-efi.wks        |   4 +-
> >  meta/classes/wic-img.bbclass                  |   6 +-
> >  .../scripts/lib/wic/plugins/isarpluginbase.py |  39 ++++
> >  .../wic/plugins/source/bootimg-efi-isar.py    | 200
> > ++++++++++++++---- .../wic/plugins/source/bootimg-pcbios-isar.py |
> > 139 ++++++------ 9 files changed, 290 insertions(+), 123
> > deletions(-) create mode 100644
> > meta-isar/scripts/lib/wic/canned-wks/sdimage-efi-sd.wks create mode
> > 100644 meta/scripts/lib/wic/plugins/isarpluginbase.py 
> 


  reply	other threads:[~2021-09-07  8:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-03 12:53 Henning Schild
2021-09-03 12:53 ` [PATCH v2 1/7] wic: reset our plugin forks to OE upstream for re-forking Henning Schild
2021-09-03 12:53 ` [PATCH v2 2/7] wic: add utility library for common bits of isar plugins Henning Schild
2021-09-03 12:53 ` [PATCH v2 3/7] wic: apply the actual fork changes to our pcbios plugin fork Henning Schild
2021-09-03 12:53 ` [PATCH v2 4/7] wic: clean up wic class in terms of isar variables Henning Schild
2021-09-03 12:53 ` [PATCH v2 5/7] wic: apply the actual fork changes to our efi plugin fork Henning Schild
2021-09-03 12:53 ` [PATCH v2 6/7] wic: mount /boot and exlude it from root for efi Henning Schild
2021-09-03 16:06   ` Henning Schild
2021-09-03 12:53 ` [PATCH v2 7/7] meta-isar: use "systemd-boot" for one test target Henning Schild
2021-09-06  5:05 ` [PATCH v2 0/7] re-fork wic pcbios and efi plugins Jan Kiszka
2021-09-06  8:59   ` Henning Schild
2021-09-06  9:48 ` Anton Mikanovich
2021-09-06 10:51   ` Henning Schild
2021-09-07  7:15 ` Bezdeka, Florian
2021-09-07  8:00   ` Henning Schild [this message]
2021-09-13 13:05 ` Anton Mikanovich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210907100009.4c6c4ee1@md1za8fc.ad001.siemens.net \
    --to=henning.schild@siemens.com \
    --cc=Vijaikumar_Kanagarajan@mentor.com \
    --cc=florian.bezdeka@siemens.com \
    --cc=isar-users@googlegroups.com \
    --cc=jan.kiszka@siemens.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox