On Mon, Feb 07, 2022 at 08:41:13PM -0800, Andrew Hewus Fresh wrote: > I believe the below should fix this as well as hiding the duplicate > error messages trying to fetch multiple times. > > I think an empty SHA256.sig is a reasonable signifier that we couldn't > fetch it successfully and to only install local files passed on the > command-line and error on files we need to detect from the device name. > > I should probably add a "failed" section to the output at the end, but > ran out of time for that today, so you get to try this. > > Perhaps the "failed" will contribute to a non-zero exit code from > fw_update. I haven't quite been able to figure out what I would expect > from it if some firmware install and others don't.
for now, it is fine. it works well and doesn't unregistered firmwares on network problem. ok semarie@ just some nits below. > On Mon, Feb 07, 2022 at 08:28:47AM +0100, Sebastien Marie wrote: > > Hi, > > > > I am seeing that fw_update(8) does unregisteration of installed > > firmwares (removing /var/db/pkg/ files) if it is unable to fetch > > SHA256.sig. > > > > The damage is low: only packages files are removed, and firmware files > > (/etc/firmware/amdgpu and /etc/firmware/vmm-bios files in my case) are > > kept. > > > > But it creates unconsistent between package database and files on > > filesystem, and it could remove information on temporary network > > issue. > > > > I would expect no action on network connectivity issue instead of > > assuming no firmware was necessary. > > > > Here the (partial) output of sysupgrade log: > > > > fw_update: connect: Connection refused > > Cannot fetch http://firmware.openbsd.org/firmware/snapshots/SHA256.sig > > fw_update: connect: Connection refused > > Cannot fetch http://firmware.openbsd.org/firmware/snapshots/SHA256.sig > > fw_update: added none; updated none; kept none; unregistered amdgpu,vmm > > > > In my case, pf on the gateway was blocking connection. > > > Index: usr.sbin/fw_update/fw_update.sh > =================================================================== > RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v > retrieving revision 1.35 > diff -u -p -r1.35 fw_update.sh > --- usr.sbin/fw_update/fw_update.sh 30 Jan 2022 02:39:19 -0000 1.35 > +++ usr.sbin/fw_update/fw_update.sh 8 Feb 2022 04:35:24 -0000 > @@ -49,6 +49,7 @@ cleanup() { > [ "${FTPPID:-}" ] && kill -TERM -"$FTPPID" 2>/dev/null > [ "${FWPKGTMP:-}" ] && rm -rf "$FWPKGTMP" > "$REMOVE_LOCALSRC" && rm -rf "$LOCALSRC" > + [ -e "${CFILE}" ] && [ ! -s "$CFILE" ] && rm -f "$CFILE" you could use only one test: # sh [ syntax [ -e "${CFILE}" -a ! -s "$CFILE" ] && rm -f "$CFILE" # ksh specific [[ syntax [[ -e "${CFILE}" && ! -s "$CFILE" ]] && rm -f "$CFILE" both are possible, or just keeping what you have too. > } > trap cleanup EXIT > > @@ -122,6 +123,21 @@ fetch() { > return 0 > } > > +# If we fail to fetch the CFILE, we don't want to try again > +# but we might be doing this in a subshell so write out > +# a blank file indicating failure. > +check_cfile() { > + if [ -e "$CFILE" ]; then > + [ -s "$CFILE" ] || return 1 > + return 0 > + fi > + if ! fetch_cfile "$@"; then > + echo -n > "$CFILE" > + return 1 > + fi > + return 0 > +} > + > fetch_cfile() { > if "$DOWNLOAD"; then > set +o noclobber # we want to get the latest CFILE > @@ -131,14 +147,14 @@ fetch_cfile() { > echo "Signature check of SHA256.sig failed" >&2 && return 1 > elif [ ! -e "$CFILE" ]; then > echo "${0##*/}: $CFILE: No such file or directory" >&2 > - return 2 > + return 1 > fi > > return 0 > } > > verify() { > - [ -e "$CFILE" ] || fetch_cfile || return 1 > + check_cfile || return 1 > # The installer sha256 lacks -C, do it by hand > if ! fgrep -qx "SHA256 (${1##*/}) = $( /bin/sha256 -qb "$1" )" > "$CFILE"; then > ((VERBOSE != 1)) && echo "Checksum test for ${1##*/} failed." > >&2 > @@ -168,7 +184,7 @@ firmware_in_dmesg() { > } > > firmware_filename() { > - [ -e "$CFILE" ] || fetch_cfile || return 1 > + check_cfile || return 1 > sed -n "s/.*(\($1-firmware-.*\.tgz\)).*/\1/p" "$CFILE" | sed '$!d' > } > > @@ -365,7 +381,7 @@ if [ "$OPT_F" ]; then > # Always check for latest CFILE and so latest firmware > if [ -e "$LOCALSRC/$CFILE" ]; then > mv "$LOCALSRC/$CFILE" "$LOCALSRC/$CFILE-OLD" > - if fetch_cfile; then > + if check_cfile; then > rm -f "$LOCALSRC/$CFILE-OLD" > else > mv "$LOCALSRC/$CFILE-OLD" "$LOCALSRC/$CFILE" > @@ -457,7 +473,7 @@ for f in "${devices[@]}"; do > > verify_existing=true > if [ "$f" = "$d" ]; then > - f=$( firmware_filename "$d" || true ) > + f=$( firmware_filename "$d" ) || continue > if [ ! "$f" ]; then > if "$INSTALL" && unregister_firmware "$d"; then > unregister="$unregister,$d" -- Sebastien Marie
