Date: Tue, 5 Jan 2016 17:48:55 -0800 From: Devin Teske <dteske@freebsd.org> To: Ian Lepore <ian@freebsd.org> Cc: Allan Jude <allanjude@freebsd.org>, Warner Losh <imp@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Devin Teske <dteske@FreeBSD.org> Subject: Re: svn commit: r293227 - head/etc Message-ID: <B77AA19A-F8BB-45D0-B7DD-D379FEA03CE7@freebsd.org> In-Reply-To: <1452043106.1320.52.camel@freebsd.org> References: <201601052120.u05LKlQw074919@repo.freebsd.org> <1452038404.1320.46.camel@freebsd.org> <1A1BB09D-2FB4-4E50-9F86-62B772855224@freebsd.org> <568C5D49.9090502@freebsd.org> <1452040072.1320.49.camel@freebsd.org> <19DAF4A2-00E8-41D5-9DB5-65854DF5D58A@freebsd.org> <1452043106.1320.52.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Jan 5, 2016, at 5:18 PM, Ian Lepore <ian@freebsd.org> wrote: >=20 > On Tue, 2016-01-05 at 16:35 -0800, Devin Teske wrote: >>> On Jan 5, 2016, at 4:27 PM, Ian Lepore <ian@freebsd.org> wrote: >>>=20 >>> On Tue, 2016-01-05 at 19:18 -0500, Allan Jude wrote: >>>> On 2016-01-05 19:16, Devin Teske wrote: >>>>>=20 >>>>>> On Jan 5, 2016, at 4:00 PM, Ian Lepore <ian@freebsd.org> >>>>>> wrote: >>>>>>=20 >>>>>> 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 >>>>>>>=20 >>>>>>> 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. >>>>>>>=20 >>>>>>> Modified: >>>>>>> head/etc/rc >>>>>>>=20 >>>>>>> Modified: head/etc/rc >>>>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>>>> =3D=3D=3D=3D >>>>>>> =3D=3D=3D=3D=3D=3D >>>>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>>>> --- 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} =3D "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} =3D "yes" ] || mount -ur / >>>>>>> kill -INT 1 >>>>>>> fi >>>>>>>=20 >>>>>>=20 >>>>>> 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. >>>>>>=20 >>>>>=20 >>>>> 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). >>>>>=20 >>>>> 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. >>>>>=20 >>>>=20 >>>> More specifically, firstboot is most likely run in situations >>>> where >>>> no=20 >>>> one will be at the console, so an interactive prompt stopping the >>>> system=20 >>>> from coming up is bad. >>>>=20 >>>=20 >>> 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. >>=20 >> What if you are in New York and the server is alone in Siberia? >>=20 >> ... Got SSH? (not if your boot stopped, you don't) >=20 > 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? >=20 > Silent failure is only a viable option for expected errors you can > recover from without intervention. >=20 Your point is valid. However, I think it unwise to rely on this: dteske@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. --=20 Devin=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B77AA19A-F8BB-45D0-B7DD-D379FEA03CE7>