[PATCH] s4:drs Create connection object (nTDSConnection)
tridge at samba.org
tridge at samba.org
Fri Nov 13 17:16:46 MST 2009
Thanks for the patch!
The problem you are getting with the fromServer attribute is because
you are treating it as a binary blob, not a string. When you're trying
to work out how to deal with a particular attribute type it can be
useful to look at how it is dealt with elsewhere in Samba. So for
example you could run this:
git grep ldb_msg_add | grep dn
and you'll see lots of places that add a dn to a message. You'll see
that they treat it as a string.
It is also arguarble that we should add a new helper function in
ldb_msg.c called ldb_msg_add_dn() which takes a 'struct ldb_dn *'
directly. It would look something like this:
int ldb_msg_add_dn(struct ldb_message *msg,
const char *attr_name, struct ldb_dn *dn)
return ldb_msg_add_string(msg, attr_name, ldb_dn_get_linearized(dn));
If you do decide to add this helper, then you could send a separate
patch that makes the other places in Samba that add DNs to also use
After you fix the above in your patch you'll find that it will create
the object successfully. There are some other issues with the patch
that you should fix up however.
1) Each time the kcc runs it re-runs this function. That means it
keeps creating new connection objects, whereas we only want one
connection object per DC that we are replicating with. We also need
to remove connection objects from DCs that we are no longer
replicating with (ie. if the nTDSDSA object of that DC is gone). So
you'll need to do a LDB_SCOPE_ONELEVEL search under our nTDSDSA
object to find all the existing connection objects we have, and then
only add missing ones, plus delete ones that should not be there. So
you'll probably need one function that works out a list of what
connections we should have, then a separate function that applies
that, adding/deleting as needed.
2) you need a lot more error checking. There are lots of functions
in kccsrv_create_connection() that could fail, and you need to
report an appropriate error if they do fail. This is especially the
case for any calls that do searches (such as
dsdb_find_dn_by_guid()). If in doubt, please err on the side of
checking error returns rather than not. When something fails we need
to know what went wrong by looking at the logs.
3) you should probably add a higher level debug message (maybe level
2) when you create a new object or delete one, saying what object
you are creating, and what server it is for. The admin can then see
what happened by looking at the logs.
4) your function kccsrv_create_connection() returns an error, but
the caller never looks at it. You should either make the function
return void, or put the return code into a variable in the caller
and print an appropriate message if it fails.
5) Why are you adding an invocationId ? I don't see one on
nTDSConnection objects on windows, and I don't see it as an
attribute for this class in the schema. (the invocationId you are
adding also looks wrong - GUIDs need to be added as NDR objects).
Once you fix the fromServer problem, you should also now compare the
resulting objects with what windows produces and look into any
other discrepancies. Also make sure you test the case when a
connection should be deleted.
While looking at your patch I also noticed that the invocationId is
not displaying as a nicely formatted GUID in ldbsearch.
> By the way, I'm not sure if I should send this patch as a top-level e-mail
> (like this one) or as a reply to the previous e-mail discussing this
It is good to do it as a new mail, with a [PATCH] prefix just like
you've done. Thanks!
More information about the samba-technical