ipq40xx: Fix Linksys upgrade, restore config step

[jeffsf]

It appears that the refactor of the upgrade process for NAND devices resulted in the nand_do_upgrade_success step not being called for devices using the linksys.sh script. As a result, configuration was not preserved over sysupgrade steps.

This was restored for some devices in commit 84ff6c9. This commit restores preservation of config for ipq40xx devices using the linksys.sh script. Other devices and targets have not been examined.

Closes: #11677
Fixes: e25e6d8 ("base-files: fix and clean up nand sysupgrade code") Tested-on: EA8300

Signed-off-by: Jeff Kletsky git-commits@allycomm.com

[Ansuel]

no new line between sob and fixes tag.

Also I think we should follow how it was done in the related commit.

Since success will reboot the system

I would use something like

tar_upgrade && nand_success
nand_fail

[Ansuel]

@robimarko what do you think ?

[jeffsf]

While I appreciate the brevity of assuming that the call to nand_do_upgrade_success reboots the system, this bug apparently arose when a refactor wasn't aware of assumptions around how consumers of generic functions expected them to behave.

Functionally, they achieve the same goal at run time. I can't get worked up about it one way or another. I was just trying to reduce the chances of this happening again. Let me know consensus and I'll update content and the commit message in one pass.

[robimarko]

I am really not any kind of an expert when it comes to sysupgrade, ideally somebody familiar should take a look

[diizzyy]

While either might not the be perfect solution can we please commit at least one soltion for now instead of having it broken?
I'd say for consistency go with what @Ansuel suggested.

[ascendbeing]

Concise and fixed (failure->failed) two liner adapted from your commit and Ansuel's pseudo code:

nand_upgrade_tar "$1" && nand_do_upgrade_success
nand_do_upgrade_failed

Tested on Kirkwood (ea3500). Same principle, replace line nand_upgrade_tar "$1" with my two liner.

Please swap in my two liner both for this target and for Kirkwood and rename the pull to reflect this fix is also for Kirkwood (at least ea3500 and ea4500 are affected on Kirkwood). Thanks.

[ascendbeing]

I am new to GitHub so my "review" was not very useful in terms of syntactical adherence to GitHub formatting and functionality please consider it 1. a request to expand this pull to include Kirkwood. 2. a 2 liner which addresses a) failure->failed (there is no failure, only failed) b) incorporates @Ansuel suggested improvements (if/else becomes && and another line)

e8625c8 just adjusted these files but did not incorporate this pull. I don't believe that any changes further than my first suggestion are needed but I will admit I haven't checked after this recent commit.

[aparcar]

@Ansuel can you take care of this?

[trinidude4]

This fix is also needed for mvebu, at least for cortexa9. I had the same issues with sysupgrade on a WRT32X and this fix corrected it. I don't know if a separate issue needs to be opened, or if mvebu can also be included here.

[Ansuel]

@jeffsf I remember this fix... I would drop the if else and use the pattern used also in the linked fix...
Consider that we are in ramfs and the system must be rebooted if something fail in the process

This is why the pattern
&& _succes
_fail

works since _success should reboot and _fail should never be reached. It's really a nitpick but it's for consistency. Ok for you ? (maybe add some comments to explain the pattern if you can)

[chunkeey]

Merged. Thanks!

[ascendbeing]

@chunkeey should I create a separate issue? This issue also applies to Kirkwood, in the same Linksys.sh file (you edit the same line, and put your fix here, or mine, whichever is better, I've tested mine against the latest version of the file).
You fixed it for ipq but not for Kirkwood. With my 1 line -> 2 lines code I posted above, Kirkwood gets fixed, but nobody has merged it into master, as of time of writing.

[jeffsf]

Thanks all - Appreciate the time in getting this merged