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

Teske, Devin Devin.Teske at fisglobal.com
Sat Oct 12 15:10:28 UTC 2013


[snip]

> 
>> 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).

I apologize... I should have gone on to explain that...

The use of camcontrol is protected by a conditional block making use of the f_have()
function which means if you rip out camcontrol completely (in any way resulting in the
binary being absent) then the value-add potentially provided by camcontrol is silently
skipped as the utility is not available.

So let's say you don't have camcontrol(8) in the media, *or* say that you have a kernel
lacking CAM knobs...

No problem.

I could have completely omitted the camcontrol value-add, ... what we had already
was sufficient enough to describe disks.

It was only upon Allan's testing that he noticed that the labels could be... better ;D

So I went scrounging for something that could supersede the description of a `da0'
that would satisfy Allan, and I found it in camcontrol -- his testing confirmed.

I don't know if he was using a kernel enhanced with CAM through custom configuration
or if he was using everything stock... and I could ask him... but it appeared to be working

*and*

I protected it with f_have() so in the event that it doesn't work for everyone... no biggie...
the descriptions will be as they were before the value-add (still there, but not as accurate
as they could be... enter jmg@ and a parsing of geom later).

I think of all people, you should understand that if something doesn't introduce a blocker,
but lacks a particular value-add (in this case, geom parsing), then it should be OK to
proceed (I'm not harping on you, but I'm specifically calling out that bsdinstall did not
implement everything needed by VICOR "out of the box"; so we're still on 8.x because
we rely on much functionality in sysinstall that bsdinstall doesn't implement). So I'm a bit
shocked to see you coming down so hard on this commit.

It will be addressed, but I also had to first address jilles@ Integer Overflow statement
(I _on purpose_ committed the code we had because it had to go in, and then *very*
quickly followed it up with a fix to the integer overflow detection). You're next in-line,
but the geom parsing may not make it in until BETA2 (I honestly need a break from that
last round of commits... it took me a solid week of sleepless nights -- the wife and I would
like some time together, please).

In light of that last paragraph... we're all Human... and what we do is volunteer. So I can't
harp on you legitimately for any short-comings. But I do request a little slack considering
I didn't break anything and this is *not* that serious in the grand scheme of things.

There are no regressions that I can see which you speak of.
-- 
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.


More information about the svn-src-all mailing list