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).

20 files changed, 270 insertions, 67 deletions

diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index 571085e5a..d3fb158f0 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -572,9 +572,10 @@ static void hbac_sysdb_save(struct tevent_req *req)
 
     ret = sysdb_transaction_commit(sysdb);
     if (ret != EOK) {
-        DEBUG(0, ("Failed to commit transaction\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
         goto fail;
     }
+    in_transaction = false;
 
     /* We don't need the rule data any longer,
      * the rest of the processing relies on
diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c
index af0000cff..341b56223 100644
--- a/src/providers/ipa/ipa_hbac_common.c
+++ b/src/providers/ipa/ipa_hbac_common.c
@@ -111,7 +111,10 @@ ipa_hbac_sysdb_save(struct sysdb_ctx *sysdb, struct sss_domain_info *domain,
 
     /* Save the entries and groups to the cache */
     ret = sysdb_transaction_start(sysdb);
-    if (ret != EOK) return ret;
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
+        goto done;
+    };
     in_transaction = true;
 
     /* First, save the specific entries */
@@ -143,7 +146,10 @@ ipa_hbac_sysdb_save(struct sysdb_ctx *sysdb, struct sss_domain_info *domain,
     }
 
     ret = sysdb_transaction_commit(sysdb);
-    if (ret != EOK) goto done;
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
     in_transaction = false;
 
 done:
diff --git a/src/providers/ipa/ipa_selinux.c b/src/providers/ipa/ipa_selinux.c
index b5a84269e..36a2bfb4a 100644
--- a/src/providers/ipa/ipa_selinux.c
+++ b/src/providers/ipa/ipa_selinux.c
@@ -123,7 +123,10 @@ static void ipa_selinux_handler_done(struct tevent_req *req)
     }
 
     ret = sysdb_transaction_start(sysdb);
-    if (ret != EOK) goto fail;
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
+        goto fail;
+    }
     in_transaction = true;
 
     ret = sysdb_delete_usermaps(breq->sysdb);
