Unify usage of sysdb transactions

Removing bad examples of usage of sysdb_transaction_start/commit/end
functions and making it more consistent (all files except of
src/db/sysdb_*.c).

6 files changed, 93 insertions, 36 deletions

diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c
index 3460b8cc6..e65356d58 100644
--- a/src/providers/ldap/ldap_id_cleanup.c
+++ b/src/providers/ldap/ldap_id_cleanup.c
@@ -192,6 +192,7 @@ struct tevent_req *ldap_id_cleanup_send(TALLOC_CTX *memctx,
 
     ret = sysdb_transaction_start(state->ctx->be->sysdb);
     if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto fail;
     }
     in_transaction = true;
@@ -209,8 +210,10 @@ struct tevent_req *ldap_id_cleanup_send(TALLOC_CTX *memctx,
 
     ret = sysdb_transaction_commit(state->ctx->be->sysdb);
     if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
         goto fail;
     }
+    in_transaction = false;
 
     tevent_req_done(req);
     tevent_req_post(req, ev);
diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 1c651c1a8..ac5057e8c 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -522,10 +522,12 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
     char *usn_value;
     bool twopass;
     int ret;
+    errno_t sret;
     int i;
     struct sysdb_attrs **saved_groups = NULL;
     int nsaved_groups = 0;
     time_t now;
