Re: Have sysupgrade run fw_update -vv

2023-08-20 Thread Andrew Hewus Fresh
On Sat, Aug 19, 2023 at 03:11:46PM -0700, Andrew Hewus Fresh wrote:
> On Sun, Aug 13, 2023 at 04:42:49PM -0600, Theo de Raadt wrote:
> > Stuart Henderson  wrote:
> > 
> > > On 2023/08/13 11:44, Andrew Hewus Fresh wrote:
> > > > My laptop doesn't have the fastest wifi and sysupgrade already uses a
> > > > progress bar to show what it's doing, so I'd really like to provide more
> > > > feedback on what it is doing:
> > > 
> > > Does a single -v give enough feedback?
> > 
> > As shown below, -vv is ridiculously verbose.
> > 
> > I think -v is also too verbose.
> 
> That makes sense to me.  Here's a fairly intrusive patch that in the
> normal case prints the status line "as we go" so you can see that it is
> doing something.  Errors from ftp are not super friendly, although I'm
> not entirely sure what it should do yet, so right now I do nothing.

Some additional detail on this.  One example is if the ftp process fails
to download a file, for example it is killed or times out or something
which results in (in the case of a SIGTERM):

$ doas ksh ./fw_update.sh 
fw_update.sh: add intel /Terminated 
Terminated 
,inteldrm,iwm,uvideo,vmm; update none
Cannot fetch 
http://firmware.openbsd.org/firmware/snapshots/intel-firmware-20230808v0.tgz

Would it be an improvement to provide more detail?

$ doas ksh ./fw_update.sh 
fw_update.sh: add intel /Terminated 
Terminated 
(intel-firmware-20230808v0.tgz download failed),inteldrm,iwm,uvideo,vmm; update 
none
Cannot fetch 
http://firmware.openbsd.org/firmware/snapshots/intel-firmware-20230808v0.tgz

Since I don't know what, if anything, was printed by the failure,
it's hard to know how much would be helpful.


> In the normal and single -v case, it also adds a spinner while it is
> downloading if there STDOUT is a tty.
> 
> We now go in two phases for install/update where we first detect what
> needs to be done and then do it so we can separate install from
> download.
> 
> In any case, I think this is ready for wider testing.
> 
> Comments, suggestions, OK?
> 
-- 
andrew

Full-time system administration is a delicate balance 
between proactiveness and laziness.
  --  jhorwitz from use.perl.org



Re: Have sysupgrade run fw_update -vv

2023-08-19 Thread Andrew Hewus Fresh
On Sun, Aug 13, 2023 at 04:42:49PM -0600, Theo de Raadt wrote:
> Stuart Henderson  wrote:
> 
> > On 2023/08/13 11:44, Andrew Hewus Fresh wrote:
> > > My laptop doesn't have the fastest wifi and sysupgrade already uses a
> > > progress bar to show what it's doing, so I'd really like to provide more
> > > feedback on what it is doing:
> > 
> > Does a single -v give enough feedback?
> 
> As shown below, -vv is ridiculously verbose.
> 
> I think -v is also too verbose.

That makes sense to me.  Here's a fairly intrusive patch that in the
normal case prints the status line "as we go" so you can see that it is
doing something.  Errors from ftp are not super friendly, although I'm
not entirely sure what it should do yet, so right now I do nothing.

In the normal and single -v case, it also adds a spinner while it is
downloading if there STDOUT is a tty.

We now go in two phases for install/update where we first detect what
needs to be done and then do it so we can separate install from
download.

In any case, I think this is ready for wider testing.

Comments, suggestions, OK?

Index: fw_update.sh
===
RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
retrieving revision 1.44
diff -u -p -r1.44 fw_update.sh
--- fw_update.sh12 Dec 2022 02:30:51 -  1.44
+++ fw_update.sh19 Aug 2023 21:49:39 -
@@ -40,18 +40,39 @@ DELETE=false
 DOWNLOAD=true
 INSTALL=true
 LOCALSRC=
+ENABLE_SPINNER=false
+[ -t 1 ] && ENABLE_SPINNER=true
+
+integer STATUS_FD=1
+integer WARN_FD=2
+FD_DIR=
 
 unset FTPPID
 unset LOCKPID
 unset FWPKGTMP
 REMOVE_LOCALSRC=false
