Message ID | 20200714142825.16889-5-freifunk@adrianschmutzler.de |
---|---|
State | Accepted |
Delegated to: | Adrian Schmutzler |
Headers | show |
Series | sysupgrade: introduce compatibility version for devices | expand |
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? 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 ideally all (correct?) devices move to DSA, we will have compat 1.1 everywhere and `supported_devices` is useless. > 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@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`. > "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
> -----Original Message----- > From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] > On Behalf Of Paul Spooren > Sent: Donnerstag, 16. Juli 2020 06:16 > To: Adrian Schmutzler <freifunk@adrianschmutzler.de>; openwrt- > devel@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@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@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
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", \ "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
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 ..."] 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@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(-)