Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 12 Oct 2013 14:32:30 +0000
From:      "Teske, Devin" <Devin.Teske@fisglobal.com>
To:        Nathan Whitehorn <nwhitehorn@freebsd.org>
Cc:        "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, Devin Teske <dteske@FreeBSD.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "Teske, Devin" <Devin.Teske@fisglobal.com>
Subject:   Re: svn commit: r256343 - in head/usr.sbin/bsdinstall: . scripts
Message-ID:  <13CA24D6AB415D428143D44749F57D720FC5B547@LTCFISWMSGMB21.FNFIS.com>
In-Reply-To: <5258F9B3.7030101@freebsd.org>
References:  <201310112041.r9BKfZeT002056@svn.freebsd.org> <5258F9B3.7030101@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

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=3Dhttp://svnweb.freebsd.=
org/changeset/base/256343&k=3D%2FbkpAUdJWZuiTILCq%2FFnQg%3D%3D%0A&r=3DMrjs6=
vR4%2Faj2Ns9%2FssHJjg%3D%3D%0A&m=3DLDzuPpXPP4D5BzfISZjw%2BXitYn4aKVzfXzcrmM=
NFo2U%3D%0A&s=3D3d0963d9c497f7bad0918888032ca62844580612dc48ab3a8a6768fe640=
c365b
>>=20
>> 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.
>>=20
>>  Submitted by:	Allan Jude <freebsd@allanjude.com>, myself
>>  Reviewed by:	Allan Jude <freebsd@allanjude.com>
>>  Approved by:	re (glebius)
>=20
> Hi Devin --
>=20
> As was discussed on the mailing list, this patch still has some issues
> that need to be resolved, for example the use of camcontrol
> unconditionally even when the disks may not be CAM and destruction of
> existing sub-partitioning for MBR disks.

The code to replace the use of camcontrol is a a *very* complex parsing
of the geom XML configuration data stashed in sysctl. jmg@ started the
ball rolling on that.



> There were some others brought
> up in the discussion as well. I am surprised you committed it,

Calm down.

The camcontrol functionality is a value-add and *not* the sole provision for
getting descriptions of the disk.

jmg@ dumped a beautiful (but needs cleaning up for integration and
optimization -- not bad for the fact that he wrote in ... a day!) piece of =
code
on me that can parse the XML geom data from sysctl.

Unfortunately, it needs some heavy integration.

So while you bring up this short-coming... _others_ have already kindly
chimed in to help.

The likelihood that it will be fixed before 10.0-R is extremely high.

You can calm down.


> especially to stable/10, before those issues were resolved.

Uh...

I don't know how to respond to that.

I'm going to ignore that (completely).




> I'm also not
> sure if people can review their own patches.
>=20

He didn't... it should have been "In collaboration with".

Allan submitted something. I completely rewrote it, and he reviewed what *I=
* wrote.



> Installer regressions are very easy to introduce and very problematic

Don't I know (looks at *you*)



> when created. Real review for installer changes is thus especially
> important this late in the release cycle.

Using camcontrol is not the end of the world -- even if the code is read-on=
ly to you...
you should be able to ... "read it?"


> Do you have any plans to fix
> these issues in the very near future?

Yes. Which has been discussed at-length, you didn't need to put a sandbag o=
n my back
(publicly no less; thanks for that).
--=20
Devin

_____________
The information contained in this message is proprietary and/or confidentia=
l. If you are not the intended recipient, please: (i) delete the message an=
d all copies; (ii) do not disclose, distribute or use the message in any ma=
nner; and (iii) notify the sender immediately. In addition, please be aware=
 that any message addressed to our domain is subject to archiving and revie=
w by persons other than the intended recipient. Thank you.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?13CA24D6AB415D428143D44749F57D720FC5B547>