Validation on upgradeprovision patches needed

Matthieu Patou mat at
Tue Jul 13 14:29:07 MDT 2010

  On 13/07/2010 03:04, Andrew Bartlett wrote:
> On Tue, 2010-07-13 at 02:58 +0400, Matthieu Patou wrote:
>> On 13/07/2010 02:41, Andrew Bartlett wrote:
>>> On Tue, 2010-07-13 at 01:16 +0400, Matthieu Patou wrote:
>>>> Hello Andrews,
>>>> As metze and Jelmer are on vacation,
>>>> can one of you had final look on
>>>> and push it if it's ok.
>>>> I normally addressed the remarks of Matthias and those made by metze on IRC.
>>> These look good, but I hope you won't mind if make some comments:
>>> In s4 dsdb: Use the changereplmetadata control
>>> You do a check for
>>> +               objectclass_el = ldb_msg_find_element(res->msgs[0], "objectClass");
>>> +               if (is_urgent&&   replmd_check_urgent_objectclass(objectclass_el,
>>> +                                                               situation)) {
>>> +                       *is_urgent = true;
>>> +               }
>>> In both arms of the if (rmd_is_provided) statement.
>>> But in any case, if we are offline doing an upgradeprovision, then it
>>> isn't urgent to replicate anything.
>> Well even though this control is used only by upgradeprovision right now
>> it's better to have this just in case ?
>> This came form the fact that I kept in this branch of the test almost
>> the same logic.
> That is quite reasonable.  Perhaps then it would be better to push it
> out of the if() statement?
Well that's not so easy, because before the if we do not have the res 
variable which is the result of the ldb_search.
So pushing it after the if looks like at a first glance like a good 
choice but in the else part (so if we do not have the replace_metadata 
there is this:

                 for (i=0; i<msg->num_elements; i++) {
                         struct ldb_message_element *old_el;
                         old_el = ldb_msg_find_element(res->msgs[0], 
                         ret = replmd_update_rpmd_element(ldb, msg, 
&msg->elements[i], old_el, &omd, schema, seq_num,
our_invocation_id, now);
                         if (ret != LDB_SUCCESS) {
                                 return ret;

                         if (is_urgent && !*is_urgent && (situation == 
                                 *is_urgent = 


So putting

                if (is_urgent&&   replmd_check_urgent_objectclass(objectclass_el,
                                                                situation)) {
                        *is_urgent = true;

After the if would change the logic, so I guess it's better to leave it 
like that.
>>> In s4 upgradeprovision: do not copy RID Set it's automaticaly created by
>>> the RID manager
>>> +    try:
>>> +        if str(reference[0].get("cn"))  == "RID Set":
>>> +            skip = True
>>> Is this the best way to identify the RID Set?
>> obviously objectClass: rIDSet is much better
> Thanks,
So I propose something like this:

          if str(reference[0].get("cn"))  == "RID Set":
-            skip = True
+            for klass in reference[0].get("objectClass"):
+                if str(klass).lower == "ridset":
+                    skip = True

I pushed my upgradeprovision-review branch.

It pass make tests on my computer.

Please push it if you think it's ok.


Matthieu Patou
Samba Team

More information about the samba-technical mailing list