[PATCH] replmd changes to handle single-valued link conflicts
Tim Beale
timbeale at catalyst.net.nz
Sun Oct 1 21:48:02 UTC 2017
Hi,
Thanks for the review Garming. Replies inline below.
Cheers,
Tim
On 29/09/17 12:38, Garming Sam via samba-technical wrote:
> Hi Tim,
>
> - For the function replmd_check_singleval_la_conflict, can you break
> down the comment a bit? It's a large block of text that I'd probably
> just skip over.
>
> - In the commit message for replmd: Fix RMD_VERSION initial value to
> match Windows, can you cite the sub-heading and section numbers for the
> MS-DRSR spec? It's just one more clue that would be useful.
>
> - In replmd: Make replmd_set_la_val() closer to replmd_build_la_val(),
> the assignment of old_dsdb_dn to NULL should have been with the original
> code. Perhaps this patch should occur at the beginning.
>
> - Bug numbers need to be on the later patches
Attached is an updated patch-set with those changes.
Note the last 3 patches are just tidy-ups (i.e. not required for bug
pack-porting), so I left the BUG number off these.
> So if an incoming link is newer, it will always clobber any existing
> links regardless of either being active or inactive?
The existing link will only be clobbered if the received link is active,
i.e.
if (active) {
ret = replmd_check_singleval_la_conflict(module, replmd_private,
If the received link is newer *and* inactive, then it just gets applied
as per normal. So we end up with the existing active link and the newer
inactive link. This seemed to be the Windows behaviour.
> The overall patches seem pretty reasonable, I think some changes could
> have been squashed in a bit more, but I don't think it matters.
Happy to squash patches further, I've left as is for now, until a second
reviewer looks at them.
> I noticed you mentioned in
> https://bugzilla.samba.org/show_bug.cgi?id=13039 that it looks like the
> object GUID is being used rather than the invocation ID, did you check
> if that was the case here? Did we just never bother to implement the
> actual object conflict algorithm at all?
I'm not sure. The current Windows spec for ResolveNameConflict() looks
significantly different to the logic that Samba currently uses. I'm not
sure if the Windows algorithm was revised after the Samba code was
implemented.
The spec suggests that the object GUID is used, rather than the
invocation ID. I haven't confirmed this. I just confirmed that the
RMD_VERSION is not used for resolving object conflicts.
>
> Cheers,
>
> Garming
>
>
> On 28/09/17 17:01, Tim Beale via samba-technical wrote:
>> Hi,
>>
>> The attached patch-set fixes the following bugs:
>> - 13055: Samba replication cannot resolve single-valued link conflicts
>> - 13059: Link attributes on Samba have different initial RMD_VERSION
>> compared to Windows
>>
>> The bulk of the changes are to handle conflicts correctly for
>> single-valued link attributes. It required a bit of refactoring in the
>> replmd code to support this.
>>
>> Patches can also be viewed here:
>> http://git.catalyst.net.nz/gw?p=samba.git;a=shortlog;h=refs/heads/tim-replmd-latest
>>
>>
>> Please let me know if you have any review comments.
>>
>> Thanks,
>> Tim
>>
>
>
-------------- next part --------------
From 2eb036375952ffaa174b58a06247ec00efb88bf2 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 18 Sep 2017 13:47:56 +1200
Subject: [PATCH 01/22] replmd: Refactor adding the backlink in
replmd_process_linked_attribute()
The code to add the backlink is the same in both the 'if' and the 'else'
case, so move it outside the if-else block.
(We're going to rework this block of code quite a bit in order to
support single-value linked attribute conflicts, aka bug #13055).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 34 ++++++++-----------------
1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 2e0f705..60a9c8c 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7301,19 +7301,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
talloc_free(tmp_ctx);
return ret;
}
-
- if (active) {
- /* add the new backlink */
- ret = replmd_add_backlink(module, replmd_private,
- schema,
- msg->dn,
- &guid, true, attr,
- parent);
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
- }
} else {
unsigned offset;
/* get a seq_num for this change */
@@ -7367,17 +7354,18 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
talloc_free(tmp_ctx);
return ret;
}
+ }
- if (active) {
- ret = replmd_add_backlink(module, replmd_private,
- schema,
- msg->dn,
- &guid, true, attr,
- parent);
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
+ /* if the new link is active, then add the new backlink */
+ if (active) {
+ ret = replmd_add_backlink(module, replmd_private,
+ schema,
+ msg->dn,
+ &guid, true, attr,
+ parent);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(tmp_ctx);
+ return ret;
}
}
--
2.7.4
From 608b763ef988c07f21fd65ad081ea00400bc38a8 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 18 Sep 2017 16:33:30 +1200
Subject: [PATCH 02/22] replmd: Refactor logic to check if replicated link is
newer
This is precursor work for supporting single-link conflicts.
Split out the code to check if the link update is newer. It's now safe
to call this from the main codepath. This also means we can combine the 2
calls to get the seqnum into a single common call.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 85 ++++++++++++++-----------
1 file changed, 49 insertions(+), 36 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 60a9c8c..f0c5ab6 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7142,6 +7142,40 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
return ret;
}
+/**
+ * @returns true if the replication linked attribute info is newer than we
+ * already have in our DB
+ * @param pdn the existing linked attribute info in our DB
+ * @param la the new linked attribute info received during replication
+ */
+static bool replmd_link_update_is_newer(struct parsed_dn *pdn,
+ struct drsuapi_DsReplicaLinkedAttribute *la)
+{
+ /* see if this update is newer than what we have already */
+ struct GUID invocation_id = GUID_zero();
+ uint32_t version = 0;
+ uint32_t originating_usn = 0;
+ NTTIME change_time = 0;
+
+ if (pdn == NULL) {
+
+ /* no existing info so update is newer */
+ return true;
+ }
+
+ dsdb_get_extended_dn_guid(pdn->dsdb_dn->dn, &invocation_id, "RMD_INVOCID");
+ dsdb_get_extended_dn_uint32(pdn->dsdb_dn->dn, &version, "RMD_VERSION");
+ dsdb_get_extended_dn_uint32(pdn->dsdb_dn->dn, &originating_usn, "RMD_ORIGINATING_USN");
+ dsdb_get_extended_dn_nttime(pdn->dsdb_dn->dn, &change_time, "RMD_CHANGETIME");
+
+ return replmd_update_is_newer(&invocation_id,
+ &la->meta_data.originating_invocation_id,
+ version,
+ la->meta_data.version,
+ change_time,
+ la->meta_data.originating_change_time);
+}
+
/*
process one linked attribute structure
*/
@@ -7244,40 +7278,24 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
return ret;
}
+ if (!replmd_link_update_is_newer(pdn, la)) {
+ DEBUG(3,("Discarding older DRS linked attribute update to %s on %s from %s\n",
+ old_el->name, ldb_dn_get_linearized(msg->dn),
+ GUID_string(tmp_ctx, &la->meta_data.originating_invocation_id)));
+ talloc_free(tmp_ctx);
+ return LDB_SUCCESS;
+ }
+
+ /* get a seq_num for this change */
+ ret = ldb_sequence_number(ldb, LDB_SEQ_NEXT, &seq_num);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(tmp_ctx);
+ return ret;
+ }
if (pdn != NULL) {
- /* see if this update is newer than what we have already */
- struct GUID invocation_id = GUID_zero();
- uint32_t version = 0;
- uint32_t originating_usn = 0;
- NTTIME change_time = 0;
uint32_t rmd_flags = dsdb_dn_rmd_flags(pdn->dsdb_dn->dn);
- dsdb_get_extended_dn_guid(pdn->dsdb_dn->dn, &invocation_id, "RMD_INVOCID");
- dsdb_get_extended_dn_uint32(pdn->dsdb_dn->dn, &version, "RMD_VERSION");
- dsdb_get_extended_dn_uint32(pdn->dsdb_dn->dn, &originating_usn, "RMD_ORIGINATING_USN");
- dsdb_get_extended_dn_nttime(pdn->dsdb_dn->dn, &change_time, "RMD_CHANGETIME");
-
- if (!replmd_update_is_newer(&invocation_id,
- &la->meta_data.originating_invocation_id,
- version,
- la->meta_data.version,
- change_time,
- la->meta_data.originating_change_time)) {
- DEBUG(3,("Discarding older DRS linked attribute update to %s on %s from %s\n",
- old_el->name, ldb_dn_get_linearized(msg->dn),
- GUID_string(tmp_ctx, &la->meta_data.originating_invocation_id)));
- talloc_free(tmp_ctx);
- return LDB_SUCCESS;
- }
-
- /* get a seq_num for this change */
- ret = ldb_sequence_number(ldb, LDB_SEQ_NEXT, &seq_num);
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
-
if (!(rmd_flags & DSDB_RMD_FLAG_DELETED)) {
/* remove the existing backlink */
ret = replmd_add_backlink(module, replmd_private,
@@ -7303,12 +7321,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
}
} else {
unsigned offset;
- /* get a seq_num for this change */
- ret = ldb_sequence_number(ldb, LDB_SEQ_NEXT, &seq_num);
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
+
/*
* We know where the new one needs to be, from the *next
* pointer into pdn_list.
--
2.7.4
From a1c680eae9a526a984524822cc779daa454261d0 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 18 Sep 2017 16:39:44 +1200
Subject: [PATCH 03/22] replmd: Remove unused originating_usn variable
The previous refactor makes it obvious that we aren't actually using
this variable for anything.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index f0c5ab6..73ec9c8 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7154,7 +7154,6 @@ static bool replmd_link_update_is_newer(struct parsed_dn *pdn,
/* see if this update is newer than what we have already */
struct GUID invocation_id = GUID_zero();
uint32_t version = 0;
- uint32_t originating_usn = 0;
NTTIME change_time = 0;
if (pdn == NULL) {
@@ -7165,7 +7164,6 @@ static bool replmd_link_update_is_newer(struct parsed_dn *pdn,
dsdb_get_extended_dn_guid(pdn->dsdb_dn->dn, &invocation_id, "RMD_INVOCID");
dsdb_get_extended_dn_uint32(pdn->dsdb_dn->dn, &version, "RMD_VERSION");
- dsdb_get_extended_dn_uint32(pdn->dsdb_dn->dn, &originating_usn, "RMD_ORIGINATING_USN");
dsdb_get_extended_dn_nttime(pdn->dsdb_dn->dn, &change_time, "RMD_CHANGETIME");
return replmd_update_is_newer(&invocation_id,
--
2.7.4
From 4c4f7712404f04f12ed5ed767fe85eb29b903d5f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 19 Sep 2017 13:41:02 +1200
Subject: [PATCH 04/22] selftest: Make sure single-link conflict retains the
deleted link
There should only ever be one active value for a single-valued link
attribute. When a conflict occurs the 'losing' value should still be
present, but should be marked as deleted.
This change is just making the test criteria stricter to make sure that
we fix the bug correctly.
Note that the only way to query the deleted link attributes present
is to send a DRS request.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/torture/drs/python/link_conflicts.py | 49 ++++++++++++++++++++++++++--
1 file changed, 47 insertions(+), 2 deletions(-)
diff --git a/source4/torture/drs/python/link_conflicts.py b/source4/torture/drs/python/link_conflicts.py
index 4af3cd3..b666ebf 100644
--- a/source4/torture/drs/python/link_conflicts.py
+++ b/source4/torture/drs/python/link_conflicts.py
@@ -35,7 +35,8 @@ from ldb import SCOPE_BASE
import random
import time
-from samba.dcerpc import drsuapi
+from drs_base import AbstractLink
+from samba.dcerpc import drsuapi, misc
# specifies the order to sync DCs in
DC1_TO_DC2 = 1
@@ -55,6 +56,9 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
"dn": self.ou,
"objectclass": "organizationalUnit"})
+ (self.drs, self.drs_handle) = self._ds_bind(self.dnsname_dc1)
+ (self.drs2, self.drs2_handle) = self._ds_bind(self.dnsname_dc2)
+
# disable replication for the tests so we can control at what point
# the DCs try to replicate
self._disable_inbound_repl(self.dnsname_dc1)
@@ -68,7 +72,7 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
super(DrsReplicaLinkConflictTestCase, self).tearDown()
def get_guid(self, samdb, dn):
- """Returns an object's GUID"""
+ """Returns an object's GUID (in string format)"""
res = samdb.search(base=dn, attrs=["objectGUID"], scope=ldb.SCOPE_BASE)
return self._GUID_string(res[0]['objectGUID'][0])
@@ -137,6 +141,37 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self.assertTrue(val in res2[0][attr],
"%s '%s' not found on DC2" %(attr, val))
+ def _check_replicated_links(self, src_obj_dn, expected_links):
+ """Checks that replication sends back the expected linked attributes"""
+
+ hwm = drsuapi.DsReplicaHighWaterMark()
+ hwm.tmp_highest_usn = 0
+ hwm.reserved_usn = 0
+ hwm.highest_usn = 0
+
+ self._check_replication([src_obj_dn],
+ drsuapi.DRSUAPI_DRS_WRIT_REP,
+ dest_dsa=None,
+ drs_error=drsuapi.DRSUAPI_EXOP_ERR_SUCCESS,
+ nc_dn_str=src_obj_dn,
+ exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ,
+ expected_links=expected_links,
+ highwatermark=hwm)
+
+ # Check DC2 as well
+ self.set_test_ldb_dc(self.ldb_dc2)
+
+ self._check_replication([src_obj_dn],
+ drsuapi.DRSUAPI_DRS_WRIT_REP,
+ dest_dsa=None,
+ drs_error=drsuapi.DRSUAPI_EXOP_ERR_SUCCESS,
+ nc_dn_str=src_obj_dn,
+ exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ,
+ expected_links=expected_links,
+ highwatermark=hwm,
+ drs=self.drs2, drs_handle=self.drs2_handle)
+ self.set_test_ldb_dc(self.ldb_dc1)
+
def _test_conflict_single_valued_link(self, sync_order):
"""
Tests a simple single-value link conflict, i.e. each DC adds a link to
@@ -176,6 +211,16 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self.assertTrue(res1[0]["managedBy"][0] == target2_ou,
"Expected most recent update to win conflict")
+ # we can't query the deleted links over LDAP, but we can check DRS
+ # to make sure the DC kept a copy of the conflicting link
+ link1 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy, 0,
+ misc.GUID(src_guid), misc.GUID(target1_guid))
+ link2 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy,
+ drsuapi.DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE,
+ misc.GUID(src_guid), misc.GUID(target2_guid))
+ self._check_replicated_links(src_ou, [link1, link2])
+
+
def test_conflict_single_valued_link(self):
# repeat the test twice, to give each DC a chance to resolve the conflict
self._test_conflict_single_valued_link(sync_order=DC1_TO_DC2)
--
2.7.4
From 0f9fb1fab3d0739f5999ae8f78c1aa3980763e8a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Sep 2017 13:55:11 +1300
Subject: [PATCH 05/22] selftest: Add test for deleted single-valued link
conflict
Currently we're only testing the case where the links have been modified
independently on 2 different DCs and both the links are active. We also
want to test the case where one link is active and the other is deleted.
Technically, this isn't really a conflict - the links involve different
target DNs, and the end result is still only one active link.
It's still probably worth having these tests to prove that fixing bug
13055 doesn't break anything.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/torture/drs/python/link_conflicts.py | 104 +++++++++++++++++++++++++++
1 file changed, 104 insertions(+)
diff --git a/source4/torture/drs/python/link_conflicts.py b/source4/torture/drs/python/link_conflicts.py
index b666ebf..95e2950 100644
--- a/source4/torture/drs/python/link_conflicts.py
+++ b/source4/torture/drs/python/link_conflicts.py
@@ -541,3 +541,107 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self._test_full_sync_link_conflict(sync_order=DC1_TO_DC2)
self._test_full_sync_link_conflict(sync_order=DC2_TO_DC1)
+ def _test_conflict_single_valued_link_deleted_winner(self, sync_order):
+ """
+ Tests a single-value link conflict where the more-up-to-date link value
+ is deleted.
+ """
+ src_ou = self.unique_dn("OU=src")
+ src_guid = self.add_object(self.ldb_dc1, src_ou)
+ self.sync_DCs()
+
+ # create a unique target on each DC
+ target1_ou = self.unique_dn("OU=target1")
+ target2_ou = self.unique_dn("OU=target2")
+
+ target1_guid = self.add_object(self.ldb_dc1, target1_ou)
+ target2_guid = self.add_object(self.ldb_dc2, target2_ou)
+
+ # add the links for the respective targets, and delete one of the links
+ self.add_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+ self.add_link_attr(self.ldb_dc2, src_ou, "managedBy", target2_ou)
+ self.ensure_unique_timestamp()
+ self.del_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+
+ # sync the 2 DCs
+ self.sync_DCs(sync_order=sync_order)
+
+ res1 = self.ldb_dc1.search(base="<GUID=%s>" % src_guid,
+ scope=SCOPE_BASE, attrs=["managedBy"])
+ res2 = self.ldb_dc2.search(base="<GUID=%s>" % src_guid,
+ scope=SCOPE_BASE, attrs=["managedBy"])
+
+ # Although the more up-to-date link value is deleted, this shouldn't
+ # trump DC1's active link
+ self.assert_attrs_match(res1, res2, "managedBy", 1)
+
+ self.assertTrue(res1[0]["managedBy"][0] == target2_ou,
+ "Expected active link win conflict")
+
+ # we can't query the deleted links over LDAP, but we can check that
+ # the deleted links exist using DRS
+ link1 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy, 0,
+ misc.GUID(src_guid), misc.GUID(target1_guid))
+ link2 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy,
+ drsuapi.DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE,
+ misc.GUID(src_guid), misc.GUID(target2_guid))
+ self._check_replicated_links(src_ou, [link1, link2])
+
+ def test_conflict_single_valued_link_deleted_winner(self):
+ # repeat the test twice, to give each DC a chance to resolve the conflict
+ self._test_conflict_single_valued_link_deleted_winner(sync_order=DC1_TO_DC2)
+ self._test_conflict_single_valued_link_deleted_winner(sync_order=DC2_TO_DC1)
+
+ def _test_conflict_single_valued_link_deleted_loser(self, sync_order):
+ """
+ Tests a single-valued link conflict, where the losing link value is deleted.
+ """
+ src_ou = self.unique_dn("OU=src")
+ src_guid = self.add_object(self.ldb_dc1, src_ou)
+ self.sync_DCs()
+
+ # create a unique target on each DC
+ target1_ou = self.unique_dn("OU=target1")
+ target2_ou = self.unique_dn("OU=target2")
+
+ target1_guid = self.add_object(self.ldb_dc1, target1_ou)
+ target2_guid = self.add_object(self.ldb_dc2, target2_ou)
+
+ # add the links - we want the link to end up deleted on DC2, but active on
+ # DC1. DC1 has the better version and DC2 has the better timestamp - the
+ # better version should win
+ self.add_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+ self.del_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+ self.add_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+ self.ensure_unique_timestamp()
+ self.add_link_attr(self.ldb_dc2, src_ou, "managedBy", target2_ou)
+ self.del_link_attr(self.ldb_dc2, src_ou, "managedBy", target2_ou)
+
+ self.sync_DCs(sync_order=sync_order)
+
+ res1 = self.ldb_dc1.search(base="<GUID=%s>" % src_guid,
+ scope=SCOPE_BASE, attrs=["managedBy"])
+ res2 = self.ldb_dc2.search(base="<GUID=%s>" % src_guid,
+ scope=SCOPE_BASE, attrs=["managedBy"])
+
+ # check the object has only have one occurence of the single-valued
+ # attribute and it matches on both DCs
+ self.assert_attrs_match(res1, res2, "managedBy", 1)
+
+ self.assertTrue(res1[0]["managedBy"][0] == target1_ou,
+ "Expected most recent update to win conflict")
+
+ # we can't query the deleted links over LDAP, but we can check DRS
+ # to make sure the DC kept a copy of the conflicting link
+ link1 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy,
+ drsuapi.DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE,
+ misc.GUID(src_guid), misc.GUID(target1_guid))
+ link2 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy, 0,
+ misc.GUID(src_guid), misc.GUID(target2_guid))
+ self._check_replicated_links(src_ou, [link1, link2])
+
+ def test_conflict_single_valued_link_deleted_loser(self):
+ # repeat the test twice, to give each DC a chance to resolve the conflict
+ self._test_conflict_single_valued_link_deleted_loser(sync_order=DC1_TO_DC2)
+ self._test_conflict_single_valued_link_deleted_loser(sync_order=DC2_TO_DC1)
+
--
2.7.4
From b1c7b2ad5b23d23b9d012c9574b70bbf6810a1b0 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 27 Sep 2017 14:43:53 +1300
Subject: [PATCH 06/22] selftest: Add conflict test where the single-valued
link already exists
As well as testing scenarios where both variants of the link are new, we
should also check the case where the received link already exists on the
DC as an inactive (i.e. previously deleted) link.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
selftest/knownfail.d/link_conflicts | 2 +
source4/torture/drs/python/link_conflicts.py | 59 ++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/selftest/knownfail.d/link_conflicts b/selftest/knownfail.d/link_conflicts
index e0c1ded..2a81b24 100644
--- a/selftest/knownfail.d/link_conflicts
+++ b/selftest/knownfail.d/link_conflicts
@@ -1,4 +1,6 @@
# Currently Samba can't resolve a conflict for a single-valued link attribute
samba4.drs.link_conflicts.python\(vampire_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_single_valued_link\(vampire_dc\)
samba4.drs.link_conflicts.python\(promoted_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_single_valued_link\(promoted_dc\)
+samba4.drs.link_conflicts.python\(vampire_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_existing_single_valued_link\(vampire_dc\)
+samba4.drs.link_conflicts.python\(promoted_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_existing_single_valued_link\(promoted_dc\)
diff --git a/source4/torture/drs/python/link_conflicts.py b/source4/torture/drs/python/link_conflicts.py
index 95e2950..c41d036 100644
--- a/source4/torture/drs/python/link_conflicts.py
+++ b/source4/torture/drs/python/link_conflicts.py
@@ -645,3 +645,62 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self._test_conflict_single_valued_link_deleted_loser(sync_order=DC1_TO_DC2)
self._test_conflict_single_valued_link_deleted_loser(sync_order=DC2_TO_DC1)
+ def _test_conflict_existing_single_valued_link(self, sync_order):
+ """
+ Tests a single-valued link conflict, where the conflicting link value
+ already exists (as inactive) on both DCs.
+ """
+ # create the link objects
+ src_ou = self.unique_dn("OU=src")
+ src_guid = self.add_object(self.ldb_dc1, src_ou)
+
+ target1_ou = self.unique_dn("OU=target1")
+ target2_ou = self.unique_dn("OU=target2")
+ target1_guid = self.add_object(self.ldb_dc1, target1_ou)
+ target2_guid = self.add_object(self.ldb_dc1, target2_ou)
+
+ # add the links, but then delete them
+ self.add_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+ self.del_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+ self.add_link_attr(self.ldb_dc1, src_ou, "managedBy", target2_ou)
+ self.del_link_attr(self.ldb_dc1, src_ou, "managedBy", target2_ou)
+ self.sync_DCs()
+
+ # re-add the links independently on each DC
+ self.add_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+ self.ensure_unique_timestamp()
+ self.add_link_attr(self.ldb_dc2, src_ou, "managedBy", target2_ou)
+
+ # try to sync the 2 DCs (this currently fails)
+ try:
+ self.sync_DCs(sync_order=sync_order)
+ except Exception, e:
+ self.fail("Replication could not resolve link conflict: %s" % e)
+
+ res1 = self.ldb_dc1.search(base="<GUID=%s>" % src_guid,
+ scope=SCOPE_BASE, attrs=["managedBy"])
+ res2 = self.ldb_dc2.search(base="<GUID=%s>" % src_guid,
+ scope=SCOPE_BASE, attrs=["managedBy"])
+
+ # check the object has only have one occurence of the single-valued
+ # attribute and it matches on both DCs
+ self.assert_attrs_match(res1, res2, "managedBy", 1)
+
+ # here we expect DC2 to win because it has the more recent link
+ self.assertTrue(res1[0]["managedBy"][0] == target2_ou,
+ "Expected most recent update to win conflict")
+
+ # we can't query the deleted links over LDAP, but we can check DRS
+ # to make sure the DC kept a copy of the conflicting link
+ link1 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy, 0,
+ misc.GUID(src_guid), misc.GUID(target1_guid))
+ link2 = AbstractLink(drsuapi.DRSUAPI_ATTID_managedBy,
+ drsuapi.DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE,
+ misc.GUID(src_guid), misc.GUID(target2_guid))
+ self._check_replicated_links(src_ou, [link1, link2])
+
+ def test_conflict_existing_single_valued_link(self):
+ # repeat the test twice, to give each DC a chance to resolve the conflict
+ self._test_conflict_existing_single_valued_link(sync_order=DC1_TO_DC2)
+ self._test_conflict_existing_single_valued_link(sync_order=DC2_TO_DC1)
+
--
2.7.4
From ee361987aa614e26a08aadbf4f923fbac450239f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 19 Sep 2017 11:59:58 +1200
Subject: [PATCH 07/22] replmd: Partial fix for single-valued link conflict
This is the first part of the fix for resolving a single-valued link
conflict.
When processing the replication data for a linked attribute, if we don't
find a match for the link target value, check if the link is a
single-valued attribute and it currently has an active link. If so, then
use the active link instead.
This change means we delete the existing active link (and backlink)
before adding the new link. This prevents the failure in the subsequent
dsdb_check_single_valued_link() check that was happening previously
(because the link would end up with 2 active values).
This is only a partial fix. It stops replication from failing completely
if we ever hit this situation (which means the test is no longer
hitting an assertion when replicating). However, ideally the existing
active link should be retained and just marked as deleted (with this
change, the existing link is overwritten completely).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 74 ++++++++++++++++++++++++-
source4/torture/drs/python/link_conflicts.py | 7 +--
2 files changed, 75 insertions(+), 6 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 73ec9c8..2a1069c 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7143,6 +7143,56 @@ static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
}
/**
+ * Finds the current active Parsed-DN value for a single-valued linked
+ * attribute, if one exists.
+ * @param ret_pdn assigned the active Parsed-DN, or NULL if none was found
+ * @returns LDB_SUCCESS (regardless of whether a match was found), unless
+ * an error occurred
+ */
+static int replmd_get_active_singleval_link(struct ldb_module *module,
+ TALLOC_CTX *mem_ctx,
+ struct parsed_dn pdn_list[],
+ unsigned int count,
+ const struct dsdb_attribute *attr,
+ struct parsed_dn **ret_pdn)
+{
+ unsigned int i;
+
+ *ret_pdn = NULL;
+
+ if (!(attr->ldb_schema_attribute->flags & LDB_ATTR_FLAG_SINGLE_VALUE)) {
+
+ /* nothing to do for multi-valued linked attributes */
+ return LDB_SUCCESS;
+ }
+
+ for (i = 0; i < count; i++) {
+ int ret = LDB_SUCCESS;
+ struct parsed_dn *pdn = &pdn_list[i];
+
+ /* skip any inactive links */
+ if (dsdb_dn_is_deleted_val(pdn->v)) {
+ continue;
+ }
+
+ /* we've found an active value for this attribute */
+ *ret_pdn = pdn;
+
+ if (pdn->dsdb_dn == NULL) {
+ struct ldb_context *ldb = ldb_module_get_ctx(module);
+
+ ret = really_parse_trusted_dn(mem_ctx, ldb, pdn,
+ attr->syntax->ldap_oid);
+ }
+
+ return ret;
+ }
+
+ /* no active link found */
+ return LDB_SUCCESS;
+}
+
+/**
* @returns true if the replication linked attribute info is newer than we
* already have in our DB
* @param pdn the existing linked attribute info in our DB
@@ -7276,6 +7326,28 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
return ret;
}
+ if (pdn == NULL && active) {
+
+ /*
+ * check if there's a conflict for single-valued links, i.e.
+ * an active linked attribute already exists, but it has a
+ * different target value
+ */
+ ret = replmd_get_active_singleval_link(module, tmp_ctx, pdn_list,
+ old_el->num_values, attr,
+ &pdn);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(tmp_ctx);
+ return ret;
+ }
+
+ if (pdn != NULL) {
+ DBG_WARNING("Link conflict for %s linked from %s\n",
+ ldb_dn_get_linearized(pdn->dsdb_dn->dn),
+ ldb_dn_get_linearized(msg->dn));
+ }
+ }
+
if (!replmd_link_update_is_newer(pdn, la)) {
DEBUG(3,("Discarding older DRS linked attribute update to %s on %s from %s\n",
old_el->name, ldb_dn_get_linearized(msg->dn),
@@ -7299,7 +7371,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
ret = replmd_add_backlink(module, replmd_private,
schema,
msg->dn,
- &guid, false, attr,
+ &pdn->guid, false, attr,
parent);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
diff --git a/source4/torture/drs/python/link_conflicts.py b/source4/torture/drs/python/link_conflicts.py
index c41d036..036472b 100644
--- a/source4/torture/drs/python/link_conflicts.py
+++ b/source4/torture/drs/python/link_conflicts.py
@@ -193,11 +193,8 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self.ensure_unique_timestamp()
self.add_link_attr(self.ldb_dc2, src_ou, "managedBy", target2_ou)
- # try to sync the 2 DCs (this currently fails)
- try:
- self.sync_DCs(sync_order=sync_order)
- except Exception, e:
- self.fail("Replication could not resolve link conflict: %s" % e)
+ # sync the 2 DCs
+ self.sync_DCs(sync_order=sync_order)
res1 = self.ldb_dc1.search(base="<GUID=%s>" % src_guid,
scope=SCOPE_BASE, attrs=["managedBy"])
--
2.7.4
From 1299ea575e62d521b2af0916011af7be1f25ce7f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 27 Sep 2017 13:44:29 +1300
Subject: [PATCH 08/22] replmd: Handle conflicts for single-valued link
attributes better
If 2 DCs independently set a single-valued linked attribute to differing
values, Samba should be able to resolve this problem when replication
occurs.
If the received information is better, then we want to set the existing
link attribute in our DB as inactive.
If our own information is better, then we still want to add the received
link attribute, but mark it as inactive so that it doesn't clobber our
own link.
This still isn't a complete solution. When we add the received attribute
as inactive, we really should be incrementing the version, updating the
USN, etc. Also this only deals with the case where the received link is
completely new (i.e. a received link conflicting with an existing
inactive link isn't handled).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
selftest/knownfail.d/link_conflicts | 2 -
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 89 +++++++++++++++++++++++--
2 files changed, 82 insertions(+), 9 deletions(-)
diff --git a/selftest/knownfail.d/link_conflicts b/selftest/knownfail.d/link_conflicts
index 2a81b24..a927997 100644
--- a/selftest/knownfail.d/link_conflicts
+++ b/selftest/knownfail.d/link_conflicts
@@ -1,6 +1,4 @@
# Currently Samba can't resolve a conflict for a single-valued link attribute
-samba4.drs.link_conflicts.python\(vampire_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_single_valued_link\(vampire_dc\)
-samba4.drs.link_conflicts.python\(promoted_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_single_valued_link\(promoted_dc\)
samba4.drs.link_conflicts.python\(vampire_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_existing_single_valued_link\(vampire_dc\)
samba4.drs.link_conflicts.python\(promoted_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_existing_single_valued_link\(promoted_dc\)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 2a1069c..80a5cad 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7224,6 +7224,54 @@ static bool replmd_link_update_is_newer(struct parsed_dn *pdn,
la->meta_data.originating_change_time);
}
+/**
+ * Marks an existing linked attribute value as deleted in the DB
+ * @param pdn the parsed-DN of the target-value to delete
+ */
+static int replmd_delete_link_value(struct ldb_module *module,
+ struct replmd_private *replmd_private,
+ TALLOC_CTX *mem_ctx,
+ struct ldb_dn *src_obj_dn,
+ const struct dsdb_schema *schema,
+ const struct dsdb_attribute *attr,
+ uint64_t seq_num,
+ bool is_active,
+ struct GUID *target_guid,
+ struct dsdb_dn *target_dsdb_dn,
+ struct ldb_val *output_val)
+{
+ struct ldb_context *ldb = ldb_module_get_ctx(module);
+ time_t t;
+ NTTIME now;
+ const struct GUID *invocation_id = NULL;
+ int ret;
+
+ t = time(NULL);
+ unix_to_nt_time(&now, t);
+
+ invocation_id = samdb_ntds_invocation_id(ldb);
+ if (invocation_id == NULL) {
+ return LDB_ERR_OPERATIONS_ERROR;
+ }
+
+ /* if the existing link is active, remove its backlink */
+ if (is_active) {
+
+ ret = replmd_add_backlink(module, replmd_private, schema,
+ src_obj_dn, target_guid, false,
+ attr, NULL);
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+ }
+
+ /* mark the existing value as deleted */
+ ret = replmd_update_la_val(mem_ctx, output_val, target_dsdb_dn,
+ target_dsdb_dn, invocation_id, seq_num,
+ seq_num, now, true);
+ return ret;
+}
+
/*
process one linked attribute structure
*/
@@ -7244,6 +7292,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
struct ldb_message_element *old_el;
time_t t = time(NULL);
struct parsed_dn *pdn_list, *pdn, *next;
+ struct parsed_dn *conflict_pdn = NULL;
struct GUID guid = GUID_zero();
bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
bool ignore_link;
@@ -7335,17 +7384,11 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
*/
ret = replmd_get_active_singleval_link(module, tmp_ctx, pdn_list,
old_el->num_values, attr,
- &pdn);
+ &conflict_pdn);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
}
-
- if (pdn != NULL) {
- DBG_WARNING("Link conflict for %s linked from %s\n",
- ldb_dn_get_linearized(pdn->dsdb_dn->dn),
- ldb_dn_get_linearized(msg->dn));
- }
}
if (!replmd_link_update_is_newer(pdn, la)) {
@@ -7363,6 +7406,38 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
return ret;
}
+ /* resolve any single-valued link conflicts */
+ if (conflict_pdn != NULL) {
+
+ DBG_WARNING("Link conflict for %s attribute on %s\n",
+ attr->lDAPDisplayName, ldb_dn_get_linearized(msg->dn));
+
+ if (replmd_link_update_is_newer(conflict_pdn, la)) {
+ DBG_WARNING("Using received value %s, over existing target %s\n",
+ ldb_dn_get_linearized(dsdb_dn->dn),
+ ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn));
+
+ ret = replmd_delete_link_value(module, replmd_private,
+ old_el, msg->dn, schema,
+ attr, seq_num, true,
+ &conflict_pdn->guid,
+ conflict_pdn->dsdb_dn,
+ conflict_pdn->v);
+
+ if (ret != LDB_SUCCESS) {
+ talloc_free(tmp_ctx);
+ return ret;
+ }
+ } else {
+ DBG_WARNING("Using existing target %s, over received value %s\n",
+ ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
+ ldb_dn_get_linearized(dsdb_dn->dn));
+
+ /* don't add the link as active */
+ active = false;
+ }
+ }
+
if (pdn != NULL) {
uint32_t rmd_flags = dsdb_dn_rmd_flags(pdn->dsdb_dn->dn);
--
2.7.4
From 6c035c75769b920a3a123e3fe5b0a08752d84924 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 15:58:16 +1300
Subject: [PATCH 09/22] replmd: Make replmd_set_la_val() closer to
replmd_build_la_val()
These two functions are almost identical. The main difference between
them is the RMD_ADDTIME. replmd_set_la_val() tries to use the
RMD_ADDTIME of the old_dsdb_dn. Whereas replmd_build_la_val() always
uses the time passed in.
Change replmd_set_la_val() so it can accept a NULL old_dsdb_dn (i.e. if
it's a new linked attribute that's being set). If so, it'll end up using
the nttime parameter passed in, same as replmd_build_la_val() does.
Also update replmd_process_linked_attribute (which used to use
replmd_build_la_val()) to now pass in a NULL old_dsdb_dn. There
shouldn't be a difference in behaviour either way, but this exercises
the code change.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 80a5cad..a7ee2d7 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2305,7 +2305,7 @@ static int replmd_set_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb
struct ldb_val iid;
struct ldb_val usnv, local_usnv;
struct ldb_val vers, flagsv;
- const struct ldb_val *old_addtime;
+ const struct ldb_val *old_addtime = NULL;
NTSTATUS status;
int ret;
const char *dnstring;
@@ -2345,7 +2345,10 @@ static int replmd_set_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb
if (ret != LDB_SUCCESS) return ret;
/* get the ADDTIME from the original */
- old_addtime = ldb_dn_get_extended_component(old_dsdb_dn->dn, "RMD_ADDTIME");
+ if (old_dsdb_dn != NULL) {
+ old_addtime = ldb_dn_get_extended_component(old_dsdb_dn->dn,
+ "RMD_ADDTIME");
+ }
if (old_addtime == NULL) {
old_addtime = &tval;
}
--
2.7.4
From ed4053f7f894983f115fb3686db3e744e549fd65 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 16:19:29 +1300
Subject: [PATCH 10/22] replmd: Fix talloc inconsistency in replmd_set_la_val()
All the other talloc_asprintf()s in this function use the mem_ctx, but
for some reason the vstring was using the dsdb_dn->dn. This probably
isn't a big deal, but might have unintentional side-effects.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index a7ee2d7..18fc097 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -2373,7 +2373,7 @@ static int replmd_set_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb
ret = ldb_dn_set_extended_component(dn, "RMD_LOCAL_USN", &local_usnv);
if (ret != LDB_SUCCESS) return ret;
- vstring = talloc_asprintf(dn, "%lu", (unsigned long)version);
+ vstring = talloc_asprintf(mem_ctx, "%lu", (unsigned long)version);
vers = data_blob_string_const(vstring);
ret = ldb_dn_set_extended_component(dn, "RMD_VERSION", &vers);
if (ret != LDB_SUCCESS) return ret;
--
2.7.4
From c8bc4cbe934bf36b7c439c2c638c4d3b6743541b Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 27 Sep 2017 16:23:29 +1300
Subject: [PATCH 11/22] replmd: Use replmd_set_la_val() when adding new links
replmd_set_la_val() and replmd_build_la_val() are almost identical. When
we were processing the replicated link attributes we were calling one
function if the link was new, and a different one if the link existed.
I think we should be able to get away with using replmd_set_la_val() in
both cases.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 39 ++++++++++++-------------
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 18fc097..d37137b 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7300,6 +7300,8 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
bool ignore_link;
enum deletion_state deletion_state = OBJECT_NOT_DELETED;
+ struct dsdb_dn *old_dsdb_dn = NULL;
+ struct ldb_val *val_to_update = NULL;
/*
* get the attribute being modified, the search result for the source object,
@@ -7457,16 +7459,9 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
}
}
- ret = replmd_set_la_val(tmp_ctx, pdn->v, dsdb_dn, pdn->dsdb_dn,
- &la->meta_data.originating_invocation_id,
- la->meta_data.originating_usn, seq_num,
- la->meta_data.originating_change_time,
- la->meta_data.version,
- !active);
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
+ val_to_update = pdn->v;
+ old_dsdb_dn = pdn->dsdb_dn;
+
} else {
unsigned offset;
@@ -7505,16 +7500,20 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
old_el->num_values++;
- ret = replmd_build_la_val(tmp_ctx, &old_el->values[offset], dsdb_dn,
- &la->meta_data.originating_invocation_id,
- la->meta_data.originating_usn, seq_num,
- la->meta_data.originating_change_time,
- la->meta_data.version,
- !active);
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
+ val_to_update = &old_el->values[offset];
+ old_dsdb_dn = NULL;
+ }
+
+ /* set the link attribute's value to the info that was received */
+ ret = replmd_set_la_val(tmp_ctx, val_to_update, dsdb_dn, old_dsdb_dn,
+ &la->meta_data.originating_invocation_id,
+ la->meta_data.originating_usn, seq_num,
+ la->meta_data.originating_change_time,
+ la->meta_data.version,
+ !active);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(tmp_ctx);
+ return ret;
}
/* if the new link is active, then add the new backlink */
--
2.7.4
From b58a61770c2b272928ac24433a8f11078e75056e Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 27 Sep 2017 16:37:59 +1300
Subject: [PATCH 12/22] replmd: Mark link conflicts as inactive correctly
The previous patch to handle link conflicts was simply overriding the
received information and marking the link as deleted. We should be doing
this as a separate operation to make it clear what has happened, and so
that the new (i.e. inactive) link details get replicated out.
This patch changes it so that when a conflict occurs, we immediately
overwrite the received information to mark it as deleted, and to update
the version/USN/timestamp/originating_invocation_id to make it clear
that this is a new change.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 26 +++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index d37137b..4f34793 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7302,6 +7302,7 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
enum deletion_state deletion_state = OBJECT_NOT_DELETED;
struct dsdb_dn *old_dsdb_dn = NULL;
struct ldb_val *val_to_update = NULL;
+ bool add_as_inactive = false;
/*
* get the attribute being modified, the search result for the source object,
@@ -7438,8 +7439,11 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
ldb_dn_get_linearized(dsdb_dn->dn));
- /* don't add the link as active */
- active = false;
+ /*
+ * we want to keep our existing active link and add the
+ * received link as inactive
+ */
+ add_as_inactive = true;
}
}
@@ -7516,8 +7520,22 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
return ret;
}
- /* if the new link is active, then add the new backlink */
- if (active) {
+ if (add_as_inactive) {
+
+ /* Set the new link as inactive/deleted to avoid conflicts */
+ ret = replmd_delete_link_value(module, replmd_private, old_el,
+ msg->dn, schema, attr, seq_num,
+ false, &guid, dsdb_dn,
+ val_to_update);
+
+ if (ret != LDB_SUCCESS) {
+ talloc_free(tmp_ctx);
+ return ret;
+ }
+
+ } else if (active) {
+
+ /* if the new link is active, then add the new backlink */
ret = replmd_add_backlink(module, replmd_private,
schema,
msg->dn,
--
2.7.4
From 5bb706a5310a69067dcf05a02d0d062953612680 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 09:42:14 +1300
Subject: [PATCH 13/22] replmd: Handle single-valued conflicts for an existing
link
Currently the code only handles the case where the received link
attribute is a new link (i.e. pdn == NULL). As well as this, we need to
handle the case where the conflicting link already exists, i.e. it's a
deleted link that has been re-added on another DC.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
selftest/knownfail.d/link_conflicts | 4 ----
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 18 +++++++++++-------
source4/torture/drs/python/link_conflicts.py | 7 ++-----
3 files changed, 13 insertions(+), 16 deletions(-)
delete mode 100644 selftest/knownfail.d/link_conflicts
diff --git a/selftest/knownfail.d/link_conflicts b/selftest/knownfail.d/link_conflicts
deleted file mode 100644
index a927997..0000000
--- a/selftest/knownfail.d/link_conflicts
+++ /dev/null
@@ -1,4 +0,0 @@
-# Currently Samba can't resolve a conflict for a single-valued link attribute
-samba4.drs.link_conflicts.python\(vampire_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_existing_single_valued_link\(vampire_dc\)
-samba4.drs.link_conflicts.python\(promoted_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_conflict_existing_single_valued_link\(promoted_dc\)
-
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 4f34793..7d7ecc8 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7381,20 +7381,24 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
return ret;
}
- if (pdn == NULL && active) {
+ /*
+ * check if there's a conflict for single-valued links, i.e. an active
+ * linked attribute already exists, but it has a different target value
+ */
+ if (active) {
+ struct parsed_dn *active_pdn = NULL;
- /*
- * check if there's a conflict for single-valued links, i.e.
- * an active linked attribute already exists, but it has a
- * different target value
- */
ret = replmd_get_active_singleval_link(module, tmp_ctx, pdn_list,
old_el->num_values, attr,
- &conflict_pdn);
+ &active_pdn);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
}
+
+ if (active_pdn != pdn) {
+ conflict_pdn = active_pdn;
+ }
}
if (!replmd_link_update_is_newer(pdn, la)) {
diff --git a/source4/torture/drs/python/link_conflicts.py b/source4/torture/drs/python/link_conflicts.py
index 036472b..c8b6556 100644
--- a/source4/torture/drs/python/link_conflicts.py
+++ b/source4/torture/drs/python/link_conflicts.py
@@ -668,11 +668,8 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self.ensure_unique_timestamp()
self.add_link_attr(self.ldb_dc2, src_ou, "managedBy", target2_ou)
- # try to sync the 2 DCs (this currently fails)
- try:
- self.sync_DCs(sync_order=sync_order)
- except Exception, e:
- self.fail("Replication could not resolve link conflict: %s" % e)
+ # try to sync the 2 DCs
+ self.sync_DCs(sync_order=sync_order)
res1 = self.ldb_dc1.search(base="<GUID=%s>" % src_guid,
scope=SCOPE_BASE, attrs=["managedBy"])
--
2.7.4
From b5dce691ff09ce062d147d7170d771f0f5d3e914 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 10:22:55 +1300
Subject: [PATCH 14/22] replmd: Move link conflict handling into separate
function
Link conflict handling is a corner-case. The logic in
replmd_process_linked_attribute() is already reasonably busy/complex.
Split out the handling of link conflicts into a separate function so
that it doesn't detract from the core replmd_process_linked_attribute()
logic too much.
This refactor should not alter functionality.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 161 ++++++++++++++++--------
1 file changed, 108 insertions(+), 53 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 7d7ecc8..11275dd 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7275,6 +7275,100 @@ static int replmd_delete_link_value(struct ldb_module *module,
return ret;
}
+/**
+ * Checks for a conflict in single-valued link attributes, and tries to
+ * resolve the problem if possible.
+ *
+ * Single-valued links should only ever have one active value. If we already
+ * have an active link value, and during replication we receive an active link
+ * value for a different target DN, then we need to resolve this inconsistency
+ * and determine which value should be active. If the received info is better/
+ * newer than the existing link attribute, then we need to set our existing
+ * link as deleted. If the received info is worse/older, then we should continue
+ * to add it, but set it as an inactive link.
+ *
+ * Note that this is a corner-case that is unlikely to happen (but if it does
+ * happen, we don't want it to break replication completely).
+ *
+ * @param pdn the parsed DN corresponding to the received link
+ * target (note this is NULL if the link does not already exist in our DB)
+ * @param pdn_list all the source object's Parsed-DNs for this attribute, i.e.
+ * any existing active or inactive values for the attribute in our DB.
+ * @param dsdb_dn the target DN for the received link attribute
+ * @param add_as_inactive gets set to true if the received link is worse than
+ * the existing link - it should still be added, but as an inactive link.
+ */
+static int replmd_check_singleval_la_conflict(struct ldb_module *module,
+ struct replmd_private *replmd_private,
+ TALLOC_CTX *mem_ctx,
+ struct ldb_dn *src_obj_dn,
+ struct drsuapi_DsReplicaLinkedAttribute *la,
+ struct dsdb_dn *dsdb_dn,
+ struct parsed_dn *pdn,
+ struct parsed_dn *pdn_list,
+ struct ldb_message_element *old_el,
+ const struct dsdb_schema *schema,
+ const struct dsdb_attribute *attr,
+ uint64_t seq_num,
+ bool *add_as_inactive)
+{
+ struct parsed_dn *active_pdn = NULL;
+ struct parsed_dn *conflict_pdn = NULL;
+ int ret;
+
+ /*
+ * check if there's a conflict for single-valued links, i.e. an active
+ * linked attribute already exists, but it has a different target value
+ */
+ ret = replmd_get_active_singleval_link(module, mem_ctx, pdn_list,
+ old_el->num_values, attr,
+ &active_pdn);
+
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+
+ if (active_pdn != pdn) {
+ conflict_pdn = active_pdn;
+ }
+
+ /* resolve any single-valued link conflicts */
+ if (conflict_pdn != NULL) {
+
+ DBG_WARNING("Link conflict for %s attribute on %s\n",
+ attr->lDAPDisplayName, ldb_dn_get_linearized(src_obj_dn));
+
+ if (replmd_link_update_is_newer(conflict_pdn, la)) {
+ DBG_WARNING("Using received value %s, over existing target %s\n",
+ ldb_dn_get_linearized(dsdb_dn->dn),
+ ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn));
+
+ ret = replmd_delete_link_value(module, replmd_private,
+ old_el, src_obj_dn, schema,
+ attr, seq_num, true,
+ &conflict_pdn->guid,
+ conflict_pdn->dsdb_dn,
+ conflict_pdn->v);
+
+ if (ret != LDB_SUCCESS) {
+ return ret;
+ }
+ } else {
+ DBG_WARNING("Using existing target %s, over received value %s\n",
+ ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
+ ldb_dn_get_linearized(dsdb_dn->dn));
+
+ /*
+ * we want to keep our existing active link and add the
+ * received link as inactive
+ */
+ *add_as_inactive = true;
+ }
+ }
+
+ return LDB_SUCCESS;
+}
+
/*
process one linked attribute structure
*/
@@ -7295,7 +7389,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
struct ldb_message_element *old_el;
time_t t = time(NULL);
struct parsed_dn *pdn_list, *pdn, *next;
- struct parsed_dn *conflict_pdn = NULL;
struct GUID guid = GUID_zero();
bool active = (la->flags & DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE)?true:false;
bool ignore_link;
@@ -7381,26 +7474,6 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
return ret;
}
- /*
- * check if there's a conflict for single-valued links, i.e. an active
- * linked attribute already exists, but it has a different target value
- */
- if (active) {
- struct parsed_dn *active_pdn = NULL;
-
- ret = replmd_get_active_singleval_link(module, tmp_ctx, pdn_list,
- old_el->num_values, attr,
- &active_pdn);
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
-
- if (active_pdn != pdn) {
- conflict_pdn = active_pdn;
- }
- }
-
if (!replmd_link_update_is_newer(pdn, la)) {
DEBUG(3,("Discarding older DRS linked attribute update to %s on %s from %s\n",
old_el->name, ldb_dn_get_linearized(msg->dn),
@@ -7416,38 +7489,20 @@ static int replmd_process_linked_attribute(struct ldb_module *module,
return ret;
}
- /* resolve any single-valued link conflicts */
- if (conflict_pdn != NULL) {
-
- DBG_WARNING("Link conflict for %s attribute on %s\n",
- attr->lDAPDisplayName, ldb_dn_get_linearized(msg->dn));
-
- if (replmd_link_update_is_newer(conflict_pdn, la)) {
- DBG_WARNING("Using received value %s, over existing target %s\n",
- ldb_dn_get_linearized(dsdb_dn->dn),
- ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn));
-
- ret = replmd_delete_link_value(module, replmd_private,
- old_el, msg->dn, schema,
- attr, seq_num, true,
- &conflict_pdn->guid,
- conflict_pdn->dsdb_dn,
- conflict_pdn->v);
-
- if (ret != LDB_SUCCESS) {
- talloc_free(tmp_ctx);
- return ret;
- }
- } else {
- DBG_WARNING("Using existing target %s, over received value %s\n",
- ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
- ldb_dn_get_linearized(dsdb_dn->dn));
-
- /*
- * we want to keep our existing active link and add the
- * received link as inactive
- */
- add_as_inactive = true;
+ /*
+ * check for single-valued link conflicts, i.e. an active linked
+ * attribute already exists, but it has a different target value
+ */
+ if (active) {
+ ret = replmd_check_singleval_la_conflict(module, replmd_private,
+ tmp_ctx, msg->dn, la,
+ dsdb_dn, pdn, pdn_list,
+ old_el, schema, attr,
+ seq_num,
+ &add_as_inactive);
+ if (ret != LDB_SUCCESS) {
+ talloc_free(tmp_ctx);
+ return ret;
}
}
--
2.7.4
From dd29c3d34f87494a23a766054a7e299685ed37c2 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 12:01:34 +1300
Subject: [PATCH 15/22] replmd: Change replmd_check_singleval_la_conflict()
logic flow
Return immediately if there's no conflict, which reduces nesting.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 54 ++++++++++++-------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 11275dd..f18415a 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7332,38 +7332,38 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
conflict_pdn = active_pdn;
}
- /* resolve any single-valued link conflicts */
- if (conflict_pdn != NULL) {
-
- DBG_WARNING("Link conflict for %s attribute on %s\n",
- attr->lDAPDisplayName, ldb_dn_get_linearized(src_obj_dn));
+ if (conflict_pdn == NULL) {
+ return LDB_SUCCESS;
+ }
- if (replmd_link_update_is_newer(conflict_pdn, la)) {
- DBG_WARNING("Using received value %s, over existing target %s\n",
- ldb_dn_get_linearized(dsdb_dn->dn),
- ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn));
+ /* resolve any single-valued link conflicts */
+ DBG_WARNING("Link conflict for %s attribute on %s\n",
+ attr->lDAPDisplayName, ldb_dn_get_linearized(src_obj_dn));
- ret = replmd_delete_link_value(module, replmd_private,
- old_el, src_obj_dn, schema,
- attr, seq_num, true,
- &conflict_pdn->guid,
- conflict_pdn->dsdb_dn,
- conflict_pdn->v);
+ if (replmd_link_update_is_newer(conflict_pdn, la)) {
+ DBG_WARNING("Using received value %s, over existing target %s\n",
+ ldb_dn_get_linearized(dsdb_dn->dn),
+ ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn));
- if (ret != LDB_SUCCESS) {
- return ret;
- }
- } else {
- DBG_WARNING("Using existing target %s, over received value %s\n",
- ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
- ldb_dn_get_linearized(dsdb_dn->dn));
+ ret = replmd_delete_link_value(module, replmd_private, old_el,
+ src_obj_dn, schema, attr,
+ seq_num, true, &conflict_pdn->guid,
+ conflict_pdn->dsdb_dn,
+ conflict_pdn->v);
- /*
- * we want to keep our existing active link and add the
- * received link as inactive
- */
- *add_as_inactive = true;
+ if (ret != LDB_SUCCESS) {
+ return ret;
}
+ } else {
+ DBG_WARNING("Using existing target %s, over received value %s\n",
+ ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
+ ldb_dn_get_linearized(dsdb_dn->dn));
+
+ /*
+ * we want to keep our existing active link and add the
+ * received link as inactive
+ */
+ *add_as_inactive = true;
}
return LDB_SUCCESS;
--
2.7.4
From 653f1375385fbf2f9e82f7d39b3b91253807c5fe Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 12:10:11 +1300
Subject: [PATCH 16/22] replmd: Small refactor to
replmd_check_singleval_la_conflict()
Now that the code is all in one place we can refactor it to make it
slightly more readable.
- added more code comments
- tweaked the 'no conflict' return logic to try to make what it's checking
for more obvious
- removed conflict_pdn (we can just use active_pdn instead)
- added a placeholder variable and tweaked a parameter name
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13055
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 36 ++++++++++++++-----------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index f18415a..d21ae97 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -7290,7 +7290,7 @@ static int replmd_delete_link_value(struct ldb_module *module,
* Note that this is a corner-case that is unlikely to happen (but if it does
* happen, we don't want it to break replication completely).
*
- * @param pdn the parsed DN corresponding to the received link
+ * @param pdn_being_modified the parsed DN corresponding to the received link
* target (note this is NULL if the link does not already exist in our DB)
* @param pdn_list all the source object's Parsed-DNs for this attribute, i.e.
* any existing active or inactive values for the attribute in our DB.
@@ -7304,7 +7304,7 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
struct ldb_dn *src_obj_dn,
struct drsuapi_DsReplicaLinkedAttribute *la,
struct dsdb_dn *dsdb_dn,
- struct parsed_dn *pdn,
+ struct parsed_dn *pdn_being_modified,
struct parsed_dn *pdn_list,
struct ldb_message_element *old_el,
const struct dsdb_schema *schema,
@@ -7313,7 +7313,7 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
bool *add_as_inactive)
{
struct parsed_dn *active_pdn = NULL;
- struct parsed_dn *conflict_pdn = NULL;
+ bool update_is_newer = false;
int ret;
/*
@@ -7328,35 +7328,41 @@ static int replmd_check_singleval_la_conflict(struct ldb_module *module,
return ret;
}
- if (active_pdn != pdn) {
- conflict_pdn = active_pdn;
- }
-
- if (conflict_pdn == NULL) {
+ /*
+ * If no active value exists (or the received info is for the currently
+ * active value), then no conflict exists
+ */
+ if (active_pdn == NULL || active_pdn == pdn_being_modified) {
return LDB_SUCCESS;
}
- /* resolve any single-valued link conflicts */
DBG_WARNING("Link conflict for %s attribute on %s\n",
attr->lDAPDisplayName, ldb_dn_get_linearized(src_obj_dn));
- if (replmd_link_update_is_newer(conflict_pdn, la)) {
+ /* Work out how to resolve the conflict based on which info is better */
+ update_is_newer = replmd_link_update_is_newer(active_pdn, la);
+
+ if (update_is_newer) {
DBG_WARNING("Using received value %s, over existing target %s\n",
ldb_dn_get_linearized(dsdb_dn->dn),
- ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn));
+ ldb_dn_get_linearized(active_pdn->dsdb_dn->dn));
+ /*
+ * Delete our existing active link. The received info will then
+ * be added (through normal link processing) as the active value
+ */
ret = replmd_delete_link_value(module, replmd_private, old_el,
src_obj_dn, schema, attr,
- seq_num, true, &conflict_pdn->guid,
- conflict_pdn->dsdb_dn,
- conflict_pdn->v);
+ seq_num, true, &active_pdn->guid,
+ active_pdn->dsdb_dn,
+ active_pdn->v);
if (ret != LDB_SUCCESS) {
return ret;
}
} else {
DBG_WARNING("Using existing target %s, over received value %s\n",
- ldb_dn_get_linearized(conflict_pdn->dsdb_dn->dn),
+ ldb_dn_get_linearized(active_pdn->dsdb_dn->dn),
ldb_dn_get_linearized(dsdb_dn->dn));
/*
--
2.7.4
From f28f9757125844a16e52cb1e35e17d6431b13302 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 14:42:08 +1300
Subject: [PATCH 17/22] selftest: Add test for initial link attribute
RMD_VERSION value
While testing link conflicts I noticed that links on Windows start from
a different RMD_VERSION compared to Samba. This adds a simple test to
highlight the problem.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13059
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
selftest/knownfail.d/link_conflicts | 5 ++++
source4/torture/drs/python/link_conflicts.py | 37 ++++++++++++++++++++++++----
2 files changed, 37 insertions(+), 5 deletions(-)
create mode 100644 selftest/knownfail.d/link_conflicts
diff --git a/selftest/knownfail.d/link_conflicts b/selftest/knownfail.d/link_conflicts
new file mode 100644
index 0000000..8c1c94f
--- /dev/null
+++ b/selftest/knownfail.d/link_conflicts
@@ -0,0 +1,5 @@
+# Samba starts its RMD_VERSION from zero. Windows starts from one.
+samba4.drs.link_conflicts.python\(vampire_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_link_attr_version\(vampire_dc\)
+samba4.drs.link_conflicts.python\(promoted_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_link_attr_version\(promoted_dc\)
+
+
diff --git a/source4/torture/drs/python/link_conflicts.py b/source4/torture/drs/python/link_conflicts.py
index c8b6556..6522fb6 100644
--- a/source4/torture/drs/python/link_conflicts.py
+++ b/source4/torture/drs/python/link_conflicts.py
@@ -141,14 +141,16 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self.assertTrue(val in res2[0][attr],
"%s '%s' not found on DC2" %(attr, val))
- def _check_replicated_links(self, src_obj_dn, expected_links):
- """Checks that replication sends back the expected linked attributes"""
-
+ def zero_highwatermark(self):
+ """Returns a zeroed highwatermark so that all DRS data gets returned"""
hwm = drsuapi.DsReplicaHighWaterMark()
hwm.tmp_highest_usn = 0
hwm.reserved_usn = 0
hwm.highest_usn = 0
+ return hwm
+ def _check_replicated_links(self, src_obj_dn, expected_links):
+ """Checks that replication sends back the expected linked attributes"""
self._check_replication([src_obj_dn],
drsuapi.DRSUAPI_DRS_WRIT_REP,
dest_dsa=None,
@@ -156,7 +158,7 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
nc_dn_str=src_obj_dn,
exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ,
expected_links=expected_links,
- highwatermark=hwm)
+ highwatermark=self.zero_highwatermark())
# Check DC2 as well
self.set_test_ldb_dc(self.ldb_dc2)
@@ -168,7 +170,7 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
nc_dn_str=src_obj_dn,
exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ,
expected_links=expected_links,
- highwatermark=hwm,
+ highwatermark=self.zero_highwatermark(),
drs=self.drs2, drs_handle=self.drs2_handle)
self.set_test_ldb_dc(self.ldb_dc1)
@@ -698,3 +700,28 @@ class DrsReplicaLinkConflictTestCase(drs_base.DrsBaseTestCase):
self._test_conflict_existing_single_valued_link(sync_order=DC1_TO_DC2)
self._test_conflict_existing_single_valued_link(sync_order=DC2_TO_DC1)
+ def test_link_attr_version(self):
+ """
+ Checks the link attribute version starts from the correct value
+ """
+ # create some objects and add a link
+ src_ou = self.unique_dn("OU=src")
+ src_guid = self.add_object(self.ldb_dc1, src_ou)
+ target1_ou = self.unique_dn("OU=target1")
+ target1_guid = self.add_object(self.ldb_dc1, target1_ou)
+ self.add_link_attr(self.ldb_dc1, src_ou, "managedBy", target1_ou)
+
+ # get the link info via replication
+ ctr6 = self._get_replication(drsuapi.DRSUAPI_DRS_WRIT_REP,
+ dest_dsa=None,
+ drs_error=drsuapi.DRSUAPI_EXOP_ERR_SUCCESS,
+ exop=drsuapi.DRSUAPI_EXOP_REPL_OBJ,
+ highwatermark=self.zero_highwatermark(),
+ nc_dn_str=src_ou)
+
+ self.assertTrue(ctr6.linked_attributes_count == 1,
+ "DRS didn't return a link")
+ link = ctr6.linked_attributes[0]
+ self.assertTrue(link.meta_data.version == 1,
+ "Link version started from %u, not 1" % link.meta_data.version)
+
--
2.7.4
From d8c0a55b0c74dbabcace9a98c2e21cc2909ca459 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 15:01:21 +1300
Subject: [PATCH 18/22] replmd: Remove static values passed to
replmd_build_la_val()
replmd_build_la_val() is used to populate a new link attribute value
from scratch. The version parameter is always passed in as the initial
value (zero), and deleted is always passed in as false.
For cases (like replication) where we want to set version/deleted to
something other than the defaults, we can use replmd_set_la_val()
instead.
This patch changes these 2 parameters to variables instead.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13059
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index d21ae97..f4cad0d 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -923,7 +923,7 @@ static void replmd_ldb_message_sort(struct ldb_message *msg,
static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb_dn *dsdb_dn,
const struct GUID *invocation_id, uint64_t seq_num,
- uint64_t local_usn, NTTIME nttime, uint32_t version, bool deleted);
+ uint64_t local_usn, NTTIME nttime);
static int parsed_dn_compare(struct parsed_dn *pdn1, struct parsed_dn *pdn2);
@@ -994,7 +994,7 @@ static int replmd_add_fix_la(struct ldb_module *module, TALLOC_CTX *mem_ctx,
}
ret = replmd_build_la_val(el->values, p->v, p->dsdb_dn,
&ac->our_invocation_id,
- ac->seq_num, ac->seq_num, now, 0, false);
+ ac->seq_num, ac->seq_num, now);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
@@ -2138,7 +2138,7 @@ static int get_parsed_dns_trusted(struct ldb_module *module,
*/
static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb_dn *dsdb_dn,
const struct GUID *invocation_id, uint64_t seq_num,
- uint64_t local_usn, NTTIME nttime, uint32_t version, bool deleted)
+ uint64_t local_usn, NTTIME nttime)
{
struct ldb_dn *dn = dsdb_dn->dn;
const char *tstring, *usn_string, *flags_string;
@@ -2150,7 +2150,8 @@ static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct ds
int ret;
const char *dnstring;
char *vstring;
- uint32_t rmd_flags = deleted?DSDB_RMD_FLAG_DELETED:0;
+ uint32_t version = 0;
+ uint32_t rmd_flags = 0;
tstring = talloc_asprintf(mem_ctx, "%llu", (unsigned long long)nttime);
if (!tstring) {
@@ -2600,8 +2601,7 @@ static int replmd_modify_la_add(struct ldb_module *module,
/* Make the new linked attribute ldb_val. */
ret = replmd_build_la_val(new_values, &new_values[num_values],
dns[i].dsdb_dn, invocation_id,
- seq_num, seq_num,
- now, 0, false);
+ seq_num, seq_num, now);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
@@ -3083,8 +3083,7 @@ static int replmd_modify_la_replace(struct ldb_module *module,
new_p->v,
new_p->dsdb_dn,
invocation_id,
- seq_num, seq_num,
- now, 0, false);
+ seq_num, seq_num, now);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
--
2.7.4
From 3838c06cb231720f2699ae5e6a6ed6d295dc9609 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 15:09:34 +1300
Subject: [PATCH 19/22] replmd: Fix RMD_VERSION inital value to match Windows
The initial value for RMD_VERSION is one on Windows. The MS-DRSR spec
states the following in section 5.11 AttributeStamp:
dwVersion: A 32-bit integer. Set to 1 when a value for the attribute is
set for the first time. On each subsequent originating update, if the
current value of dwVersion is less than 0xFFFFFFFF, then increment it
by 1; otherwise set it to 0
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13059
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
selftest/knownfail.d/link_conflicts | 5 -----
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 7 +++++--
2 files changed, 5 insertions(+), 7 deletions(-)
delete mode 100644 selftest/knownfail.d/link_conflicts
diff --git a/selftest/knownfail.d/link_conflicts b/selftest/knownfail.d/link_conflicts
deleted file mode 100644
index 8c1c94f..0000000
--- a/selftest/knownfail.d/link_conflicts
+++ /dev/null
@@ -1,5 +0,0 @@
-# Samba starts its RMD_VERSION from zero. Windows starts from one.
-samba4.drs.link_conflicts.python\(vampire_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_link_attr_version\(vampire_dc\)
-samba4.drs.link_conflicts.python\(promoted_dc\).link_conflicts.DrsReplicaLinkConflictTestCase.test_link_attr_version\(promoted_dc\)
-
-
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index f4cad0d..4737222 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -54,6 +54,9 @@
#undef DBGC_CLASS
#define DBGC_CLASS DBGC_DRS_REPL
+/* the RMD_VERSION for linked attributes starts from 1 */
+#define RMD_VERSION_INITIAL 1
+
/*
* It's 29/12/9999 at 23:59:59 UTC as specified in MS-ADTS 7.1.1.4.2
* Deleted Objects Container
@@ -2150,7 +2153,7 @@ static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct ds
int ret;
const char *dnstring;
char *vstring;
- uint32_t version = 0;
+ uint32_t version = RMD_VERSION_INITIAL;
uint32_t rmd_flags = 0;
tstring = talloc_asprintf(mem_ctx, "%llu", (unsigned long long)nttime);
@@ -2397,7 +2400,7 @@ static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct d
bool deleted)
{
uint32_t old_version;
- uint32_t version = 0;
+ uint32_t version = RMD_VERSION_INITIAL;
NTSTATUS status;
/*
--
2.7.4
From 803a0a5e713a3e2dfbf03621b69477cc9fa613b8 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 15:48:58 +1300
Subject: [PATCH 20/22] replmd: Get rid of duplicated replmd_build_la_val()
code
replmd_build_la_val() and replmd_set_la_val() are pretty much identical.
Keep the replmd_build_la_val() API (as it makes it clearer we're
creating a new linked attribute), but replace the code with a call to
replmd_set_la_val().
This isn't required for any bug fix, but is just a general tidy-up to
avoid code duplication.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 77 +++----------------------
1 file changed, 7 insertions(+), 70 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 4737222..a719d6b 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -122,6 +122,10 @@ static int replmd_check_upgrade_links(struct ldb_context *ldb,
const char *ldap_oid);
static int replmd_verify_linked_attribute(struct replmd_replicated_request *ar,
struct la_entry *la);
+static int replmd_set_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb_dn *dsdb_dn,
+ struct dsdb_dn *old_dsdb_dn, const struct GUID *invocation_id,
+ uint64_t usn, uint64_t local_usn, NTTIME nttime,
+ uint32_t version, bool deleted);
enum urgent_situation {
REPL_URGENT_ON_CREATE = 1,
@@ -2143,76 +2147,9 @@ static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct ds
const struct GUID *invocation_id, uint64_t seq_num,
uint64_t local_usn, NTTIME nttime)
{
- struct ldb_dn *dn = dsdb_dn->dn;
- const char *tstring, *usn_string, *flags_string;
- struct ldb_val tval;
- struct ldb_val iid;
- struct ldb_val usnv, local_usnv;
- struct ldb_val vers, flagsv;
- NTSTATUS status;
- int ret;
- const char *dnstring;
- char *vstring;
- uint32_t version = RMD_VERSION_INITIAL;
- uint32_t rmd_flags = 0;
-
- tstring = talloc_asprintf(mem_ctx, "%llu", (unsigned long long)nttime);
- if (!tstring) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- tval = data_blob_string_const(tstring);
-
- usn_string = talloc_asprintf(mem_ctx, "%llu", (unsigned long long)seq_num);
- if (!usn_string) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- usnv = data_blob_string_const(usn_string);
-
- usn_string = talloc_asprintf(mem_ctx, "%llu", (unsigned long long)local_usn);
- if (!usn_string) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- local_usnv = data_blob_string_const(usn_string);
-
- vstring = talloc_asprintf(mem_ctx, "%lu", (unsigned long)version);
- if (!vstring) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- vers = data_blob_string_const(vstring);
-
- status = GUID_to_ndr_blob(invocation_id, dn, &iid);
- if (!NT_STATUS_IS_OK(status)) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
-
- flags_string = talloc_asprintf(mem_ctx, "%u", rmd_flags);
- if (!flags_string) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- flagsv = data_blob_string_const(flags_string);
-
- ret = ldb_dn_set_extended_component(dn, "RMD_FLAGS", &flagsv);
- if (ret != LDB_SUCCESS) return ret;
- ret = ldb_dn_set_extended_component(dn, "RMD_ADDTIME", &tval);
- if (ret != LDB_SUCCESS) return ret;
- ret = ldb_dn_set_extended_component(dn, "RMD_INVOCID", &iid);
- if (ret != LDB_SUCCESS) return ret;
- ret = ldb_dn_set_extended_component(dn, "RMD_CHANGETIME", &tval);
- if (ret != LDB_SUCCESS) return ret;
- ret = ldb_dn_set_extended_component(dn, "RMD_LOCAL_USN", &local_usnv);
- if (ret != LDB_SUCCESS) return ret;
- ret = ldb_dn_set_extended_component(dn, "RMD_ORIGINATING_USN", &usnv);
- if (ret != LDB_SUCCESS) return ret;
- ret = ldb_dn_set_extended_component(dn, "RMD_VERSION", &vers);
- if (ret != LDB_SUCCESS) return ret;
-
- dnstring = dsdb_dn_get_extended_linearized(mem_ctx, dsdb_dn, 1);
- if (dnstring == NULL) {
- return LDB_ERR_OPERATIONS_ERROR;
- }
- *v = data_blob_string_const(dnstring);
-
- return LDB_SUCCESS;
+ return replmd_set_la_val(mem_ctx, v, dsdb_dn, NULL, invocation_id,
+ seq_num, local_usn, nttime,
+ RMD_VERSION_INITIAL, false);
}
static int replmd_update_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb_dn *dsdb_dn,
--
2.7.4
From 4bb1416015dd7f3fafe2ffe01cad54d3d1f4dc57 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 28 Sep 2017 16:13:05 +1300
Subject: [PATCH 21/22] replmd: Remove unnecessary replmd_build_la_val() param
replmd_build_la_val() is creating a new link attribute. In this case,
the RMD_ORIGINATING_USN and RMD_LOCAL_USN are always going to be the
same thing, so we don't need to pass them in as 2 separate parameters.
This isn't required for any bug fix, but is just a general code
tidy-up.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index a719d6b..1901ee1 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -929,7 +929,7 @@ static void replmd_ldb_message_sort(struct ldb_message *msg,
}
static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb_dn *dsdb_dn,
- const struct GUID *invocation_id, uint64_t seq_num,
+ const struct GUID *invocation_id,
uint64_t local_usn, NTTIME nttime);
static int parsed_dn_compare(struct parsed_dn *pdn1, struct parsed_dn *pdn2);
@@ -1001,7 +1001,7 @@ static int replmd_add_fix_la(struct ldb_module *module, TALLOC_CTX *mem_ctx,
}
ret = replmd_build_la_val(el->values, p->v, p->dsdb_dn,
&ac->our_invocation_id,
- ac->seq_num, ac->seq_num, now);
+ ac->seq_num, now);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
@@ -2143,12 +2143,13 @@ static int get_parsed_dns_trusted(struct ldb_module *module,
RMD_LOCAL_USN = local_usn
RMD_VERSION = version
*/
-static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v, struct dsdb_dn *dsdb_dn,
- const struct GUID *invocation_id, uint64_t seq_num,
+static int replmd_build_la_val(TALLOC_CTX *mem_ctx, struct ldb_val *v,
+ struct dsdb_dn *dsdb_dn,
+ const struct GUID *invocation_id,
uint64_t local_usn, NTTIME nttime)
{
return replmd_set_la_val(mem_ctx, v, dsdb_dn, NULL, invocation_id,
- seq_num, local_usn, nttime,
+ local_usn, local_usn, nttime,
RMD_VERSION_INITIAL, false);
}
@@ -2541,7 +2542,7 @@ static int replmd_modify_la_add(struct ldb_module *module,
/* Make the new linked attribute ldb_val. */
ret = replmd_build_la_val(new_values, &new_values[num_values],
dns[i].dsdb_dn, invocation_id,
- seq_num, seq_num, now);
+ seq_num, now);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
@@ -3023,7 +3024,7 @@ static int replmd_modify_la_replace(struct ldb_module *module,
new_p->v,
new_p->dsdb_dn,
invocation_id,
- seq_num, seq_num, now);
+ seq_num, now);
if (ret != LDB_SUCCESS) {
talloc_free(tmp_ctx);
return ret;
--
2.7.4
From ad727ca7e4ceef1296bfb7615be27332a6af4333 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 27 Sep 2017 11:21:48 +1300
Subject: [PATCH 22/22] selftest: Print link meta-data when developer debugging
is used
For Windows, DRS is the only way to see the RMD_VERSION of a link, or to
tell what inactive links the DC. Add some debug to display this
information. By default, this debug is turned off.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
source4/torture/drs/python/drs_base.py | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/source4/torture/drs/python/drs_base.py b/source4/torture/drs/python/drs_base.py
index c2760e2..10f2e63 100644
--- a/source4/torture/drs/python/drs_base.py
+++ b/source4/torture/drs/python/drs_base.py
@@ -253,10 +253,22 @@ class DrsBaseTestCase(SambaToolCmdTest):
next_object = next_object.next_object
print("Linked Attributes: %d" % ctr6.linked_attributes_count)
- ctr6_links = self._get_ctr6_links(ctr6)
- for link in ctr6_links:
+ for lidx in range(0, ctr6.linked_attributes_count):
+ l = ctr6.linked_attributes[lidx]
+ try:
+ target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3,
+ l.value.blob)
+ except:
+ target = ndr_unpack(drsuapi.DsReplicaObjectIdentifier3Binary,
+ l.value.blob)
+
print("Link Tgt %s... <-- Src %s"
- %(link.targetDN[:25], link.identifier))
+ %(target.dn[:25], l.identifier.guid))
+ state = "Del"
+ if l.flags & drsuapi.DRSUAPI_DS_LINKED_ATTRIBUTE_FLAG_ACTIVE:
+ state = "Act"
+ print(" v%u %s changed %u" %(l.meta_data.version, state,
+ l.meta_data.originating_change_time))
print("HWM: %d" %(ctr6.new_highwatermark.highest_usn))
print("Tmp HWM: %d" %(ctr6.new_highwatermark.tmp_highest_usn))
--
2.7.4
More information about the samba-technical
mailing list