From: Andrew Hewus Fresh Subject: Re: Better SIGINFO for fw_update(8) To: tech@openbsd.org Date: Mon, 27 Oct 2025 20:39:46 -0700 On Sun, Oct 26, 2025 at 03:24:57PM -0700, Andrew Hewus Fresh wrote: > Theo mentioned that sometimes fw_update takes a while and sending a > SIGINFO doesn't provide anthing useful. Unfortunately, because when > VERBOSE < 2 we save ftp output for later it's not super straight > forward. Saving the partially output lines so we can print them again > after the info was also a bit challenging, but it looks much nicer. > Hopefully we don't get a partially truncated INFO output in that case, > but I'm not sure how to guarantee that. > > One issue is that I am not sure what is catching the SIGINFO and > printing "load: 0.19 cmd: ksh 97111 [running] 0.02u 0.04s 0% 190k", but > I haven't looked terribly hard. `trap "" INFO` doesn't stop it though. > > Right now we it only traps during "fetch", but we could add another more > global handler and variable that prints out like "Install > whatever-firmware" or "Remove whatever" if it were useful. > > Comments, suggestions (especially on the "load:..." output), OK? I got some feedback that the "load: ..." was coming from the kernel and `stty nokerninfo` would let me toggle it. Here's the updated patch with, what I think, looks pretty nice, but not sure if we need other info and could use a brilliant solution for making sure that we get all the info out of _ftp_errors. It is probably good enough, but correct would be nice. Index: fw_update.sh =================================================================== RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v diff -u -p -r1.65 fw_update.sh --- fw_update.sh 12 May 2025 23:48:12 -0000 1.65 +++ fw_update.sh 28 Oct 2025 03:34:35 -0000 @@ -46,8 +46,11 @@ unset FWPKGTMP REMOVE_LOCALSRC=false DROP_PRIVS=true -status() { echo -n "$*" >&"$STATUS_FD"; } -warn() { echo "$*" >&"$WARN_FD"; } +status() { + [ ! "${FD_DIR:-}" ] || echo -n "$*" >> "$FD_DIR/last_status" + echo -n "$*" >&"$STATUS_FD" +} +warn() { echo "$*" >&"$WARN_FD"; } cleanup() { set +o errexit # ignore errors from killing ftp @@ -101,6 +104,42 @@ spin() { }>/dev/tty } +ftp_info() { + local _src="$1" _ftp_errors="$2" + local _timeout=30 # tenths of a second + + # If there was a last_status start the info on the next line + # and maybe overwrite the spinner. + [ ! "${FD_DIR:-}" ] || [ ! -s "$FD_DIR/last_status" ] || echo " " + + echo "Fetching $_src" >&2 + + kill -INFO -"$FTPPID" 2>/dev/null || return 0 + + # If VERBOSE is than 2, we are storing its STDERR in _ftp_errors, + # and we need special handling, otherwise it outputs as normal. + (( VERBOSE < 2 )) || return 0 + + # Normally in this case ftp is quiet until it prints INFO + # so give up after a while in case it never shows up. + # This is a bit of a race and the full output may not be there, + # but I don't have a good solution for that. + while [ ! -s "$_ftp_errors" ]; do + (( _timeout-- > 0 )) || return 0 + sleep 0.1 + done + + # Output the info and truncate so we can print additional output later + cat "$_ftp_errors" >&2 + >|"$_ftp_errors" + + # Re-print the last_status so the line looks right. + [ ! "${FD_DIR:-}" ] || [ ! -s "$FD_DIR/last_status" ] || + cat "$FD_DIR/last_status" + + return 0 +} + fetch() { local _src="${FWURL}/${1##*/}" _dst=$1 _user=_file _exit _error='' local _ftp_errors="$FD_DIR/ftp_errors" @@ -127,6 +166,9 @@ fetch() { ) & FTPPID=$! set +o monitor + trap 'ftp_info "$_src" "$_ftp_errors"' INFO + stty nokerninfo + SECONDS=0 _last=0 while kill -0 -"$FTPPID" 2>/dev/null; do @@ -150,6 +192,8 @@ fetch() { _exit=$? set -o errexit + stty kerninfo + trap - INFO unset FTPPID if ((_exit != 0)); then @@ -764,24 +808,28 @@ if [ "${devices[*]:-}" ]; then if "$verify_existing" && [ -e "$f" ]; then pending_status=false if ((VERBOSE == 1)); then - echo -n "Verify ${f##*/} ..." + echo -n "Verify ${f##*/} ..." | + tee "$FD_DIR/last_status" pending_status=true elif ((VERBOSE > 1)) && ! "$INSTALL"; then echo "Keep/Verify ${f##*/}" fi if "$DRYRUN" || verify_existing "$f"; then - "$pending_status" && echo " done." + "$pending_status" && echo " done." && + rm -f "$FD_DIR/last_status" if ! "$INSTALL"; then kept="$kept,$d" continue fi elif "$DOWNLOAD"; then - "$pending_status" && echo " failed." + "$pending_status" && echo " failed." && + rm -f "$FD_DIR/last_status" ((VERBOSE > 1)) && echo "Refetching $f" rm -f "$f" else - "$pending_status" && echo " failed." + "$pending_status" && echo " failed." && + rm -f "$FD_DIR/last_status" continue fi fi @@ -831,7 +879,8 @@ for f in "${add[@]}" _update_ "${update[ ((VERBOSE)) && echo "$action ${f##*/}" else if ((VERBOSE == 1)); then - echo -n "Install ${f##*/} ..." + echo -n "Install ${f##*/} ..." | + tee "$FD_DIR/last_status" pending_status=true fi fi @@ -840,14 +889,16 @@ for f in "${add[@]}" _update_ "${update[ ((VERBOSE)) && echo "Get/Verify ${f##*/}" else if ((VERBOSE == 1)); then - echo -n "Get/Verify ${f##*/} ..." + echo -n "Get/Verify ${f##*/} ..." | + tee "$FD_DIR/last_status" pending_status=true fi fetch "$f" && verify "$f" || { integer e=$? - "$pending_status" && echo " failed." + "$pending_status" && echo " failed." && + rm -f "$FD_DIR/last_status" status " failed (${f##*/})" if ((VERBOSE)) && [ -s "$FD_DIR/warn" ]; then @@ -868,7 +919,8 @@ for f in "${add[@]}" _update_ "${update[ fi if ! "$INSTALL"; then - "$pending_status" && echo " done." + "$pending_status" && echo " done." && + rm -f "$FD_DIR/last_status" continue fi @@ -888,7 +940,8 @@ for f in "${add[@]}" _update_ "${update[ fi add_firmware "$f" "$action" || { - "$pending_status" && echo " failed." + "$pending_status" && echo " failed." && + rm -f "$FD_DIR/last_status" status " failed (${f##*/})" continue } @@ -900,6 +953,7 @@ for f in "${add[@]}" _update_ "${update[ else echo " updated." fi + rm -f "$FD_DIR/last_status" fi done