PEP8: fix E703: statement ends with a semicolon
[nivanova/samba-autobuild/.git] / python / samba / upgrade.py
index e013d2c831aa931fb129901180237ef965c6b3d3..7207835b95c8373cade65e2b200d2cfd5c0aa86d 100644 (file)
@@ -26,7 +26,8 @@ import pwd
 
 from samba import Ldb, registry
 from samba.param import LoadParm
-from samba.provision import provision, FILL_FULL, ProvisioningError, setsysvolacl
+from samba.provision import provision, ProvisioningError, setsysvolacl
+from samba.provision.common import FILL_FULL
 from samba.samba3 import passdb
 from samba.samba3 import param as s3param
 from samba.dcerpc import lsa, samr, security
@@ -59,17 +60,17 @@ def import_sam_policy(samdb, policy, logger):
 
     if 'min password length' in policy:
         m['a01'] = ldb.MessageElement(str(policy['min password length']),
-            ldb.FLAG_MOD_REPLACE, 'minPwdLength')
+                                      ldb.FLAG_MOD_REPLACE, 'minPwdLength')
 
     if 'password history' in policy:
         m['a02'] = ldb.MessageElement(str(policy['password history']),
-            ldb.FLAG_MOD_REPLACE, 'pwdHistoryLength')
+                                      ldb.FLAG_MOD_REPLACE, 'pwdHistoryLength')
 
     if 'minimum password age' in policy:
         min_pw_age_unix = policy['minimum password age']
         min_pw_age_nt = int(-min_pw_age_unix * (1e7))
         m['a03'] = ldb.MessageElement(str(min_pw_age_nt), ldb.FLAG_MOD_REPLACE,
-            'minPwdAge')
+                                      'minPwdAge')
 
     if 'maximum password age' in policy:
         max_pw_age_unix = policy['maximum password age']
@@ -86,16 +87,16 @@ def import_sam_policy(samdb, policy, logger):
         lockout_duration_nt = unix2nttime(lockout_duration_mins * 60)
 
         m['a05'] = ldb.MessageElement(str(lockout_duration_nt),
-            ldb.FLAG_MOD_REPLACE, 'lockoutDuration')
+                                      ldb.FLAG_MOD_REPLACE, 'lockoutDuration')
 
     try:
         samdb.modify(m)
-    except ldb.LdbError, e:
+    except ldb.LdbError as e:
         logger.warn("Could not set account policy, (%s)", str(e))
 
 
 def add_posix_attrs(logger, samdb, sid, name, nisdomain, xid_type, home=None,
-        shell=None, pgid=None):
+                    shell=None, pgid=None):
     """Add posix attributes for the user/group
 
     :param samdb: Samba4 sam.ldb database
@@ -123,11 +124,12 @@ def add_posix_attrs(logger, samdb, sid, name, nisdomain, xid_type, home=None,
             str(nisdomain), ldb.FLAG_MOD_REPLACE, 'msSFU30NisDomain')
 
         samdb.modify(m)
-    except ldb.LdbError, e:
+    except ldb.LdbError as e:
         logger.warn(
             'Could not add posix attrs for AD entry for sid=%s, (%s)',
             str(sid), str(e))
 
+
 def add_ad_posix_idmap_entry(samdb, sid, xid, xid_type, logger):
     """Create idmap entry
 
@@ -153,7 +155,7 @@ def add_ad_posix_idmap_entry(samdb, sid, xid, xid_type, logger):
                 "posixGroup", ldb.FLAG_MOD_ADD, 'objectClass')
 
         samdb.modify(m)
-    except ldb.LdbError, e:
+    except ldb.LdbError as e:
         logger.warn(
             'Could not modify AD idmap entry for sid=%s, id=%s, type=%s (%s)',
             str(sid), str(xid), xid_type, str(e))
@@ -184,19 +186,19 @@ def add_idmap_entry(idmapdb, sid, xid, xid_type, logger):
             m['type'] = ldb.MessageElement(
                 xid_type, ldb.FLAG_MOD_REPLACE, 'type')
             idmapdb.modify(m)
