On 06/16/2015 11:41 AM, Ludwig Krispenz wrote:
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




This is looking good to me with few comments

 * in ipa_topo_cfg_replica_segment_find, if 'dir=0' or
   'dir=bidirectionnal' the reverse direction is bidirectionnal. Is it
   the expected result ?
 *   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).
 * 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.
 * 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 ?
 * 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 ?


Thanks
thierry
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to