+    bool in_transaction = false;
 
     switch (opts->schema_type) {
     case SDAP_SCHEMA_RFC2307:
@@ -549,8 +551,10 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
+    in_transaction = true;
 
     if (twopass && !populate_members) {
         saved_groups = talloc_array(tmpctx, struct sysdb_attrs *,
@@ -616,15 +620,22 @@ static int sdap_save_groups(TALLOC_CTX *memctx,
 
     ret = sysdb_transaction_commit(sysdb);
     if (ret) {
-        DEBUG(1, ("Failed to commit transaction!\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction!\n"));
         goto done;
     }
+    in_transaction = false;
 
     if (_usn_value) {
         *_usn_value = talloc_steal(memctx, higher_usn);
     }
 
 done:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
+    }
     talloc_zfree(tmpctx);
     return ret;
 }
@@ -1864,6 +1875,7 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
     hash_key_t key;
     hash_value_t value;
     size_t count;
+    bool in_transaction = false;
 
     if (_ghosts == NULL) {
         return EINVAL;
@@ -1886,9 +1898,10 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
-        DEBUG(1, ("Failed to start transaction!\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction!\n"));
         goto done;
     }
+    in_transaction = true;
 
     for (i = 0; i < num_users; i++) {
         ret = sysdb_attrs_primary_name(sysdb, users[i],
@@ -1973,17 +1986,21 @@ static errno_t sdap_nested_group_populate_users(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_transaction_commit(sysdb);
     if (ret) {
-        DEBUG(1, ("Failed to commit transaction!\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction!\n"));
         goto done;
     }
+    in_transaction = false;
 
     ret = EOK;
 done:
-    if (ret != EOK) {
+    if (in_transaction) {
         sret = sysdb_transaction_cancel(sysdb);
         if (sret != EOK) {
-            DEBUG(2, ("Could not cancel transaction\n"));
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
         }
+    }
+
+    if (ret != EOK) {
         *_ghosts = NULL;
     } else {
         *_ghosts = talloc_steal(mem_ctx, ghosts);
diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c
index a1c73f960..d55f661ff 100644
--- a/src/providers/ldap/sdap_async_initgroups.c
+++ b/src/providers/ldap/sdap_async_initgroups.c
@@ -42,6 +42,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
     char **missing;
     gid_t gid;
     int ret;
+    errno_t sret;
     bool in_transaction = false;
     bool posix;
     time_t now;
@@ -57,18 +58,10 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
     missing = talloc_array(tmp_ctx, char *, ldap_groups_count+1);
     if (!missing) {
         ret = ENOMEM;
-        goto fail;
+        goto done;
     }
     mi = 0;
 
-    ret = sysdb_transaction_start(sysdb);
-    if (ret != EOK) {
-        DEBUG(1, ("Cannot start sysdb transaction [%d]: %s\n",
-                   ret, strerror(ret)));
-        goto fail;
-    }
-    in_transaction = true;
-
     for (i=0; groupnames[i]; i++) {
         ret = sysdb_search_group_by_name(tmp_ctx, sysdb, groupnames[i], NULL, &msg);
         if (ret == EOK) {
@@ -82,7 +75,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
         } else if (ret != ENOENT) {
             DEBUG(1, ("search for group failed [%d]: %s\n",
                       ret, strerror(ret)));
-            goto fail;
+            goto done;
         }
     }
     missing[mi] = NULL;
@@ -93,6 +86,16 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
         goto done;
     }
 
+    ret = sysdb_transaction_start(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Cannot start sysdb transaction [%d]: %s\n",
+               ret, strerror(ret)));
+        goto done;
+    }
+    in_transaction = true;
+
+
     now = time(NULL);
     for (i=0; missing[i]; i++) {
         /* The group is not in sysdb, need to add a fake entry */
@@ -102,7 +105,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                                            &name);
             if (ret != EOK) {
                 DEBUG(1, ("The group has no name attribute\n"));
-                goto fail;
+                goto done;
             }
 
             if (strcmp(name, missing[i]) == 0) {
@@ -116,7 +119,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                             tmp_ctx, opts->idmap_ctx, ldap_groups[ai],
                             opts->group_map[SDAP_AT_GROUP_OBJECTSID].sys_name,
                             &sid_str);
-                    if (ret != EOK) goto fail;
+                    if (ret != EOK) goto done;
 
                     DEBUG(SSSDBG_TRACE_INTERNAL,
                           ("Group [%s] has objectSID [%s]\n",
@@ -151,7 +154,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                         posix = false;
                     } else if (ret) {
                         DEBUG(1, ("The GID attribute is malformed\n"));
-                        goto fail;
+                        goto done;
                     }
                 }
 
@@ -167,7 +170,7 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
                 ret = sysdb_add_incomplete_group(sysdb, name, gid, original_dn,
                                                  posix, now);
                 if (ret != EOK) {
-                    goto fail;
+                    goto done;
                 }
                 break;
             }
@@ -176,21 +179,24 @@ static errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb,
         if (ai == ldap_groups_count) {
             DEBUG(2, ("Group %s not present in LDAP\n", missing[i]));
             ret = EINVAL;
-            goto fail;
+            goto done;
         }
     }
 
-done:
     ret = sysdb_transaction_commit(sysdb);
     if (ret != EOK) {
-        DEBUG(1, ("sysdb_transaction_commit failed.\n"));
-        goto fail;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("sysdb_transaction_commit failed.\n"));
+        goto done;
     }
     in_transaction = false;
     ret = EOK;
-fail:
+
+done:
     if (in_transaction) {
-        sysdb_transaction_cancel(sysdb);
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
     }
     talloc_free(tmp_ctx);
     return ret;
@@ -1964,6 +1970,7 @@ errno_t save_rfc2307bis_user_memberships(
     DEBUG(7, ("Save parent groups to sysdb\n"));
     ret = sysdb_transaction_start(state->sysdb);
     if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto error;
     }
     in_transaction = true;
@@ -2012,6 +2019,7 @@ errno_t save_rfc2307bis_user_memberships(
 
     ret = sysdb_transaction_commit(state->sysdb);
     if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
         goto error;
     }
     in_transaction = false;
@@ -2630,8 +2638,10 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
     struct sysdb_attrs **usr_attrs;
     size_t count;
     int ret;
+    errno_t sret;
     const char *orig_dn;
     const char *cname;
+    bool in_transaction = false;
 
     DEBUG(9, ("Receiving info for the user\n"));
 
@@ -2666,9 +2676,10 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
 
     ret = sysdb_transaction_start(state->sysdb);
     if (ret) {
-        tevent_req_error(req, ret);
-        return;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
+        goto fail;
     }
+    in_transaction = true;
 
     DEBUG(9, ("Storing the user\n"));
 
@@ -2677,18 +2688,17 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
                          state->orig_user,
                          true, NULL, 0);
     if (ret) {
-        sysdb_transaction_cancel(state->sysdb);
-        tevent_req_error(req, ret);
-        return;
+        goto fail;
     }
 
     DEBUG(9, ("Commit change\n"));
 
     ret = sysdb_transaction_commit(state->sysdb);
     if (ret) {
-        tevent_req_error(req, ret);
-        return;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto fail;
     }
