Unify usage of sysdb transactions (part 2).

9 files changed, 330 insertions, 270 deletions

diff --git a/src/db/sysdb_autofs.c b/src/db/sysdb_autofs.c
index eff4e2969..df5f33980 100644
--- a/src/db/sysdb_autofs.c
+++ b/src/db/sysdb_autofs.c
@@ -345,8 +345,9 @@ sysdb_autofs_map_update_members(struct sysdb_ctx *sysdb,
                                 const char *const *add_entries,
                                 const char *const *del_entries)
 {
-    errno_t ret;
+    errno_t ret, sret;
     int i;
+    bool in_transaction = false;
 
     TALLOC_CTX *tmp_ctx = talloc_new(NULL);
     if(!tmp_ctx) {
@@ -360,6 +361,8 @@ sysdb_autofs_map_update_members(struct sysdb_ctx *sysdb,
         goto done;
     }
 
+    in_transaction = true;
+
     if (add_entries) {
         /* Add the all te add_entries to the map */
         for (i = 0; add_entries[i]; i++) {
@@ -389,9 +392,20 @@ sysdb_autofs_map_update_members(struct sysdb_ctx *sysdb,
     }
 
     ret = sysdb_transaction_commit(sysdb);
-done:
     if (ret != EOK) {
-        sysdb_transaction_cancel(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, ("Could not cancel transaction\n"));
+        }
     }
     talloc_free(tmp_ctx);
     return ret;
diff --git a/src/db/sysdb_idmap.c b/src/db/sysdb_idmap.c
index 94566d773..eca3d1bd6 100644
--- a/src/db/sysdb_idmap.c
+++ b/src/db/sysdb_idmap.c
@@ -53,221 +53,225 @@ sysdb_idmap_store_mapping(struct sysdb_ctx *sysdb,
                           const char *dom_sid,
                           id_t slice_num)
 {
-   errno_t ret, sret;
-   int lret;
-   bool in_transaction = false;
-   TALLOC_CTX *tmp_ctx;
-   struct ldb_dn *dn;
-   static const char *attrs[] = SYSDB_IDMAP_ATTRS;
-   size_t count;
-   struct ldb_message *update_msg;
-   struct ldb_message **msgs;
-   const char *old_name;
-   id_t old_slice;
-
-   tmp_ctx = talloc_new(NULL);
-   if (!tmp_ctx) return ENOMEM;
-
-   dn = sysdb_idmap_dn(tmp_ctx, sysdb, dom_sid);
-   if (!dn) {
-       ret = ENOMEM;
-       goto done;
-   }
-
-   update_msg = ldb_msg_new(tmp_ctx);
-   if (!update_msg) {
-       ret = ENOMEM;
-       goto done;
-   }
-   update_msg->dn = dn;
-
-   ret = sysdb_transaction_start(sysdb);
-   if (ret != EOK) goto done;
-   in_transaction = true;
-
-
-   /* Check for an existing mapping */
-   ret = sysdb_search_entry(tmp_ctx, sysdb, dn, LDB_SCOPE_BASE,
-                            NULL, attrs, &count, &msgs);
-   if (ret != EOK && ret != ENOENT) goto done;
-
-   if (ret == EOK && count != 1) {
-       /* More than one reply for a base search? */
-       ret = EIO;
-       goto done;
-   } else if (ret == ENOENT) {
-       /* Create a new mapping */
-       DEBUG(SSSDBG_CONF_SETTINGS,
-             ("Adding new ID mapping [%s][%s][%lu]\n",
-              dom_name, dom_sid, (unsigned long)slice_num));
-
-       /* Add the objectClass */
-       lret = ldb_msg_add_empty(update_msg, SYSDB_OBJECTCLASS,
-                                LDB_FLAG_MOD_ADD,
-                                NULL);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       lret = ldb_msg_add_string(update_msg, SYSDB_OBJECTCLASS,
-                                 SYSDB_IDMAP_MAPPING_OC);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       /* Add the domain objectSID */
-       lret = ldb_msg_add_empty(update_msg, SYSDB_IDMAP_SID_ATTR,
-                                LDB_FLAG_MOD_ADD,
-                                NULL);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       lret = ldb_msg_add_string(update_msg, SYSDB_IDMAP_SID_ATTR, dom_sid);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       /* Add the domain name */
-       lret = ldb_msg_add_empty(update_msg, SYSDB_NAME,
-                                LDB_FLAG_MOD_ADD,
-                                NULL);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       lret = ldb_msg_add_string(update_msg, SYSDB_NAME, dom_name);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       /* Add the slice number */
-       lret = ldb_msg_add_empty(update_msg, SYSDB_IDMAP_SLICE_ATTR,
-                                LDB_FLAG_MOD_ADD,
-                                NULL);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       lret = ldb_msg_add_fmt(update_msg, SYSDB_IDMAP_SLICE_ATTR,
-                              "%lu", (unsigned long)slice_num);
-       if (lret != LDB_SUCCESS) {
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-
-       lret = ldb_add(sysdb->ldb, update_msg);
-       if (lret != LDB_SUCCESS) {
-           DEBUG(SSSDBG_MINOR_FAILURE,
-                 ("Failed to add mapping: [%s]\n",
-                  ldb_strerror(lret)));
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-   } else {
-       /* Update the existing mapping */
-
-       /* Check whether the slice has changed
-        * This should never happen, and it's a recipe for
-        * disaster. We'll throw an error if it does.
-        */
-       old_slice = ldb_msg_find_attr_as_int(msgs[0],
-                                            SYSDB_IDMAP_SLICE_ATTR,
-                                            -1);
-       if (old_slice == -1) {
-           DEBUG(SSSDBG_CRIT_FAILURE,
-                 ("Could not identify original slice for SID [%s]\n",
-                  dom_sid));
-           ret = ENOENT;
-           goto done;
-       }
-
-       if (slice_num != old_slice) {
-           DEBUG(SSSDBG_FATAL_FAILURE,
-                 ("Detected attempt to change slice value for sid [%s] "
-                  "This will break existing users. Refusing to perform.\n"));
-           ret = EINVAL;
-           goto done;
-       }
-
-       /* Check whether the name has changed. This may happen
-        * if we're told the real name of a domain and want to
-        * replace the SID as placeholder.
-        */
-       old_name = ldb_msg_find_attr_as_string(msgs[0], SYSDB_NAME, NULL);
-       if (!old_name) {
-           DEBUG(SSSDBG_CRIT_FAILURE,
-                 ("Could not identify original domain name of SID [%s]\n",
-                  dom_sid));
-           ret = ENOENT;
-           goto done;
-       }
-
-       if (strcmp(old_name, dom_name) == 0) {
-           /* There's nothing to be done. We don't need to
-            * make any changes here. Just return success.
-            */
-           DEBUG(SSSDBG_TRACE_LIBS,
-                 ("No changes needed, canceling transaction\n"));
-           ret = EOK;
-           goto done;
-       } else {
-           /* The name has changed. Replace it */
-           DEBUG(SSSDBG_CONF_SETTINGS,
-                 ("Changing domain name of SID [%s] from [%s] to [%s]\n",
-                  dom_sid, old_name, dom_name));
-
-           /* Set the new name */
-           lret = ldb_msg_add_empty(update_msg, SYSDB_NAME,
-                                    LDB_FLAG_MOD_REPLACE,
-                                    NULL);
-           if (lret != LDB_SUCCESS) {
-               ret = sysdb_error_to_errno(lret);
-               goto done;
-           }
-
-           lret = ldb_msg_add_string(update_msg, SYSDB_NAME, dom_name);
-           if (lret != LDB_SUCCESS) {
-               ret = sysdb_error_to_errno(lret);
-               goto done;
-           }
-       }
-
-       lret = ldb_modify(sysdb->ldb, update_msg);
-       if (lret != LDB_SUCCESS) {
-           DEBUG(SSSDBG_MINOR_FAILURE,
-                 ("Failed to update mapping: [%s]\n",
-                  ldb_strerror(lret)));
-           ret = sysdb_error_to_errno(lret);
-           goto done;
-       }
-   }
-
-   ret = sysdb_transaction_commit(sysdb);
-   if (ret != EOK) {
-       DEBUG(SSSDBG_CRIT_FAILURE,
-             ("Could not commit transaction: [%s]\n", strerror(ret)));
-       goto done;
-   }
-   in_transaction = false;
+    errno_t ret, sret;
+    int lret;
+    bool in_transaction = false;
+    TALLOC_CTX *tmp_ctx;
+    struct ldb_dn *dn;
+    static const char *attrs[] = SYSDB_IDMAP_ATTRS;
+    size_t count;
+    struct ldb_message *update_msg;
+    struct ldb_message **msgs;
+    const char *old_name;
+    id_t old_slice;
+
+    tmp_ctx = talloc_new(NULL);
+    if (!tmp_ctx) return ENOMEM;
+
+    dn = sysdb_idmap_dn(tmp_ctx, sysdb, dom_sid);
+    if (!dn) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    update_msg = ldb_msg_new(tmp_ctx);
+    if (!update_msg) {
+        ret = ENOMEM;
+        goto done;
+    }
+    update_msg->dn = dn;
+
+    ret = sysdb_transaction_start(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
+        goto done;
+    }
+
+    in_transaction = true;
+
+
+    /* Check for an existing mapping */
+    ret = sysdb_search_entry(tmp_ctx, sysdb, dn, LDB_SCOPE_BASE,
+                             NULL, attrs, &count, &msgs);
+    if (ret != EOK && ret != ENOENT) goto done;
+
+    if (ret == EOK && count != 1) {
+        /* More than one reply for a base search? */
+        ret = EIO;
+        goto done;
+    } else if (ret == ENOENT) {
+        /* Create a new mapping */
+        DEBUG(SSSDBG_CONF_SETTINGS,
+              ("Adding new ID mapping [%s][%s][%lu]\n",
+               dom_name, dom_sid, (unsigned long)slice_num));
+
+        /* Add the objectClass */
+        lret = ldb_msg_add_empty(update_msg, SYSDB_OBJECTCLASS,
+                                 LDB_FLAG_MOD_ADD,
+                                 NULL);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        lret = ldb_msg_add_string(update_msg, SYSDB_OBJECTCLASS,
+                                  SYSDB_IDMAP_MAPPING_OC);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        /* Add the domain objectSID */
+        lret = ldb_msg_add_empty(update_msg, SYSDB_IDMAP_SID_ATTR,
+                                 LDB_FLAG_MOD_ADD,
+                                 NULL);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        lret = ldb_msg_add_string(update_msg, SYSDB_IDMAP_SID_ATTR, dom_sid);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        /* Add the domain name */
+        lret = ldb_msg_add_empty(update_msg, SYSDB_NAME,
+                                 LDB_FLAG_MOD_ADD,
+                                 NULL);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        lret = ldb_msg_add_string(update_msg, SYSDB_NAME, dom_name);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        /* Add the slice number */
+        lret = ldb_msg_add_empty(update_msg, SYSDB_IDMAP_SLICE_ATTR,
+                                 LDB_FLAG_MOD_ADD,
+                                 NULL);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        lret = ldb_msg_add_fmt(update_msg, SYSDB_IDMAP_SLICE_ATTR,
+                               "%lu", (unsigned long)slice_num);
+        if (lret != LDB_SUCCESS) {
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+
+        lret = ldb_add(sysdb->ldb, update_msg);
+        if (lret != LDB_SUCCESS) {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("Failed to add mapping: [%s]\n",
+                   ldb_strerror(lret)));
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+    } else {
+        /* Update the existing mapping */
+
+        /* Check whether the slice has changed
+         * This should never happen, and it's a recipe for
+         * disaster. We'll throw an error if it does.
+         */
+        old_slice = ldb_msg_find_attr_as_int(msgs[0],
+                                             SYSDB_IDMAP_SLICE_ATTR,
+                                             -1);
+        if (old_slice == -1) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Could not identify original slice for SID [%s]\n",
+                   dom_sid));
+            ret = ENOENT;
+            goto done;
+        }
+
+        if (slice_num != old_slice) {
+            DEBUG(SSSDBG_FATAL_FAILURE,
+                  ("Detected attempt to change slice value for sid [%s] "
+                   "This will break existing users. Refusing to perform.\n"));
+            ret = EINVAL;
+            goto done;
+        }
+
+        /* Check whether the name has changed. This may happen
+         * if we're told the real name of a domain and want to
+         * replace the SID as placeholder.
+         */
+        old_name = ldb_msg_find_attr_as_string(msgs[0], SYSDB_NAME, NULL);
+        if (!old_name) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Could not identify original domain name of SID [%s]\n",
+                   dom_sid));
+            ret = ENOENT;
+            goto done;
+        }
+
+        if (strcmp(old_name, dom_name) == 0) {
+            /* There's nothing to be done. We don't need to
+             * make any changes here. Just return success.
+             */
+            DEBUG(SSSDBG_TRACE_LIBS,
+                  ("No changes needed, canceling transaction\n"));
+            ret = EOK;
+            goto done;
+        } else {
+            /* The name has changed. Replace it */
+            DEBUG(SSSDBG_CONF_SETTINGS,
+                  ("Changing domain name of SID [%s] from [%s] to [%s]\n",
+                   dom_sid, old_name, dom_name));
+
+            /* Set the new name */
+            lret = ldb_msg_add_empty(update_msg, SYSDB_NAME,
+                                     LDB_FLAG_MOD_REPLACE,
+                                     NULL);
+            if (lret != LDB_SUCCESS) {
+                ret = sysdb_error_to_errno(lret);
+                goto done;
+            }
+
+            lret = ldb_msg_add_string(update_msg, SYSDB_NAME, dom_name);
+            if (lret != LDB_SUCCESS) {
+                ret = sysdb_error_to_errno(lret);
+                goto done;
+            }
+        }
+
+        lret = ldb_modify(sysdb->ldb, update_msg);
+        if (lret != LDB_SUCCESS) {
+            DEBUG(SSSDBG_MINOR_FAILURE,
+                  ("Failed to update mapping: [%s]\n",
+                   ldb_strerror(lret)));
+            ret = sysdb_error_to_errno(lret);
+            goto done;
+        }
+    }
+
+    ret = sysdb_transaction_commit(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE,
+              ("Could not commit transaction: [%s]\n", strerror(ret)));
+        goto done;
+    }
+    in_transaction = false;
 
 done:
