important: Wrong logic (to my mind) in schema_set.c
Matthieu Patou
mat+Informatique.Samba at matws.net
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
b/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
transaction
A transaction is still active in ldb context [0x8ea8190] on
/home/mat/test/private/sam.ldb
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
CN=401,CN=DisplaySpecifiers,CN=Configuration,DC=home,DC=matws,DC=net
Traceback (most recent call last):
File "scripting/bin/upgradeprovision", line 770, in <module>
update_samdb(newpaths,paths,creds,session,names)
File "scripting/bin/upgradeprovision", line 711, in update_samdb
hashSD =
check_diff_name(newpaths,paths,creds,session,str(names.rootdn),names,0)
File "scripting/bin/upgradeprovision", line 554, in check_diff_name
sam_ldb.add(delta,["relax:0"])
_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).
Matthieu.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: log
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091130/4406ac81/attachment.ksh>
More information about the samba-technical
mailing list