From owner-svn-src-head@freebsd.org Wed Jan 6 01:48:57 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8666AA631A9; Wed, 6 Jan 2016 01:48:57 +0000 (UTC) (envelope-from devin@shxd.cx) Received: from shxd.cx (mail.shxd.cx [64.201.244.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 741821D68; Wed, 6 Jan 2016 01:48:57 +0000 (UTC) (envelope-from devin@shxd.cx) Received: from 50-196-156-133-static.hfc.comcastbusiness.net ([50.196.156.133]:53910 helo=tinkerbell.pixel8networks.com) by shxd.cx with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.77 (FreeBSD)) (envelope-from ) id 1aGUuJ-000OUS-1n; Tue, 05 Jan 2016 08:56:27 -0800 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Subject: Re: svn commit: r293227 - head/etc From: Devin Teske In-Reply-To: <1452043106.1320.52.camel@freebsd.org> Date: Tue, 5 Jan 2016 17:48:55 -0800 Cc: Allan Jude , Warner Losh , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Devin Teske Message-Id: 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> To: Ian Lepore X-Mailer: Apple Mail (2.2104) Sender: devin@shxd.cx Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jan 2016 01:48:57 -0000 > On Jan 5, 2016, at 5:18 PM, Ian Lepore wrote: >=20 > On Tue, 2016-01-05 at 16:35 -0800, Devin Teske wrote: >>> On Jan 5, 2016, at 4:27 PM, Ian Lepore 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 >>>>>> 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=