-   if (in_transaction) {
-       sret = sysdb_transaction_cancel(sysdb);
-       if (sret != EOK) {
-           DEBUG(SSSDBG_CRIT_FAILURE,
-                 ("Could not cancel transaction\n"));
-       }
-   }
-   talloc_free(tmp_ctx);
-   return ret;
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Could not cancel transaction\n"));
+        }
+    }
+    talloc_free(tmp_ctx);
+    return ret;
 }
 
 errno_t
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index ed7b37e32..1b8627692 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -1526,7 +1526,10 @@ int sysdb_store_user(struct sysdb_ctx *sysdb,
     }
 
     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;
 
@@ -1618,22 +1621,22 @@ int sysdb_store_user(struct sysdb_ctx *sysdb,
         }
     }
 
+    ret = sysdb_transaction_commit(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+
+    in_transaction = false;
+
 done:
     if (in_transaction) {
-        if (ret == EOK) {
-            sret = sysdb_transaction_commit(sysdb);
-            if (sret != EOK) {
-                DEBUG(2, ("Could not commit transaction\n"));
-            }
-        }
-
-        if (ret != EOK || sret != EOK){
-            sret = sysdb_transaction_cancel(sysdb);
-            if (sret != EOK) {
-                DEBUG(2, ("Could not cancel transaction\n"));
-            }
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
         }
     }
+
     if (ret) {
         DEBUG(6, ("Error: %d (%s)\n", ret, strerror(ret)));
     }
@@ -2889,7 +2892,9 @@ errno_t sysdb_update_members(struct sysdb_ctx *sysdb,
                              const char *const *del_groups)
 {
     errno_t ret;
+    errno_t sret;
     int i;
+    bool in_transaction = false;
 
     TALLOC_CTX *tmp_ctx = talloc_new(NULL);
     if(!tmp_ctx) {
@@ -2902,6 +2907,8 @@ errno_t sysdb_update_members(struct sysdb_ctx *sysdb,
         goto done;
     }
 
+    in_transaction = true;
+
     if (add_groups) {
         /* Add the user to all add_groups */
         for (i = 0; add_groups[i]; i++) {
@@ -2929,10 +2936,19 @@ errno_t sysdb_update_members(struct sysdb_ctx *sysdb,
     }
 
     ret = sysdb_transaction_commit(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+
+    in_transaction = false;
 
 done:
-    if (ret != EOK) {
-        sysdb_transaction_cancel(sysdb);
+    if (in_transaction) {
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
+        }
     }
     talloc_free(tmp_ctx);
     return ret;
@@ -3103,7 +3119,10 @@ errno_t sysdb_remove_attrs(struct sysdb_ctx *sysdb,
     }
 
     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;
 
@@ -3138,20 +3157,19 @@ errno_t sysdb_remove_attrs(struct sysdb_ctx *sysdb,
 
     ret = EOK;
 
+    ret = sysdb_transaction_commit(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+
+    in_transaction = false;
+
 done:
     if (in_transaction) {
-        if (ret == EOK) {
-            sret = sysdb_transaction_commit(sysdb);
-            if (sret != EOK) {
-                DEBUG(2, ("Could not commit transaction\n"));
-            }
-        }
-
-        if (ret != EOK || sret != EOK){
-            sret = sysdb_transaction_cancel(sysdb);
-            if (sret != EOK) {
-                DEBUG(2, ("Could not cancel transaction\n"));
-            }
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
         }
     }
     talloc_free(msg);
diff --git a/src/db/sysdb_ranges.c b/src/db/sysdb_ranges.c
index 99518a28f..416bcdd02 100644
--- a/src/db/sysdb_ranges.c
+++ b/src/db/sysdb_ranges.c
@@ -327,11 +327,11 @@ errno_t sysdb_update_ranges(struct sysdb_ctx *sysdb,
     }
 
     ret = sysdb_transaction_commit(sysdb);
-    if (ret == EOK) {
-        in_transaction = false;
-    } else {
-        DEBUG(SSSDBG_MINOR_FAILURE, ("Could not commit transaction\n"));
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Could not commit transaction\n"));
+        goto done;
     }
+    in_transaction = false;
 
 done:
     if (in_transaction) {
diff --git a/src/db/sysdb_selinux.c b/src/db/sysdb_selinux.c
index 976489503..565b7612f 100644
--- a/src/db/sysdb_selinux.c
+++ b/src/db/sysdb_selinux.c
@@ -174,7 +174,10 @@ static errno_t sysdb_store_selinux_entity(struct sysdb_ctx *sysdb,
     }
 
     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;
 
@@ -212,22 +215,21 @@ static errno_t sysdb_store_selinux_entity(struct sysdb_ctx *sysdb,
         ret = ldb_modify(sysdb->ldb, rm_msg);
     }
 
+    ret = sysdb_transaction_commit(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+    in_transaction = false;
+
 done:
     if (in_transaction) {
-        if (ret == EOK) {
-            sret = sysdb_transaction_commit(sysdb);
-            if (sret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, ("Could not commit transaction\n"));
-            }
-        }
-
-        if (ret != EOK || sret != EOK){
-            sret = sysdb_transaction_cancel(sysdb);
-            if (sret != EOK) {
-                DEBUG(SSSDBG_OP_FAILURE, ("Could not cancel transaction\n"));
-            }
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
         }
     }
+
     if (ret) {
         DEBUG(SSSDBG_MINOR_FAILURE, ("Error: %d (%s)\n", ret, strerror(ret)));
     }
diff --git a/src/db/sysdb_services.c b/src/db/sysdb_services.c
index 9a5e57a02..f3ed48ed8 100644
--- a/src/db/sysdb_services.c
+++ b/src/db/sysdb_services.c
@@ -202,7 +202,10 @@ sysdb_store_service(struct sysdb_ctx *sysdb,
     if (!tmp_ctx) return ENOMEM;
 
     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;
 
@@ -403,7 +406,11 @@ sysdb_store_service(struct sysdb_ctx *sysdb,
     }
 
     ret = sysdb_transaction_commit(sysdb);
-    if (ret == EOK) in_transaction = false;
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
+    in_transaction = false;
 
 done:
     if (in_transaction) {
@@ -660,7 +667,10 @@ sysdb_svc_delete(struct sysdb_ctx *sysdb,
     }
 
     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;
 
@@ -692,6 +702,10 @@ sysdb_svc_delete(struct sysdb_ctx *sysdb,
     }
 
     ret = sysdb_transaction_commit(sysdb);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
+    }
     in_transaction = false;
 
 done:
@@ -702,6 +716,7 @@ done:
                   ("Could not cancel transaction\n"));
         }
     }
+
     if (ret != EOK && ret != ENOENT) {
         DEBUG(SSSDBG_TRACE_INTERNAL,
               ("Error: %d (%s)\n", ret, strerror(ret)));
diff --git a/src/db/sysdb_ssh.c b/src/db/sysdb_ssh.c
index a4d4345b6..5b956cdd1 100644
--- a/src/db/sysdb_ssh.c
+++ b/src/db/sysdb_ssh.c
@@ -31,6 +31,7 @@ sysdb_store_ssh_host(struct sysdb_ctx *sysdb,
 {
     TALLOC_CTX *tmp_ctx;
     errno_t ret;
+    errno_t sret;
     struct ldb_message **hosts;
     size_t num_hosts;
     struct ldb_message_element *el;
@@ -129,7 +130,10 @@ sysdb_store_ssh_host(struct sysdb_ctx *sysdb,
 
 done:
     if (in_transaction) {
-        sysdb_transaction_cancel(sysdb);
+        sret = sysdb_transaction_cancel(sysdb);
+        if (sret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
+        }
     }
 
     talloc_free(tmp_ctx);
diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c
index 8489041bd..96b8b4b21 100644
--- a/src/db/sysdb_subdomains.c
+++ b/src/db/sysdb_subdomains.c
@@ -528,11 +528,11 @@ errno_t sysdb_update_subdomains(struct sysdb_ctx *sysdb,
     }
 
     ret = sysdb_transaction_commit(sysdb);
-    if (ret == EOK) {
-        in_transaction = false;
-    } else {
+    if (ret != EOK) {
         DEBUG(SSSDBG_MINOR_FAILURE, ("Could not commit transaction\n"));
+        goto done;
     }
+    in_transaction = false;
 
 done:
     if (in_transaction) {
diff --git a/src/db/sysdb_sudo.c b/src/db/sysdb_sudo.c
index 8a9539492..e1434e254 100644
--- a/src/db/sysdb_sudo.c
+++ b/src/db/sysdb_sudo.c
@@ -607,6 +607,7 @@ errno_t sysdb_sudo_purge_byfilter(struct sysdb_ctx *sysdb,
 
     ret = sysdb_transaction_start(sysdb);
     if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to start transaction\n"));
         goto done;
     }
     in_transaction = true;
@@ -627,9 +628,11 @@ errno_t sysdb_sudo_purge_byfilter(struct sysdb_ctx *sysdb,
     }
 
     ret = sysdb_transaction_commit(sysdb);
-    if (ret == EOK) {
-        in_transaction = false;
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to commit transaction\n"));
+        goto done;
     }
+    in_transaction = false;
 
 done:
     if (in_transaction) {