Samba4 wearing out usn

[Matthias Dieter Wallnöfer]

Hi ekacnet,

first of all I would like to point up the fact that the 
"dcesrv_netr_LogonGetDomainInfo" was designed in an incredibly complex 
manner and at the moment it works for sure not perfect, but acceptable.

The behaviour seems to be that we always have to update (or delete when 
none provided) the old OS values when we perform this call, but as you 
say it should be possible to do it in one replace operation and not in a 
"remove" and afterwards an "add".

I will look into this.

Greets,
Matthias

Matthieu Patou wrote:
>  Hello,
>
> I witnessed a high speed wearout of usn in my production system after 
> some investigation I noticed through an investigation on 
> replPropertyMetaData that's it's operatingSystemVersion and 
> operatingSystemServicePack that are quite often changed when obviously 
> they shouldn't ....
>
>
> I find this code in ./rpc_server/netlogon/dcerpc_netlogon.c :
>
>                 /* Deletes old OS version values */
>                 samdb_msg_add_delete(sam_ctx, mem_ctx, new_msg,
>                         "operatingSystemServicePack");
>                 samdb_msg_add_delete(sam_ctx, mem_ctx, new_msg,
>                         "operatingSystemVersion");
>
>                 if (dsdb_replace(sam_ctx, new_msg, 0) != LDB_SUCCESS) {
>                         DEBUG(3,("Impossible to update samdb: %s\n",
>                                 ldb_errstring(sam_ctx)));
>                 }
>
>                 talloc_free(new_msg);
>
>                 new_msg = ldb_msg_new(mem_ctx);
>                 NT_STATUS_HAVE_NO_MEMORY(new_msg);
>
>                 new_msg->dn = workstation_dn;
>
>                 /* Sets the OS name */
>                 samdb_msg_set_string(sam_ctx, mem_ctx, new_msg,
>                         "operatingSystem",
>                         r->in.query->workstation_info->os_name.string);
>
>                 /*
>                  * Sets informations from "os_version". On a empty 
> structure
>                  * the values are cleared.
>                  */
>                 if (r->in.query->workstation_info->os_version.os != 
> NULL) {
>                         os_version = 
> &r->in.query->workstation_info->os_version.os->os;
>
>                         samdb_msg_set_string(sam_ctx, mem_ctx, new_msg,
>                                              
> "operatingSystemServicePack",
>                                              os_version->CSDVersion);
>
>                         samdb_msg_set_string(sam_ctx, mem_ctx, new_msg,
>                                 "operatingSystemVersion",
>                                 talloc_asprintf(mem_ctx, "%d.%d (%d)",
>                                         os_version->MajorVersion,
>                                         os_version->MinorVersion,
>                                         os_version->BuildNumber
>                                 )
>                         );
>                 }
>
> I'm pretty sure that the pb comes from the fact that the code has a 
> simplistic logic and do remove/add instead of replace (or add the 
> first time).
>
> Mathias any reason for this to be done that way ?
> Could we change this ?
>
> Cheers.
> Matthieu.
>