<font face="Default Sans Serif,Verdana,Arial,Helvetica,sans-serif" size="2"><br>On 06/22/2016 around 11:58PM, Andrew Jeffery wrote:<br>>On Wed, 2016-06-22 at 19:30 -0500, OpenBMC Patches wrote:<br>>> From: "Milton D. Miller II" <<a target="_blank" href="mailto:miltonm@us.ibm.com">miltonm@us.ibm.com</a>><br>>> <br>>> When update was written it was exec'd from the shutdown script<br>>> and hence took over pid 1. Since exiting in that environment was<br>>> a panic situation, the script instead started a rescue shell with<br>>> its output presumably on the console.<br>>> <br>>> The calling convention was updated to be a simple invocation in<br>>> commit dbacf104885c ("obmc-initfs: run update as a sub-script")<br>>> but the error handling was not updated. That error handling is<br>>> now becoming a hinderance<br>><br>>hindrance<br><br>fixed<br><br>>> findmtd() {<br>>> m=$(grep -xl "$1" /sys/class/mtd/*/name)<br>>> @@ -130,7 +127,7 @@ do<br>>> if test -z "$m"<br>>> then<br>>> echo 1>&2 "Unable to find mtd partiton for ${f##*/}."<br>>> - exec /bin/sh<br>>> + exit 1<br>><br>>Is there any value differentiating between the failure cases? I.e.<br>>could we `exit 2` here instead?<br><br>The problem with differentiated exit codes is knowing what they <br>mean and keeping the callers updated with new codes. After this <br>series we can recognize the following exceptional conditions (1) <br>unrecognized options (but stops checking on the first argument <br>without a leading hyphen, additional arguments are accepted and <br>ignored), (2) can't match image suffix to a mtd label (3) image too <br>big for partition (4) mtd or a sub-mtd partition is in /proc/self/mounts (5) <br>unknown error from flash_cp (could be a erase, write, or verify <br>error, or its size check) (6) trying to restore without any saved files (7) <br>bad entry in whitelist (8) trying to flash with no images. Of these, <br>(7) and (8) print a message to stdout, (5) and (6) are totally silent, <br>while (1), (2), (3), and (4) exit 1 on first encounter stopping further <br>checks. I have some rational for the choices, mostly I want to flash <br>as much as possible if the set of images passed a first scan to <br>plausible success (and the arg parsing is to allow future extension <br>to pass the files to flash) and not caring between whitelist didn't <br>match anything vs whitelist entries are bad when flashing is the <br>primary function. Unknown error (5) is silent because I already had <br>remove on success and didn't bother to add code to remember the <br>error while wanting as much success as possible.<br><br>I could see differentiating between unrecognized options (1), image <br>file doesn't have an mtd or is too big for the mtd (2 or 3), and flash <br>update error (5), but what would be the use case? If from a user they <br>would run --help or read the error message and correct, and hopefully <br>calls from REST would return the error. The bad whitelist (7) and no <br>files saved before trying to restore or copy files (6) will be debugged <br>when someone notices the contents not there after an upgrade (the <br>default is currently to save and restore). The check for a busy <br>mount (4) vs check for size and name (3) can be handled by calling <br>again with --ignore-mount if the script wants to know if deferred update <br>is likely to work.<br><br>So for now I will stay with one error and only report when checks fail. <br>Let me know if you think an exit code for (8) is important and I'll add <br>that. I suppose a --verfy-whitelist could complain if any entry doesn't <br>match a file or directory but that seems low priority.<br><br>Unfortunately the check for mounted device (4) is easy to defeat (by calling <br>umount) and I can't see how to recognize that the mtd partiton is busy. The <br>block layer has sysfs holders and slaves that show block device references <br>but jffs2 uses mtd directly. UBI has exclusive open but MTD does not. <br>Perhaps I should propose a patch to the Linux kernel.<br><br>milton<br></font><BR>