From owner-svn-src-head@FreeBSD.ORG Sun Oct 13 07:41:16 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 07A3E723; Sun, 13 Oct 2013 07:41:16 +0000 (UTC) (envelope-from nwhitehorn@anacreon.physics.wisc.edu) Received: from anacreon.physics.wisc.edu (unknown [IPv6:2607:f388:101c:0:216:cbff:fe39:3fae]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id CF1B12742; Sun, 13 Oct 2013 07:41:14 +0000 (UTC) Received: from anacreon.physics.wisc.edu (localhost [127.0.0.1]) by anacreon.physics.wisc.edu (8.14.7/8.14.7) with ESMTP id r9D7fB4m033822; Sun, 13 Oct 2013 02:41:11 -0500 (CDT) (envelope-from nwhitehorn@anacreon.physics.wisc.edu) Received: from localhost (nwhitehorn@localhost) by anacreon.physics.wisc.edu (8.14.7/8.14.7/Submit) with ESMTP id r9D7fA9l033819; Sun, 13 Oct 2013 02:41:10 -0500 (CDT) (envelope-from nwhitehorn@anacreon.physics.wisc.edu) Date: Sun, 13 Oct 2013 02:41:10 -0500 (CDT) From: Nathan Whitehorn To: Devin Teske Subject: Re: svn commit: r256343 - in head/usr.sbin/bsdinstall: . scripts In-Reply-To: <13CA24D6AB415D428143D44749F57D720FC5BB95@LTCFISWMSGMB21.FNFIS.com> Message-ID: References: <201310112041.r9BKfZeT002056@svn.freebsd.org> <5258F9B3.7030101@freebsd.org> <13CA24D6AB415D428143D44749F57D720FC5BB95@LTCFISWMSGMB21.FNFIS.com> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" , Nathan Whitehorn , "Teske, Devin" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 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: Sun, 13 Oct 2013 07:41:16 -0000 On Sat, 12 Oct 2013, Teske, Devin wrote: > > On Oct 12, 2013, at 12:26 AM, Nathan Whitehorn wrote: > >> On 10/11/13 22:41, Devin Teske wrote: >>> Author: dteske >>> Date: Fri Oct 11 20:41:35 2013 >>> New Revision: 256343 >>> URL: https://urldefense.proofpoint.com/v1/url?u=http://svnweb.freebsd.org/changeset/base/256343&k=%2FbkpAUdJWZuiTILCq%2FFnQg%3D%3D%0A&r=Mrjs6vR4%2Faj2Ns9%2FssHJjg%3D%3D%0A&m=LDzuPpXPP4D5BzfISZjw%2BXitYn4aKVzfXzcrmMNFo2U%3D%0A&s=3d0963d9c497f7bad0918888032ca62844580612dc48ab3a8a6768fe640c365b >>> >>> Log: >>> Add zfsboot module as an option for automatic configuration. Default is >>> to run interactively but it can be scripted too (optinally completely >>> non-interactive). Currently supports GELI and all ZFS vdev types. Also >>> performs validation on selections/settings providing error messages if >>> necessary, explaining (in plain language) what the issue is. Currently >>> the auto partitioning of naked disks only supports GPT and MBR (VTOC8 >>> pending for sparc64), so is only available for i386/amd64 install. >>> >>> Submitted by: Allan Jude , myself >>> Reviewed by: Allan Jude >>> Approved by: re (glebius) >> >> Hi Devin -- >> >> As was discussed on the mailing list, this patch still has some issues >> that need to be resolved, > > Can you kindly provide links? I'm crawling through the mailing lists and > not finding anything for the October, (current, stable, sysinstall, ... ?? others?) > > Do I need to be looking back in September? I wouldn't think so, because that > bit wasn't even in our development tree until October 1st: This was discussion on freebsd-current from yesterday and the day before. > http://druidbsd.cvs.sf.net/viewvc/druidbsd/bsdinstall_zfs/usr.sbin%3A%3Absdconfig%3A%3Ashare%3A%3Adevice.subr.patch?revision=1.1&view=markup > > So there couldn't have been any discussion on it before then. So I'm just not > able to find the mailing lists where all the action is that they're discussing it. > Would be nice to find where the action is, so I could participate. > > >> for example the use of camcontrol >> unconditionally even when the disks may not be CAM > > Allan Adds: > 9.2 should have all disks listed in camcontrol, so it shouldn't be an issue No it shouldn't. Not all disks are interfaced to CAM. MFI comes to mind, nvme, VM block devices, SD cards. There are many other examples. Just because you don't have them does not justify a phenomenological approach here. > And: > I think the only systems without cam based disks are old 8.x - we're only targeting 10 anyway. Not true at all. > I tend to agree with those statements. > > >> and destruction of >> existing sub-partitioning for MBR disks. > > I think we both (Allan and I) actually responded directly to you on this one. > > We have code that handles that. It's in there. To me, yes, but I was wrong in my initial comment as pointed out by some others. In particular, you need to run gpart -F destroy recursively on the disk instead of just on the root node. There were several other issues and bugs mentioned in people's cursory review of the patch. I never dreamed you would then just commit it. I really really don't want to have to subject installer changes to explicit approval requirements, but *please* request review of non-trivial changes before commits, especially to the disk partitioning code, and especially again before insta-MFCing them to a stable branch right before a release. It is much, much better than having to do this after the fact as we are doing now. -Nathan