+
+status() { echo -n "$*" >&"$STATUS_FD"; }
+warn()   { echo"$*" >&"$WARN_FD"; }
+
 cleanup() {
set +o errexit # ignore errors from killing ftp
+
+   if [ -d "$FD_DIR" ]; then
+   echo "" >&"$STATUS_FD"
+   exec 4>&-
+
+   [ -s "$FD_DIR/status" ] && cat "$FD_DIR/status"
+   [ -s "$FD_DIR/warn"   ] && cat "$FD_DIR/warn" >&2
+
+   rm -rf "$FD_DIR"
+   fi
+
[ "${FTPPID:-}" ] && kill -TERM -"$FTPPID" 2>/dev/null
[ "${LOCKPID:-}" ] && kill -TERM -"$LOCKPID" 2>/dev/null
[ "${FWPKGTMP:-}" ] && rm -rf "$FWPKGTMP"
"$REMOVE_LOCALSRC" && rm -rf "$LOCALSRC"
-   [ -e "${CFILE}" ] && [ ! -s "$CFILE" ] && rm -f "$CFILE"
+   [ -e "$CFILE" ] && [ ! -s "$CFILE" ] && rm -f "$CFILE"
 }
 trap cleanup EXIT
 
@@ -70,6 +91,22 @@ tmpdir() {
echo "$_dir"
 }
 