-        except ldb.LdbError, e:
+        except ldb.LdbError as e:
             logger.warn(
                 'Could not modify idmap entry for sid=%s, id=%s, type=%s (%s)',
                 str(sid), str(xid), xid_type, str(e))
     else:
         try:
             idmapdb.add({"dn": "CN=%s" % str(sid),
-                        "cn": str(sid),
-                        "objectClass": "sidMap",
-                        "objectSid": ndr_pack(sid),
-                        "type": xid_type,
-                        "xidNumber": str(xid)})
-        except ldb.LdbError, e:
+                         "cn": str(sid),
+                         "objectClass": "sidMap",
+                         "objectSid": ndr_pack(sid),
+                         "type": xid_type,
+                         "xidNumber": str(xid)})
+        except ldb.LdbError as e:
             logger.warn(
                 'Could not add idmap entry for sid=%s, id=%s, type=%s (%s)',
                 str(sid), str(xid), xid_type, str(e))
@@ -212,7 +214,7 @@ def import_idmap(idmapdb, samba3, logger):
 
     try:
         samba3_idmap = samba3.get_idmap_db()
-    except IOError, e:
+    except IOError as e:
         logger.warn('Cannot open idmap database, Ignoring: %s', str(e))
         return
 
@@ -254,7 +256,8 @@ def add_group_from_mapping_entry(samdb, groupmap, logger):
         msg = samdb.search(
             base='<SID=%s>' % str(groupmap.sid), scope=ldb.SCOPE_BASE)
         found = True
-    except ldb.LdbError, (ecode, emsg):
+    except ldb.LdbError as e1:
+        (ecode, emsg) = e1.args
         if ecode == ldb.ERR_NO_SUCH_OBJECT:
             found = False
         else:
@@ -262,7 +265,7 @@ def add_group_from_mapping_entry(samdb, groupmap, logger):
 
     if found:
         logger.warn('Group already exists sid=%s, groupname=%s existing_groupname=%s, Ignoring.',
-                            str(groupmap.sid), groupmap.nt_name, msg[0]['sAMAccountName'][0])
+                    str(groupmap.sid), groupmap.nt_name, msg[0]['sAMAccountName'][0])
     else:
         if groupmap.sid_name_use == lsa.SID_NAME_WKN_GRP:
             # In a lot of Samba3 databases, aliases are marked as well known groups
@@ -271,26 +274,28 @@ def add_group_from_mapping_entry(samdb, groupmap, logger):
                 return
 
         m = ldb.Message()
-        m.dn = ldb.Dn(samdb, "CN=%s,CN=Users,%s" % (groupmap.nt_name, samdb.get_default_basedn()))
-        m['cn'] = ldb.MessageElement(groupmap.nt_name, ldb.FLAG_MOD_ADD, 'cn')
+        # We avoid using the format string to avoid needing to escape the CN values
+        m.dn = ldb.Dn(samdb, "CN=X,CN=Users")
+        m.dn.set_component(0, "CN", groupmap.nt_name)
+        m.dn.add_base(samdb.get_default_basedn())
         m['objectClass'] = ldb.MessageElement('group', ldb.FLAG_MOD_ADD, 'objectClass')
         m['objectSid'] = ldb.MessageElement(ndr_pack(groupmap.sid), ldb.FLAG_MOD_ADD,
-            'objectSid')
+                                            'objectSid')
         m['sAMAccountName'] = ldb.MessageElement(groupmap.nt_name, ldb.FLAG_MOD_ADD,
-            'sAMAccountName')
+                                                 'sAMAccountName')
 
         if groupmap.comment:
             m['description'] = ldb.MessageElement(groupmap.comment, ldb.FLAG_MOD_ADD,
-                'description')
+                                                  'description')
 
         # Fix up incorrect 'well known' groups that are actually builtin (per test above) to be aliases
         if groupmap.sid_name_use == lsa.SID_NAME_ALIAS or groupmap.sid_name_use == lsa.SID_NAME_WKN_GRP:
             m['groupType'] = ldb.MessageElement(str(dsdb.GTYPE_SECURITY_DOMAIN_LOCAL_GROUP),
-                ldb.FLAG_MOD_ADD, 'groupType')
+                                                ldb.FLAG_MOD_ADD, 'groupType')
 
         try:
             samdb.add(m, controls=["relax:0"])
