[U-Boot] [PATCH] fastboot: Fix slot names reported by getvar

Fri Jun 14 15:52:50 UTC 2019

Please drop this patch in favor of:

    [PATCH v2 0/2] fastboot: Support new A/B slot format

Thanks!

On Fri, Jun 14, 2019 at 5:37 PM Sam Protsenko
<semen.protsenko at linaro.org> wrote:
>
> Hi Eugeniu,
>
> On Thu, Jun 13, 2019 at 8:57 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
> >
> > Hi Sam,
> >
> > Thanks for the detailed answer. Some comments below.
> >
> > On Thu, Jun 13, 2019 at 04:59:40PM +0300, Sam Protsenko wrote:
> > > Hi Eugeniu,
> > >
> > > On Thu, Jun 13, 2019 at 12:31 PM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
> > > >
> > > > Hi Sam,
> > > >
> > > > On Thu, Jun 13, 2019 at 12:49:45AM +0300, Sam Protsenko wrote:
> > > > > In commit [1] fastboot tool was changed w.r.t. new A/B specification [2],
> > > > > and now we should report slot names in "a" format instead of "_a".
> > > > > Latter is now considered legacy and we shouldn't rely on that anymore.
> > > >
> > > > This looks like a change which advantages the users who are always on
> > > > the tip/HEAD of all relevant components (fastboot and U-Boot), but that
> > > > rarely happens in the industry. Suppliers and hardening vendors often
> > > > deliver obsoleted versions because they can't keep up with upstream
> > > > development. Can you please document the behavior of 'fastboot flash'
> > > > (and anything else relying on
> > > > 'fastboot getvar (current-slot|slot-suffixes) in below scenarios:
> > > >
> > > > A. fastboot >= [1] && U-Boot + this patch
> > > > B. fastboot >= [1] && U-Boot - this patch
> > > > C. fastboot <  [1] && U-Boot + this patch
> > > > D. fastboot <  [1] && U-Boot - this patch
> > > >
> > > > Would it be possible to keep U-Boot backward-compatible, such that
> > > > regardless of the scenario enumerated above, 'fastboot flash' will
> > > > always succeed?
> > > >
> > >
> > > I'm afraid in this particular case we weren't given any choice, and we
> > > won't be able to provide backward-compatibility for older Android
> > > releases. After this commit:
> > >
> > >     [3] https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f
> >
> > Thumbs up for pointing out this specific commit.
> >
> > >
> > > they dropped support for "_a" format completely (in fastboot tool). So:
> > >   * if user runs new fastboot tool ( >= [3]), then the only way to
> > > make "fastboot flash" work is to return slot in "a" format from
> > > bootloader
> > >   * if user runs old fastboot tool (< [1]), then the only way to make
> > > "fastboot flash" work is to return slot in "_a" format from bootloader
> >
> > This interface change seems to carry no semantic value, so it's
> > unfortunate to see such kind of non-backward-compatible update
> > contributed by the maintainers of the user-space tool.
> >
>
> Yeah, I was pretty frustrated too :) Anyway, let's try to handle this
> in U-Boot as good as we can.
>
> > > Good news is that user can basically downgrade or upgrade fastboot
> > > tool, to be in sync with U-Boot version in use. Bad news, we need to
> > > decide which Android version to break in U-Boot/master.
> > >
> > > I suggest we track AOSP/master in U-Boot/master. Please let me know if
> > > you agree, or maybe there is some better way I'm missing.
> >
> > I can't come up with a better proposal.
> >
>
> Well, theoretically we *can* provide some config option like
> AB_NEW/AB_OLD, or check similar variable in U-Boot shell, to figure
> out which format to use ("_a" or "a"). But right now A/B is not
> implemented in upstream U-Boot anyway, so we probably don't have much
> users for that feature yet. So let's implement it for the latest
> AOSP/master, and if the need arises, we can handle old format support
> later. The crucial thing is, this patch fixes "fastboot flash" in
> AOSP/master, when user has slotted partitions.
>
> > Reflecting on fastboot in general, I think there are many more
> > questions which lack a straightforward answer:
> >  - How accurately the fastboot protocol [4] reflects the AOSP
> >    user-space implementation (and viceversa)?
> >  - Some basic git grepping reveals that the fastboot protocol [4]
> >    was once upgraded from v0.3 to v0.4, but that's pretty much it. It
> >    definitely doesn't cover the evolution of fastboot usage/parameters.
> >    For example, none of the AB/slot-related parameters (e.g.
> >    "slot-suffixes", "current-slot") has ever been documented in [4].
> >  - Since the protocol and the actual development are out of sync,
> >    staying aligned to AOSP means asserting about the fastboot
> >    interface updates by reviewing the AOSP patches (which is time
> >    consuming and cannot be easily automated).
> >  - OTOH, it is also not clear to which degree U-Boot/master is aligned
> >    to AOSP/master even after integrating this patch. As example,
> >    commit [3] mentioned by you dropped the support for 'slot-suffixes'
> >    altogether while the option is still being processed in U-Boot.
> >
>
> Agree. Someone should check AOSP patches and provide corresponding
> support for the latest fastboot tool feature. I will probably look
> into that later, as we want this for our platforms to be implemented.
>
> I'm gonna send v2 soon, addressing your comments (will remove
> slot-suffixes variable and improve commit message). Thanks for your
> input!
>
>
> > In a nutshell, I think we have no other choice but to apply this fix
> > (still not clear if w/ or w/o keeping the support for "slot-suffixes").
> > I think it's also a must to warn the users that this patch
> > assumes/depends on AOSP fastboot version/commit [3] or higher.
> >
> > > > > [1] https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5
> > > > > [2] https://source.android.com/devices/tech/ota/ab/ab_implement#partitions
> >
> > [4] https://android.googlesource.com/platform/system/core/+/master/fastboot/README.md
> >
> > --
> > Best Regards,
> > Eugeniu.