[illumos-Developer] issue 968 request for code review

Garrett D'Amore garrett at nexenta.com
Sat Apr 30 13:14:42 PDT 2011


On Sat, 2011-04-30 at 15:31 -0400, Albert Lee wrote:
> I'm strongly in favour of replacing the "preample" with a packed
> structure with named fields. I see no compelling reason to make the
> contents tunable since any necessary change implies an implementation
> bug.

I disagree.  I hate packing because it is compiler sensitive.  In this
case the structure itself is not compile time tunable, and I'm happy
with it.

I think the code can integrate as is.

	- Garrett

> 
> -Albert
> 
> On Sat, Apr 30, 2011 at 11:49 AM, Thomas Joy <t.joy at deepserv.com> wrote:
> > Since the parameter is obsolete, I should think hard-coded would be
> > all right, but in the interest of robustness I'll see about putting it
> > in fct.conf.  And fixing the spelling.
> >
> > In the meantime, I'd appreciate some feedback on whether this causes
> > problems on newer fibre channel networks.
> >
> >
> > ---------- Forwarded message ----------
> > From: Steve Gonczi <gonczi at comcast.net>
> > Date: Fri, Apr 29, 2011 at 2:39 PM
> > Subject: Re: [illumos-Developer] issue 968 request for code review
> > To: Thomas Joy <t.joy at deepserv.com>
> >
> >
> > Ideally, this value should be configurable instead of hard-coded.
> >
> > BTW what the heck is preample?
> > (I am guessing whoever wrote the code meant
> > preamble )
> >
> > /sG/
> >
> > ----- "Thomas Joy" <t.joy at deepserv.com> wrote:
> >
> > yesterday I submitted this bug for the fct driver that affected my
> > organization's SAN, in particular a protocol beef between OI and a
> > brocade silkworm 3800.
> >
> > t10.org released a document on 2005 that amended the FC standard to
> > increase interoperability, i've linked it in the bug report.
> >
> > https://www.illumos.org/issues/968
> >
> > I'd like to ultimately submit a patch to bring fct in line with the
> > 2005 FC amendment.  I've built on this patch in the lab and it appears
> > to fix my problem very well.
> >
> > here's the webrev:
> >
> > http://cr.illumos.org/view/mwxjxrcc/
> >
> > _______________________________________________
> > Developer mailing list
> > Developer at lists.illumos.org
> > http://lists.illumos.org/m/listinfo/developer
> >
> 
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer





More information about the Developer mailing list