-        except ldb.LdbError, e:
+        except ldb.LdbError as e:
             logger.warn('Could not add group name=%s (%s)', groupmap.nt_name, str(e))
 
 
@@ -309,7 +314,8 @@ def add_users_to_group(samdb, group, members, logger):
 
         try:
             samdb.modify(m)
-        except ldb.LdbError, (ecode, emsg):
+        except ldb.LdbError as e:
+            (ecode, emsg) = e.args
             if ecode == ldb.ERR_ENTRY_ALREADY_EXISTS:
                 logger.debug("skipped re-adding member '%s' to group '%s': %s", member_sid, group.sid, emsg)
             elif ecode == ldb.ERR_NO_SUCH_OBJECT:
@@ -346,9 +352,9 @@ def import_wins(samba4_winsdb, samba3_winsdb):
                 rType = 0x0
 
         if ttl > time.time():
-            rState = 0x0 # active
+            rState = 0x0  # active
         else:
-            rState = 0x1 # released
+            rState = 0x1  # released
 
         nType = ((nb_flags & 0x60) >> 5)
 
@@ -370,140 +376,6 @@ def import_wins(samba4_winsdb, samba3_winsdb):
                        "maxVersion": str(version_id)})
 
 
-def enable_samba3sam(samdb, ldapurl):
-    """Enable Samba 3 LDAP URL database.
-
-    :param samdb: SAM Database.
-    :param ldapurl: Samba 3 LDAP URL
-    """
-    samdb.modify_ldif("""
-dn: @MODULES
-changetype: modify
-replace: @LIST
-@LIST: samldb,operational,objectguid,rdn_name,samba3sam
-""")
-
-    samdb.add({"dn": "@MAP=samba3sam", "@MAP_URL": ldapurl})
-
-
-smbconf_keep = [
-    "dos charset",
-    "unix charset",
-    "display charset",
-    "comment",
-    "path",
-    "directory",
-    "workgroup",
-    "realm",
-    "netbios name",
-    "netbios aliases",
-    "netbios scope",
-    "server string",
-    "interfaces",
-    "bind interfaces only",
-    "security",
-    "auth methods",
-    "encrypt passwords",
-    "null passwords",
-    "obey pam restrictions",
-    "password server",
-    "smb passwd file",
-    "private dir",
-    "passwd chat",
-    "password level",
-    "lanman auth",
-    "ntlm auth",
-    "client NTLMv2 auth",
-    "client lanman auth",
-    "client plaintext auth",
-    "read only",
-    "hosts allow",
-    "hosts deny",
-    "log level",
-    "debuglevel",
-    "log file",
-    "smb ports",
-    "large readwrite",
-    "max protocol",
-    "min protocol",
-    "unicode",
-    "read raw",
-    "write raw",
-    "disable netbios",
-    "nt status support",
-    "max mux",
-    "max xmit",
-    "name resolve order",
-    "max wins ttl",
-    "min wins ttl",
-    "time server",
-    "unix extensions",
-    "use spnego",
-    "server signing",
-    "client signing",
-    "max connections",
-    "paranoid server security",
-    "socket options",
-    "strict sync",
-    "max print jobs",
-    "printable",
-    "print ok",
-    "printer name",
-    "printer",
-    "map system",
-    "map hidden",
-    "map archive",
-    "preferred master",
-    "prefered master",
-    "local master",
-    "browseable",
-    "browsable",
-    "wins server",
-    "wins support",
-    "csc policy",
-    "strict locking",
-    "preload",
-    "auto services",
-    "lock dir",
-    "lock directory",
-    "pid directory",
-    "socket address",
-    "copy",
-    "include",
-    "available",
-    "volume",
-    "fstype",
-    "panic action",
-    "msdfs root",
-    "host msdfs",
-    "winbind separator"]
-
-
-def upgrade_smbconf(oldconf, mark):
-    """Remove configuration variables not present in Samba4
-
-    :param oldconf: Old configuration structure
-    :param mark: Whether removed configuration variables should be
-        kept in the new configuration as "samba3:<name>"
-    """
-    data = oldconf.data()
-    newconf = LoadParm()
-
-    for s in data:
-        for p in data[s]:
-            keep = False
-            for k in smbconf_keep:
-                if smbconf_keep[k] == p:
-                    keep = True
-                    break
-
-            if keep:
-                newconf.set(s, p, oldconf.get(s, p))
-            elif mark:
-                newconf.set(s, "samba3:" + p, oldconf.get(s, p))
-
-    return newconf
-
 SAMBA3_PREDEF_NAMES = {
         'HKLM': registry.HKEY_LOCAL_MACHINE,
 }
