On 06/29/2015 12:47 PM, Martin Basti wrote:
On 17/06/15 11:05, Ludwig Krispenz wrote:
On 06/17/2015 10:35 AM, thierry bordaz wrote:
good point, in preop we cannot rely on schema been checked, need to
add a check.
On 06/17/2015 09:25 AM, Ludwig Krispenz wrote:
thanks for review, see answers inline.
On 06/16/2015 05:17 PM, thierry bordaz wrote:
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.
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 ?
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.
* 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
thanks for your explanations. All of them makes sense and so for me
the patch is valid.
I have a minor question about schema violation. When we add an
entry, in preop we did not yet check the schema.
So ipa_topo_pre_add->ipa_topo_check_segment_is_valid may be called
with an invalid segment entry where some attributes are missing
no :-) one directional segments are a bit confusing. a replication
agreement B-->A can be represented by a segment (A,B,right-left) or
(B,A,left-right). when doing segment_find (A,B,right-left) we are
looking if any segment covers this and teh result could be a segment
Also something that is not clear to.
I have a segment seg=ipa_topo_cfg_replica_segment_find(.., A, B,
SEGMENT_RIGHT_LEFT, ..);. my understanding is that seg->right != 0
and seg->left == 0. is that correct ?
(B,A,left right with seg->left !=0
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.
* 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
no, we don't know if it is a right-left segment. 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
* 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 ?
if it is bidirectional check_reverse is set to 0 and reveres is not
* 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 ?
Hello, what is status of this patch?
Also there are 2 whitespace errors.
Ludwig's PATCH 15 depends on this patch, would be nice to have this
acked, to unblock review.
The fix is good except some sanity checks that Ludwig will add with
Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code