[openib-general] [PATCH 10/10] osm: QoS in OpenSM

[Sasha Khapyorsky]

On 00:36 Thu 01 Feb     , Yevgeny Kliteynik wrote:
> Hi Sasha,
> 
> Sasha Khapyorsky wrote:
> > On 17:33 Tue 30 Jan     , Yevgeny Kliteynik wrote:
> >> Checking PathRecord query for QoS constraints
> >>
> >> The QoS-aware path selection logic is implemented in a
> >> separate function that is called only when QoS in OpenSM
> >> is on. It causes some code duplication, but the idea is
> >> to minimize the changes in the existing logic in OSM.
> >> Eventually, these two function (the old path selection
> >> and the new QoS-aware path selection) will be merged
> >> into a single function.
> > 
> > Please merge __osm_pr_rcv_get_path_parms() and
> > __osm_pr_rcv_get_path_parms_qos() functions into single one - as you
> > stated most code is duplicated there.
> > 
> > In fact __osm_pr_rcv_get_path_parms() is most "changeable" function in
> > SA PR processor, and it is not good idea to make this twice. IMHO it
> > creates more ground for future bugs comparing to the risk of possible
> > impacts to existing functionality.
> 
> As I said, this actually won't be a "merge" - the original function 
> will be removed, and the new function will have a few if() statements
> for cases when QoS in osm is down.

I call this merge, but doesn't matter... Those functions are similar
(after removing some extra identation characters) and have identical
meaning and we don't want to track two versions of the same.

> However, this will be a bunch of new code that is running as part of
> the usual flow, and since this code didn't have enough time to be tested
> before feature freeze, and because we discussed the necessity of implementing
> QoS-aware PathRecord the way that it won't change the usual path (again, 
> until it will be tested), I think that it would be better right now to 
> leave it in two separate functions.

For version control we are using git instead of keeping old and new
function versions in the source files.

> Trust me, I want to get rid of this code duplication much more than you do :)
> And I'll do it as soon as I get to test the new code properly.

Great, so please just do it in this way. Then we will see the patch with
the actual changes instead of untested code which is good just because it
is "off".

Sasha