diff mbox series

[OpenWrt-Devel,3/3] treewide: sysupgrade: use $UPGRADE_BACKUP to check for backup

Message ID 20190906051054.18311-4-zajec5@gmail.com
State Accepted
Delegated to: Rafał Miłecki
Headers show
Series sysupgrade: support custom backup paths | expand

Commit Message

Rafał Miłecki Sept. 6, 2019, 5:10 a.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

Now that $UPGRADE_BACKUP is set conditionally there is no need to check
the $UPGRADE_OPT_SAVE_CONFIG anymore. All conditions can be simplified.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 package/base-files/files/lib/upgrade/common.sh          | 2 +-
 package/base-files/files/lib/upgrade/do_stage2          | 2 +-
 package/base-files/files/sbin/sysupgrade                | 1 -
 target/linux/ar71xx/base-files/lib/upgrade/dir825.sh    | 2 +-
 target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh  | 2 +-
 target/linux/ar71xx/base-files/lib/upgrade/platform.sh  | 4 ++--
 target/linux/ath25/base-files/lib/upgrade/platform.sh   | 2 +-
 target/linux/ath79/base-files/lib/upgrade/platform.sh   | 4 ++--
 target/linux/imx6/base-files/lib/upgrade/platform.sh    | 2 +-
 target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh | 2 +-
 target/linux/ipq40xx/base-files/lib/upgrade/platform.sh | 2 +-
 target/linux/ixp4xx/base-files/lib/upgrade/platform.sh  | 2 +-
 12 files changed, 13 insertions(+), 14 deletions(-)

Comments

Adrian Schmutzler Sept. 6, 2019, 1:05 p.m. UTC | #1
Hi,

just a nitpick:

> diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> index e313562017..8e02186eb8 100644
> --- a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> +++ b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> @@ -74,7 +74,7 @@ platform_do_upgrade_openmesh() {
>  	#
> 
>  	# take care of restoring a saved config
> -	[ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"
> +	[ -n "$UPGRADE_BACKUP" ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"

Any reason for the curly braces here?

If not, I'd consider removing them with this patch ...

Best

Adrian
Rafał Miłecki Sept. 6, 2019, 1:12 p.m. UTC | #2
On 2019-09-06 15:05, Adrian Schmutzler wrote:
>> diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh 
>> b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
>> index e313562017..8e02186eb8 100644
>> --- a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
>> +++ b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
>> @@ -74,7 +74,7 @@ platform_do_upgrade_openmesh() {
>>  	#
>> 
>>  	# take care of restoring a saved config
>> -	[ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && 
>> restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"
>> +	[ -n "$UPGRADE_BACKUP" ] && restore_backup="${MTD_CONFIG_ARGS} -j 
>> ${UPGRADE_BACKUP}"
> 
> Any reason for the curly braces here?
> 
> If not, I'd consider removing them with this patch ...

I just left existing coding style
Adrian Schmutzler Sept. 6, 2019, 1:16 p.m. UTC | #3
> -----Original Message-----
> From: Rafał Miłecki [mailto:rafal@milecki.pl]
> Sent: Freitag, 6. September 2019 15:13
> To: Adrian Schmutzler <mail@adrianschmutzler.de>
> Cc: 'Rafał Miłecki' <zajec5@gmail.com>; openwrt-devel@lists.openwrt.org; 'Jonas Gorski' <jonas.gorski@gmail.com>; 'Jo-Philipp Wich'
> <jo@mein.io>; 'John Crispin' <john@phrozen.org>
> Subject: Re: [OpenWrt-Devel] [PATCH 3/3] treewide: sysupgrade: use $UPGRADE_BACKUP to check for backup
> 
> On 2019-09-06 15:05, Adrian Schmutzler wrote:
> >> diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> >> b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> >> index e313562017..8e02186eb8 100644
> >> --- a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> >> +++ b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> >> @@ -74,7 +74,7 @@ platform_do_upgrade_openmesh() {
> >>  	#
> >>
> >>  	# take care of restoring a saved config
> >> -	[ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] &&
> >> restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"
> >> +	[ -n "$UPGRADE_BACKUP" ] && restore_backup="${MTD_CONFIG_ARGS} -j
> >> ${UPGRADE_BACKUP}"
> >
> > Any reason for the curly braces here?
> >
> > If not, I'd consider removing them with this patch ...
> 
> I just left existing coding style

I'm aware of that, I just saw that from all changed lines/files this is the only one using the curly braces (without necessity).

However, this was just a suggestion and it is purely cosmetical, so feel free to ignore it :-)
Adrian Schmutzler Sept. 11, 2019, 10:55 a.m. UTC | #4
Hi,

when looking at the merged patch (unfortunately only then), I found some "issues" (see below):

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On Behalf Of Rafal Milecki
> Sent: Freitag, 6. September 2019 07:11
> To: openwrt-devel@lists.openwrt.org
> Cc: Rafał Miłecki <rafal@milecki.pl>; Jonas Gorski <jonas.gorski@gmail.com>; Jo-Philipp Wich <jo@mein.io>; John Crispin
> <john@phrozen.org>
> Subject: [OpenWrt-Devel] [PATCH 3/3] treewide: sysupgrade: use $UPGRADE_BACKUP to check for backup
> 
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Now that $UPGRADE_BACKUP is set conditionally there is no need to check
> the $UPGRADE_OPT_SAVE_CONFIG anymore. All conditions can be simplified.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  package/base-files/files/lib/upgrade/common.sh          | 2 +-
>  package/base-files/files/lib/upgrade/do_stage2          | 2 +-
>  package/base-files/files/sbin/sysupgrade                | 1 -
>  target/linux/ar71xx/base-files/lib/upgrade/dir825.sh    | 2 +-
>  target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh  | 2 +-
>  target/linux/ar71xx/base-files/lib/upgrade/platform.sh  | 4 ++--
>  target/linux/ath25/base-files/lib/upgrade/platform.sh   | 2 +-
>  target/linux/ath79/base-files/lib/upgrade/platform.sh   | 4 ++--
>  target/linux/imx6/base-files/lib/upgrade/platform.sh    | 2 +-
>  target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh | 2 +-
>  target/linux/ipq40xx/base-files/lib/upgrade/platform.sh | 2 +-
>  target/linux/ixp4xx/base-files/lib/upgrade/platform.sh  | 2 +-
>  12 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/package/base-files/files/lib/upgrade/common.sh b/package/base-files/files/lib/upgrade/common.sh
> index 8e7866f698..0d3162d4fc 100644
> --- a/package/base-files/files/lib/upgrade/common.sh
> +++ b/package/base-files/files/lib/upgrade/common.sh
> @@ -220,7 +220,7 @@ indicate_upgrade() {
>  # $(2): (optional) pipe command to extract firmware, e.g. dd bs=n skip=m
>  default_do_upgrade() {
>  	sync
> -	if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ]; then
> +	if [ -n "$UPGRADE_BACKUP" ]; then

Any reason why this is "-n" and not "-f" as below?

>  		get_image "$1" "$2" | mtd $MTD_ARGS $MTD_CONFIG_ARGS -j "$UPGRADE_BACKUP" write - "${PART_NAME:-
> image}"
>  	else
>  		get_image "$1" "$2" | mtd $MTD_ARGS write - "${PART_NAME:-image}"
> diff --git a/package/base-files/files/lib/upgrade/do_stage2 b/package/base-files/files/lib/upgrade/do_stage2
> index 0e6cc1bfc3..0e32445743 100755
> --- a/package/base-files/files/lib/upgrade/do_stage2
> +++ b/package/base-files/files/lib/upgrade/do_stage2
> @@ -11,7 +11,7 @@ else
>  	default_do_upgrade "$IMAGE"
>  fi
> 
> -if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && type 'platform_copy_config' >/dev/null 2>/dev/null; then
> +if [ -n "$UPGRADE_BACKUP" ] && type 'platform_copy_config' >/dev/null 2>/dev/null; then

Here I'm not so sure about "-f" vs. "-n" ...

>  	platform_copy_config
>  fi
> 
> diff --git a/package/base-files/files/sbin/sysupgrade b/package/base-files/files/sbin/sysupgrade
> index f18143bff4..935d08048e 100755
> --- a/package/base-files/files/sbin/sysupgrade
> +++ b/package/base-files/files/sbin/sysupgrade
> @@ -371,7 +371,6 @@ else
>  		$backup_attr
>  		\"command\": $(json_string "$COMMAND"),
>  		\"options\": {
> -			\"save_config\": $SAVE_CONFIG,
>  			\"save_partitions\": $SAVE_PARTITIONS
>  		}
>  	}"
> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
> index c694c2e6f2..e991a06b7a 100644
> --- a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
> +++ b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
> @@ -75,7 +75,7 @@ dir825b_do_upgrade_combined() {
> 
>  	if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then
>  		local append=""
> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
> 
>  		sync
>  		dd if="$fw_file" bs=64k skip=1 count=$fw_blocks 2>/dev/null | \
> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
> index 8536d4ba4a..f43bdcea7f 100644
> --- a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
> +++ b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
> @@ -159,7 +159,7 @@ platform_do_upgrade_openmesh()
>  	local cfg_size= kernel_size= rootfs_size=
>  	local append=""
> 
> -	[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
> +	[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
> 
>  	cfg_size=$(dd if="$img_path" bs=2 skip=35 count=4 2>/dev/null)
>  	kernel_size=$(dd if="$img_path" bs=2 skip=71 count=4 2>/dev/null)
> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
> index 726163291d..86e7b6386b 100755
> --- a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
> @@ -65,7 +65,7 @@ platform_do_upgrade_combined() {
>  	then
>  		local rootfspart=$(platform_find_rootfspart "$partitions" "$kernelpart")
>  		local append=""
> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
> 
>  		if [ "$PLATFORM_DO_UPGRADE_COMBINED_SEPARATE_MTD" -ne 1 ]; then
>  		    ( dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null; \
> @@ -164,7 +164,7 @@ platform_do_upgrade_compex() {
> 
>  	if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then
>  		local append=""
> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUPs"
> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUPs"

Is there a reason for the trailing "s" here or is this a typo: ="-j $UPGRADE_BACKUPs" ?

> 
>  		sync
>  		dd if="$fw_file" bs=64k skip=1 count=$fw_blocks 2>/dev/null | \
> diff --git a/target/linux/ath25/base-files/lib/upgrade/platform.sh b/target/linux/ath25/base-files/lib/upgrade/platform.sh
> index 0dde103605..778bbf5a39 100644
> --- a/target/linux/ath25/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/ath25/base-files/lib/upgrade/platform.sh
> @@ -67,7 +67,7 @@ platform_do_upgrade() {
>  	   [ ${erase_size:-0} -gt 0 ];
>  	then
>  		local append=""
> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
> 
>  		( dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null; \
>  		  dd if="$1" bs=$CI_BLKSZ skip=$((1+$kern_blocks)) count=$root_blocks 2>/dev/null ) | \
> diff --git a/target/linux/ath79/base-files/lib/upgrade/platform.sh b/target/linux/ath79/base-files/lib/upgrade/platform.sh
> index f7e62143e7..f4fca06384 100644
> --- a/target/linux/ath79/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/ath79/base-files/lib/upgrade/platform.sh
> @@ -14,7 +14,7 @@ redboot_fis_do_upgrade() {
>  	if [ "$magic" = "4349" ]; then
>  		local kern_length=0x$(dd if="$sysup_file" bs=2 skip=1 count=4 2>/dev/null)
> 
> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j
> $UPGRADE_BACKUP"
> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
>  		dd if="$sysup_file" bs=64k skip=1 2>/dev/null | \
>  			mtd -r $append -F$kern_part:$kern_length:0x80060000,rootfs write - $kern_part:rootfs
> 
> @@ -22,7 +22,7 @@ redboot_fis_do_upgrade() {
>  		local board_dir=$(tar tf $sysup_file | grep -m 1 '^sysupgrade-.*/$')
>  		local kern_length=$(tar xf $sysup_file ${board_dir}kernel -O | wc -c)
> 
> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j
> $UPGRADE_BACKUP"
> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
>  		tar xf $sysup_file ${board_dir}kernel ${board_dir}root -O | \
>  			mtd -r $append -F$kern_part:$kern_length:0x80060000,rootfs write - $kern_part:rootfs
> 
> diff --git a/target/linux/imx6/base-files/lib/upgrade/platform.sh b/target/linux/imx6/base-files/lib/upgrade/platform.sh
> index 9414b18710..a090cc080b 100755
> --- a/target/linux/imx6/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/imx6/base-files/lib/upgrade/platform.sh
> @@ -75,7 +75,7 @@ platform_pre_upgrade() {
> 
>  	case "$board" in
>  	apalis*)
> -		[ "$UPGRADE_OPT_SAVE_CONFIG" -eq 0 ] && {
> +		[ -z "$UPGRADE_BACKUP" ] && {

Really "-z" or "! -f"?

>  			jffs2reset -y
>  			umount /overlay
>  		}
> diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> index e313562017..8e02186eb8 100644
> --- a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> +++ b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
> @@ -74,7 +74,7 @@ platform_do_upgrade_openmesh() {
>  	#
> 
>  	# take care of restoring a saved config
> -	[ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"
> +	[ -n "$UPGRADE_BACKUP" ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"

"-f" here, too?

> 
>  	mtd -q erase inactive
>  	tar xf $tar_file ${board_dir}/root -O | mtd -n -p $kernel_length $restore_backup write - $PART_NAME
> diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh b/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh
> index 6b9858beb0..c12508c437 100644
> --- a/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh
> @@ -37,7 +37,7 @@ zyxel_do_upgrade() {
> 
>  	tar Oxf $tar_file ${board_dir}/kernel | mtd write - kernel
> 
> -	if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ]; then
> +	if [ -n "$UPGRADE_BACKUP" ]; then

"-f" here, too?

Sorry for being late with this.

Best

Adrian

>  		tar Oxf $tar_file ${board_dir}/root | mtd -j "$UPGRADE_BACKUP" write - rootfs
>  	else
>  		tar Oxf $tar_file ${board_dir}/root | mtd write - rootfs
> diff --git a/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh b/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh
> index 557f43ce6f..f83aa430cf 100644
> --- a/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh
> +++ b/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh
> @@ -68,7 +68,7 @@ platform_do_upgrade_combined() {
>  	   [ ${erase_size:-0} -gt 0 ];
>  	then
>  		local append=""
> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
> 
>  		# write the kernel
>  		dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null | \
> --
> 2.21.0
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Adrian Schmutzler Sept. 15, 2019, 9:54 a.m. UTC | #5
Hi,

 

please also backport this to 19.07, since the variables for ath79 are still wrong there.

 

Despite, maybe have a look at my annotations below, at least one of them might require a fix…

 

Best

 

Adrian

 

From: Adrian Schmutzler [mailto:mail@adrianschmutzler.de] 
Sent: Mittwoch, 11. September 2019 12:56
To: 'Rafał Miłecki' <zajec5@gmail.com>; openwrt-devel@lists.openwrt.org
Cc: 'Rafał Miłecki' <rafal@milecki.pl>; 'Jonas Gorski' <jonas.gorski@gmail.com>; 'Jo-Philipp Wich' <jo@mein.io>; 'John Crispin' <john@phrozen.org>
Subject: RE: [OpenWrt-Devel] [PATCH 3/3] treewide: sysupgrade: use $UPGRADE_BACKUP to check for backup

 

Hi, 

when looking at the merged patch (unfortunately only then), I found some "issues" (see below): 

> -----Original Message----- 
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On Behalf Of Rafal Milecki 
> Sent: Freitag, 6. September 2019 07:11 
> To: openwrt-devel@lists.openwrt.org <mailto:openwrt-devel@lists.openwrt.org>  
> Cc: Rafał Miłecki <rafal@milecki.pl <mailto:rafal@milecki.pl> >; Jonas Gorski <jonas.gorski@gmail.com <mailto:jonas.gorski@gmail.com> >; Jo-Philipp Wich <jo@mein.io <mailto:jo@mein.io> >; John Crispin

> <john@phrozen.org <mailto:john@phrozen.org> > 
> Subject: [OpenWrt-Devel] [PATCH 3/3] treewide: sysupgrade: use $UPGRADE_BACKUP to check for backup 
> 
> From: Rafał Miłecki <rafal@milecki.pl <mailto:rafal@milecki.pl> > 
> 
> Now that $UPGRADE_BACKUP is set conditionally there is no need to check 
> the $UPGRADE_OPT_SAVE_CONFIG anymore. All conditions can be simplified. 
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl <mailto:rafal@milecki.pl> > 
> --- 
>  package/base-files/files/lib/upgrade/common.sh          | 2 +- 
>  package/base-files/files/lib/upgrade/do_stage2          | 2 +- 
>  package/base-files/files/sbin/sysupgrade                | 1 - 
>  target/linux/ar71xx/base-files/lib/upgrade/dir825.sh    | 2 +- 
>  target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh  | 2 +- 
>  target/linux/ar71xx/base-files/lib/upgrade/platform.sh  | 4 ++-- 
>  target/linux/ath25/base-files/lib/upgrade/platform.sh   | 2 +- 
>  target/linux/ath79/base-files/lib/upgrade/platform.sh   | 4 ++-- 
>  target/linux/imx6/base-files/lib/upgrade/platform.sh    | 2 +- 
>  target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh | 2 +- 
>  target/linux/ipq40xx/base-files/lib/upgrade/platform.sh | 2 +- 
>  target/linux/ixp4xx/base-files/lib/upgrade/platform.sh  | 2 +- 
>  12 files changed, 13 insertions(+), 14 deletions(-) 
> 
> diff --git a/package/base-files/files/lib/upgrade/common.sh b/package/base-files/files/lib/upgrade/common.sh 
> index 8e7866f698..0d3162d4fc 100644 
> --- a/package/base-files/files/lib/upgrade/common.sh 
> +++ b/package/base-files/files/lib/upgrade/common.sh 
> @@ -220,7 +220,7 @@ indicate_upgrade() { 
>  # $(2): (optional) pipe command to extract firmware, e.g. dd bs=n skip=m 
>  default_do_upgrade() { 
>       sync 
> -     if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ]; then 
> +     if [ -n "$UPGRADE_BACKUP" ]; then 

Any reason why this is "-n" and not "-f" as below? 

>               get_image "$1" "$2" | mtd $MTD_ARGS $MTD_CONFIG_ARGS -j "$UPGRADE_BACKUP" write - "${PART_NAME:- 
> image}" 
>       else 
>               get_image "$1" "$2" | mtd $MTD_ARGS write - "${PART_NAME:-image}" 
> diff --git a/package/base-files/files/lib/upgrade/do_stage2 b/package/base-files/files/lib/upgrade/do_stage2 
> index 0e6cc1bfc3..0e32445743 100755 
> --- a/package/base-files/files/lib/upgrade/do_stage2 
> +++ b/package/base-files/files/lib/upgrade/do_stage2 
> @@ -11,7 +11,7 @@ else 
>       default_do_upgrade "$IMAGE" 
>  fi 
> 
> -if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && type 'platform_copy_config' >/dev/null 2>/dev/null; then 
> +if [ -n "$UPGRADE_BACKUP" ] && type 'platform_copy_config' >/dev/null 2>/dev/null; then 

Here I'm not so sure about "-f" vs. "-n" ... 

>       platform_copy_config 
>  fi 
> 
> diff --git a/package/base-files/files/sbin/sysupgrade b/package/base-files/files/sbin/sysupgrade 
> index f18143bff4..935d08048e 100755 
> --- a/package/base-files/files/sbin/sysupgrade 
> +++ b/package/base-files/files/sbin/sysupgrade 
> @@ -371,7 +371,6 @@ else 
>               $backup_attr 
>               \"command\": $(json_string "$COMMAND"), 
>               \"options\": { 
> -                     \"save_config\": $SAVE_CONFIG, 
>                       \"save_partitions\": $SAVE_PARTITIONS 
>               } 
>       }" 
> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh

> index c694c2e6f2..e991a06b7a 100644 
> --- a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh 
> +++ b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh 
> @@ -75,7 +75,7 @@ dir825b_do_upgrade_combined() { 
> 
>       if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then 
>               local append="" 
> -             [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP" 
> +             [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP" 
> 
>               sync 
>               dd if="$fw_file" bs=64k skip=1 count=$fw_blocks 2>/dev/null | \ 
> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh

> index 8536d4ba4a..f43bdcea7f 100644 
> --- a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh 
> +++ b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh 
> @@ -159,7 +159,7 @@ platform_do_upgrade_openmesh() 
>       local cfg_size= kernel_size= rootfs_size= 
>       local append="" 
> 
> -     [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP" 
> +     [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP" 
> 
>       cfg_size=$(dd if="$img_path" bs=2 skip=35 count=4 2>/dev/null) 
>       kernel_size=$(dd if="$img_path" bs=2 skip=71 count=4 2>/dev/null) 
> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh

> index 726163291d..86e7b6386b 100755 
> --- a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh 
> +++ b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh 
> @@ -65,7 +65,7 @@ platform_do_upgrade_combined() { 
>       then 
>               local rootfspart=$(platform_find_rootfspart "$partitions" "$kernelpart") 
>               local append="" 
> -             [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP" 
> +             [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP" 
> 
>               if [ "$PLATFORM_DO_UPGRADE_COMBINED_SEPARATE_MTD" -ne 1 ]; then 
>                   ( dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null; \ 
> @@ -164,7 +164,7 @@ platform_do_upgrade_compex() { 
> 
>       if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then 
>               local append="" 
> -             [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUPs" 
> +             [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUPs" 

Is there a reason for the trailing "s" here or is this a typo: ="-j $UPGRADE_BACKUPs" ? 

> 
>               sync 
>               dd if="$fw_file" bs=64k skip=1 count=$fw_blocks 2>/dev/null | \ 
> diff --git a/target/linux/ath25/base-files/lib/upgrade/platform.sh b/target/linux/ath25/base-files/lib/upgrade/platform.sh

> index 0dde103605..778bbf5a39 100644 
> --- a/target/linux/ath25/base-files/lib/upgrade/platform.sh 
> +++ b/target/linux/ath25/base-files/lib/upgrade/platform.sh 
> @@ -67,7 +67,7 @@ platform_do_upgrade() { 
>          [ ${erase_size:-0} -gt 0 ]; 
>       then 
>               local append="" 
> -             [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP" 
> +             [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP" 
> 
>               ( dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null; \ 
>                 dd if="$1" bs=$CI_BLKSZ skip=$((1+$kern_blocks)) count=$root_blocks 2>/dev/null ) | \ 
> diff --git a/target/linux/ath79/base-files/lib/upgrade/platform.sh b/target/linux/ath79/base-files/lib/upgrade/platform.sh

> index f7e62143e7..f4fca06384 100644 
> --- a/target/linux/ath79/base-files/lib/upgrade/platform.sh 
> +++ b/target/linux/ath79/base-files/lib/upgrade/platform.sh 
> @@ -14,7 +14,7 @@ redboot_fis_do_upgrade() { 
>       if [ "$magic" = "4349" ]; then 
>               local kern_length=0x$(dd if="$sysup_file" bs=2 skip=1 count=4 2>/dev/null) 
> 
> -             [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j 
> $UPGRADE_BACKUP" 
> +             [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP" 
>               dd if="$sysup_file" bs=64k skip=1 2>/dev/null | \ 
>                       mtd -r $append -F$kern_part:$kern_length:0x80060000,rootfs write - $kern_part:rootfs 
> 
> @@ -22,7 +22,7 @@ redboot_fis_do_upgrade() { 
>               local board_dir=$(tar tf $sysup_file | grep -m 1 '^sysupgrade-.*/$') 
>               local kern_length=$(tar xf $sysup_file ${board_dir}kernel -O | wc -c) 
> 
> -             [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j 
> $UPGRADE_BACKUP" 
> +             [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP" 
>               tar xf $sysup_file ${board_dir}kernel ${board_dir}root -O | \ 
>                       mtd -r $append -F$kern_part:$kern_length:0x80060000,rootfs write - $kern_part:rootfs 
> 
> diff --git a/target/linux/imx6/base-files/lib/upgrade/platform.sh b/target/linux/imx6/base-files/lib/upgrade/platform.sh

> index 9414b18710..a090cc080b 100755 
> --- a/target/linux/imx6/base-files/lib/upgrade/platform.sh 
> +++ b/target/linux/imx6/base-files/lib/upgrade/platform.sh 
> @@ -75,7 +75,7 @@ platform_pre_upgrade() { 
> 
>       case "$board" in 
>       apalis*) 
> -             [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 0 ] && { 
> +             [ -z "$UPGRADE_BACKUP" ] && { 

Really "-z" or "! -f"? 

>                       jffs2reset -y 
>                       umount /overlay 
>               } 
> diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh

> index e313562017..8e02186eb8 100644 
> --- a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh 
> +++ b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh 
> @@ -74,7 +74,7 @@ platform_do_upgrade_openmesh() { 
>       # 
> 
>       # take care of restoring a saved config 
> -     [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}" 
> +     [ -n "$UPGRADE_BACKUP" ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}" 

"-f" here, too? 

> 
>       mtd -q erase inactive 
>       tar xf $tar_file ${board_dir}/root -O | mtd -n -p $kernel_length $restore_backup write - $PART_NAME 
> diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh b/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh

> index 6b9858beb0..c12508c437 100644 
> --- a/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh 
> +++ b/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh 
> @@ -37,7 +37,7 @@ zyxel_do_upgrade() { 
> 
>       tar Oxf $tar_file ${board_dir}/kernel | mtd write - kernel 
> 
> -     if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ]; then 
> +     if [ -n "$UPGRADE_BACKUP" ]; then 

"-f" here, too? 

Sorry for being late with this. 

Best 

Adrian 

>               tar Oxf $tar_file ${board_dir}/root | mtd -j "$UPGRADE_BACKUP" write - rootfs 
>       else 
>               tar Oxf $tar_file ${board_dir}/root | mtd write - rootfs 
> diff --git a/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh b/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh

> index 557f43ce6f..f83aa430cf 100644 
> --- a/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh 
> +++ b/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh 
> @@ -68,7 +68,7 @@ platform_do_upgrade_combined() { 
>          [ ${erase_size:-0} -gt 0 ]; 
>       then 
>               local append="" 
> -             [ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP" 
> +             [ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP" 
> 
>               # write the kernel 
>               dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null | \ 
> -- 
> 2.21.0 
> 
> 
> _______________________________________________ 
> openwrt-devel mailing list 
> openwrt-devel@lists.openwrt.org <mailto:openwrt-devel@lists.openwrt.org>  
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Rafał Miłecki Sept. 16, 2019, 4:37 a.m. UTC | #6
Hi,

please use text/plain for your e-mails

On 2019-09-15 11:54, mail@adrianschmutzler.de wrote:
> please also backport this to 19.07, since the variables for ath79 are 
> still wrong there.

That was on my list and I've just done that
Rafał Miłecki Sept. 16, 2019, 4:40 a.m. UTC | #7
On 2019-09-11 12:55, Adrian Schmutzler wrote:
>> diff --git a/package/base-files/files/lib/upgrade/common.sh 
>> b/package/base-files/files/lib/upgrade/common.sh
>> index 8e7866f698..0d3162d4fc 100644
>> --- a/package/base-files/files/lib/upgrade/common.sh
>> +++ b/package/base-files/files/lib/upgrade/common.sh
>> @@ -220,7 +220,7 @@ indicate_upgrade() {
>>  # $(2): (optional) pipe command to extract firmware, e.g. dd bs=n 
>> skip=m
>>  default_do_upgrade() {
>>  	sync
>> -	if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ]; then
>> +	if [ -n "$UPGRADE_BACKUP" ]; then
> 
> Any reason why this is "-n" and not "-f" as below?

I kept the old logic which wasn't checking for file existence. Feel free
to develop, test & send a patch changing that.


>>  		get_image "$1" "$2" | mtd $MTD_ARGS $MTD_CONFIG_ARGS -j 
>> "$UPGRADE_BACKUP" write - "${PART_NAME:-
>> image}"
>>  	else
>>  		get_image "$1" "$2" | mtd $MTD_ARGS write - "${PART_NAME:-image}"
>> diff --git a/package/base-files/files/lib/upgrade/do_stage2 
>> b/package/base-files/files/lib/upgrade/do_stage2
>> index 0e6cc1bfc3..0e32445743 100755
>> --- a/package/base-files/files/lib/upgrade/do_stage2
>> +++ b/package/base-files/files/lib/upgrade/do_stage2
>> @@ -11,7 +11,7 @@ else
>>  	default_do_upgrade "$IMAGE"
>>  fi
>> 
>> -if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && type 
>> 'platform_copy_config' >/dev/null 2>/dev/null; then
>> +if [ -n "$UPGRADE_BACKUP" ] && type 'platform_copy_config' >/dev/null 
>> 2>/dev/null; then
> 
> Here I'm not so sure about "-f" vs. "-n" ...
> 
>>  	platform_copy_config
>>  fi
>> 
>> diff --git a/package/base-files/files/sbin/sysupgrade 
>> b/package/base-files/files/sbin/sysupgrade
>> index f18143bff4..935d08048e 100755
>> --- a/package/base-files/files/sbin/sysupgrade
>> +++ b/package/base-files/files/sbin/sysupgrade
>> @@ -371,7 +371,6 @@ else
>>  		$backup_attr
>>  		\"command\": $(json_string "$COMMAND"),
>>  		\"options\": {
>> -			\"save_config\": $SAVE_CONFIG,
>>  			\"save_partitions\": $SAVE_PARTITIONS
>>  		}
>>  	}"
>> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh 
>> b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
>> index c694c2e6f2..e991a06b7a 100644
>> --- a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
>> +++ b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
>> @@ -75,7 +75,7 @@ dir825b_do_upgrade_combined() {
>> 
>>  	if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then
>>  		local append=""
>> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && 
>> append="-j $UPGRADE_BACKUP"
>> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
>> 
>>  		sync
>>  		dd if="$fw_file" bs=64k skip=1 count=$fw_blocks 2>/dev/null | \
>> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh 
>> b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
>> index 8536d4ba4a..f43bdcea7f 100644
>> --- a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
>> +++ b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
>> @@ -159,7 +159,7 @@ platform_do_upgrade_openmesh()
>>  	local cfg_size= kernel_size= rootfs_size=
>>  	local append=""
>> 
>> -	[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && 
>> append="-j $UPGRADE_BACKUP"
>> +	[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
>> 
>>  	cfg_size=$(dd if="$img_path" bs=2 skip=35 count=4 2>/dev/null)
>>  	kernel_size=$(dd if="$img_path" bs=2 skip=71 count=4 2>/dev/null)
>> diff --git a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh 
>> b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
>> index 726163291d..86e7b6386b 100755
>> --- a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
>> +++ b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
>> @@ -65,7 +65,7 @@ platform_do_upgrade_combined() {
>>  	then
>>  		local rootfspart=$(platform_find_rootfspart "$partitions" 
>> "$kernelpart")
>>  		local append=""
>> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && 
>> append="-j $UPGRADE_BACKUP"
>> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
>> 
>>  		if [ "$PLATFORM_DO_UPGRADE_COMBINED_SEPARATE_MTD" -ne 1 ]; then
>>  		    ( dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 
>> 2>/dev/null; \
>> @@ -164,7 +164,7 @@ platform_do_upgrade_compex() {
>> 
>>  	if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then
>>  		local append=""
>> -		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && 
>> append="-j $UPGRADE_BACKUPs"
>> +		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUPs"
> 
> Is there a reason for the trailing "s" here or is this a typo: ="-j
> $UPGRADE_BACKUPs" ?

Try to check when it has appeared there:
git log -p target/linux/ar71xx/base-files/lib/upgrade/platform.sh

It's a typo.
diff mbox series

Patch

diff --git a/package/base-files/files/lib/upgrade/common.sh b/package/base-files/files/lib/upgrade/common.sh
index 8e7866f698..0d3162d4fc 100644
--- a/package/base-files/files/lib/upgrade/common.sh
+++ b/package/base-files/files/lib/upgrade/common.sh
@@ -220,7 +220,7 @@  indicate_upgrade() {
 # $(2): (optional) pipe command to extract firmware, e.g. dd bs=n skip=m
 default_do_upgrade() {
 	sync
-	if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ]; then
+	if [ -n "$UPGRADE_BACKUP" ]; then
 		get_image "$1" "$2" | mtd $MTD_ARGS $MTD_CONFIG_ARGS -j "$UPGRADE_BACKUP" write - "${PART_NAME:-image}"
 	else
 		get_image "$1" "$2" | mtd $MTD_ARGS write - "${PART_NAME:-image}"
diff --git a/package/base-files/files/lib/upgrade/do_stage2 b/package/base-files/files/lib/upgrade/do_stage2
index 0e6cc1bfc3..0e32445743 100755
--- a/package/base-files/files/lib/upgrade/do_stage2
+++ b/package/base-files/files/lib/upgrade/do_stage2
@@ -11,7 +11,7 @@  else
 	default_do_upgrade "$IMAGE"
 fi
 
-if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && type 'platform_copy_config' >/dev/null 2>/dev/null; then
+if [ -n "$UPGRADE_BACKUP" ] && type 'platform_copy_config' >/dev/null 2>/dev/null; then
 	platform_copy_config
 fi
 
diff --git a/package/base-files/files/sbin/sysupgrade b/package/base-files/files/sbin/sysupgrade
index f18143bff4..935d08048e 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -371,7 +371,6 @@  else
 		$backup_attr
 		\"command\": $(json_string "$COMMAND"),
 		\"options\": {
-			\"save_config\": $SAVE_CONFIG,
 			\"save_partitions\": $SAVE_PARTITIONS
 		}
 	}"
diff --git a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
index c694c2e6f2..e991a06b7a 100644
--- a/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
+++ b/target/linux/ar71xx/base-files/lib/upgrade/dir825.sh
@@ -75,7 +75,7 @@  dir825b_do_upgrade_combined() {
 
 	if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then
 		local append=""
-		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
+		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
 
 		sync
 		dd if="$fw_file" bs=64k skip=1 count=$fw_blocks 2>/dev/null | \
diff --git a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
index 8536d4ba4a..f43bdcea7f 100644
--- a/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
+++ b/target/linux/ar71xx/base-files/lib/upgrade/openmesh.sh
@@ -159,7 +159,7 @@  platform_do_upgrade_openmesh()
 	local cfg_size= kernel_size= rootfs_size=
 	local append=""
 
-	[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
+	[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
 
 	cfg_size=$(dd if="$img_path" bs=2 skip=35 count=4 2>/dev/null)
 	kernel_size=$(dd if="$img_path" bs=2 skip=71 count=4 2>/dev/null)
diff --git a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
index 726163291d..86e7b6386b 100755
--- a/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
+++ b/target/linux/ar71xx/base-files/lib/upgrade/platform.sh
@@ -65,7 +65,7 @@  platform_do_upgrade_combined() {
 	then
 		local rootfspart=$(platform_find_rootfspart "$partitions" "$kernelpart")
 		local append=""
-		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
+		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
 
 		if [ "$PLATFORM_DO_UPGRADE_COMBINED_SEPARATE_MTD" -ne 1 ]; then
 		    ( dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null; \
@@ -164,7 +164,7 @@  platform_do_upgrade_compex() {
 
 	if [ -n "$fw_mtd" ] &&  [ ${fw_blocks:-0} -gt 0 ]; then
 		local append=""
-		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUPs"
+		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUPs"
 
 		sync
 		dd if="$fw_file" bs=64k skip=1 count=$fw_blocks 2>/dev/null | \
diff --git a/target/linux/ath25/base-files/lib/upgrade/platform.sh b/target/linux/ath25/base-files/lib/upgrade/platform.sh
index 0dde103605..778bbf5a39 100644
--- a/target/linux/ath25/base-files/lib/upgrade/platform.sh
+++ b/target/linux/ath25/base-files/lib/upgrade/platform.sh
@@ -67,7 +67,7 @@  platform_do_upgrade() {
 	   [ ${erase_size:-0} -gt 0 ];
 	then
 		local append=""
-		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
+		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
 
 		( dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null; \
 		  dd if="$1" bs=$CI_BLKSZ skip=$((1+$kern_blocks)) count=$root_blocks 2>/dev/null ) | \
diff --git a/target/linux/ath79/base-files/lib/upgrade/platform.sh b/target/linux/ath79/base-files/lib/upgrade/platform.sh
index f7e62143e7..f4fca06384 100644
--- a/target/linux/ath79/base-files/lib/upgrade/platform.sh
+++ b/target/linux/ath79/base-files/lib/upgrade/platform.sh
@@ -14,7 +14,7 @@  redboot_fis_do_upgrade() {
 	if [ "$magic" = "4349" ]; then
 		local kern_length=0x$(dd if="$sysup_file" bs=2 skip=1 count=4 2>/dev/null)
 
-		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
+		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
 		dd if="$sysup_file" bs=64k skip=1 2>/dev/null | \
 			mtd -r $append -F$kern_part:$kern_length:0x80060000,rootfs write - $kern_part:rootfs
 
@@ -22,7 +22,7 @@  redboot_fis_do_upgrade() {
 		local board_dir=$(tar tf $sysup_file | grep -m 1 '^sysupgrade-.*/$')
 		local kern_length=$(tar xf $sysup_file ${board_dir}kernel -O | wc -c)
 
-		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
+		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
 		tar xf $sysup_file ${board_dir}kernel ${board_dir}root -O | \
 			mtd -r $append -F$kern_part:$kern_length:0x80060000,rootfs write - $kern_part:rootfs
 
diff --git a/target/linux/imx6/base-files/lib/upgrade/platform.sh b/target/linux/imx6/base-files/lib/upgrade/platform.sh
index 9414b18710..a090cc080b 100755
--- a/target/linux/imx6/base-files/lib/upgrade/platform.sh
+++ b/target/linux/imx6/base-files/lib/upgrade/platform.sh
@@ -75,7 +75,7 @@  platform_pre_upgrade() {
 
 	case "$board" in
 	apalis*)
-		[ "$UPGRADE_OPT_SAVE_CONFIG" -eq 0 ] && {
+		[ -z "$UPGRADE_BACKUP" ] && {
 			jffs2reset -y
 			umount /overlay
 		}
diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
index e313562017..8e02186eb8 100644
--- a/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
+++ b/target/linux/ipq40xx/base-files/lib/upgrade/openmesh.sh
@@ -74,7 +74,7 @@  platform_do_upgrade_openmesh() {
 	#
 
 	# take care of restoring a saved config
-	[ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"
+	[ -n "$UPGRADE_BACKUP" ] && restore_backup="${MTD_CONFIG_ARGS} -j ${UPGRADE_BACKUP}"
 
 	mtd -q erase inactive
 	tar xf $tar_file ${board_dir}/root -O | mtd -n -p $kernel_length $restore_backup write - $PART_NAME
diff --git a/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh b/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh
index 6b9858beb0..c12508c437 100644
--- a/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh
+++ b/target/linux/ipq40xx/base-files/lib/upgrade/platform.sh
@@ -37,7 +37,7 @@  zyxel_do_upgrade() {
 
 	tar Oxf $tar_file ${board_dir}/kernel | mtd write - kernel
 
-	if [ "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ]; then
+	if [ -n "$UPGRADE_BACKUP" ]; then
 		tar Oxf $tar_file ${board_dir}/root | mtd -j "$UPGRADE_BACKUP" write - rootfs
 	else
 		tar Oxf $tar_file ${board_dir}/root | mtd write - rootfs
diff --git a/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh b/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh
index 557f43ce6f..f83aa430cf 100644
--- a/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh
+++ b/target/linux/ixp4xx/base-files/lib/upgrade/platform.sh
@@ -68,7 +68,7 @@  platform_do_upgrade_combined() {
 	   [ ${erase_size:-0} -gt 0 ];
 	then
 		local append=""
-		[ -f "$UPGRADE_BACKUP" -a "$UPGRADE_OPT_SAVE_CONFIG" -eq 1 ] && append="-j $UPGRADE_BACKUP"
+		[ -f "$UPGRADE_BACKUP" ] && append="-j $UPGRADE_BACKUP"
 
 		# write the kernel
 		dd if="$1" bs=$CI_BLKSZ skip=1 count=$kern_blocks 2>/dev/null | \