[brussels-dev] [crossbow-discuss] code review Brussels persistence

[Michael Lim]

Artem Kachitchkine wrote:
> Testing is still in progress, minor changes might still happen and debug 
> printf's need to be removed, but overall it is ready for review.
>
> 	http://cr.opensolaris.org/~artem/pers-0421/
>
> Anyone is welcome to provide constructive comments, gentle critique, 
> witty remarks and naughty limericks. Following folks might be especially 
> interested:
>
> Cathy and/or Seb: dlmgmtd, dls
> Eric Cheng and/or Michael Lim: mac, dld, dls
> Ted You: bge
> Sowmini: mac, dld, libdladm, dladm, mdb
>
> I'd appreciate your inputs before next Tuesday, April 29th.
>   
I'm looking at the changes from the perspective of a consumer of the new
*_MAC_ONLY path for handling mac level properties.

linkprop.c  471

Any reason why you don't add a flags argument to 
dladm_datalink_id2info() instead
of creating i_dladm_datalink_id2nfo()?

linkprop.c 1656

The new flag, DLADM_OPT_PROP_MAC_ONLY is passed through from the 
application
all the way to  dld_set_public_prop().  Is there a way to have that flag 
attached to the specific
property instead of being decided by the application?

mac.c 101

You might want to add a note that explains the case where the properties 
are handled
by the mac layer and not passed down to the driver.

dld_drv.c 536-543

Can you bypass this for cases when setting properties at the mac layer?

dld_drv.c 558

It looks like DLD_PROP_FLAG_MAC_ONLY needs to also have DLD_PROP_MAC_NAME
set for proper handling (if the first is set but the second is not, err 
is set to EINVAL).  Is this intentional? 
For crossbow,  we'd expect to set the first without the second.


-Mike