diff --git a/src/providers/ipa/ipa_selinux_common.c b/src/providers/ipa/ipa_selinux_common.c
index a01e0b6cb..45794388f 100644
--- a/src/providers/ipa/ipa_selinux_common.c
+++ b/src/providers/ipa/ipa_selinux_common.c
@@ -32,12 +32,16 @@ errno_t ipa_save_user_maps(struct sysdb_ctx *sysdb,
                            struct sysdb_attrs **maps)
 {
     errno_t ret;
+    errno_t sret;
+    bool in_transaction = false;
     int i;
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
+    in_transaction = true;
 
     for (i = 0; i < map_count; i++) {
         ret = sysdb_store_selinux_usermap(sysdb, maps[i]);
@@ -54,9 +58,15 @@ errno_t ipa_save_user_maps(struct sysdb_ctx *sysdb,
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction!\n"));
         goto done;
     }
-
+    in_transaction = false;
     ret = EOK;
 
 done:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction"));
+        }
+    }
     return ret;
 }
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 1da1d0253..c3a9e62de 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -121,6 +121,8 @@ static int krb5_mod_ccname(TALLOC_CTX *mem_ctx,
     TALLOC_CTX *tmpctx;
     struct sysdb_attrs *attrs;
     int ret;
+    errno_t sret;
+    bool in_transaction = false;
 
     if (name == NULL || ccname == NULL) {
         DEBUG(1, ("Missing user or ccache name.\n"));
@@ -154,9 +156,11 @@ static int krb5_mod_ccname(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret != EOK) {
-        DEBUG(6, ("Error %d starting transaction (%s)\n", ret, strerror(ret)));
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Error %d starting transaction (%s)\n", ret, strerror(ret)));
         goto done;
     }
+    in_transaction = true;
 
     ret = sysdb_set_user_attr(sysdb, name, attrs, mod_op);
     if (ret != EOK) {
@@ -167,10 +171,18 @@ static int krb5_mod_ccname(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_transaction_commit(sysdb);
     if (ret != EOK) {
-        DEBUG(1, ("Failed to commit transaction!\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction!\n"));
+        goto done;
     }
+    in_transaction = false;
 
 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;
 }
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;
 }
diff --git a/src/providers/proxy/proxy_id.c b/src/providers/proxy/proxy_id.c
index 9a179209d..451bdff5b 100644
--- a/src/providers/proxy/proxy_id.c
+++ b/src/providers/proxy/proxy_id.c
@@ -353,6 +353,7 @@ static int enum_users(TALLOC_CTX *mem_ctx,
     char *buffer;
     char *newbuf;
     int ret;
+    errno_t sret;
     bool again;
 
     DEBUG(SSSDBG_TRACE_LIBS, ("Enumerating users\n"));
@@ -377,6 +378,7 @@ static int enum_users(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
     in_transaction = true;
@@ -420,6 +422,10 @@ static int enum_users(TALLOC_CTX *mem_ctx,
                 DEBUG(SSSDBG_TRACE_LIBS, ("Enumeration completed.\n"));
 
                 ret = sysdb_transaction_commit(sysdb);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+                    goto done;
+                }
                 in_transaction = false;
                 break;
 
@@ -468,7 +474,10 @@ static int enum_users(TALLOC_CTX *mem_ctx,
 done:
     talloc_zfree(tmpctx);
     if (in_transaction) {
-        sysdb_transaction_cancel(sysdb);
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
     }
     ctx->ops.endpwent();
     return ret;
@@ -518,7 +527,10 @@ static int save_group(struct sysdb_ctx *sysdb, struct sss_domain_info *dom,
     DEBUG_GR_MEM(7, grp);
 
     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;
 
     if (grp->gr_mem && grp->gr_mem[0]) {
@@ -948,6 +960,7 @@ static int enum_groups(TALLOC_CTX *mem_ctx,
     char *buffer;
     char *newbuf;
     int ret;
+    errno_t sret;
     bool again;
 
     DEBUG(SSSDBG_TRACE_LIBS, ("Enumerating groups\n"));
@@ -972,6 +985,7 @@ static int enum_groups(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
     in_transaction = true;
@@ -1015,6 +1029,10 @@ static int enum_groups(TALLOC_CTX *mem_ctx,
                 DEBUG(SSSDBG_TRACE_LIBS, ("Enumeration completed.\n"));
 
                 ret = sysdb_transaction_commit(sysdb);
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+                    goto done;
+                }
                 in_transaction = false;
                 break;
 
@@ -1062,7 +1080,10 @@ static int enum_groups(TALLOC_CTX *mem_ctx,
 done:
     talloc_zfree(tmpctx);
     if (in_transaction) {
-        sysdb_transaction_cancel(sysdb);
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
     }
     ctx->ops.endgrent();
     return ret;
@@ -1090,6 +1111,7 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
     char *buffer;
     size_t buflen;
     int ret;
+    errno_t sret;
     bool del_user;
     uid_t uid;
     struct ldb_result *cached_pwd = NULL;
@@ -1115,6 +1137,7 @@ static int get_initgr(TALLOC_CTX *mem_ctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto fail;
     }
     in_transaction = true;
@@ -1212,7 +1235,10 @@ done:
 fail:
     talloc_zfree(tmpctx);
     if (in_transaction) {
-        sysdb_transaction_cancel(sysdb);
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
     }
     return ret;
 }
diff --git a/src/providers/proxy/proxy_services.c b/src/providers/proxy/proxy_services.c
index aa19ccb68..4f8a379bb 100644
--- a/src/providers/proxy/proxy_services.c
+++ b/src/providers/proxy/proxy_services.c
@@ -223,6 +223,7 @@ enum_services(struct proxy_id_ctx *ctx,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
     in_transaction = true;
@@ -266,7 +267,10 @@ enum_services(struct proxy_id_ctx *ctx,
                 DEBUG(SSSDBG_TRACE_FUNC, ("Enumeration completed.\n"));
 
                 ret = sysdb_transaction_commit(sysdb);
-                if (ret != EOK) goto done;
+                if (ret != EOK) {
+                    DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+                    goto done;
+                }
 
                 in_transaction = false;
                 break;
diff --git a/src/python/pysss.c b/src/python/pysss.c
index 45725c0c9..842c1b5e4 100644
--- a/src/python/pysss.c
+++ b/src/python/pysss.c
@@ -170,6 +170,7 @@ static PyObject *py_sss_useradd(PySssLocalObject *self,
     PyObject *py_groups = Py_None;
     PyObject *py_create_home = Py_None;
     int create_home = 0;
+    bool in_transaction = false;
 
     /* parse arguments */
     if (!PyArg_ParseTupleAndKeywords(args, kwds,
@@ -232,12 +233,11 @@ static PyObject *py_sss_useradd(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = true;
 
     /* useradd */
     tctx->error = useradd(tctx, tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
@@ -247,6 +247,7 @@ static PyObject *py_sss_useradd(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = false;
 
     /* Create user's home directory and/or mail spool */
     if (tctx->octx->create_homedir) {
@@ -285,6 +286,12 @@ static PyObject *py_sss_useradd(PySssLocalObject *self,
     Py_RETURN_NONE;
 
 fail:
+    if (in_transaction) {
+        /* We do not handle return value of sysdb_transaction_cancel()
+         * because we don't want to overwrite previous error code.
+         */
+        sysdb_transaction_cancel(tctx->sysdb);
+    }
     talloc_zfree(tctx);
     return NULL;
 }
@@ -428,6 +435,7 @@ static PyObject *py_sss_usermod(PySssLocalObject *self,
     const char * const kwlist[] = { "username", "uid", "gid", "lock",
                                     "gecos",  "homedir", "shell",
                                     "addgroups", "rmgroups", NULL };
+    bool in_transaction = false;
 
     /* parse arguments */
     if (!PyArg_ParseTupleAndKeywords(args, kwds,
@@ -491,12 +499,11 @@ static PyObject *py_sss_usermod(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = true;
 
     /* usermod */
     tctx->error = usermod(tctx, tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
@@ -506,11 +513,18 @@ static PyObject *py_sss_usermod(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = false;
 
     talloc_zfree(tctx);
     Py_RETURN_NONE;
 
 fail:
+    if (in_transaction) {
+        /* We do not handle return value of sysdb_transaction_cancel()
+         * because we don't want to overwrite previous error code.
+         */
+        sysdb_transaction_cancel(tctx->sysdb);
+    }
     talloc_zfree(tctx);
     return NULL;
 }
@@ -533,6 +547,7 @@ static PyObject *py_sss_groupadd(PySssLocalObject *self,
     char *groupname;
     unsigned long gid = 0;
     const char * const kwlist[] = { "groupname", "gid", NULL };
+    bool in_transaction = false;
 
     /* parse arguments */
     if (!PyArg_ParseTupleAndKeywords(args, kwds,
@@ -558,12 +573,11 @@ static PyObject *py_sss_groupadd(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = true;
 
     /* groupadd */
     tctx->error = groupadd(tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
@@ -573,11 +587,18 @@ static PyObject *py_sss_groupadd(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = false;
 
     talloc_zfree(tctx);
     Py_RETURN_NONE;
 
 fail:
+    if (in_transaction) {
+        /* We do not handle return value of sysdb_transaction_cancel()
+         * because we don't want to overwrite previous error code.
+         */
+        sysdb_transaction_cancel(tctx->sysdb);
+    }
     talloc_zfree(tctx);
     return NULL;
 }
@@ -647,6 +668,7 @@ static PyObject *py_sss_groupmod(PySssLocalObject *self,
     char *groupname = NULL;
     const char * const kwlist[] = { "groupname", "gid", "addgroups",
                                     "rmgroups", NULL };
+    bool in_transaction = false;
 
     /* parse arguments */
     if (!PyArg_ParseTupleAndKeywords(args, kwds,
@@ -694,12 +716,11 @@ static PyObject *py_sss_groupmod(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = true;
 
     /* groupmod */
     tctx->error = groupmod(tctx, tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
@@ -709,11 +730,18 @@ static PyObject *py_sss_groupmod(PySssLocalObject *self,
         PyErr_SetSssError(tctx->error);
         goto fail;
     }
+    in_transaction = false;
 
     talloc_zfree(tctx);
     Py_RETURN_NONE;
 
 fail:
+    if (in_transaction) {
+        /* We do not handle return value of sysdb_transaction_cancel()
+         * because we don't want to overwrite previous error code.
+         */
+        sysdb_transaction_cancel(tctx->sysdb);
+    }
     talloc_zfree(tctx);
     return NULL;
 }
diff --git a/src/tools/sss_cache.c b/src/tools/sss_cache.c
index 1b2b29fe7..950ff1c16 100644
--- a/src/tools/sss_cache.c
+++ b/src/tools/sss_cache.c
@@ -131,7 +131,10 @@ int main(int argc, const char *argv[])
         ret = sysdb_transaction_commit(sysdb);
         if (ret != EOK) {
             DEBUG(SSSDBG_CRIT_FAILURE, ("Could not commit the transaction!\n"));
-            sysdb_transaction_cancel(sysdb);
+            ret = sysdb_transaction_cancel(sysdb);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+            }
         }
 
         if (skipped == true) {
diff --git a/src/tools/sss_groupadd.c b/src/tools/sss_groupadd.c
index f2e09a100..2a01f392c 100644
--- a/src/tools/sss_groupadd.c
+++ b/src/tools/sss_groupadd.c
@@ -46,7 +46,9 @@ int main(int argc, const char **argv)
     poptContext pc = NULL;
     struct tools_ctx *tctx = NULL;
     int ret = EXIT_SUCCESS;
+    errno_t sret;
     const char *pc_groupname = NULL;
+    bool in_transaction = false;
 
     debug_prg_name = argv[0];
 
@@ -106,20 +108,32 @@ int main(int argc, const char **argv)
 
     tctx->error = sysdb_transaction_start(tctx->sysdb);
     if (tctx->error != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
+    in_transaction = true;
 
     /* groupadd */
     tctx->error = groupadd(tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         goto done;
     }
 
     tctx->error = sysdb_transaction_commit(tctx->sysdb);
+    if (tctx->error != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+    in_transaction = false;
 
 done:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(tctx->sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
+    }
+
     if (tctx->error) {
         ret = tctx->error;
         switch (ret) {
diff --git a/src/tools/sss_groupmod.c b/src/tools/sss_groupmod.c
index abab4f57f..c210bad75 100644
--- a/src/tools/sss_groupmod.c
+++ b/src/tools/sss_groupmod.c
@@ -52,8 +52,10 @@ int main(int argc, const char **argv)
     struct tools_ctx *tctx = NULL;
     char *addgroups = NULL, *rmgroups = NULL;
     int ret;
+    errno_t sret;
     const char *pc_groupname = NULL;
     char *badgroup = NULL;
+    bool in_transaction = false;
 
     debug_prg_name = argv[0];
 
@@ -194,20 +196,31 @@ int main(int argc, const char **argv)
 
     tctx->error = sysdb_transaction_start(tctx->sysdb);
     if (tctx->error != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
+    in_transaction = true;
 
     /* groupmod */
     tctx->error = groupmod(tctx, tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         goto done;
     }
 
     tctx->error = sysdb_transaction_commit(tctx->sysdb);
+    if (tctx->error != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+    in_transaction = false;
 
 done:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(tctx->sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
+    }
     if (tctx->error) {
         ret = tctx->error;
         DEBUG(1, ("sysdb operation failed (%d)[%s]\n", ret, strerror(ret)));
diff --git a/src/tools/sss_seed.c b/src/tools/sss_seed.c
index 9136de34f..372678963 100644
--- a/src/tools/sss_seed.c
+++ b/src/tools/sss_seed.c
@@ -678,6 +678,7 @@ static int seed_cache_user(struct seed_ctx *sctx)
 {
     bool in_transaction = false;
     int ret = EOK;
+    errno_t sret;
 
     ret = sysdb_transaction_start(sctx->sysdb);
     if (ret != EOK) {
@@ -720,8 +721,8 @@ static int seed_cache_user(struct seed_ctx *sctx)
 
 done:
     if (in_transaction == true) {
-        ret = sysdb_transaction_cancel(sctx->sysdb);
-        if (ret != EOK) {
+        sret = sysdb_transaction_cancel(sctx->sysdb);
+        if (sret != EOK) {
             DEBUG(SSSDBG_OP_FAILURE, ("Failed to cancel transaction\n"));
         }
     }
diff --git a/src/tools/sss_useradd.c b/src/tools/sss_useradd.c
index 4df7c098e..1bf174c51 100644
--- a/src/tools/sss_useradd.c
+++ b/src/tools/sss_useradd.c
@@ -62,6 +62,8 @@ int main(int argc, const char **argv)
     char *groups = NULL;
     char *badgroup = NULL;
     int ret;
+    errno_t sret;
+    bool in_transaction = false;
 
     debug_prg_name = argv[0];
 
@@ -179,21 +181,23 @@ int main(int argc, const char **argv)
 
     tctx->error = sysdb_transaction_start(tctx->sysdb);
     if (tctx->error != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
+    in_transaction = true;
 
     /* useradd */
     tctx->error = useradd(tctx, tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         goto done;
     }
 
     tctx->error = sysdb_transaction_commit(tctx->sysdb);
     if (tctx->error) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
         goto done;
     }
+    in_transaction = false;
 
     /* Set SELinux login context - must be done after transaction is done
      * b/c libselinux calls getpwnam */
@@ -249,6 +253,13 @@ int main(int argc, const char **argv)
     }
 
 done:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(tctx->sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
+    }
+
     if (tctx->error) {
         switch (tctx->error) {
             case ERANGE:
diff --git a/src/tools/sss_usermod.c b/src/tools/sss_usermod.c
index b761de225..a45005caa 100644
--- a/src/tools/sss_usermod.c
+++ b/src/tools/sss_usermod.c
@@ -60,9 +60,11 @@ int main(int argc, const char **argv)
     poptContext pc = NULL;
     char *addgroups = NULL, *rmgroups = NULL;
     int ret;
+    errno_t sret;
     const char *pc_username = NULL;
     struct tools_ctx *tctx = NULL;
     char *badgroup = NULL;
+    bool in_transaction = false;
 
     debug_prg_name = argv[0];
 
@@ -216,18 +218,23 @@ int main(int argc, const char **argv)
 
     tctx->error = sysdb_transaction_start(tctx->sysdb);
     if (tctx->error != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
+    in_transaction = true;
 
     /* usermod */
     tctx->error = usermod(tctx, tctx->sysdb, tctx->octx);
     if (tctx->error) {
-        /* cancel transaction */
-        sysdb_transaction_cancel(tctx->sysdb);
         goto done;
     }
 
     tctx->error = sysdb_transaction_commit(tctx->sysdb);
+    if (tctx->error) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+    in_transaction = false;
 
     /* Set SELinux login context - must be done after transaction is done
      * b/c libselinux calls getpwnam */
@@ -239,6 +246,13 @@ int main(int argc, const char **argv)
     }
 
 done:
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(tctx->sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to cancel transaction\n"));
+        }
+    }
+
     if (tctx->error) {
         ret = tctx->error;
         switch (ret) {