Index | Thread | Search

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:
Sun, 18 May 2025 08:32:21 +0000

Download raw body.

Thread
13.05.2025 09:15, Klemens Nanni пишет:
> 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?

Rather early ping so as to not forget about it.

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 is 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)