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: Tue, 13 May 2025 06:15:17 +0000

09.05.2025 04:00, Andrew Hewus Fresh пишет:
> On Thu, May 08, 2025 at 08:47:34AM +0000, Klemens Nanni wrote:
>> 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.
> 
> I still think that before this patch, in the `ifconfig netboot` case, it
> preferred http over cd0, and after it prefers cd0 over http.

You're right, I must have misunderstood and checked the wrong thing the first time.

Now I have verified that my diff indeed yields this change when installing with
available network, but without CGI aka ftplist.o.o answering;  that crucial and
apparently too easy to miss in my testing, because once the installer got an CGI
response, it takes that over all else.

> 
> If that is OK with folks, then it is (grudgingly) OK with me.

The fix is easy:  handle CD after HTTP.

Now it really just omits network sources if there are not interfaces and otherwise
behaves like before.

Thanks for the input!

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	13 May 2025 06:10:25 -0000
@@ -2806,27 +2806,28 @@ 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
+	# Offer network methods if there are physical interfaces.
+        if [[ -n $(get_ifs) ]]; then
+		_locs="$_locs http"
 
-	# Set default location to method recorded last time.
-	_d=$CGI_METHOD
+		# Offer NFS if the ramdisk supports it.
+		[[ -x /sbin/mount_nfs ]] && _locs="$_locs nfs"
 
-	# Set default location to HTTP in case we netbooted.
-	ifconfig netboot >/dev/null 2>&1 && : ${_d:=http}
+		# Prefer HTTP to any existing default if installurl(5) exists.
+		[[ -s /mnt/etc/installurl ]] && _d=http
 
-	# Set default location to HTTP if installurl(5) exists.
-	[[ -s /mnt/etc/installurl ]] && _d=http
+		# Default to HTTP, whether netbooted or not.
+		: ${_d:=http}
+	fi
 
-	# Set default location to the first cdrom device if any are found.
+        # Default to CD, assuming it is installXY.iso containing sets.
 	[[ -n $_cddevs ]] && : ${_d:=cd0}
 
-	# Add NFS to set locations if the boot kernel supports it.
-	[[ -x /sbin/mount_nfs ]] && _locs="$_locs nfs"
-
-	# In case none of the above applied, set HTTP as default location.
-	: ${_d:=http}
+	# Without network or CD, only disk remains valid and available.
+	: ${_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 +2837,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)