On Mon, 2008-06-16 at 17:05 +0300, Jack Morgenstein wrote:
> On Saturday 19 April 2008 00:54, Roland Dreier wrote:
> > Thanks, I added a man page and changed things a little and committed the
> > following:
> > 
> > commit 1c0b7ac0a6bbbe4d246ef4cf50ae31bde4929ba3
> > Author: Ira Weiny <[EMAIL PROTECTED]>
> > Date:   Tue Apr 15 13:35:48 2008 -0700
> > 
> >     Add functions to convert enum values to strings
> >     
> >     Add ibv_xxx_str() functions to convert node type, port state, event
> >     type and wc status enum values to strings.
> >     
> >     Signed-off-by: Ira K. Weiny <[EMAIL PROTECTED]>
> >     Signed-off-by: Roland Dreier <[EMAIL PROTECTED]>
> > 
> >  
> The change below (in the output format of the port state string)
> is causing us problems (with all sorts of scripts based upon ibv_devinfo).
> This only surfaced now because we're only just now bringing up the OFED 1.4 
> tree
> based upon the current libibverbs and kernel 2.6.26.
> 
> Unfortunately, I missed this change when you posted it to the list in April.
> 
> I assume that the only reason for the change below, aside from adding the 
> defer state,

Is it needed to support active defer ? Isn't active defer a SNO
condition in terms of PortState ?

-- Hal

>  was cosmetic.
> Is it possible to change the output strings so that for previously defined 
> port states,
> the output remains what it was previously?
> 
> (I don't see this as any different from other userspace library 
> backwards-compatibility issues,
> the change breaks existing scripts).
> 
> To avoid breaking the scripts, we would need as a minimum: 
> 
> +     static const char *const port_state_str[] = {
> +             [IBV_PORT_NOP]          = "no state change (NOP)",
> +             [IBV_PORT_DOWN]         = "PORT_DOWN",
> +             [IBV_PORT_INIT]         = "PORT_INIT",
> +             [IBV_PORT_ARMED]        = "PORT_ARMED",
> +             [IBV_PORT_ACTIVE]       = "PORT_ACTIVE",
> +             [IBV_PORT_ACTIVE_DEFER] = "active defer"
> +     };
> +
> +     if (port_state < IBV_PORT_NOP || port_state > IBV_PORT_ACTIVE_DEFER)
> +             return "invalid state";
> 
> I have no preferences for "active defer", but you might consider
> "PORT_ACTIVE_DEFER" for consistency. Our scripts do not work with the 
> ACTIVE_DEFER
> state. (100% backwards compatibility would dictate that the active_defer 
> state and
> the NOP state also return "invalid state" -- but this is clearly impossible 
> to do).
> 
> Please note also that "unknown" was previously "invalid state".
> - Jack
> 
> > -static const char *port_state_str(enum ibv_port_state pstate)
> > -{
> > -   switch (pstate) {
> > -   case IBV_PORT_DOWN:   return "PORT_DOWN";
> > -   case IBV_PORT_INIT:   return "PORT_INIT";
> > -   case IBV_PORT_ARMED:  return "PORT_ARMED";
> > -   case IBV_PORT_ACTIVE: return "PORT_ACTIVE";
> > -   default:              return "invalid state";
> > -   }
> > -}
> > -
> > +
> > +const char *ibv_port_state_str(enum ibv_port_state port_state)
> > +{
> > +   static const char *const port_state_str[] = {
> > +           [IBV_PORT_NOP]          = "no state change (NOP)",
> > +           [IBV_PORT_DOWN]         = "down",
> > +           [IBV_PORT_INIT]         = "init",
> > +           [IBV_PORT_ARMED]        = "armed",
> > +           [IBV_PORT_ACTIVE]       = "active",
> > +           [IBV_PORT_ACTIVE_DEFER] = "active defer"
> > +   };
> > +
> > +   if (port_state < IBV_PORT_NOP || port_state > IBV_PORT_ACTIVE_DEFER)
> > +           return "unknown";
> > +
> > +   return port_state_str[port_state];
> > +}
> _______________________________________________
> general mailing list
> [email protected]
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to