svn commit: r293227 - head/etc

[Devin Teske]

> On Jan 5, 2016, at 5:18 PM, Ian Lepore <ian at freebsd.org> wrote:
> 
> On Tue, 2016-01-05 at 16:35 -0800, Devin Teske wrote:
>>> On Jan 5, 2016, at 4:27 PM, Ian Lepore <ian at freebsd.org> wrote:
>>> 
>>> On Tue, 2016-01-05 at 19:18 -0500, Allan Jude wrote:
>>>> On 2016-01-05 19:16, Devin Teske wrote:
>>>>> 
>>>>>> On Jan 5, 2016, at 4:00 PM, Ian Lepore <ian at freebsd.org>
>>>>>> wrote:
>>>>>> 
>>>>>> On Tue, 2016-01-05 at 21:20 +0000, Warner Losh wrote:
>>>>>>> Author: imp
>>>>>>> Date: Tue Jan  5 21:20:47 2016
>>>>>>> New Revision: 293227
>>>>>>> URL: https://svnweb.freebsd.org/changeset/base/293227
>>>>>>> 
>>>>>>> Log:
>>>>>>> Use the more proper -f. Leave /bin/rm in place since
>>>>>>> that's
>>>>>>> what
>>>>>>> other rc scripts have, though it isn't strictly necessary.
>>>>>>> 
>>>>>>> Modified:
>>>>>>> head/etc/rc
>>>>>>> 
>>>>>>> Modified: head/etc/rc
>>>>>>> ===========================================================
>>>>>>> ====
>>>>>>> ======
>>>>>>> =========
>>>>>>> --- head/etc/rc	Tue Jan  5 21:20:46 2016	(r29
>>>>>>> 3226
>>>>>>> )
>>>>>>> +++ head/etc/rc	Tue Jan  5 21:20:47 2016	(r29
>>>>>>> 3227
>>>>>>> )
>>>>>>> @@ -132,9 +132,9 @@ done
>>>>>>> # Remove the firstboot sentinel, and reboot if it was
>>>>>>> requested.
>>>>>>> if [ -e ${firstboot_sentinel} ]; then
>>>>>>> 	[ ${root_rw_mount} = "yes" ] || mount -uw /
>>>>>>> -	/bin/rm ${firstboot_sentinel}
>>>>>>> +	/bin/rm -f ${firstboot_sentinel}
>>>>>>> 	if [ -e ${firstboot_sentinel}-reboot ]; then
>>>>>>> -		/bin/rm ${firstboot_sentinel}-reboot
>>>>>>> +		/bin/rm -f ${firstboot_sentinel}-reboot
>>>>>>> 		[ ${root_rw_mount} = "yes" ] || mount -ur /
>>>>>>> 		kill -INT 1
>>>>>>> 	fi
>>>>>>> 
>>>>>> 
>>>>>> Using rm -f to suppress an error message seems like a bad
>>>>>> idea
>>>>>> here --
>>>>>> if the sentinel file can't be removed that implies it's going
>>>>>> to
>>>>>> do
>>>>>> firstboot behavior every time it boots, and that's the sort
>>>>>> of
>>>>>> error
>>>>>> that should be in-your-face.  Especially on the reboot one
>>>>>> because
>>>>>> you're going to be stuck in a reboot loop with no error
>>>>>> message.
>>>>>> 
>>>>> 
>>>>> Leaving off -f so that the user gets prompted isn't quite as
>>>>> helpful
>>>>> as, say, using -f but then testing to make sure the file is
>>>>> really
>>>>> gone
>>>>> (if it still exists after a silent "rm -f", put up an
>>>>> informative
>>>>> warning
>>>>> instead of asking the user if they would like to delete it).
>>>>> 
>>>>> The end-result of having something thrown in your face seems
>>>>> desirable. Having a prompt that asks you if you'd like to
>>>>> delete it
>>>>> (even if there is an error immediately above it explaining it
>>>>> could
>>>>> not be deleted) seems nonsensical.
>>>>> 
>>>> 
>>>> More specifically, firstboot is most likely run in situations
>>>> where
>>>> no 
>>>> one will be at the console, so an interactive prompt stopping the
>>>> system 
>>>> from coming up is bad.
>>>> 
>>> 
>>> I couldn't possibly disagree more.  If you're not paying attention
>>> to
>>> what happens the first time you boot a freshly installed system,
>>> you
>>> deserve whatever happens to you.
>> 
>> What if you are in New York and the server is alone in Siberia?
>> 
>> ... Got SSH? (not if your boot stopped, you don't)
> 
> Unh huh.  And what are you going to do when the server goes
> unresponsive because it silently failed to delete firstboot-reboot and
> now it's just in an endless reboot loop?
> 
> Silent failure is only a viable option for expected errors you can
> recover from without intervention.
> 

Your point is valid. However, I think it unwise to rely on this:

dteske at porridge wwwww $ rm foo
override rw-rw-r--  dteske/dteske schg,uarch for foo? y
rm: foo: Operation not permitted

As you can see above, the prompt put forth by rm really has nothing to do with "failure" but rather it has performed a cursory check and is asking you if it is OK to proceed.

The condition in which rm puts forth the prompt is _NOT_ the condition in which you want to halt the boot process.

You're absolutely right that we ought to prevent an infinite reboot-cycle.
Relying on rm to do it by not using "-f" is the wrong approach.

This is the right approach:

	rm -f "${firstboot_sentinel}-reboot"
	if [ -e "${firstboot_sentinel}-reboot" ]; then
		read -p "Ruh roh; I smell an infinite reboot in your future!" IGNORED
	fi

(if lovable Scooby Doo had coded it)
Funny error message aside, I earnestly think that's the approach we should take.

...

Quick note, should the code be updated to handle this:

$ mkdir $firstboot_sentinel
$ mkdir !$-reboot
$ reboot

This too:

$ touch $firstboot_sentinel
$ chflags schg !$
$ touch !$-reboot
$ chflags schg !$
$ reboot


Both of which would lead to infinite reboot cycle.
-- 
Devin