@@ -528,6 +400,7 @@ def import_registry(samba4_registry, samba3_regdb):
         for (value_name, (value_type, value_data)) in samba3_regdb.values(key).items():
             key_handle.set_value(value_name, value_type, value_data)
 
+
 def get_posix_attr_from_ldap_backend(logger, ldb_object, base_dn, user, attr):
     """Get posix attributes from a samba3 ldap backend
     :param ldbs: a list of ldb connection objects
@@ -537,10 +410,10 @@ def get_posix_attr_from_ldap_backend(logger, ldb_object, base_dn, user, attr):
     """
     try:
         msg = ldb_object.search(base_dn, scope=ldb.SCOPE_SUBTREE,
-                        expression=("(&(objectClass=posixAccount)(uid=%s))"
-                        % (user)), attrs=[attr])
-    except ldb.LdbError, e:
-        raise ProvisioningError("Failed to retrieve attribute %s for user %s, the error is: %s", attr, user, e)
+                                expression=("(&(objectClass=posixAccount)(uid=%s))"
+                                            % (user)), attrs=[attr])
+    except ldb.LdbError as e:
+        raise ProvisioningError("Failed to retrieve attribute %s for user %s, the error is: %s" % (attr, user, e))
     else:
         if msg.count <= 1:
             # This will raise KeyError (which is what we want) if there isn't a entry for this user
@@ -551,7 +424,7 @@ def get_posix_attr_from_ldap_backend(logger, ldb_object, base_dn, user, attr):
 
 
 def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
