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:m...@adrianschmutzler.de] Sent: Mittwoch, 11. September 2019 12:56 To: 'Rafał Miłecki' <zaj...@gmail.com>; openwrt-devel@lists.openwrt.org Cc: 'Rafał Miłecki' <ra...@milecki.pl>; 'Jonas Gorski' <jonas.gor...@gmail.com>; 'Jo-Philipp Wich' <j...@mein.io>; 'John Crispin' <j...@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-boun...@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 <ra...@milecki.pl <mailto:ra...@milecki.pl> >; Jonas Gorski > <jonas.gor...@gmail.com <mailto:jonas.gor...@gmail.com> >; Jo-Philipp Wich > <j...@mein.io <mailto:j...@mein.io> >; John Crispin > <j...@phrozen.org <mailto:j...@phrozen.org> > > Subject: [OpenWrt-Devel] [PATCH 3/3] treewide: sysupgrade: use > $UPGRADE_BACKUP to check for backup > > From: Rafał Miłecki <ra...@milecki.pl <mailto:ra...@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 <ra...@milecki.pl <mailto:ra...@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
openpgp-digital-signature.asc
Description: PGP signature
_______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel