Review on patches for multi-ip handling

Andrew Tridgell tridge at samba.org
Thu Nov 10 20:30:36 MST 2011


Hi Matthieu,

I've had a look at these patches, and the others in drs_timebased.

The connect_multi_next_socket() changes look good to me. 

It would probably be even clearer to setup an array of structures where
each structure contains an address and port, then just increment an
index into that array. That would avoid the slightly confusing division
needed to work out the right port for a given value of 'next'. I think
however that the code as you have written it is correct, so if you want
to put the patch in as it is then that's fine.

The "s4-librpc: do not limit to the first IP when trying to do a rpc
connection" also looks good to me. I'm surprised we haven't had more
problems with this!

The DRS_CRITICAL_ONLY changes are probably OK, but I am a bit concerned
about how they will affect dbcheck. When we do critical only replication
we end up with DN links that point to non-existant objects. Andrew and I
deliberatly didn't use critical-only initial replication because of this
problem.

With the recent changes to DN link handling this may now be all OK, but
please check that dbcheck doesn't give any errors after a domain join
with these changes.

The DRS_GET_ANC changes and the ldb_msg_find_attr_as_* changes look good
to me. 

The timeout handling changes in getncchanges could be a bit neater I
think. I've pushed an alternative implementation to my drs_timebased
branch for you to take a look at. It adds a flag to the existing
get_nc_changes_build_object() call instead of having a new
function.  (I haven't tested it, sorry)

Cheers, Tridge


More information about the samba-technical mailing list