From: Mark Kettenis Subject: Re: Better SIGINFO for fw_update(8) To: Andrew Hewus Fresh Cc: tech@openbsd.org Date: Tue, 28 Oct 2025 11:05:12 +0100 > Date: Mon, 27 Oct 2025 20:39:46 -0700 > From: Andrew Hewus Fresh > > 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. So what happens if I interrupt the script with ^C? Do I end up with my terminal in the "stty nokerninfo" state? To me this all seems a lot of complexity for little gain... > 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 > > >