On 2022/02/09 19:30, Andrew Hewus Fresh wrote:
> I was reminded that fw_update(8) updates the package database without
> locking currently. That can cause issues when running it concurrently
> with pkg_add, for example starting `pkg_add -u` in one terminal and
> `sysupgrade` in another.
>
> This diff checks to see if perl is available and if so starts a perl
> co-process that locks the database and then kills it when exiting.
>
> (Figuring out how to wait for the lock to be acquired and find out the
> pid of the co-process was a bit of a challenge, so if someone has a
> suggestion on improving that, I'm open to suggestions)
>
> Comments, OK?
Works for me, and as fw_update is using the system package database
then it does need to lock it. (Also it prints feedback, which is good
- SIGINFO is handled by the sleep or ksh process so it's normally
hard to see what's happening when fw_update is taking a long time).
FWIW OK with me.
$ doas fw_update
Package database already locked... awaiting release... done!
fw_update: added none; updated iwm; kept bwfm,intel,inteldrm,iwn,iwx,uvideo,vmm
> Index: usr.sbin/fw_update/fw_update.sh
> ===================================================================
> RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
> retrieving revision 1.36
> diff -u -p -r1.36 fw_update.sh
> --- usr.sbin/fw_update/fw_update.sh 10 Feb 2022 00:29:32 -0000 1.36
> +++ usr.sbin/fw_update/fw_update.sh 10 Feb 2022 03:22:20 -0000
> @@ -42,11 +42,13 @@ INSTALL=true
> LOCALSRC=
>
> unset FTPPID
> +unset LOCKPID
> unset FWPKGTMP
> REMOVE_LOCALSRC=false
> cleanup() {
> set +o errexit # ignore errors from killing ftp
> [ "${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"
> @@ -194,6 +196,36 @@ firmware_devicename() {
> echo "$_d"
> }
>
> +lock_db() {
> + [ "${LOCKPID:-}" ] && return 0
> +
> + # The installer doesn't have perl, so we can't lock there
> + [ -e /usr/bin/perl ] || return 0
> +
> + set -o monitor
> + perl <<'EOL' |&
> + use v5.16;
> + use warnings;
> + use OpenBSD::PackageInfo qw< lock_db unlock_db >;
> + use OpenBSD::State;
> +
> + $|=1;
> +
> + lock_db(0, OpenBSD::State->new);
> + END { unlock_db }
> + $SIG{TERM} = sub { exit };
> +
> + say $$;
> + sleep;
> +EOL
> + set +o monitor
> +
> + read -rp LOCKPID
> +
> + return 0
> +}
> +
> +
> installed_firmware() {
> local _pre="$1" _match="$2" _post="$3" _firmware _fw
> set -sA _firmware -- $(
> @@ -401,6 +433,7 @@ set -sA devices -- "$@"
>
> if "$DELETE"; then
> [ "$OPT_F" ] && echo "Cannot use -F and -d" >&2 && usage
> + lock_db
>
> # Show the "Uninstall" message when just deleting not upgrading
> ((VERBOSE)) && VERBOSE=3
> @@ -463,6 +496,8 @@ else
> fi
>
> [ "${devices[*]:-}" ] || exit
> +
> +lock_db
>
> added=''
> updated=''
>
