svn commit: r256343 - in head/usr.sbin/bsdinstall: . scripts

[Teske, Devin]

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 <freebsd at allanjude.com>, myself
>>  Reviewed by:	Allan Jude <freebsd at allanjude.com>
>>  Approved by:	re (glebius)
> 
> Hi Devin --
> 
> 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.
> 

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-only 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 on my back
(publicly no less; thanks for that).
-- 
Devin

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