From 21d485184df986e1a123f70c689517386e51a5ce Mon Sep 17 00:00:00 2001 From: Michal Zidek Date: Thu, 16 Aug 2012 18:48:53 +0200 Subject: 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). --- src/providers/ipa/ipa_access.c | 3 +- src/providers/ipa/ipa_hbac_common.c | 10 +++- src/providers/ipa/ipa_selinux.c | 5 +- src/providers/ipa/ipa_selinux_common.c | 12 ++++- src/providers/krb5/krb5_auth.c | 16 ++++++- src/providers/ldap/ldap_id_cleanup.c | 3 ++ src/providers/ldap/sdap_async_groups.c | 27 +++++++++-- src/providers/ldap/sdap_async_initgroups.c | 74 +++++++++++++++++++----------- src/providers/ldap/sdap_async_services.c | 5 +- src/providers/ldap/sdap_async_sudo.c | 7 ++- src/providers/ldap/sdap_async_users.c | 13 +++++- src/providers/proxy/proxy_id.c | 34 ++++++++++++-- src/providers/proxy/proxy_services.c | 6 ++- src/python/pysss.c | 44 ++++++++++++++---- src/tools/sss_cache.c | 5 +- src/tools/sss_groupadd.c | 18 +++++++- src/tools/sss_groupmod.c | 17 ++++++- src/tools/sss_seed.c | 5 +- src/tools/sss_useradd.c | 15 +++++- src/tools/sss_usermod.c | 18 +++++++- 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) { -- cgit