+spin() {
+   if ! "$ENABLE_SPINNER"; then
+   sleep 1
+   return 0
+   fi
+
+   {
+   echo -n '  '
+   for p in '/' '-' '\\' '|'; do
+   echo -n '\010'"$p"
+   sleep 0.25
+   done
+   echo -n "\010 \010\010"
+   }>/dev/tty
+}
+
 fetch() {
local _src="${FWURL}/${1##*/}" _dst=$1 _user=_file _exit _error=''
 
@@ -99,13 +136,13 @@ fetch() {
if [[ $_last -ne $5 ]]; then
_last=$5
SECONDS=0
-   sleep 1
+   spin
else
kill -INT -"$FTPPID" 2>/dev/null
_error=" (timed out)"
fi
else
-   sleep 1
+   spin
fi
done
 
@@ -118,7 +155,7 @@ fetch() {
 
if [ "$_exit" -ne 0 ]; then
rm -f "$_dst"
-   echo "Cannot fetch $_src$_error" >&2
+   warn "Cannot fetch $_src$_error"
return 1
fi
 
@@ -133,7 +170,7 @@ check_cfile() {
[ -s "$CFILE" ] || return 1
return 0
fi
-   if ! fetch_cfile "$@"; then
+   if ! fetch_cfile; then
echo -n > "$CFILE"
return 1
fi
@@ -146,10 +183,10 @@ fetch_cfile() {
fetch "$CFILE" || return 1
set -o noclobber
! signify -qVep "$FWPUB_KEY" -x "$CFILE" -m "$CFILE" &&
-   echo "Signature check of SHA256.sig failed" >&2 &&
+   warn "Signature check of SHA256.sig failed" &&
rm -f "$CFILE" && return 1
elif [ ! -e "$CFILE" ]; then
-   echo "${0##*/}: $CFILE: No such file or directory" >&2
+   warn "${0##*/}: $CFILE: No such file or directory"
return 1
fi
 
@@ -159,14 +196,25 @@ fetch_cfile() {
 verify() {
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
+   if ! grep -Fqx "SHA256 (${1##*/}) = $( /bin/sha256 -qb "$1" )" "$CFILE"
+   then
+   

Re: Have sysupgrade run fw_update -vv

2023-08-14 Thread Mikhail
On Sun, Aug 13, 2023 at 10:02:26PM -0700, Kevin Williams wrote:
> What about giving sysupgrade the same verbosity options as fw_update?
> This would mean -v and -vv I sysupgrade would pass through the same
> options to fw_update.
>
> And for the installer, maybe another question: “Verbose download &
> install progress?” Or no, as default.

I think it's overengineering for such simple task.

Regarding installer - probably simple notice about what's happening will
be enough too - user will either understand that some network activity
in process or will get immediate message that installer can't resolve
whatever etc.

Proposed patch:

diff /usr/src
commit - 8afcf90fb39e4a84606e93137c2b6c20f44312cb
path + /usr/src
blob - 4386ec9873c433a99fa83b9a9091c06bd952
file + distrib/miniroot/install.sub
--- distrib/miniroot/install.sub
+++ distrib/miniroot/install.sub
@@ -3008,7 +3008,9 @@ __EOT
 fi
 __EOT
 
-   [ -x /mnt/usr/sbin/fw_update ] && DESTDIR=/mnt /mnt/usr/sbin/fw_update
+   [ -x /mnt/usr/sbin/fw_update ] &&
+   echo "Downloading firmware..." &&
+   DESTDIR=/mnt /mnt/usr/sbin/fw_update
 
if [[ -f $_kernel_dir.tgz ]]; then
echo -n "Relinking to create unique kernel..."



Re: Have sysupgrade run fw_update -vv

2023-08-13 Thread Kevin Williams
What about giving sysupgrade the same verbosity options as fw_update? This 
would mean -v and -vv I sysupgrade would pass through the same options to 
fw_update.

And for the installer, maybe another question: “Verbose download & install 
progress?” Or no, as default.

On Sun, Aug 13, 2023, at 15:42, Theo de Raadt wrote:
> Stuart Henderson  wrote:
> 
> > On 2023/08/13 11:44, Andrew Hewus Fresh wrote:
> > > My laptop doesn't have the fastest wifi and sysupgrade already uses a
> > > progress bar to show what it's doing, so I'd really like to provide more
> > > feedback on what it is doing:
> > 
> > Does a single -v give enough feedback?
> 
> As shown below, -vv is ridiculously verbose.
> 
> I think -v is also too verbose.  If you are going to do a progress bar,
> I think it should be one progress bar.
> 
> I am sure there is a machine out there where -vv will do more than 24
> lines of output.  If not today, eventually.
> 
> So I don't think we want -vv or -v in in sysupgrade or the installer,
> unless the output is changed to be less verbose.  The IMPORTANT information
> from sysupgrade or the installer... is not this wall of text.
> 
> > It's a fair bit quieter (it
> > doesn't include the time estimates from ftp, but does still prints
> > what it's doing before it starts downloading anything potentially
> > large, so at least you aren't sat there downloading 12Mb of intel-
> > or iwx-firmware or 25Mb of amdgpu-firmware with zero idea about
> > what it's doing).
> > 
> > # fw_update -vv
> > Detect firmware ... found.
> > Get/Verify SHA256.sig   100% |**|  2371   00:00 
> >
> > Get/Verify intel-firmware-2023080... 100% |*| 12155 KB00:07 
> >
> > Install intel-firmware-2023080... 100% || 12155 KB00:00 
> >
> > Get/Verify iwx-firmware-20230330.tgz 100% |*| 12890 KB00:48 
> >
> > Install iwx-firmware-20230330.tgz 100% || 12890 KB00:00 
> >
> > Get/Verify vmm-firmware-1.14.0p4.tgz 100% |*| 42927   00:00 
> >
> > Install vmm-firmware-1.14.0p4.tgz 100% || 42927   00:00 
> >
> > fw_update: added intel,iwx,vmm; updated none; kept inteldrm,uvideo
> > 
> > vs.
> > 
> > # fw_update -v
> > Get/Verify intel-firmware-20230808v0.tgz ... installed.
> > Get/Verify iwx-firmware-20230330.tgz ... installed.
> > Get/Verify vmm-firmware-1.14.0p4.tgz ... installed.
> > fw_update: added intel,iwx,vmm; updated none; kept inteldrm,uvideo
> > 
> > > $ time doas fw_update 
> > > fw_update: added intel; updated none; kept inteldrm,iwm,uvideo,vmm
> > > 0m58.45s real 0m00.51s user 0m00.35s system
> > 
> > firmware.openbsd.org is handled entirely by DNS round-robin with no
> > geographical awareness, so even with good local network and internet
> > connection, it can sometimes still take a very long time. For example,
> > times from two consecutive runs fetching intel-firmware on a 550M
> > download connection:
> > 
> > 0m10.11s real 0m00.71s user 0m00.77s system
> > 1m17.47s real 0m01.28s user 0m01.22s system
> > 
> 
> 


Re: Have sysupgrade run fw_update -vv

2023-08-13 Thread Theo de Raadt
Stuart Henderson  wrote:

> On 2023/08/13 11:44, Andrew Hewus Fresh wrote:
> > My laptop doesn't have the fastest wifi and sysupgrade already uses a
> > progress bar to show what it's doing, so I'd really like to provide more
> > feedback on what it is doing:
> 
> Does a single -v give enough feedback?

As shown below, -vv is ridiculously verbose.

I think -v is also too verbose.  If you are going to do a progress bar,
I think it should be one progress bar.

I am sure there is a machine out there where -vv will do more than 24
lines of output.  If not today, eventually.

So I don't think we want -vv or -v in in sysupgrade or the installer,
unless the output is changed to be less verbose.  The IMPORTANT information
from sysupgrade or the installer... is not this wall of text.

> It's a fair bit quieter (it
> doesn't include the time estimates from ftp, but does still prints
> what it's doing before it starts downloading anything potentially
> large, so at least you aren't sat there downloading 12Mb of intel-
> or iwx-firmware or 25Mb of amdgpu-firmware with zero idea about
> what it's doing).
> 
> # fw_update -vv
> Detect firmware ... found.
> Get/Verify SHA256.sig   100% |**|  2371   00:00   
>  
> Get/Verify intel-firmware-2023080... 100% |*| 12155 KB00:07   
>  
> Install intel-firmware-2023080... 100% || 12155 KB00:00   
>  
> Get/Verify iwx-firmware-20230330.tgz 100% |*| 12890 KB00:48   
>  
> Install iwx-firmware-20230330.tgz 100% || 12890 KB00:00   
>  
> Get/Verify vmm-firmware-1.14.0p4.tgz 100% |*| 42927   00:00   
>  
> Install vmm-firmware-1.14.0p4.tgz 100% || 42927   00:00   
>  
> fw_update: added intel,iwx,vmm; updated none; kept inteldrm,uvideo
> 
> vs.
> 
> # fw_update -v
> Get/Verify intel-firmware-20230808v0.tgz ... installed.
> Get/Verify iwx-firmware-20230330.tgz ... installed.
> Get/Verify vmm-firmware-1.14.0p4.tgz ... installed.
> fw_update: added intel,iwx,vmm; updated none; kept inteldrm,uvideo
> 
> > $ time doas fw_update 
> > fw_update: added intel; updated none; kept inteldrm,iwm,uvideo,vmm
> > 0m58.45s real 0m00.51s user 0m00.35s system
> 
> firmware.openbsd.org is handled entirely by DNS round-robin with no
> geographical awareness, so even with good local network and internet
> connection, it can sometimes still take a very long time. For example,
> times from two consecutive runs fetching intel-firmware on a 550M
> download connection:
> 
> 0m10.11s real 0m00.71s user 0m00.77s system
> 1m17.47s real 0m01.28s user 0m01.22s system
> 



Re: Have sysupgrade run fw_update -vv

2023-08-13 Thread Stuart Henderson
On 2023/08/13 11:44, Andrew Hewus Fresh wrote:
> My laptop doesn't have the fastest wifi and sysupgrade already uses a
> progress bar to show what it's doing, so I'd really like to provide more
> feedback on what it is doing:

Does a single -v give enough feedback? It's a fair bit quieter (it
doesn't include the time estimates from ftp, but does still prints
what it's doing before it starts downloading anything potentially
large, so at least you aren't sat there downloading 12Mb of intel-
or iwx-firmware or 25Mb of amdgpu-firmware with zero idea about
what it's doing).

# fw_update -vv
Detect firmware ... found.
Get/Verify SHA256.sig   100% |**|  2371   00:00
Get/Verify intel-firmware-2023080... 100% |*| 12155 KB00:07
Install intel-firmware-2023080... 100% || 12155 KB00:00
Get/Verify iwx-firmware-20230330.tgz 100% |*| 12890 KB00:48
Install iwx-firmware-20230330.tgz 100% || 12890 KB00:00
Get/Verify vmm-firmware-1.14.0p4.tgz 100% |*| 42927   00:00
Install vmm-firmware-1.14.0p4.tgz 100% || 42927   00:00
fw_update: added intel,iwx,vmm; updated none; kept inteldrm,uvideo

vs.

# fw_update -v
Get/Verify intel-firmware-20230808v0.tgz ... installed.
Get/Verify iwx-firmware-20230330.tgz ... installed.
Get/Verify vmm-firmware-1.14.0p4.tgz ... installed.
fw_update: added intel,iwx,vmm; updated none; kept inteldrm,uvideo

> $ time doas fw_update 
> fw_update: added intel; updated none; kept inteldrm,iwm,uvideo,vmm
> 0m58.45s real 0m00.51s user 0m00.35s system

firmware.openbsd.org is handled entirely by DNS round-robin with no
geographical awareness, so even with good local network and internet
connection, it can sometimes still take a very long time. For example,
times from two consecutive runs fetching intel-firmware on a 550M
download connection:

0m10.11s real 0m00.71s user 0m00.77s system
1m17.47s real 0m01.28s user 0m01.22s system



Re: Have sysupgrade run fw_update -vv

2023-08-13 Thread David Coppa
Il Dom 13 Ago 2023, 21:17 Mikhail  ha scritto:

This will be useful in the installer too, when I first installed OpenBSD
> with network connection I thought installation was stuck after
> "Multiprocessor machine; using bsd.mp instead of bsd.", only after
> some time I understood that the installer was downloading firmware.
>

+1 for this.

Cheers,
David


Re: Have sysupgrade run fw_update -vv

2023-08-13 Thread Mikhail
On Sun, Aug 13, 2023 at 11:44:33AM -0700, Andrew Hewus Fresh wrote:
> My laptop doesn't have the fastest wifi and sysupgrade already uses a
> progress bar to show what it's doing, so I'd really like to provide more
> feedback on what it is doing:
> 
> $ doas fw_update -d intel
> fw_update: deleted intel
> $ time doas fw_update 
> fw_update: added intel; updated none; kept inteldrm,iwm,uvideo,vmm
> 0m58.45s real 0m00.51s user 0m00.35s system
> $ doas fw_update -d intel 
> fw_update: deleted intel
> $ time doas fw_update -vv 
> Detect firmware ... found.
> Get/Verify SHA256.sig   100% |**|  2371   00:00   
>  
> Get/Verify intel-firmware-2023080... 100% |*| 12155 KB01:04   
>  
> Install intel-firmware-2023080... 100% || 12155 KB00:00   
>  
> fw_update: added intel; updated none; kept inteldrm,iwm,uvideo,vmm
> 1m17.46s real 0m00.56s user 0m00.34s system
> 
> 
> Comments, OK?
> 
> Index: usr.sbin/sysupgrade/sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.48
> diff -u -p -r1.48 sysupgrade.sh
> --- usr.sbin/sysupgrade/sysupgrade.sh 8 Jun 2022 09:03:11 -   1.48
> +++ usr.sbin/sysupgrade/sysupgrade.sh 13 Aug 2023 18:22:02 -
> @@ -205,7 +205,7 @@ if [[ ${_NEXTKERNV[1]} == '-current' ]];
>  else
>   FW_URL=http://firmware.openbsd.org/firmware/${_NEXTKERNV[0]}/
>  fi
> -VNAME="${_NEXTKERNV[0]}" fw_update -p ${FW_URL} || true
> +VNAME="${_NEXTKERNV[0]}" fw_update -vv -p ${FW_URL} || true
>  
>  install -F -m 700 bsd.rd /bsd.upgrade
>  logger -t sysupgrade -p kern.info "installed new /bsd.upgrade. Old kernel 
> version: $(sysctl -n kern.version)"

This will be useful in the installer too, when I first installed OpenBSD
with network connection I thought installation was stuck after
"Multiprocessor machine; using bsd.mp instead of bsd.", only after
some time I understood that the installer was downloading firmware.

(untested patch)

diff /usr/src
commit - 8afcf90fb39e4a84606e93137c2b6c20f44312cb
path + /usr/src
blob - 4386ec9873c433a99fa83b9a9091c06bd952
file + distrib/miniroot/install.sub
--- distrib/miniroot/install.sub
+++ distrib/miniroot/install.sub
@@ -3008,7 +3008,7 @@ __EOT
 fi
 __EOT
 
-   [ -x /mnt/usr/sbin/fw_update ] && DESTDIR=/mnt /mnt/usr/sbin/fw_update
+   [ -x /mnt/usr/sbin/fw_update ] && DESTDIR=/mnt /mnt/usr/sbin/fw_update 
-vv
 
if [[ -f $_kernel_dir.tgz ]]; then
echo -n "Relinking to create unique kernel..."