<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hi,<br>
    thanks for review, see answers inline.<br>
    <br>
    <div class="moz-cite-prefix">On 06/16/2015 05:17 PM, thierry bordaz
      wrote:<br>
    </div>
    <blockquote cite="mid:55803DFF.7040805@redhat.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      <div class="moz-cite-prefix">On 06/16/2015 11:41 AM, Ludwig
        Krispenz wrote:<br>
      </div>
      <blockquote cite="mid:557FEF38.9000700@redhat.com" type="cite">this

        patch adresses issues in checking existing segments for one
        directional segments and correctly handles the merging of
        segments, so that all agreements will be removed when the merged
        segment is deleted <br>
        <br>
        <fieldset class="mimeAttachmentHeader"></fieldset>
        <br>
      </blockquote>
      <br>
      <font face="Times New Roman, Times, serif">This is looking good to
        me with few comments<br>
        <br>
      </font>
      <ul>
        <li><font face="Times New Roman, Times, serif">in
            ipa_topo_cfg_replica_segment_find, if 'dir=0' or
            'dir=bidirectionnal' the reverse direction is
            bidirectionnal. Is it the expected result ?</font></li>
      </ul>
    </blockquote>
    <font face="Times New Roman, Times, serif">yes. 0 does not exist as
      valid direct and if we are looking for (A,B,both) this could als
      be expressed as (B,A,both). we do not really look for a opposite
      direction of (A,B,dir) but for a segment (B,A,revdir) which covers
      this segment.</font><br>
    <blockquote cite="mid:55803DFF.7040805@redhat.com" type="cite">
      <ul>
        <li><font face="Times New Roman, Times, serif"> in
            ipa_topo_check_segment_is_valid and
            ipa_topo_util_find_segment, may be hardening
            leftnode,rightnode,dir if they are NULL. (if the entry
            violate schema).</font></li>
      </ul>
    </blockquote>
    <font face="Times New Roman, Times, serif">if we can arrive at a
      state where an entry violates the schema I think we have more
      trouble, I want to avoid adding code for handling errors which
      cannot exist.</font><br>
    <blockquote cite="mid:55803DFF.7040805@redhat.com" type="cite">
      <ul>
        <li><font face="Times New Roman, Times, serif">ipa_topo_util_segm_dir

            if direction does not match any of the strings, it returns
            -1. 0 would be better if we decide to test bit mask.</font></li>
      </ul>
    </blockquote>
    <font face="Times New Roman, Times, serif">yes, but in preop we
      check that only valid directions are added, so it might be
      unnecesarry to handle it, but if you want I can change it.</font><br>
    <blockquote cite="mid:55803DFF.7040805@redhat.com" type="cite">
      <ul>
        <li><font face="Times New Roman, Times, serif">in
            ipa_topo_util_segment_update:810, ex_segm is a rigth_left
            segment. Why trying to call ipa_topo_cfg_agmt_dup with
            ex_segm->left in priority. Why not ex_segm->right
            first ? <br>
          </font></li>
      </ul>
    </blockquote>
    <font face="Times New Roman, Times, serif">no, we don't know if it
      is a right-left segment. </font>we have (A,B,left-right), the
    segment for the other direction could be (A.B,right-left) or
    (B,A,left-right). All we know is that it is not bidirectional,
    otherwise (A,B,left-right) would have been rejected in the preop
    test. So there is one agmt, left or right and take the existing one.<br>
    <blockquote cite="mid:55803DFF.7040805@redhat.com" type="cite">
      <ul>
        <li><font face="Times New Roman, Times, serif"> </font><br>
        </li>
        <li><font face="Times New Roman, Times, serif">in
            ipa_topo_util_delete_segments_for_host, If segment
            localhost->delhost is bidirectional, how can it exists a
            reverse segment delhost->localhost ? I thought those
            segments have been merged ?</font></li>
      </ul>
    </blockquote>
    <font face="Times New Roman, Times, serif">if it is bidirectional
      check_reverse is set to 0 and reveres is not attempted</font><br>
    <blockquote cite="mid:55803DFF.7040805@redhat.com" type="cite">
      <ul>
      </ul>
      <font face="Times New Roman, Times, serif"><br>
        Thanks<br>
        thierry </font><br>
    </blockquote>
    <br>
  </body>
</html>