[PATCH v2 4/6] base-files: fwtool: make compat_version backward compatible

[mail at adrianschmutzler.de]

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Paul Spooren
> Sent: Donnerstag, 16. Juli 2020 06:16
> To: Adrian Schmutzler <freifunk at adrianschmutzler.de>; openwrt-
> devel at lists.openwrt.org
> Subject: Re: [PATCH v2 4/6] base-files: fwtool: make compat_version
> backward compatible
> 
> On 14.07.20 04:28, Adrian Schmutzler wrote:
> 
> > So far, the compatibility mechanism only works if both device and
> > image are already updated to the new routines. This patch extends the
> > sysupgrade metadata and fwtool_check_image() to account for "older"
> > images as well:
> >
> > The basic mechanism for older devices to check for image compatibility
> > is the supported_devices entry. This can be exploited by putting a
> > custom message into this variable of the metadata, so older FW will
> > produce a mismatch and print the message as it thinks it's the list of
> > supported devices. So, we have two cases:
> >
> > device 1.0, image 1.0:
> >    The metadata will just contain supported_devices as before.
> >
> > device 1.0, image 1.1:
> >    The metadata will contain:
> >
> >    "new_supported_devices":["device_string1", "device_string2", ...],
> >    "supported_devices":["Upgrade incompatible. Please check Wiki ..."]
> 
> Does that mean starting from 1.1 all sysupgrade images will carry a
> informative only "supported_device" array until the end of time?

Yes. That's the only way I could think of to make this "backwards-compatible".
I just don't think it will hurt much, except for aesthetic reasons.

> This is maybe a bit of a big request but as there will be 19.07.4 release
> without DSA, couldn't some firmware migration patches be added there and
> anyone trying to upgrade from 19.07.x will receive a message like "Please
> upgrade to 19.07.4 before installing 20.x"? Maybe it's not feasible, but as

Despite, that I generally don't like the "update-to-X-first approach", I don't see how we will gain anything there for user before 19.07.4?
How would they receive the message then?

> ideally all (correct?) devices move to DSA, we will have compat 1.1
> everywhere and `supported_devices` is useless.

Generally yes, we have some devices that maybe won't be touched; but this change isn't really limited to DSA.
We just don't have any mechanism to prevent sysupgrade with kept config so far, and therefore I tried to implement this as general as possible.
Just think of the eth0/eth1 swap for ar71xx->ath79 without migration - that would be another example where we could have used such a mechanism if we already had it back then.
Maybe some devices will be at 1.3 in five years, as we introduce other incompatible changes.

It's just that so far we have nothing to treat incompatible changes, and I've been missing a comparable feature quite frequently already.

We just have to start at some point, and abusing supported_devices for backwards compatibility here seems to be the least painful way, as it won't hurt us practically, but just be a little bit ugly after all.

> 
> >    If the device is "legacy", i.e. does not have the updated fwtool.sh,
> >    it will just fail with image check and print the content of
> >    supported_devices. Upgrade can still be performed with -F like when
> >    SUPPORTED_DEVICES has been removed to prevent bricking.
> >
> >    If the device has updated fwtool.sh (but is 1.0), it will just use
> >    the new_supported_devices instead, and work as intended (flashing
> >    with -n will work, flashing without will print the appropriate
> >    warning).
> >
> > This mechanism should provide a fair tradeoff between simplicity and
> > functionality.
> >
> > Signed-off-by: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> >
> > ---
> >
> > Changes in v2:
> > - Add DEVICE_COMPAT_MESSAGE and reword comment for legacy
> warning.
> > ---
> >   include/image-commands.mk                      | 5 ++++-
> >   package/base-files/files/lib/upgrade/fwtool.sh | 7 ++++++-
> >   2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/image-commands.mk b/include/image-commands.mk
> > index 9da712e733..e21472a659 100644
> > --- a/include/image-commands.mk
> > +++ b/include/image-commands.mk
> > @@ -393,7 +393,10 @@ metadata_json = \
> >   		"metadata_version": "1.0", \
> By replacing the meaning of `supported_devices` to
> `new_supported_devices` we should also modify `metadata_version`.

Fair point. I have to check where it's actually used.

Best

Adrian

> >   		"compat_version": "$(call json_quote,$(compat_version))", \
> >   		$(if $(DEVICE_COMPAT_MESSAGE),"compat_message":
> "$(call json_quote,$(DEVICE_COMPAT_MESSAGE))"$(comma)) \
> > -		"supported_devices":[$(call
> metadata_devices,$(SUPPORTED_DEVICES))], \
> > +		$(if $(filter-out
> 1.0,$(compat_version)),"new_supported_devices":[$(call
> metadata_devices,$(SUPPORTED_DEVICES))]$(comma)) \
> > +		$(if $(filter-out 1.0,$(compat_version)),"supported_devices":
> \
> > +			["$(call json_quote,Image version $(compat_version)
> incompatible to device: $(if
> $(DEVICE_COMPAT_MESSAGE),$(DEVICE_COMPAT_MESSAGE),Please check
> documentation ...)"]$(comma)) \
> > +		$(if $(filter
> 1.0,$(compat_version)),"supported_devices":[$(call
> > +metadata_devices,$(SUPPORTED_DEVICES))]$(comma)) \
> >   		"version": { \
> >   			"dist": "$(call json_quote,$(VERSION_DIST))", \
> >   			"version": "$(call
> json_quote,$(VERSION_NUMBER))", \ diff --git
> > a/package/base-files/files/lib/upgrade/fwtool.sh
> > b/package/base-files/files/lib/upgrade/fwtool.sh
> > index 1807aecd6d..59f981f6c3 100644
> > --- a/package/base-files/files/lib/upgrade/fwtool.sh
> > +++ b/package/base-files/files/lib/upgrade/fwtool.sh
> > @@ -51,7 +51,12 @@ fwtool_check_image() {
> >   	json_get_var compatmessage compat_message
> >   	[ -n "$imagecompat" ] || imagecompat="1.0"
> >
> > -	json_select supported_devices || return 1
> > +	# select correct supported list based on compat_version
> > +	# (using this ensures that compatibility check works for devices
> > +	#  not knowing about compat-version)
> > +	local supported=supported_devices
> > +	[ "$imagecompat" != "1.0" ] && supported=new_supported_devices
> > +	json_select $supported || return 1
> >
> >   	json_get_keys dev_keys
> >   	for k in $dev_keys; do
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20200716/9d24c96b/attachment.sig>