upgrade: Properly cancel/commit transactions in a couple more places.

2 files changed, 136 insertions, 123 deletions

diff --git a/source4/scripting/bin/upgradeprovision b/source4/scripting/bin/upgradeprovision
index 518bba68c7..4ab4c007ed 100755
--- a/source4/scripting/bin/upgradeprovision
+++ b/source4/scripting/bin/upgradeprovision
@@ -328,7 +328,7 @@ def update_secrets(newpaths, paths, creds, session):
     :param creds: credential for the authentification
     :param session: session for connexion"""
 
-    message(SIMPLE,"update secrets.ldb")
+    message(SIMPLE, "update secrets.ldb")
     newsecrets_ldb = Ldb(newpaths.secrets, session_info=session,
         credentials=creds,lp=lp)
     secrets_ldb = Ldb(paths.secrets, session_info=session,
@@ -371,9 +371,9 @@ def update_secrets(newpaths, paths, creds, session):
         delta = secrets_ldb.msg_diff(empty,reference[0])
         for att in hashAttrNotCopied.keys():
             delta.remove(att)
-        message(CHANGE,"Entry %s is missing from secrets.ldb"%reference[0].dn)
+        message(CHANGE, "Entry %s is missing from secrets.ldb"%reference[0].dn)
         for att in delta:
-            message(CHANGE," Adding attribute %s"%att)
+            message(CHANGE, " Adding attribute %s"%att)
         delta.dn = reference[0].dn
         secrets_ldb.add(delta)
 
@@ -385,7 +385,7 @@ def update_secrets(newpaths, paths, creds, session):
             delta.remove(att)
         for att in delta:
             if att == "name":
-                message(CHANGE,"Found attribute name on  %s, must rename the DN "%(current[0].dn))
+                message(CHANGE, "Found attribute name on  %s, must rename the DN "%(current[0].dn))
                 identic_rename(secrets_ldb,reference[0].dn)
             else:
                 delta.remove(att)
@@ -398,7 +398,7 @@ def update_secrets(newpaths, paths, creds, session):
             delta.remove(att)
         for att in delta:
             if att != "dn":
-                message(CHANGE," Adding/Changing attribute %s to %s"%(att,current[0].dn))
+                message(CHANGE, " Adding/Changing attribute %s to %s"%(att,current[0].dn))
 
         delta.dn = current[0].dn
         secrets_ldb.modify(delta)
@@ -417,16 +417,16 @@ def dump_denied_change(dn,att,flagtxt,current,reference):
     if att != "objectSid" :
         i = 0
         for e in range(0,len(current)):
-            message(CHANGE,"old %d : %s"%(i,str(current[e])))
+            message(CHANGE, "old %d : %s"%(i,str(current[e])))
             i+=1
         if reference != None:
             i = 0
             for e in range(0,len(reference)):
-                message(CHANGE,"new %d : %s"%(i,str(reference[e])))
+                message(CHANGE, "new %d : %s"%(i,str(reference[e])))
                 i+=1
     else:
-        message(CHANGE,"old : %s"%str(ndr_unpack( security.dom_sid,current[0])))
-        message(CHANGE,"new : %s"%str(ndr_unpack( security.dom_sid,reference[0])))
+        message(CHANGE, "old : %s"%str(ndr_unpack( security.dom_sid,current[0])))
+        message(CHANGE, "new : %s"%str(ndr_unpack( security.dom_sid,reference[0])))
 
 
 def handle_special_add(sam_ldb,dn,names):
@@ -452,7 +452,7 @@ def handle_special_add(sam_ldb,dn,names):
     if dntoremove != None:
         res = sam_ldb.search(expression="objectClass=*",base=dntoremove, scope=SCOPE_BASE,attrs=["dn"],controls=["search_options:1:2"])
         if len(res) > 0:
-            message(CHANGE,"Existing object %s must be replaced by %s, removing old object"%(dntoremove,str(dn)))
+            message(CHANGE, "Existing object %s must be replaced by %s, removing old object"%(dntoremove,str(dn)))
             sam_ldb.delete(res[0]["dn"])
 
 
@@ -502,11 +502,11 @@ def add_missing_object(newsam_ldb, sam_ldb, dn, names, basedn, hash, index):
     for att in dn_syntax_att:
         depend_on_yet_tobecreated = check_dn_nottobecreated(hash,index,delta.get(str(att)))
         if depend_on_yet_tobecreated != None:
-            message(CHANGE,"Object %s depends on %s in attribute %s, delaying the creation"
+            message(CHANGE, "Object %s depends on %s in attribute %s, delaying the creation"
                             %(str(dn),depend_on_yet_tobecreated,str(att)))
             return False
     delta.dn = dn
-    message(CHANGE,"Object %s will be added"%dn)
+    message(CHANGE, "Object %s will be added"%dn)
     sam_ldb.add(delta,["relax:0"])
     return True
 
@@ -580,14 +580,18 @@ def check_diff_name(newpaths, paths, creds, session, basedn, names, ischema):
     newsam_ldb = Ldb(newpaths.samdb, session_info=session, credentials=creds,lp=lp)
     sam_ldb = Ldb(paths.samdb, session_info=session, credentials=creds,lp=lp, options=["modules:samba_dsdb"])
     sam_ldb.transaction_start()
-    if ischema:
-        reference = newsam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"])
-        current = sam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"])
+    try:
+        if ischema:
+            reference = newsam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"])
+            current = sam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"])
+        else:
+            reference = newsam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"],controls=["search_options:1:2"])
+            current = sam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"],controls=["search_options:1:2"])
+    except:
+        sam_ldb.transaction_cancel()
+        raise
     else:
-        reference = newsam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"],controls=["search_options:1:2"])
-        current = sam_ldb.search(expression="objectClass=*",base=basedn, scope=SCOPE_SUBTREE,attrs=["dn"],controls=["search_options:1:2"])
-
-    sam_ldb.transaction_commit()
+        sam_ldb.transaction_commit()
     # Create a hash for speeding the search of new object
     for i in range(0,len(reference)):
         hash_new[str(reference[i]["dn"]).lower()] = reference[i]["dn"]
@@ -629,58 +633,62 @@ def check_diff_name(newpaths, paths, creds, session, basedn, names, ischema):
         sam_ldb = Ldb(paths.samdb, session_info=session, credentials=creds,lp=lp, options=["modules:samba_dsdb"])
 
     sam_ldb.transaction_start()
-    # XXX: This needs to be wrapped in try/except so we
-    # abort on exceptions.
-    message(SIMPLE,"There are %d missing objects"%(len(listMissing)))
-    add_missing_entries(newsam_ldb,sam_ldb,names,basedn,listMissing)
-    changed = 0
-    for dn in listPresent:
-        reference = newsam_ldb.search(expression="dn=%s"%(str(dn)),base=basedn, scope=SCOPE_SUBTREE,controls=["search_options:1:2"])
-        current = sam_ldb.search(expression="dn=%s"%(str(dn)),base=basedn, scope=SCOPE_SUBTREE,controls=["search_options:1:2"])
-        if ((str(current[0].dn) != str(reference[0].dn)) and (str(current[0].dn).upper() == str(reference[0].dn).upper())):
-            message(CHANGE,"Name are the same but case change, let's rename %s to %s"%(str(current[0].dn),str(reference[0].dn)))
-            identic_rename(sam_ldb,reference[0].dn)
+    try:
+        # XXX: This needs to be wrapped in try/except so we
+        # abort on exceptions.
+        message(SIMPLE, "There are %d missing objects"%(len(listMissing)))
+        add_missing_entries(newsam_ldb,sam_ldb,names,basedn,listMissing)
+        changed = 0
+        for dn in listPresent:
+            reference = newsam_ldb.search(expression="dn=%s"%(str(dn)),base=basedn, scope=SCOPE_SUBTREE,controls=["search_options:1:2"])
             current = sam_ldb.search(expression="dn=%s"%(str(dn)),base=basedn, scope=SCOPE_SUBTREE,controls=["search_options:1:2"])
+            if ((str(current[0].dn) != str(reference[0].dn)) and (str(current[0].dn).upper() == str(reference[0].dn).upper())):
+                message(CHANGE, "Name are the same but case change, let's rename %s to %s"%(str(current[0].dn),str(reference[0].dn)))
+                identic_rename(sam_ldb,reference[0].dn)
+                current = sam_ldb.search(expression="dn=%s"%(str(dn)),base=basedn, scope=SCOPE_SUBTREE,controls=["search_options:1:2"])
 
-        delta = sam_ldb.msg_diff(current[0],reference[0])
-        for att in hashAttrNotCopied.keys():
-            delta.remove(att)
-        for att in backlinked:
-            delta.remove(att)
-        delta.remove("parentGUID")
-        nb = 0
-        
-        for att in delta:
-            msgElt = delta.get(att)
-            if att == "dn":
-                continue
-            if att == "name":
+            delta = sam_ldb.msg_diff(current[0],reference[0])
+            for att in hashAttrNotCopied.keys():
                 delta.remove(att)
-                continue
-            if not handle_security_desc(ischema,att,msgElt,hashallSD,current,reference):
+            for att in backlinked:
                 delta.remove(att)
-                continue
-            if (not hashOverwrittenAtt.has_key(att) or not (hashOverwrittenAtt.get(att)&2^msgElt.flags())):
-                if  hashOverwrittenAtt.has_key(att) and hashOverwrittenAtt.get(att)==never:
+            delta.remove("parentGUID")
+            nb = 0
+            
+            for att in delta:
+                msgElt = delta.get(att)
+                if att == "dn":
+                    continue
+                if att == "name":
                     delta.remove(att)
                     continue
-                if not handle_special_case(att,delta,reference,current,ischema) and msgElt.flags()!=FLAG_MOD_ADD:
-                    if opts.debugchange or opts.debugall:
-                        try:
-                            dump_denied_change(dn,att,messageEltFlagToString(msgElt.flags()),current[0][att],reference[0][att])
-                        except:
-                            # FIXME: Should catch an explicit exception here
-                            dump_denied_change(dn,att,messageEltFlagToString(msgElt.flags()),current[0][att],None)
+                if not handle_security_desc(ischema,att,msgElt,hashallSD,current,reference):
                     delta.remove(att)
-        delta.dn = dn
-        if len(delta.items()) >1:
-            attributes=",".join(delta.keys())
-            message(CHANGE,"%s is different from the reference one, changed attributes: %s"%(dn,attributes))
-            changed = changed + 1
-            sam_ldb.modify(delta)
-
-    sam_ldb.transaction_commit()
-    message(SIMPLE,"There are %d changed objects"%(changed))
+                    continue
+                if (not hashOverwrittenAtt.has_key(att) or not (hashOverwrittenAtt.get(att)&2^msgElt.flags())):
+                    if  hashOverwrittenAtt.has_key(att) and hashOverwrittenAtt.get(att)==never:
+                        delta.remove(att)
+                        continue
+                    if not handle_special_case(att,delta,reference,current,ischema) and msgElt.flags()!=FLAG_MOD_ADD:
+                        if opts.debugchange or opts.debugall:
+                            try:
+                                dump_denied_change(dn,att,messageEltFlagToString(msgElt.flags()),current[0][att],reference[0][att])
+                            except:
+                                # FIXME: Should catch an explicit exception here
+                                dump_denied_change(dn,att,messageEltFlagToString(msgElt.flags()),current[0][att],None)
+                        delta.remove(att)
+            delta.dn = dn
+            if len(delta.items()) >1:
+                attributes=",".join(delta.keys())
+                message(CHANGE, "%s is different from the reference one, changed attributes: %s"%(dn,attributes))
+                changed = changed + 1
+                sam_ldb.modify(delta)
+    except:
+        sam_ldb.transaction_cancel()
+        raise
+    else:
+        sam_ldb.transaction_commit()
+    message(SIMPLE, "There are %d changed objects"%(changed))
     return hashallSD
 
 
@@ -724,57 +732,62 @@ def update_sd(paths, creds, session, names):
 
     sam_ldb = Ldb(paths.samdb, session_info=session, credentials=creds,lp=lp,options=["modules:samba_dsdb"])
     sam_ldb.transaction_start()
-    # First update the SD for the rootdn
-    sam_ldb.set_session_info(session)
-    res = sam_ldb.search(expression="objectClass=*", base=str(names.rootdn), scope=SCOPE_BASE,\
-                         attrs=["dn", "whenCreated"], controls=["search_options:1:2"])
-    delta = Message()
-    delta.dn = Dn(sam_ldb,str(res[0]["dn"]))
-    descr = get_domain_descriptor(names.domainsid)
-    delta["nTSecurityDescriptor"] = MessageElement(descr, FLAG_MOD_REPLACE, "nTSecurityDescriptor")
-    sam_ldb.modify(delta,["recalculate_sd:0"])
-    # Then the config dn
-    res = sam_ldb.search(expression="objectClass=*",base=str(names.configdn), scope=SCOPE_BASE,attrs=["dn","whenCreated"],controls=["search_options:1:2"])
-    delta = Message()
-    delta.dn = Dn(sam_ldb,str(res[0]["dn"]))
-    descr = get_config_descriptor(names.domainsid)
-    delta["nTSecurityDescriptor"] = MessageElement(descr, FLAG_MOD_REPLACE, "nTSecurityDescriptor" )
-    sam_ldb.modify(delta,["recalculate_sd:0"])
-    # Then the schema dn
-    res = sam_ldb.search(expression="objectClass=*",base=str(names.schemadn), scope=SCOPE_BASE,attrs=["dn","whenCreated"],controls=["search_options:1:2"])
-    delta = Message()
-    delta.dn = Dn(sam_ldb,str(res[0]["dn"]))
-    descr = get_schema_descriptor(names.domainsid)
-    delta["nTSecurityDescriptor"] = MessageElement(descr, FLAG_MOD_REPLACE, "nTSecurityDescriptor" )
-    sam_ldb.modify(delta,["recalculate_sd:0"])
-
-    # Then the rest
-    hash = {}
-    res = sam_ldb.search(expression="objectClass=*",base=str(names.rootdn), scope=SCOPE_SUBTREE,attrs=["dn","whenCreated"],controls=["search_options:1:2"])
-    for obj in res:
-        if not (str(obj["dn"]) == str(names.rootdn) or
-            str(obj["dn"]) == str(names.configdn) or \
-            str(obj["dn"]) == str(names.schemadn)):
-            hash[str(obj["dn"])] = obj["whenCreated"]
-
-    listkeys = hash.keys()
-    listkeys.sort(dn_sort)
-
-    for key in listkeys:
-        try:
-            delta = Message()
-            delta.dn = Dn(sam_ldb,key)
-            delta["whenCreated"] = MessageElement(hash[key], FLAG_MOD_REPLACE, "whenCreated" )
-            sam_ldb.modify(delta,["recalculate_sd:0"])
-        except:
-            # XXX: We should always catch an explicit exception.
-            # What could go wrong here?
-            sam_ldb.transaction_cancel()
-            res = sam_ldb.search(expression="objectClass=*", base=str(names.rootdn), scope=SCOPE_SUBTREE,\
-                                 attrs=["dn","nTSecurityDescriptor"], controls=["search_options:1:2"])
-            print "bad stuff" +ndr_unpack(security.descriptor,str(res[0]["nTSecurityDescriptor"])).as_sddl(names.domainsid)
-            return
-    sam_ldb.transaction_commit()
+    try:
+        # First update the SD for the rootdn
+        sam_ldb.set_session_info(session)
+        res = sam_ldb.search(expression="objectClass=*", base=str(names.rootdn), scope=SCOPE_BASE,\
+                             attrs=["dn", "whenCreated"], controls=["search_options:1:2"])
+        delta = Message()
+        delta.dn = Dn(sam_ldb,str(res[0]["dn"]))
+        descr = get_domain_descriptor(names.domainsid)
+        delta["nTSecurityDescriptor"] = MessageElement(descr, FLAG_MOD_REPLACE, "nTSecurityDescriptor")
+        sam_ldb.modify(delta,["recalculate_sd:0"])
+        # Then the config dn
+        res = sam_ldb.search(expression="objectClass=*",base=str(names.configdn), scope=SCOPE_BASE,attrs=["dn","whenCreated"],controls=["search_options:1:2"])
+        delta = Message()
+        delta.dn = Dn(sam_ldb,str(res[0]["dn"]))
+        descr = get_config_descriptor(names.domainsid)
+        delta["nTSecurityDescriptor"] = MessageElement(descr, FLAG_MOD_REPLACE, "nTSecurityDescriptor" )
+        sam_ldb.modify(delta,["recalculate_sd:0"])
+        # Then the schema dn
+        res = sam_ldb.search(expression="objectClass=*",base=str(names.schemadn), scope=SCOPE_BASE,attrs=["dn","whenCreated"],controls=["search_options:1:2"])
+        delta = Message()
+        delta.dn = Dn(sam_ldb,str(res[0]["dn"]))
+        descr = get_schema_descriptor(names.domainsid)
+        delta["nTSecurityDescriptor"] = MessageElement(descr, FLAG_MOD_REPLACE, "nTSecurityDescriptor" )
+        sam_ldb.modify(delta,["recalculate_sd:0"])
+
+        # Then the rest
+        hash = {}
+        res = sam_ldb.search(expression="objectClass=*",base=str(names.rootdn), scope=SCOPE_SUBTREE,attrs=["dn","whenCreated"],controls=["search_options:1:2"])
+        for obj in res:
+            if not (str(obj["dn"]) == str(names.rootdn) or
+                str(obj["dn"]) == str(names.configdn) or \
+                str(obj["dn"]) == str(names.schemadn)):
+                hash[str(obj["dn"])] = obj["whenCreated"]
+
+        listkeys = hash.keys()
+        listkeys.sort(dn_sort)
+
+        for key in listkeys:
+            try:
+                delta = Message()
+                delta.dn = Dn(sam_ldb,key)
+                delta["whenCreated"] = MessageElement(hash[key], FLAG_MOD_REPLACE, "whenCreated" )
+                sam_ldb.modify(delta,["recalculate_sd:0"])
+            except:
+                # XXX: We should always catch an explicit exception.
+                # What could go wrong here?
+                sam_ldb.transaction_cancel()
+                res = sam_ldb.search(expression="objectClass=*", base=str(names.rootdn), scope=SCOPE_SUBTREE,\
+                                     attrs=["dn","nTSecurityDescriptor"], controls=["search_options:1:2"])
+                print "bad stuff" +ndr_unpack(security.descriptor,str(res[0]["nTSecurityDescriptor"])).as_sddl(names.domainsid)
+                return
+    except:
+        sam_ldb.transaction_cancel()
+        raise
+    else:
+        sam_ldb.transaction_commit()
 
 
 def update_basesamdb(newpaths, paths, names):
@@ -784,10 +797,10 @@ def update_basesamdb(newpaths, paths, names):
     :param paths: List of paths for different provision objects from the upgraded provision
     :param names: List of key provision parameters"""
 
