| Message ID | 20181204195932.29505-3-jonathan@navigue.com |
|---|---|
| State | Changes Requested |
| Delegated to: | John Crispin |
| Headers | show |
| Series | Allow sysupgrade restore on ER | expand |
Jonathan Thibault <jonathan@navigue.com> [2018-12-04 14:59:32]: Hi, sorry, that I've missed previous review rounds. > diff --git a/target/linux/octeon/base-files/lib/preinit/79_move_config b/target/linux/octeon/base-files/lib/preinit/79_move_config > index ec63d9f5ff..470cbfe005 100644 > --- a/target/linux/octeon/base-files/lib/preinit/79_move_config > +++ b/target/linux/octeon/base-files/lib/preinit/79_move_config > @@ -5,6 +5,11 @@ move_config() { > . /lib/functions.sh > > case "$(board_name)" in > + er) > + mount -t vfat /dev/mmcblk0p1 /mnt > + [ -f /mnt/sysupgrade.tgz ] && mv -f /mnt/sysupgrade.tgz / > + umount /mnt > + ;; > erlite) > mount -t vfat /dev/sda1 /mnt > [ -f /mnt/sysupgrade.tgz ] && mv -f /mnt/sysupgrade.tgz / Please try to follow DRY principles and don't copy&paste the same code over again. I mean something like this: er_move_config() { local dev="$1" mount -t vfat "$dev" /mnt [ -f /mnt/sysupgrade.tgz ] && mv -f /mnt/sysupgrade.tgz / umount /mnt } move_config() { . /lib/functions.sh case "$(board_name)" in er) er_move_config /dev/mmcblk0p1 ;; erlite) er_move_config /dev/sda1 ;; esac } > diff --git a/target/linux/octeon/base-files/lib/upgrade/platform.sh b/target/linux/octeon/base-files/lib/upgrade/platform.sh > index 6d258dbb0f..009eae7a2c 100755 > --- a/target/linux/octeon/base-files/lib/upgrade/platform.sh > +++ b/target/linux/octeon/base-files/lib/upgrade/platform.sh > @@ -23,6 +23,11 @@ platform_get_rootfs() { > > platform_copy_config() { > case "$(board_name)" in > + er) > + mount -t vfat /dev/mmcblk0p1 /mnt > + cp -af "$CONF_TAR" /mnt/ > + umount /mnt > + ;; > erlite) > mount -t vfat /dev/sda1 /mnt > cp -af "$CONF_TAR" /mnt/ Ditto. -- ynezz
On 04/12/18 04:53 PM, Petr Štetiar wrote: > > Please try to follow DRY principles and don't copy&paste the same code over > again. I mean something like this: > > er_move_config() { > local dev="$1" > mount -t vfat "$dev" /mnt > [ -f /mnt/sysupgrade.tgz ] && mv -f /mnt/sysupgrade.tgz / > umount /mnt > } > > move_config() { > . /lib/functions.sh > > case "$(board_name)" in > er) > er_move_config /dev/mmcblk0p1 > ;; > erlite) > er_move_config /dev/sda1 > ;; > esac > } No problem. I was thinking along those lines too initially but as this isn't my code I was trying my best to match what's already there and not step on any toes. Same for the board_name eval order. I had matched what was already in the file until it was requested that I put them in order.
diff --git a/target/linux/octeon/base-files/lib/preinit/79_move_config b/target/linux/octeon/base-files/lib/preinit/79_move_config index ec63d9f5ff..470cbfe005 100644 --- a/target/linux/octeon/base-files/lib/preinit/79_move_config +++ b/target/linux/octeon/base-files/lib/preinit/79_move_config @@ -5,6 +5,11 @@ move_config() { . /lib/functions.sh case "$(board_name)" in + er) + mount -t vfat /dev/mmcblk0p1 /mnt + [ -f /mnt/sysupgrade.tgz ] && mv -f /mnt/sysupgrade.tgz / + umount /mnt + ;; erlite) mount -t vfat /dev/sda1 /mnt [ -f /mnt/sysupgrade.tgz ] && mv -f /mnt/sysupgrade.tgz / diff --git a/target/linux/octeon/base-files/lib/upgrade/platform.sh b/target/linux/octeon/base-files/lib/upgrade/platform.sh index 6d258dbb0f..009eae7a2c 100755 --- a/target/linux/octeon/base-files/lib/upgrade/platform.sh +++ b/target/linux/octeon/base-files/lib/upgrade/platform.sh @@ -23,6 +23,11 @@ platform_get_rootfs() { platform_copy_config() { case "$(board_name)" in + er) + mount -t vfat /dev/mmcblk0p1 /mnt + cp -af "$CONF_TAR" /mnt/ + umount /mnt + ;; erlite) mount -t vfat /dev/sda1 /mnt cp -af "$CONF_TAR" /mnt/