From: Klemens Nanni <kn@openbsd.org>
Subject: Re: installer: sets: do not offer http/nfs without network
To: Andrew Hewus Fresh <andrew@afresh1.com>
Cc: OpenBSD tech <tech@openbsd.org>
Date: Thu,  8 May 2025 08:47:34 +0000

03.05.2025 22:21, Klemens Nanni пишет:
> 03.05.2025 05:30, Andrew Hewus Fresh пишет:
>> Does this change to prefer cd0 over http if both cddev and network are
>> available?  I'm not sure I like that choice.  I like the existing order
>> of http, cd, nfs, disk.
> 
> No, both -current and my diff follow the same order of first two defaults:
> 
> 1. $CGI_METHOD - whatever ftplist.o.o remembers, if at all, if there's internet
> 2. cd0 - if there is a CD present
> 
> Only then is networking considered and my diff changes behaviour.
> 
> So whether or not HTTP is actually possible, once cd0 attached, it always is the
> default unless ftplist.o.o already said otherwise.
> 
> The installer picks HTTP over CD if and only if /mnt/etc/installurl exists, i.e.
> on upgrades where users did not remove the file.
> 
> 
> It could make sense to prefer HTTP over CD for install as well, but that's another diff.

Ping.  Same diff rebased after the recent commit.

---

Offline installs/upgrades, i.e. when there are no interfaces, still default
to fetching sets over HTTP, which makes no sense (minimal reproducer):

	# vmctl create -1g disk; vmctl start -c -b /bsd.rd -d disk offline
	...
	Let's install the sets!
	Location of sets? (disk http nfs or 'done') [http] 

At this point, interfaces are configured already and state is basically known,
yet NFS and HTTP are offered unconditionally.

Skip these two methodos unless there is a physical interface around:

	Let's install the sets!
	Location of sets? (disk or 'done') [disk] 

Shorten and clarify the comments while here.


NB: The shell idiom ${var:=val} means "set $var to 'val' unless already non-empty".

    This diff reads much better with `sdiff -w165 old new' than in unified form.


Feedback? OK?

Index: install.sub
===================================================================
RCS file: /cvs/src/distrib/miniroot/install.sub,v
diff -u -p -r1.1270 install.sub
--- install.sub	4 May 2025 12:32:41 -0000	1.1270
+++ install.sub	8 May 2025 08:46:13 -0000
@@ -2806,27 +2806,30 @@ feed_random() {
 # selects from that location. Repeat as many times as the user needs to get all
 # desired sets.
 install_sets() {
-	local _cddevs=$(get_cddevs) _d _im _locs="disk http" _src
+	# Default to method recorded last time, if any.
+	local _cddevs=$(get_cddevs) _d=$CGI_METHOD _im _locs=disk _src
 
-	echo
+	[[ -n $_cddevs ]] && : ${_d:=cd0}
 
-	# Set default location to method recorded last time.
-	_d=$CGI_METHOD
+	# Offer network methods if there are physical interfaces.
+        if [[ -n $(get_ifs) ]]; then
+		_locs="$_locs http"
 
-	# Set default location to HTTP in case we netbooted.
-	ifconfig netboot >/dev/null 2>&1 && : ${_d:=http}
+		# Default to HTTP if netbooted.
+		[[ -n $(get_ifs netboot) ]] && : ${_d:=http}
 
-	# Set default location to HTTP if installurl(5) exists.
-	[[ -s /mnt/etc/installurl ]] && _d=http
+		# Prefer HTTP to any existing default if installurl(5) exists.
+		[[ -s /mnt/etc/installurl ]] && _d=http
 
-	# Set default location to the first cdrom device if any are found.
-	[[ -n $_cddevs ]] && : ${_d:=cd0}
+		# Offer NFS if the ramdisk supports it.
+		[[ -x /sbin/mount_nfs ]] && _locs="$_locs nfs"
 
-	# Add NFS to set locations if the boot kernel supports it.
-	[[ -x /sbin/mount_nfs ]] && _locs="$_locs nfs"
+		# Default to HTTP.
+		: ${_d:=http}
+	fi
 
-	# In case none of the above applied, set HTTP as default location.
-	: ${_d:=http}
+	# Without CD and network, disk is the only option.
+	: ${_d:=disk}
 
 	# If the default location set so far is not one of the cdrom devices or
 	# is not in the list of valid locations, set a sane default.
@@ -2836,7 +2839,7 @@ install_sets() {
 		done
 	fi
 
-	echo "Let's $MODE the sets!"
+	echo "\nLet's $MODE the sets!"
 	while :; do
 		# Get list of cdroms again in case one just got plugged in.
 		_cddevs=$(get_cddevs)