-    message(SIMPLE,"Copy samdb")
+    message(SIMPLE, "Copy samdb")
     shutil.copy(newpaths.samdb,paths.samdb)
 
-    message(SIMPLE,"Update partitions filename if needed")
+    message(SIMPLE, "Update partitions filename if needed")
     schemaldb = os.path.join(paths.private_dir, "schema.ldb")
     configldb = os.path.join(paths.private_dir, "configuration.ldb")
     usersldb = os.path.join(paths.private_dir, "users.ldb")
@@ -828,10 +841,10 @@ def update_samdb(newpaths, paths, creds, session, names):
 
     message(SIMPLE, "Doing schema update")
     hashdef = check_diff_name(newpaths,paths,creds,session,str(names.schemadn),names,1)
-    message(SIMPLE,"Done with schema update")
-    message(SIMPLE,"Scanning whole provision for updates and additions")
+    message(SIMPLE, "Done with schema update")
+    message(SIMPLE, "Scanning whole provision for updates and additions")
     hashSD = check_diff_name(newpaths,paths,creds,session,str(names.rootdn),names,0)
-    message(SIMPLE,"Done with scanning")
+    message(SIMPLE, "Done with scanning")
 
 
 def update_machine_account_password(paths, creds, session, names):
@@ -925,12 +938,12 @@ if __name__ == '__main__':
 
     names = find_provision_key_parameters(param, creds, session, paths, smbconf)
     if not sanitychecks(creds,session,names,paths):
