Kirkwood configuration not kept via sysupgrade

[ascendbeing]

Describe the bug

In file target/linux/kirkwood/base-files/lib/upgrade/linksys.sh there's a line that goes:
nand_upgrade_tar "$1"

Replace it with either:
nand_upgrade_tar "$1" && nand_do_upgrade_success
nand_do_upgrade_failed or with how it was done in 8634c10

I have personally tested successfully the former to success many times on a Kirkwood Linksys ea3.5k. without the former change, sysupgrade becomes sysupgrade + dont save config

I have not tested the latter against it. I proposed resolving this issue before that commit was ever made, in my comments in #11692, with the former proposed fix.

OpenWrt version

master

OpenWrt target/subtarget

kirkwood

Device

Linksys ea3500

Image kind

Official downloaded image

Steps to reproduce

No response

Actual behaviour

No response

Expected behaviour

No response

Additional info

No response

Diffconfig

No response

Terms

• I am reporting an issue for OpenWrt, not an unsupported fork.

[dseven]

There seems to be some unfortunate duplication of code going here. The same thing appears under ipq806x and mvebu/cortexa9 too. Probably someone should evaluate if the fix pertains to them too.

[trinidude4]

I'm trying to look through all the related bug reports. It looks like this affects anyone calling nand_upgrade_tar, and that looks like it's only called in linksys.sh. It would impact ipq40xx, ipq80xx, kirkwood, and mvebu/cortexa9. This was already fixed for ipq40xx and it looks like we opened pull requests for ipq80xx and mvebu/cortexa9. Kirkwood has this bug report.

I don't know if the bug causes problems for anyone else that doesn't have the linksys.sh file, but the fix identified here should cover all of us who need the linksys.sh file modified just to have our configurations restored.

[ascendbeing]

@trinidude4 that's my understanding. The reason i was petitioning to have this fix for ipq also done for Kirkwood is because I personally had the experience of the sysup on my ea3500 not saving config, whereas on my ath79 it works fine, so I looked around and Linksys.sh hadn't changed in a while (it has since changed recently but the changes dony have anything to do with causing or resolving this), so I found that ipq pull request.
So after I found that I read the thread and @Ansuel was pitching some improvements. He provided pseudo code for what would work better than an if/else. I also noticed that the original pull had referenced a function that doesn't exist so I synthesized the pseudo code and original pull code into a 2 liner that should incorporate the best of all worlds as far as effectively addressing the issue.

I wasn't aware until the other day that this Linksys.sh file exists on more than these original 2 targets, but barring anything exotic done on the other targets, my proposed code here should work to resolve the issue in each case.

I have to call out @chunkeey for not using my code. Because he used _failure instead of _failed. _failure as in nand_do_upgrade_failure DOES NOT EXIST. I provided working code, and you committed code that on such a serious subject of firmware upgrading, refers to nonexistent functions. nand_do_upgrade_failed it's supposed to be.

So to summarize, the ipq commit is flawed because if the flash doesn't succeed, it calls a nonexistent function. Fix it for all the targets mentioned by @trinidude4 barring any exotic existing conflicts or workarounds, by putting in my proposed code in the manner I described at the top of this bug report.

[trinidude4]

I put the mvebu pull request back in draft because of that nand_do_upgrade_failure typo. Maybe we should put the fix for all the targets in one pull request. The replacement text you have above matches the way it was fixed in package/base-files/files/lib/upgrade/nand.sh, so I think that would be best.