cr50-updater.sh needs to be fixed once util-linux is fixed |
|||
Issue descriptioncurrently logger does not handle command line options properly, the cr50-update.sh script submitted under https://chromium-review.googlesource.com/c/459119/ includes a workaround, which will have to be removed when not necessary any more.
,
May 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/653148d7d5a2f8fd2b78a8a8aa3ce54ad39958a4 commit 653148d7d5a2f8fd2b78a8a8aa3ce54ad39958a4 Author: Vadim Bendebury <vbendeb@chromium.org> Date: Tue May 16 17:41:46 2017 cr50: script for background firmware updates When required, the new script will be invoked at the postinstall phase of Chrome OS install process to copy the new cr50 image to the H1. The update would happen only if two conditions are met: - the currently running cr50 version is 0.0.19 or older, otherwise the system could reset unexpectedly for the user after the update. This check is done by trunks_send internally. - the resident version of trunks_send is capable of sending the image to the H1 If either of these conditions is not true, the update would happen on the next system restart, under the init scripts' control. The H1 is guaranteed not to allow fallbacks and to ignore images which are of the same version as the currently running. CQ-DEPEND=CL:505251 BUG=b:35580805, chromium:721612 TEST=in concert with CL:49391 verified the following: - successful update of cr50 from RW18 to RW19 when updating a reef running M57 to M60 (the actual update happens at boot time and the system reboots twice) - successful update from 60 to 60 with a newer image of cr50, the new image was installed in the background, no two restarts were required. Change-Id: Ida5019554d1ac67b2cb74a69819b88c15def3e93 Signed-off-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/505459 Reviewed-by: Ben Chan <benchan@chromium.org> Reviewed-by: Andrey Pronin <apronin@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/653148d7d5a2f8fd2b78a8a8aa3ce54ad39958a4/chromeos-base/chromeos-cr50/chromeos-cr50-0.0.1.ebuild [add] https://crrev.com/653148d7d5a2f8fd2b78a8a8aa3ce54ad39958a4/chromeos-base/chromeos-cr50/files/cr50-update.sh [rename] https://crrev.com/653148d7d5a2f8fd2b78a8a8aa3ce54ad39958a4/chromeos-base/chromeos-cr50/chromeos-cr50-0.0.1-r30.ebuild
,
May 22 2017
with the util-linux fix merged, how should cr50-update.sh be modified?
I suggest this:
< logger "${script_name}[${pid}]:" "$@"
---
> logger --id "${script_name}:${pid}" "$@"
because using -t causes the logger to show the script name and the function PID (i.e. it is still different for every invocation).
,
May 22 2017
i already wrote the exact command to use: https://chromium-review.googlesource.com/c/459119/9/chromeos-base/chromeos-cr50/files/cr50-update.sh#15
,
May 22 2017
-t "${script_name}" --id "${pid}"
result in a somewhat misleading and ugly log contents:
2017-05-22T13:26:03.287500-07:00 NOTICE cr50-update.sh[14409]: 14407 Starting
2017-05-22T13:26:03.314411-07:00 NOTICE cr50-update.sh[14412]: 14407 using /usr/sbin/trunks_send for update
2017-05-22T13:26:03.356186-07:00 NOTICE cr50-update.sh[14414]: 14407 success
i.e. it still shows cr50-update.sh running on different PIDs.
the form I suggest results in a clearer log messages:
2017-05-22T13:24:48.561043-07:00 NOTICE root[14155]: cr50-update.sh[14153]: Starting
2017-05-22T13:24:48.620301-07:00 NOTICE root[14158]: cr50-update.sh[14153]: using /usr/sbin/trunks_send for update
2017-05-22T13:24:48.673391-07:00 NOTICE root[14160]: cr50-update.sh[14153]: success
i.e. it is immediately clear what cr50_update process left the messages.
,
May 22 2017
that isn't what i wrote. here's exactly what i pasted earlier:
logger -t "${script_name}" --id="${pid}" "$@"
,
May 22 2017
wow, isn't omitting '=' when specifying command line options a common occurrence, i.e. isn't '--option=value' and '--option value' usually the same thing? It sure makes a difference in this case, is this a bug or a feature?
,
May 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/6879c8d50b1ef260c14a2c552c79881afd844374 commit 6879c8d50b1ef260c14a2c552c79881afd844374 Author: Vadim Bendebury <vbendeb@chromium.org> Date: Wed May 24 04:59:41 2017 chromeos-cr50: use proper logger command line optioins With logger fix in place proper command line options can be used. BRANCH=none BUG= chromium:721612 TEST=verifed that the new script results in proper log file messages: 2017-05-22T16:02:41.579017-07:00 NOTICE cr50-update.sh[6393]: Starting 2017-05-22T16:02:41.609123-07:00 NOTICE cr50-update.sh[6393]: using /usr/sbin/trunks_send for update 2017-05-22T16:02:41.651837-07:00 NOTICE cr50-update.sh[6393]: success Change-Id: Ic34e7d1c66cb6894c389fae74e57a1c7f53e0bb2 Reviewed-on: https://chromium-review.googlesource.com/511289 Commit-Ready: Vadim Bendebury <vbendeb@chromium.org> Tested-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [rename] https://crrev.com/6879c8d50b1ef260c14a2c552c79881afd844374/chromeos-base/chromeos-cr50/chromeos-cr50-0.0.1-r32.ebuild [modify] https://crrev.com/6879c8d50b1ef260c14a2c552c79881afd844374/chromeos-base/chromeos-cr50/files/cr50-update.sh
,
May 24 2017
it's a feature. getopt supports optional arguments. that is, a flag that may accept an argument, or just be "on". in the case of logger, the -i/--id flag is such an option. if you pass "-i" or "--id", it'll do one thing. but you can pass in an argument, you can set the id directly. since it's optional, getopt needs to know how to handle something like "-i -T" or "--id --tcp" -- is the "-T"/"--tcp" a standalone option, or should it be the optional arg to "-i"/"--id" ? it resolves this by requiring they be placed together like "-iT" or "--id=--tcp". if the argument were required, then it'd know that "-T"/"--tcp" were the argument to the "-i"/"--id" flag. it also means that "-iT" and "-Ti" are not equivalent ;).
,
May 24 2017
,
Aug 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/6a3c95945466be76544d683aa102cae11fb60c80 commit 6a3c95945466be76544d683aa102cae11fb60c80 Author: Vadim Bendebury <vbendeb@chromium.org> Date: Mon Aug 14 02:40:12 2017 cr50: script for background firmware updates When required, the new script will be invoked at the postinstall phase of Chrome OS install process to copy the new cr50 image to the H1. The update would happen only if two conditions are met: - the currently running cr50 version is 0.0.19 or older, otherwise the system could reset unexpectedly for the user after the update. This check is done by trunks_send internally. - the resident version of trunks_send is capable of sending the image to the H1 If either of these conditions is not true, the update would happen on the next system restart, under the init scripts' control. The H1 is guaranteed not to allow fallbacks and to ignore images which are of the same version as the currently running. CQ-DEPEND=CL:505251 BUG=b:35580805, chromium:721612 TEST=in concert with CL:49391 verified the following: - successful update of cr50 from RW18 to RW19 when updating a reef running M57 to M60 (the actual update happens at boot time and the system reboots twice) - successful update from 60 to 60 with a newer image of cr50, the new image was installed in the background, no two restarts were required. Original-Change-Id: Ida5019554d1ac67b2cb74a69819b88c15def3e93 Signed-off-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/505459 Reviewed-by: Ben Chan <benchan@chromium.org> Reviewed-by: Andrey Pronin <apronin@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> Change-Id: I82c7a2659c71dded38ff292a13c884c90d130714 Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> Reviewed-on: https://chromium-review.googlesource.com/606853 Reviewed-by: Philip Chen <philipchen@chromium.org> [rename] https://crrev.com/6a3c95945466be76544d683aa102cae11fb60c80/chromeos-base/chromeos-cr50/chromeos-cr50-0.0.1-r29.ebuild [add] https://crrev.com/6a3c95945466be76544d683aa102cae11fb60c80/chromeos-base/chromeos-cr50/files/cr50-update.sh [modify] https://crrev.com/6a3c95945466be76544d683aa102cae11fb60c80/chromeos-base/chromeos-cr50/chromeos-cr50-0.0.1.ebuild
,
Aug 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/120513895b56e6bce6a00bac71c6233bfafafbd5 commit 120513895b56e6bce6a00bac71c6233bfafafbd5 Author: Vadim Bendebury <vbendeb@chromium.org> Date: Mon Aug 14 02:41:59 2017 chromeos-cr50: use proper logger command line optioins With logger fix in place proper command line options can be used. BRANCH=none BUG= chromium:721612 TEST=verifed that the new script results in proper log file messages: 2017-05-22T16:02:41.579017-07:00 NOTICE cr50-update.sh[6393]: Starting 2017-05-22T16:02:41.609123-07:00 NOTICE cr50-update.sh[6393]: using /usr/sbin/trunks_send for update 2017-05-22T16:02:41.651837-07:00 NOTICE cr50-update.sh[6393]: success Original-Change-Id: Ic34e7d1c66cb6894c389fae74e57a1c7f53e0bb2 Reviewed-on: https://chromium-review.googlesource.com/511289 Commit-Ready: Vadim Bendebury <vbendeb@chromium.org> Tested-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> Change-Id: If5c3cc2730645979e8fc1aed51ba32790a3cdabe Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> Reviewed-on: https://chromium-review.googlesource.com/606855 Reviewed-by: Philip Chen <philipchen@chromium.org> [modify] https://crrev.com/120513895b56e6bce6a00bac71c6233bfafafbd5/chromeos-base/chromeos-cr50/files/cr50-update.sh [rename] https://crrev.com/120513895b56e6bce6a00bac71c6233bfafafbd5/chromeos-base/chromeos-cr50/chromeos-cr50-0.0.1-r31.ebuild |
|||
►
Sign in to add a comment |
|||
Comment 1 by benchan@chromium.org
, May 12 2017