-        message(SIMPLE,"Sanity checks for the upgrade fails, checks messages and correct it before rerunning upgradeprovision")
+        message(SIMPLE, "Sanity checks for the upgrade fails, checks messages and correct it before rerunning upgradeprovision")
         sys.exit(1)
     # Let's see them
     print_provision_key_parameters(names)
     # With all this information let's create a fresh new provision used as reference
-    message(SIMPLE,"Creating a reference provision")
+    message(SIMPLE, "Creating a reference provision")
     provisiondir = tempfile.mkdtemp(dir=paths.private_dir, prefix="referenceprovision")
     newprovision(names, setup_dir, creds, session, smbconf, provisiondir, messageprovision)
     # Get file paths of this new provision
diff --git a/source4/scripting/python/samba/upgradehelpers.py b/source4/scripting/python/samba/upgradehelpers.py
index a9b020285e..a9efa9ed5a 100755
--- a/source4/scripting/python/samba/upgradehelpers.py
+++ b/source4/scripting/python/samba/upgradehelpers.py
@@ -173,7 +173,7 @@ def newprovision(names, setup_dir, creds, session, smbconf, provdir, logger):
     :param session: Session object
     :param smbconf: Path to the smb.conf file
     :param provdir: Directory where the provision will be stored
-    :param messagefunc: A function for displaying the message of the provision
+    :param logger: A `Logger`
     """
     if os.path.isdir(provdir):
         shutil.rmtree(provdir)