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

[Sasha Khapyorsky]

On 09:14 Tue 23 May     , Eitan Zahavi wrote:
> > 
> > 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.

You need to keep the file in order to know that there is no information
in this file? (Am I missing something?)

> If I do not have such an h file to learn about the partition manager
> where would I look for that info?

Is "no file" - "no info" not clear enough?

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

I agree with last paragraph ("AS IS" this may be good "rule" IMO), but
cannot see how it is related to our discussion or to the proposed
patches. The goal of those patches is not to move object definition to
some unclear "secret" place, but to remove the useless object and
associated with it obsolete header file.

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

Great. I am happy that you are agree to not keep unused objects.

> But
> the case is that there is a partition manager so my objection is to
> having a manager without an h file.

But why we need to keep the file without information?

(somehow it reminds the joke about a programmer with two glasses - one
with water and one without water... :))

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

As programmer you should know that such kind of cleanup is safe and
cannot cause instability. And if you are referring such kind of
instability where moving any character in the source code may cause
errors then I would prefer to "shake" the code just in order to find
and fix more "hidden" bugs.

> What
> regression testing are you running before you posting these patches?

I don't have too much serious test facilities, so my generic test case is
simple - I'm trying to keep OpenSM running 24x7 on one of my machines
(I need to stop it from time to time in order to rerun with fresh
executable, to test some specific feature or for 32/64 bit change) under
'osmtest' and 'kill -HUP' in loop.

Sasha