-        useeadb=False, dns_backend=None, use_ntvfs=False):
+                        useeadb=False, dns_backend=None, use_ntvfs=False):
     """Upgrade from samba3 database to samba4 AD database
 
     :param samba3: samba3 object
@@ -571,13 +444,13 @@ def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
     # secrets db
     try:
         secrets_db = samba3.get_secrets_db()
-    except IOError, e:
+    except IOError as e:
         raise ProvisioningError("Could not open '%s', the Samba3 secrets database: %s.  Perhaps you specified the incorrect smb.conf, --testparm or --dbdir option?" % (samba3.privatedir_path("secrets.tdb"), str(e)))
 
     if not domainname:
         domainname = secrets_db.domains()[0]
         logger.warning("No workgroup specified in smb.conf file, assuming '%s'",
-                domainname)
+                       domainname)
 
     if not realm:
         if serverrole == "ROLE_DOMAIN_BDC" or serverrole == "ROLE_DOMAIN_PDC":
@@ -585,7 +458,7 @@ def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
         else:
             realm = domainname.upper()
             logger.warning("No realm specified in smb.conf file, assuming '%s'",
-                    realm)
+                           realm)
 
     # Find machine account and password
     next_rid = 1000
@@ -596,7 +469,7 @@ def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
         machinepass = None
 
     if samba3.lp.get("passdb backend").split(":")[0].strip() == "ldapsam":
-        base_dn =  samba3.lp.get("ldap suffix")
+        base_dn = samba3.lp.get("ldap suffix")
         ldapuser = samba3.lp.get("ldap admin dn")
         ldappass = secrets_db.get_ldap_bind_pw(ldapuser)
         if ldappass is None:
@@ -649,7 +522,7 @@ def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
             try:
                 members = s3db.enum_aliasmem(group.sid)
                 groupmembers[str(group.sid)] = members
-            except passdb.error, e:
+            except passdb.error as e:
                 logger.warn("Ignoring group '%s' %s listed but then not found: %s",
                             group.nt_name, group.sid, e)
                 continue
@@ -657,7 +530,7 @@ def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
             try:
                 members = s3db.enum_group_members(group.sid)
                 groupmembers[str(group.sid)] = members
-            except passdb.error, e:
+            except passdb.error as e:
                 logger.warn("Ignoring group '%s' %s listed but then not found: %s",
                             group.nt_name, group.sid, e)
                 continue
@@ -671,7 +544,7 @@ def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
             try:
                 members = s3db.enum_aliasmem(group.sid)
                 groupmembers[str(group.sid)] = members
-            except passdb.error, e:
+            except passdb.error as e:
                 logger.warn("Ignoring group '%s' %s listed but then not found: %s",
                             group.nt_name, group.sid, e)
                 continue
@@ -697,32 +570,40 @@ def upgrade_from_samba3(samba3, logger, targetdir, session_info=None,
             next_rid = entry['rid'] + 1
 
         user = s3db.getsampwnam(username)
-        acct_type = (user.acct_ctrl & (samr.ACB_NORMAL|samr.ACB_WSTRUST|samr.ACB_SVRTRUST|samr.ACB_DOMTRUST))
-        if (acct_type == samr.ACB_NORMAL or acct_type == samr.ACB_WSTRUST):
-            pass
-
-        elif acct_type == samr.ACB_SVRTRUST:
-            logger.warn("  Demoting BDC account trust for %s, this DC must be elevated to an AD DC using 'samba-tool domain promote'" % username[:-1])
+        acct_type = (user.acct_ctrl & (samr.ACB_NORMAL |samr.ACB_WSTRUST |samr.ACB_SVRTRUST |samr.ACB_DOMTRUST))
+        if acct_type == samr.ACB_SVRTRUST:
+            logger.warn("  Demoting BDC account trust for %s, this DC must be elevated to an AD DC using 'samba-tool domain dcpromo'" % username[:-1])
             user.acct_ctrl = (user.acct_ctrl & ~samr.ACB_SVRTRUST) | samr.ACB_WSTRUST
 
         elif acct_type == samr.ACB_DOMTRUST:
             logger.warn("  Skipping inter-domain trust from domain %s, this trust must be re-created as an AD trust" % username[:-1])
+            continue
+
+        elif acct_type == (samr.ACB_WSTRUST) and username[-1] != '$':
+            logger.warn("  Skipping account %s that has ACB_WSTRUST (W) set but does not end in $.  This account can not have worked, and is probably left over from a misconfiguration." % username)
+            continue
 
-        elif acct_type == (samr.ACB_NORMAL|samr.ACB_WSTRUST) and username[-1] == '$':
+        elif acct_type == (samr.ACB_NORMAL |samr.ACB_WSTRUST) and username[-1] == '$':
             logger.warn("  Fixing account %s which had both ACB_NORMAL (U) and ACB_WSTRUST (W) set.  Account will be marked as ACB_WSTRUST (W), i.e. as a domain member" % username)
             user.acct_ctrl = (user.acct_ctrl & ~samr.ACB_NORMAL)
 
-        elif acct_type == (samr.ACB_NORMAL|samr.ACB_SVRTRUST) and username[-1] == '$':
+        elif acct_type == (samr.ACB_NORMAL |samr.ACB_SVRTRUST) and username[-1] == '$':
             logger.warn("  Fixing account %s which had both ACB_NORMAL (U) and ACB_SVRTRUST (S) set.  Account will be marked as ACB_WSTRUST (S), i.e. as a domain member" % username)
             user.acct_ctrl = (user.acct_ctrl & ~samr.ACB_NORMAL)
 
+        elif acct_type == 0 and username[-1] != '$':
+            user.acct_ctrl = (user.acct_ctrl | samr.ACB_NORMAL)
+
+        elif (acct_type == samr.ACB_NORMAL or acct_type == samr.ACB_WSTRUST):
+            pass
+
         else:
             raise ProvisioningError("""Failed to upgrade due to invalid account %s, account control flags 0x%08X must have exactly one of
 ACB_NORMAL (N, 0x%08X), ACB_WSTRUST (W 0x%08X), ACB_SVRTRUST (S 0x%08X) or ACB_DOMTRUST (D 0x%08X).
 
 Please fix this account before attempting to upgrade again
 """
-                                    % (user.acct_flags, username,
+                                    % (username, user.acct_ctrl,
                                        samr.ACB_NORMAL, samr.ACB_WSTRUST, samr.ACB_SVRTRUST, samr.ACB_DOMTRUST))
 
         userdata[username] = user
@@ -740,14 +621,14 @@ Please fix this account before attempting to upgrade again
             admin_user = username
 
         try:
-            group_memberships = s3db.enum_group_memberships(user);
+            group_memberships = s3db.enum_group_memberships(user)
             for group in group_memberships:
                 if str(group) in groupmembers:
                     if user.user_sid not in groupmembers[str(group)]:
                         groupmembers[str(group)].append(user.user_sid)
                 else:
-                    groupmembers[str(group)] = [user.user_sid];
-        except passdb.error, e:
+                    groupmembers[str(group)] = [user.user_sid]
+        except passdb.error as e:
             logger.warn("Ignoring group memberships of '%s' %s: %s",
                         username, user.user_sid, e)
 
@@ -786,12 +667,12 @@ Please fix this account before attempting to upgrade again
         creds.guess(samba3.lp)
         creds.set_bind_dn(ldapuser)
         creds.set_password(ldappass)
-        urls = samba3.lp.get("passdb backend").split(":",1)[1].strip('"')
+        urls = samba3.lp.get("passdb backend").split(":", 1)[1].strip('"')
         for url in urls.split():
             try:
                 ldb_object = Ldb(url, credentials=creds)
-            except ldb.LdbError, e:
-                logger.warning("Could not open ldb connection to %s, the error message is: %s", url, e)
+            except ldb.LdbError as e:
+                raise ProvisioningError("Could not open ldb connection to %s, the error message is: %s" % (url, e))
             else:
                 break
     logger.info("Exporting posix attributes")
@@ -833,7 +714,7 @@ Please fix this account before attempting to upgrade again
     samba3_winsdb = None
     try:
         samba3_winsdb = samba3.get_wins_db()
-    except IOError, e:
+    except IOError as e:
         logger.warn('Cannot open wins database, Ignoring: %s', str(e))
 
     if not (serverrole == "ROLE_DOMAIN_BDC" or serverrole == "ROLE_DOMAIN_PDC"):
@@ -848,10 +729,10 @@ Please fix this account before attempting to upgrade again
         adminpass = None
 
     # Do full provision
-    result = provision(logger, session_info, None,
+    result = provision(logger, session_info,
                        targetdir=targetdir, realm=realm, domain=domainname,
-                       domainsid=str(domainsid), next_rid=next_rid,
-                       dc_rid=machinerid, adminpass = adminpass,
+                       domainsid=domainsid, next_rid=next_rid,
+                       dc_rid=machinerid, adminpass=adminpass,
                        dom_for_fun_level=dsdb.DS_DOMAIN_FUNCTION_2003,
                        hostname=netbiosname.lower(), machinepass=machinepass,
                        serverrole=serverrole, samdb_fill=FILL_FULL,
@@ -883,14 +764,29 @@ Please fix this account before attempting to upgrade again
     # Connect to samba4 backend
     s4_passdb = passdb.PDB(new_lp_ctx.get("passdb backend"))
 
-    # Export groups to samba4 backend
-    logger.info("Importing groups")
-    for g in grouplist:
-        # Ignore uninitialized groups (gid = -1)
-        if g.gid != -1:
-            add_group_from_mapping_entry(result.samdb, g, logger)
-            add_ad_posix_idmap_entry(result.samdb, g.sid, g.gid, "ID_TYPE_GID", logger)
-            add_posix_attrs(samdb=result.samdb, sid=g.sid, name=g.nt_name, nisdomain=domainname.lower(), xid_type="ID_TYPE_GID", logger=logger)
+    # Start a new transaction (should speed this up a little, due to index churn)
+    result.samdb.transaction_start()
+
+    logger.info("Adding groups")
+    try:
+        # Export groups to samba4 backend
+        logger.info("Importing groups")
+        for g in grouplist:
+            # Ignore uninitialized groups (gid = -1)
+            if g.gid != -1:
+                add_group_from_mapping_entry(result.samdb, g, logger)
+                add_ad_posix_idmap_entry(result.samdb, g.sid, g.gid, "ID_TYPE_GID", logger)
+                add_posix_attrs(samdb=result.samdb, sid=g.sid, name=g.nt_name, nisdomain=domainname.lower(), xid_type="ID_TYPE_GID", logger=logger)
+
+    except:
+        # We need this, so that we do not give even more errors due to not cancelling the transaction
+        result.samdb.transaction_cancel()
+        raise
+
+    logger.info("Committing 'add groups' transaction to disk")
+    result.samdb.transaction_commit()
+
+    logger.info("Adding users")
 
     # Export users to samba4 backend
     logger.info("Importing users")
@@ -914,9 +810,21 @@ Please fix this account before attempting to upgrade again
                 add_posix_attrs(samdb=result.samdb, sid=userdata[username].user_sid, name=username, nisdomain=domainname.lower(), xid_type="ID_TYPE_UID", home=homes[username], shell=shells[username], pgid=pgids[username], logger=logger)
 
     logger.info("Adding users to groups")
-    for g in grouplist:
-        if str(g.sid) in groupmembers:
-            add_users_to_group(result.samdb, g, groupmembers[str(g.sid)], logger)
+    # Start a new transaction (should speed this up a little, due to index churn)
+    result.samdb.transaction_start()
+
+    try:
+        for g in grouplist:
+            if str(g.sid) in groupmembers:
+                add_users_to_group(result.samdb, g, groupmembers[str(g.sid)], logger)
+
+    except:
+        # We need this, so that we do not give even more errors due to not cancelling the transaction
+        result.samdb.transaction_cancel()
+        raise
+
+    logger.info("Committing 'add users to groups' transaction to disk")
+    result.samdb.transaction_commit()
 
     # Set password for administrator
     if admin_user:
@@ -933,9 +841,9 @@ Please fix this account before attempting to upgrade again
 
     if result.server_role == "active directory domain controller":
         setsysvolacl(result.samdb, result.paths.netlogon, result.paths.sysvol,
-                result.paths.root_uid, result.paths.root_gid,
-                security.dom_sid(result.domainsid), result.names.dnsdomain,
-                result.names.domaindn, result.lp, use_ntvfs)
+                     result.paths.root_uid, result.paths.root_gid,
+                     security.dom_sid(result.domainsid), result.names.dnsdomain,
+                     result.names.domaindn, result.lp, use_ntvfs)
 
     # FIXME: import_registry(registry.Registry(), samba3.get_registry())
     # FIXME: shares