[openib-general] RE: [PATCH] opensm: remove osm_pkey_mgr.h

[Eitan Zahavi]

Hi Sasha 

> On 13:55 Mon 22 May     , Eitan Zahavi wrote:
> > Hi Sasha,
> >
> > My point is simple:
> >
> > OpenSM has a very structured skeleton:
> > 1. All mad receivers have two c files and two h files
> > 1.1  mad receive controller which deals with dispatcher registration
.
> > 1.2 Mad receiver which deals with all the action happening after
such a
> > mad is received
> > 2. All algorithm stages (managers) have a c file and h file
> >     An algorithm stage might be lid assignment, routing, partition
> > enforcement etc
> > 3. All SMDB objects have a c file and h files.
> >     Examples are Nodes, Ports, Multicast registrations etc
> >
> > These are the structural code rules for OpenSM.
> >
> > Even if you think it is better to merge functionality and avoid
having
> > some of these h files and you might even be able to save some lines
of
> > code by doing that, you break the code structure. If you personally
like
> > to work with flat code - this is your preference.
> > I prefer having clear structure. So if I need to know where is the
pkey
> > manager object defined? What are its internal state variables? What
are
> > the algorithms it uses? Etc I can simply open up osm_pkey_mgr.h and
find
> > this out.
> 
> I would be agree with your last example, but it is not the case - what
> you will actually find in osm_pkey_mgr.h is some object with no
related
> to pkey management fields, but instead with four duplicated from
> somewhere pointers (and you will need to dig the whole tree in order
to
> find from where actually it was copied). Do you call this "clear
> structure"?
[EZ] What is important is not what the manager specific data or
functions are but the fact anybody knows where to find them. So once it
is clear the osm_partition_mgr is described in osm_partition_mgr.h I can
know with one glance that it does not have any special data or function.
If I do not have such an h file to learn about the partition manager
where would I look for that info?

If you keep track of the structure you do not need to dig so much to
find where the manager pointers are defined. You should KNOW they must
be passed to the manager on its initialization at the osm_sm . 

And yes - I call the state where you can know by simple rule what file
to look for some object definition a clear structure. And I call the
state when you can not tell which object should be defined in what file
- a mess.

> 
> > If you want to redesign OpenSM structure to be "simpler" or more
> > "effective" you can propose doing that. But doing it in the salami
way
> > is just going to hurt stability and leave us with no structure at
all.
> > Unless we are willing to re-architect the code in a clean manner and
> > spend the years of development and validation for these changes -
lets
> > keep the code with clear structure.
> 
> So your proposition is to wait years in order to remove unused object?
[EZ] If the pkey manager is not used - yes go ahead and remove it. But
the case is that there is a partition manager so my objection is to
having a manager without an h file.

Regarding code flattening and structure rules violations my position is
to avoid messing one project structure for obscure reasons, killing its
stability and structure. 
> 
> Also note that this cleanup has nothing similar with to re-architect
the
> code, it does not even touch OpenSM architecture.
[EZ] Call it whatever you like - if you continuously going to modify the
structure it is a major re-write which will impact stability. What
regression testing are you running before you posting these patches?