[PATCH] Do not deleted the deleted objects container, and never write out a 0000-0000... GUID

[Matthieu Patou]

Hi Andrew,

Few remarks :

There is a couple of white space here and there:


Applying: python/drs: Ensure to pass in the local invocationID during 
the domain join
/usr/local/src/samba/.git/rebase-apply/patch:148: trailing whitespace.

warning: 1 line applied after fixing whitespace errors.
Applying: dsdb-repl_meta_data: Check for a NULL invocationID and do not 
proceed
/usr/local/src/samba/.git/rebase-apply/patch:15: trailing whitespace.
         return replmd_replicated_request_werror(ar, 
WERR_DS_DRA_INTERNAL_ERROR);
warning: 1 line applied after fixing whitespace errors.
Applying: dsdb: Refuse to return an all-zero invocationID
/usr/local/src/samba/.git/rebase-apply/patch:15: trailing whitespace.
             DEBUG(0, ("Failed to find our own NTDS Settings 
invocationId in the ldb!\n"));
warning: 1 line applied after fixing whitespace errors.
Applying: dsdb-repl_meta_data: Do not re-delete the Deleted Objects DN 
during replication
Applying: dsdb-repl_meta_data: Make handling of Deleted Objects DN 
clearer in delete
Applying: lib/messaging: Check the GUID type correctly

--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -1302,6 +1302,7 @@ const struct GUID *samdb_ntds_invocation_id(struct ldb_context *ldb)
  	/* see if we have a cached copy */
  	invocation_id = (struct GUID *)ldb_get_opaque(ldb, "cache.invocation_id");
  	if (invocation_id) {
+		SMB_ASSERT(!GUID_all_zero(invocation_id));
  		return invocation_id;
  	}


Do we really want to have an assert here ?
If you have systems in the wild with this, samba will suddenly stop work 
for them.


Also in Patch 4 you moved some code that was in replmd_delete_internals, 
which is fine but can you also move the comments explaining why.
I attached a small variation on your patch to this email.

Apart from this minor remarks, it's ok.
Also I think we should check that when we replicate that we don't have a 
replication coming with NULL invocation id (0000-000...) for every 
single attribute.

Matthieu.


On 09/19/2013 09:30 AM, Andrew Bartlett wrote:
> This patch fixes a significant issue in our AD DC.  Without this patch,
> on initial replication (ie, the join) we would 'delete' the Deleted
> Objects containers, and we would write in an invalid invocationID into
> the replPropertyMetaData.  This broke replication with Windows 2008R2
> domains.
>
> We still need to do two things:
>
> We need to get wintest back as a regular part of our testing.  It would
> have found this bug, if correctly configured and running.
>
> We need to write a unit test for this, at the very least write a
> dbchecker that can find and fix these (so flagging if we ever find such
> an entry in our database), but preferably also confirm the invocationID
> in replPropertyMetaData at 'runtime' somehow.
>
> This (bug #10157) will shortly be marked a blocker on all our 4.x
> releases, it is a serious regression :-(
>
> The primary regression is in the below, but much of the issue goes all
> the way back to the rewrite of the domain join code to use join.py,
> which did not pass in the invocationID.
>
> commit d3aad891c5759f66bd891cb47866d908a0562a8a
> Author: Andrew Bartlett <abartlet at samba.org>
> Date:   Fri May 31 20:01:17 2013 +1000
>
>      dsdb: Prune deleted objects of links and extra attributes of replicated deletes
>      
>      When an object is deleted, the links to be removed are not propogated,
>      you have to watch out for them manually!
>      
>      We do this by calling back into the originating update delete code(ie
>      what is called if you ldb_delete() locally) so that any extra
>      attribute found locally and not on the remote server becomes removed
>      remotely too.
>      
>      We currently do the same with links, but that isn't strictly correct,
>      but for now our getNCChanges server code filters these out, so only
>      the usn is bumped.
>      
>      Andrew Bartlett
>      
>      Signed-off-by: Andrew Bartlett <abartlet at samba.org>
>      Reviewed-by: Stefan Metzmacher <metze at samba.org>
>
> Andrew Bartlett


-- 
Matthieu Patou
Samba Team
http://samba.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-dsdb-repl_meta_data-Do-not-re-delete-the-Deleted-Obj.patch
Type: text/x-diff
Size: 2173 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130919/9b40e40b/attachment.patch>