important: Wrong logic (to my mind) in schema_set.c

Matthieu Patou mat+Informatique.Samba at
Mon Nov 30 01:41:46 MST 2009

On 30/11/2009 02:57, Andrew Bartlett wrote:
> On Mon, 2009-11-30 at 01:07 +0300, Matthieu Patou wrote:
>> On 30/11/2009 00:30, Andrew Bartlett wrote:
>>> On Sun, 2009-11-29 at 17:02 +0300, Matthieu Patou wrote:
>>>> Dear all, I've been trying to upgrade a very old provision (alpha3) I've
>>>> been hit by errors like this:
>>>> _ldb.LdbError: (80, 'schema_load_init: dsdb_set_schema() failed: 32:No
>>>> such object')
>>>> After a careful investigation I am 99% sure that those two changes
>>>> (introduced in changeset 6b05a907) are wrong
>>>> @@ -135,7 +135,7 @@ static int dsdb_schema_set_attributes(struct
>>>> ldb_context *ldb, struct dsdb_schem
>>>>                    mod_msg = ldb_msg_diff(ldb, res->msgs[0], msg);
>>>>                    if (mod_msg->num_elements>   0) {
>>>> -                       ret = ldb_modify(ldb, mod_msg);
>>>> +                       ret = samdb_replace(ldb, mem_ctx, mod_msg);
>>>>                    }
>>>>            }
>>>> @@ -163,7 +163,7 @@ static int dsdb_schema_set_attributes(struct
>>>> ldb_context *ldb, struct dsdb_schem
>>>>                    mod_msg = ldb_msg_diff(ldb, res_idx->msgs[0], msg_idx);
>>>>                    if (mod_msg->num_elements>   0) {
>>>> -                       ret = ldb_modify(ldb, mod_msg);
>>>> +                       ret = samdb_replace(ldb, mem_ctx, mod_msg);
>>>>                    }
>>>> Because the different elements with different flags (add,replace,delete)
>>>> and using samdb_replace will override this flag by using just replace.
>>>> Obviously if the ldb_diff was telling you that you should add the
>>>> attribute 'foo' trying to make ldb modify the 'foo' attribute will not
>>>> be very good.
>>>> Also I'm quite puzzled by the use ldb_msg_diff and the order of
>>>> parameters. Because res->msgs[0] is an message element obtained from a
>>>> search in the LDB database represented by the ldb object.
>>>> So to my mind by doing
>>>> mod_msg = ldb_msg_diff(ldb, res->msgs[0], msg);
>>>> we are comparing res->msgs[0] and msg and using res->msgs[0] as a
>>>> reference, so there is no point trying to apply changes in the ldb
>>>> object as the attributes have already the value contained in the
>>>> different messageElement of the mod_msg object.
>>>> So I think parameters should be swapped it makes just sens from the
>>>> point of view of the comparison and the modification of the ldb but for
>>>> the overall logic I'm much more reserved.
>>> I've looked over the code, and while I agree the samdb_replace seems
>>> wrong, but before we added it, it just didn't work.  The goal of this
>>> routine is to build new @ATTRIBUTE records and @INDEX records, and to
>>> then, if they differ from the DB, replace them in a DB.
>> It's not very clear from what @ATTRIBUTE and @INDEX are built can you
>> explain me ?
> What we do is walk over the schema, and if the schema elements tell us
> that an object is case sensitive, or if it is indexed, then we add it to
> those attributes.
> The idea is then that every time we load the schema, we check that this
> on-disk record (used by ldb before the schema is loaded, and used to
> determine indexing) is in sync with the schema.
>>> The purpose of res->msgs[0] is to give the current DB as a base to the
>>> diff function.
>> Yeah I checked that ldb_msg_diff use the last ldbMessage as the ref, I
>> tried to revert to ldb_modify but it didn't make the trick, as it was
>> proposing  to add attribute to @ATTRIBUTE that were already in the database.
>> I'll try to remake trace so that we can further discuss it.
> Thanks,
So by apply this patch
diff --git a/source4/dsdb/schema/schema_set.c 
index a984c03..46cbede 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -137,3 +137,9 @@ static int dsdb_schema_set_attributes(struct 
ldb_context *ldb, struct dsdb_schem

                 mod_msg = ldb_msg_diff(ldb, res->msgs[0], msg);
                 if (mod_msg->num_elements > 0) {
+                                      int r;
for(r=0;r<mod_msg->num_elements;r++) {
+                                              fprintf(stdout,"att: %s 
flag %d\n",mod_msg->elements[r].name,mod_msg->elements[r].flags);
+                                              }
+                                      ldb_modify(ldb,mod_msg);

I produced the log attached to this email. The lines starting with att 
are produced by the upper patch. It's clear that ldb_msg_diff wants 
stuff to be added and removed to the database (and quite to change them).

Using the ldb_modify instead of samdb_replace will allow to avoid the

_ldb.LdbError: (80, 'schema_load_init: dsdb_set_schema() failed: 32:No 
such object')

But still remain the following messages that are emitted in the function 
guess_names_from_provision when trying to do
     samdb = Ldb(paths.samdb, session_info=session_info,
             credentials=credentials, lp=lp, options=["modules:samba_dsdb"])

ltdb: tdb(/home/mat/test/private/users.ldb): tdb_transaction_commit: no 
A transaction is still active in ldb context [0x8ea8190] on 

Also please note that in function check_diff_name when called with 
isschema to 0

we just open the samdb normaly but under the hood there is one more 
round of modifications in the @ATTRIBUTES and @INDEXES (because just 
before the schema was modified to add the missing classes and 
attributes) and when the script start to insert the first missing object 
I get the following output:

Adding object 

Traceback (most recent call last):
   File "scripting/bin/upgradeprovision", line 770, in <module>
   File "scripting/bin/upgradeprovision", line 711, in update_samdb
     hashSD = 
   File "scripting/bin/upgradeprovision", line 554, in check_diff_name
_ldb.LdbError: (1, 'ltdb modify without transaction')

I worked around this problem by reopening the database before adding 
missing objects but clearly it shouldn't be needed (to my mind).

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: log
URL: <>

More information about the samba-technical mailing list