| Message ID | 20190614155056.16749-3-semen.protsenko@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Lukasz Majewski |
| Headers | show |
| Series | fastboot: Support new A/B slot format | expand |
Hi Sam, > Fastboot tool recently underwent changes w.r.t. A/B slot format: > 1. In commit [1] the new slot format was introduced, according to > new A/B specification [2]. New slot format is "a", and old format "_a" > is now considered legacy. > 2. In commit [3] the legacy format "_a" was fixed and fastboot tool > should support both "a" and "_a" slot formats now. > 3. Finally, commit [4] drops the legacy slot format ("_a") > completely. That makes the latest fastboot tool incompatible with > "_a" format. > > Last change leads to next error, when running "fastboot flash" with > current U-Boot: > > $ fastboot flash boot boot.img > Sending 'boot__a' (11301 KB) OKAY [ 0.451s] > Writing 'boot__a' FAILED (remote: 'cannot find > partition') fastboot: error: Command failed > > To overcome this issue we should report slot names in "a" format > instead of "_a". Of course, this change breaks U-Boot compatibility > with older fastboot tools, but that shouldn't be a problem as A/B is > not implemented in U-Boot yet, and there are not users for slotted > partitions out there anyway. This fact can be checked like this: > > $ grep -Ir \b'boot_a\b' * > > Let's use new slot format in order to fix "fastboot flash" with > slotted partitions and to be in sync with AOSP master. > > With this patch, U-Boot depends on most recent fastboot tool (patch > [1] or later), for working with slotted partitions. Is there any "version" number scheme for the fastboot protocol specification? I do remember that in the past there were some mismatches for some "fastboot" specification depending on vendor. Now it turns out that there is a problem between AOSP versions ... (My point is that for example even lthor had version number for specification update. Is something similar for fastboot?). > > [1] > https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5 > [2] > https://source.android.com/devices/tech/ota/ab/ab_implement#partitions > [3] > https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d > [4] > https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > --- > Changes in v2: > - don't change slot format in "slot-suffixes" variable (it's now > dropped in [PATCH 1/2]) > - improve commit message > > drivers/fastboot/fb_getvar.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/fastboot/fb_getvar.c > b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644 > --- a/drivers/fastboot/fb_getvar.c > +++ b/drivers/fastboot/fb_getvar.c > @@ -174,8 +174,8 @@ static void getvar_platform(char *var_parameter, > char *response) > static void getvar_current_slot(char *var_parameter, char *response) > { > - /* A/B not implemented, for now always return _a */ > - fastboot_okay("_a", response); > + /* A/B not implemented, for now always return "a" */ > + fastboot_okay("a", response); > } > > #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz, Seems like the versioning is basically useless in fastboot USB protocol. It exists, but only host requests the version from device (bootloader), like "fastboot getvar version". It always returns "0.4" [1], and it was unchanged for 5 years or so. I didn't find any handshake with protocol version exchange. Alas, as I mentioned before, we probably can't keep the backward compatibility in universal way. The only way I see to make U-Boot work with both "a" and "_a" prefixes, is to introduce some new config option or dedicated environment variable, so that user can specify which format should be used. I suggest merging this as is, as we don't have any users for slotted partitions in upstream U-Boot anyway, at this moment. If need arises, we can add option or variable later. Of course, I don't suggest to merge this right now, as codebase is frozen, and this is not an ideal bugfix and can even break backward compatibility. Let's take it when merge window opens. Please tell me what is the preferred course of actions on this one, from your point of view. Thanks! [1] http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/fastboot/fb_getvar.c;h=4268628f5ef0210507c5d23f2e4209b2afc07029;hb=refs/heads/master#l84 [2] https://android.googlesource.com/platform/system/core/+/9bfecb0e3444306ec57d7fafe4a99a47d3848c17%5E%21/#F0 On Sat, Jun 15, 2019 at 6:29 PM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Sam, > > > Fastboot tool recently underwent changes w.r.t. A/B slot format: > > 1. In commit [1] the new slot format was introduced, according to > > new A/B specification [2]. New slot format is "a", and old format "_a" > > is now considered legacy. > > 2. In commit [3] the legacy format "_a" was fixed and fastboot tool > > should support both "a" and "_a" slot formats now. > > 3. Finally, commit [4] drops the legacy slot format ("_a") > > completely. That makes the latest fastboot tool incompatible with > > "_a" format. > > > > Last change leads to next error, when running "fastboot flash" with > > current U-Boot: > > > > $ fastboot flash boot boot.img > > Sending 'boot__a' (11301 KB) OKAY [ 0.451s] > > Writing 'boot__a' FAILED (remote: 'cannot find > > partition') fastboot: error: Command failed > > > > To overcome this issue we should report slot names in "a" format > > instead of "_a". Of course, this change breaks U-Boot compatibility > > with older fastboot tools, but that shouldn't be a problem as A/B is > > not implemented in U-Boot yet, and there are not users for slotted > > partitions out there anyway. This fact can be checked like this: > > > > $ grep -Ir \b'boot_a\b' * > > > > Let's use new slot format in order to fix "fastboot flash" with > > slotted partitions and to be in sync with AOSP master. > > > > With this patch, U-Boot depends on most recent fastboot tool (patch > > [1] or later), for working with slotted partitions. > > Is there any "version" number scheme for the fastboot protocol > specification? > > I do remember that in the past there were some mismatches for some > "fastboot" specification depending on vendor. Now it turns out that > there is a problem between AOSP versions ... > > (My point is that for example even lthor had version number for > specification update. Is something similar for fastboot?). > > > > > [1] > > https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5 > > [2] > > https://source.android.com/devices/tech/ota/ab/ab_implement#partitions > > [3] > > https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d > > [4] > > https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f > > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > --- > > Changes in v2: > > - don't change slot format in "slot-suffixes" variable (it's now > > dropped in [PATCH 1/2]) > > - improve commit message > > > > drivers/fastboot/fb_getvar.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/fastboot/fb_getvar.c > > b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644 > > --- a/drivers/fastboot/fb_getvar.c > > +++ b/drivers/fastboot/fb_getvar.c > > @@ -174,8 +174,8 @@ static void getvar_platform(char *var_parameter, > > char *response) > > static void getvar_current_slot(char *var_parameter, char *response) > > { > > - /* A/B not implemented, for now always return _a */ > > - fastboot_okay("_a", response); > > + /* A/B not implemented, for now always return "a" */ > > + fastboot_okay("a", response); > > } > > > > #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) > > > > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Tue, 18 Jun 2019 17:34:54 +0300 Sam Protsenko <semen.protsenko@linaro.org> wrote: > Hi Lukasz, > > Seems like the versioning is basically useless in fastboot USB > protocol. It exists, but only host requests the version from device > (bootloader), like "fastboot getvar version". It always returns "0.4" > [1], and it was unchanged for 5 years or so. I didn't find any > handshake with protocol version exchange. Alas, as I mentioned before, > we probably can't keep the backward compatibility in universal way. > The only way I see to make U-Boot work with both "a" and "_a" > prefixes, is to introduce some new config option or dedicated > environment variable, so that user can specify which format should be > used. > > I suggest merging this as is, as we don't have any users for slotted > partitions in upstream U-Boot anyway, at this moment. If need arises, > we can add option or variable later. Of course, I don't suggest to > merge this right now, as codebase is frozen, and this is not an ideal > bugfix and can even break backward compatibility. Let's take it when > merge window opens. > > Please tell me what is the preferred course of actions on this one, > from your point of view. As you stated in the other mail - you shall prepare some fix for current code base (without the _a suffix) for this release. Please proceed. Regarding the supported features - maybe we shall introduce new env variable - like fastboot_version = "0.5" (then - 0.6, 0.7). And in U-Boot just decide which set of features would be 0.5 or 0.6. Then, we can also use Kconfig option to enable needed features. Otherwise, we will have regressions all the time as the "fastboot" protocol changes without any notice in the versioning (inside the protocol). > > Thanks! > > [1] > http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/fastboot/fb_getvar.c;h=4268628f5ef0210507c5d23f2e4209b2afc07029;hb=refs/heads/master#l84 > [2] > https://android.googlesource.com/platform/system/core/+/9bfecb0e3444306ec57d7fafe4a99a47d3848c17%5E%21/#F0 > > On Sat, Jun 15, 2019 at 6:29 PM Lukasz Majewski <lukma@denx.de> wrote: > > > > Hi Sam, > > > > > Fastboot tool recently underwent changes w.r.t. A/B slot format: > > > 1. In commit [1] the new slot format was introduced, according > > > to new A/B specification [2]. New slot format is "a", and old > > > format "_a" is now considered legacy. > > > 2. In commit [3] the legacy format "_a" was fixed and fastboot > > > tool should support both "a" and "_a" slot formats now. > > > 3. Finally, commit [4] drops the legacy slot format ("_a") > > > completely. That makes the latest fastboot tool incompatible with > > > "_a" format. > > > > > > Last change leads to next error, when running "fastboot flash" > > > with current U-Boot: > > > > > > $ fastboot flash boot boot.img > > > Sending 'boot__a' (11301 KB) OKAY [ 0.451s] > > > Writing 'boot__a' FAILED (remote: 'cannot find > > > partition') fastboot: error: Command failed > > > > > > To overcome this issue we should report slot names in "a" format > > > instead of "_a". Of course, this change breaks U-Boot > > > compatibility with older fastboot tools, but that shouldn't be a > > > problem as A/B is not implemented in U-Boot yet, and there are > > > not users for slotted partitions out there anyway. This fact can > > > be checked like this: > > > > > > $ grep -Ir \b'boot_a\b' * > > > > > > Let's use new slot format in order to fix "fastboot flash" with > > > slotted partitions and to be in sync with AOSP master. > > > > > > With this patch, U-Boot depends on most recent fastboot tool > > > (patch [1] or later), for working with slotted partitions. > > > > Is there any "version" number scheme for the fastboot protocol > > specification? > > > > I do remember that in the past there were some mismatches for some > > "fastboot" specification depending on vendor. Now it turns out that > > there is a problem between AOSP versions ... > > > > (My point is that for example even lthor had version number for > > specification update. Is something similar for fastboot?). > > > > > > > > [1] > > > https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5 > > > [2] > > > https://source.android.com/devices/tech/ota/ab/ab_implement#partitions > > > [3] > > > https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d > > > [4] > > > https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f > > > > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > > --- > > > Changes in v2: > > > - don't change slot format in "slot-suffixes" variable (it's now > > > dropped in [PATCH 1/2]) > > > - improve commit message > > > > > > drivers/fastboot/fb_getvar.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/fastboot/fb_getvar.c > > > b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644 > > > --- a/drivers/fastboot/fb_getvar.c > > > +++ b/drivers/fastboot/fb_getvar.c > > > @@ -174,8 +174,8 @@ static void getvar_platform(char > > > *var_parameter, char *response) > > > static void getvar_current_slot(char *var_parameter, char > > > *response) { > > > - /* A/B not implemented, for now always return _a */ > > > - fastboot_okay("_a", response); > > > + /* A/B not implemented, for now always return "a" */ > > > + fastboot_okay("a", response); > > > } > > > > > > #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) > > > > > > > > > > Best regards, > > > > Lukasz Majewski > > > > -- > > > > DENX Software Engineering GmbH, Managing Director: Wolfgang > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: > > lukma@denx.de Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Wed, Jun 19, 2019 at 12:10 AM Lukasz Majewski <lukma@denx.de> wrote: > > On Tue, 18 Jun 2019 17:34:54 +0300 > Sam Protsenko <semen.protsenko@linaro.org> wrote: > > > Hi Lukasz, > > > > Seems like the versioning is basically useless in fastboot USB > > protocol. It exists, but only host requests the version from device > > (bootloader), like "fastboot getvar version". It always returns "0.4" > > [1], and it was unchanged for 5 years or so. I didn't find any > > handshake with protocol version exchange. Alas, as I mentioned before, > > we probably can't keep the backward compatibility in universal way. > > The only way I see to make U-Boot work with both "a" and "_a" > > prefixes, is to introduce some new config option or dedicated > > environment variable, so that user can specify which format should be > > used. > > > > I suggest merging this as is, as we don't have any users for slotted > > partitions in upstream U-Boot anyway, at this moment. If need arises, > > we can add option or variable later. Of course, I don't suggest to > > merge this right now, as codebase is frozen, and this is not an ideal > > bugfix and can even break backward compatibility. Let's take it when > > merge window opens. > > > > Please tell me what is the preferred course of actions on this one, > > from your point of view. > > As you stated in the other mail - you shall prepare some fix for > current code base (without the _a suffix) for this release. Please > proceed. > Already did, in this patch: [PATCH v3] fastboot: Remove "slot-suffixes" variable Hope it's what we have agreed on (it's not for v2019.07, but for next release, should be applied when merge window opens). If no, please correct me. There was a lot of confusion going on w.r.t. this patch series :) > Regarding the supported features - maybe we shall introduce new env > variable - like fastboot_version = "0.5" (then - 0.6, 0.7). > > And in U-Boot just decide which set of features would be 0.5 or 0.6. > > Then, we can also use Kconfig option to enable needed features. > > Otherwise, we will have regressions all the time as the "fastboot" > protocol changes without any notice in the versioning (inside the > protocol). > Agreed. As A/B is not implemented in U-Boot right now, and we don't have any users for A/B slotted partitions, I presume this is not going to be a regression, so we can skip this step right now. But on next major change in fastboot protocol, this should be implemented. > > > > Thanks! > > > > [1] > > http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/fastboot/fb_getvar.c;h=4268628f5ef0210507c5d23f2e4209b2afc07029;hb=refs/heads/master#l84 > > [2] > > https://android.googlesource.com/platform/system/core/+/9bfecb0e3444306ec57d7fafe4a99a47d3848c17%5E%21/#F0 > > > > On Sat, Jun 15, 2019 at 6:29 PM Lukasz Majewski <lukma@denx.de> wrote: > > > > > > Hi Sam, > > > > > > > Fastboot tool recently underwent changes w.r.t. A/B slot format: > > > > 1. In commit [1] the new slot format was introduced, according > > > > to new A/B specification [2]. New slot format is "a", and old > > > > format "_a" is now considered legacy. > > > > 2. In commit [3] the legacy format "_a" was fixed and fastboot > > > > tool should support both "a" and "_a" slot formats now. > > > > 3. Finally, commit [4] drops the legacy slot format ("_a") > > > > completely. That makes the latest fastboot tool incompatible with > > > > "_a" format. > > > > > > > > Last change leads to next error, when running "fastboot flash" > > > > with current U-Boot: > > > > > > > > $ fastboot flash boot boot.img > > > > Sending 'boot__a' (11301 KB) OKAY [ 0.451s] > > > > Writing 'boot__a' FAILED (remote: 'cannot find > > > > partition') fastboot: error: Command failed > > > > > > > > To overcome this issue we should report slot names in "a" format > > > > instead of "_a". Of course, this change breaks U-Boot > > > > compatibility with older fastboot tools, but that shouldn't be a > > > > problem as A/B is not implemented in U-Boot yet, and there are > > > > not users for slotted partitions out there anyway. This fact can > > > > be checked like this: > > > > > > > > $ grep -Ir \b'boot_a\b' * > > > > > > > > Let's use new slot format in order to fix "fastboot flash" with > > > > slotted partitions and to be in sync with AOSP master. > > > > > > > > With this patch, U-Boot depends on most recent fastboot tool > > > > (patch [1] or later), for working with slotted partitions. > > > > > > Is there any "version" number scheme for the fastboot protocol > > > specification? > > > > > > I do remember that in the past there were some mismatches for some > > > "fastboot" specification depending on vendor. Now it turns out that > > > there is a problem between AOSP versions ... > > > > > > (My point is that for example even lthor had version number for > > > specification update. Is something similar for fastboot?). > > > > > > > > > > > [1] > > > > https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5 > > > > [2] > > > > https://source.android.com/devices/tech/ota/ab/ab_implement#partitions > > > > [3] > > > > https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d > > > > [4] > > > > https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f > > > > > > > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > > > > --- > > > > Changes in v2: > > > > - don't change slot format in "slot-suffixes" variable (it's now > > > > dropped in [PATCH 1/2]) > > > > - improve commit message > > > > > > > > drivers/fastboot/fb_getvar.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/fastboot/fb_getvar.c > > > > b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644 > > > > --- a/drivers/fastboot/fb_getvar.c > > > > +++ b/drivers/fastboot/fb_getvar.c > > > > @@ -174,8 +174,8 @@ static void getvar_platform(char > > > > *var_parameter, char *response) > > > > static void getvar_current_slot(char *var_parameter, char > > > > *response) { > > > > - /* A/B not implemented, for now always return _a */ > > > > - fastboot_okay("_a", response); > > > > + /* A/B not implemented, for now always return "a" */ > > > > + fastboot_okay("a", response); > > > > } > > > > > > > > #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) > > > > > > > > > > > > > > > Best regards, > > > > > > Lukasz Majewski > > > > > > -- > > > > > > DENX Software Engineering GmbH, Managing Director: Wolfgang > > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: > > > lukma@denx.de > > > > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index f89c7f62e6..9ee5054485 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -174,8 +174,8 @@ static void getvar_platform(char *var_parameter, char *response) static void getvar_current_slot(char *var_parameter, char *response) { - /* A/B not implemented, for now always return _a */ - fastboot_okay("_a", response); + /* A/B not implemented, for now always return "a" */ + fastboot_okay("a", response); } #if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
Fastboot tool recently underwent changes w.r.t. A/B slot format: 1. In commit [1] the new slot format was introduced, according to new A/B specification [2]. New slot format is "a", and old format "_a" is now considered legacy. 2. In commit [3] the legacy format "_a" was fixed and fastboot tool should support both "a" and "_a" slot formats now. 3. Finally, commit [4] drops the legacy slot format ("_a") completely. That makes the latest fastboot tool incompatible with "_a" format. Last change leads to next error, when running "fastboot flash" with current U-Boot: $ fastboot flash boot boot.img Sending 'boot__a' (11301 KB) OKAY [ 0.451s] Writing 'boot__a' FAILED (remote: 'cannot find partition') fastboot: error: Command failed To overcome this issue we should report slot names in "a" format instead of "_a". Of course, this change breaks U-Boot compatibility with older fastboot tools, but that shouldn't be a problem as A/B is not implemented in U-Boot yet, and there are not users for slotted partitions out there anyway. This fact can be checked like this: $ grep -Ir \b'boot_a\b' * Let's use new slot format in order to fix "fastboot flash" with slotted partitions and to be in sync with AOSP master. With this patch, U-Boot depends on most recent fastboot tool (patch [1] or later), for working with slotted partitions. [1] https://android.googlesource.com/platform/system/core/+/8091947847d5e5130b09d2ac0a4bdc900f3b77c5 [2] https://source.android.com/devices/tech/ota/ab/ab_implement#partitions [3] https://android.googlesource.com/platform/system/core/+/04396f62da6150b94e02d50e5302fd980048833d [4] https://android.googlesource.com/platform/system/core/+/42b18a518bac85c3eea14206f6cbafbd1e98a31f Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> --- Changes in v2: - don't change slot format in "slot-suffixes" variable (it's now dropped in [PATCH 1/2]) - improve commit message drivers/fastboot/fb_getvar.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)