+    in_transaction = false;
 
     ret = sysdb_get_real_name(state, state->sysdb, state->name, &cname);
     if (ret != EOK) {
@@ -2760,6 +2770,16 @@ static void sdap_get_initgr_user(struct tevent_req *subreq)
         tevent_req_error(req, EINVAL);
         return;
     }
+
+    return;
+fail:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(state->sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
+    }
+    tevent_req_error(req, ret);
 }
 
 static int sdap_initgr_rfc2307bis_recv(struct tevent_req *req);
diff --git a/src/providers/ldap/sdap_async_services.c b/src/providers/ldap/sdap_async_services.c
index a59db6c66..026fa13f9 100644
--- a/src/providers/ldap/sdap_async_services.c
+++ b/src/providers/ldap/sdap_async_services.c
@@ -273,7 +273,10 @@ sdap_save_services(TALLOC_CTX *mem_ctx,
     }
 
     ret = sysdb_transaction_start(sysdb);
-    if (ret != EOK) goto done;
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
+        goto done;
+    }
 
     in_transaction = true;
 
diff --git a/src/providers/ldap/sdap_async_sudo.c b/src/providers/ldap/sdap_async_sudo.c
index db5e056d9..86edcc343 100644
--- a/src/providers/ldap/sdap_async_sudo.c
+++ b/src/providers/ldap/sdap_async_sudo.c
@@ -480,6 +480,7 @@ static void sdap_sudo_load_sudoers_done(struct tevent_req *subreq)
     /* start transaction */
     ret = sysdb_transaction_start(state->sysdb);
     if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
     in_transaction = true;
@@ -502,9 +503,11 @@ static void sdap_sudo_load_sudoers_done(struct tevent_req *subreq)
 
     /* commit transaction */
     ret = sysdb_transaction_commit(state->sysdb);
-    if (ret == EOK) {
-        in_transaction = false;
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
     }
+    in_transaction = false;
 
     DEBUG(SSSDBG_TRACE_FUNC, ("Sudoers is successfuly stored in cache\n"));
 
diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index dfce319b2..8974e6a24 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -377,8 +377,10 @@ int sdap_save_users(TALLOC_CTX *memctx,
     char *higher_usn = NULL;
     char *usn_value;
     int ret;
+    errno_t sret;
     int i;
     time_t now;
+    bool in_transaction = false;
 
     if (num_users == 0) {
         /* Nothing to do if there are no users */
@@ -392,8 +394,10 @@ int sdap_save_users(TALLOC_CTX *memctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
+    in_transaction = true;
 
     now = time(NULL);
     for (i = 0; i < num_users; i++) {
@@ -428,15 +432,22 @@ int sdap_save_users(TALLOC_CTX *memctx,
 
     ret = sysdb_transaction_commit(sysdb);
     if (ret) {
-        DEBUG(1, ("Failed to commit transaction!\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction!\n"));
         goto done;
     }
+    in_transaction = false;
 
     if (_usn_value) {
         *_usn_value = talloc_steal(memctx, higher_usn);
     }
 
 done:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
+    }
     talloc_zfree(tmpctx);
     return ret;
 }