Re: [Freeipa-devel] [PATCHES] Infrastructure fixes
On Mon, 2009-07-20 at 21:12 +0200, Sumit Bose wrote: > On Mon, Jul 20, 2009 at 02:17:00PM -0400, Simo Sorce wrote: > > On Mon, 2009-07-20 at 13:57 +, Simo Sorce wrote: > > > > > > 3/3: Rework the use of openldap libraries in the ldap driver. Se the > > > commit for a brief explanation. > > > > Sumit found a degfault bug where we were not removing the fde event > > before killing the connection resulting in a callback to be called and > > using sh->ldap, now freed an null. > > > > I reworked the sdap_handle to store the fde directly instead of the fd > > as it was originally. This way we can properly free the event before > > killing the ldap context. > > > > Patch replaces 3/3 > > > > ACK to this patch, > ACK to the second version of the timestamp patch > ACK to the raise of debuglevel patch pushed Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] Infrastructure fixes
On Mon, Jul 20, 2009 at 02:17:00PM -0400, Simo Sorce wrote: > On Mon, 2009-07-20 at 13:57 +, Simo Sorce wrote: > > > > 3/3: Rework the use of openldap libraries in the ldap driver. Se the > > commit for a brief explanation. > > Sumit found a degfault bug where we were not removing the fde event > before killing the connection resulting in a callback to be called and > using sh->ldap, now freed an null. > > I reworked the sdap_handle to store the fde directly instead of the fd > as it was originally. This way we can properly free the event before > killing the ldap context. > > Patch replaces 3/3 > ACK to this patch, ACK to the second version of the timestamp patch ACK to the raise of debuglevel patch bye, Sumit ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] Infrastructure fixes
On Mon, 2009-07-20 at 13:57 +, Simo Sorce wrote:
>
> 3/3: Rework the use of openldap libraries in the ldap driver. Se the
> commit for a brief explanation.
Sumit found a degfault bug where we were not removing the fde event
before killing the connection resulting in a callback to be called and
using sh->ldap, now freed an null.
I reworked the sdap_handle to store the fde directly instead of the fd
as it was originally. This way we can properly free the event before
killing the ldap context.
Patch replaces 3/3
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From ea6c91f6758767410c957e5585cccdad4babbffd Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Mon, 20 Jul 2009 09:25:51 -0400
Subject: [PATCH] Rework the engine that deals with openldap libraries
The way openldap libraries work, require to have a single engine per
connection as all replies are read at the same time. So we need to
always read anything that comes in from the wire and then loop to
dispatch results to the requests that are waiting.
---
server/providers/ldap/ldap_id.c| 27 +-
server/providers/ldap/sdap.h | 28 +-
server/providers/ldap/sdap_async.c | 845
3 files changed, 403 insertions(+), 497 deletions(-)
diff --git a/server/providers/ldap/ldap_id.c b/server/providers/ldap/ldap_id.c
index 83d009e..3008f9b 100644
--- a/server/providers/ldap/ldap_id.c
+++ b/server/providers/ldap/ldap_id.c
@@ -113,7 +113,7 @@ static bool connected(struct sdap_id_ctx *ctx)
struct sdap_id_connect_state {
struct tevent_context *ev;
-struct sdap_options *opts;
+struct sdap_id_ctx *ctx;
bool use_start_tls;
struct sdap_handle *sh;
@@ -124,7 +124,7 @@ static void sdap_id_anon_bind_done(struct tevent_req *subreq);
struct tevent_req *sdap_id_connect_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
-struct sdap_options *opts,
+struct sdap_id_ctx *ctx,
bool use_start_tls)
{
struct tevent_req *req, *subreq;
@@ -134,10 +134,10 @@ struct tevent_req *sdap_id_connect_send(TALLOC_CTX *memctx,
if (!req) return NULL;
state->ev = ev;
-state->opts = opts;
+state->ctx = ctx;
state->use_start_tls = use_start_tls;
-subreq = sdap_connect_send(state, ev, opts, use_start_tls);
+subreq = sdap_connect_send(state, ev, ctx->opts, use_start_tls);
if (!subreq) {
talloc_zfree(req);
return NULL;
@@ -193,8 +193,7 @@ static void sdap_id_anon_bind_done(struct tevent_req *subreq)
tevent_req_done(req);
}
-static int sdap_id_connect_recv(struct tevent_req *req,
-TALLOC_CTX *memctx, struct sdap_handle **sh)
+static int sdap_id_connect_recv(struct tevent_req *req)
{
struct sdap_id_connect_state *state = tevent_req_data(req,
struct sdap_id_connect_state);
@@ -206,8 +205,8 @@ static int sdap_id_connect_recv(struct tevent_req *req,
return EIO;
}
-*sh = talloc_steal(memctx, state->sh);
-if (!*sh) {
+state->ctx->gsh = talloc_steal(state->ctx, state->sh);
+if (!state->ctx->gsh) {
return ENOMEM;
}
return EOK;
@@ -283,7 +282,7 @@ static struct tevent_req *users_get_send(TALLOC_CTX *memctx,
/* FIXME: add option to decide if tls should be used
* or SASL/GSSAPI, etc ... */
-subreq = sdap_id_connect_send(state, ev, ctx->opts, false);
+subreq = sdap_id_connect_send(state, ev, ctx, false);
if (!subreq) {
ret = ENOMEM;
goto fail;
@@ -320,7 +319,7 @@ static void users_get_connect_done(struct tevent_req *subreq)
struct users_get_state);
int ret;
-ret = sdap_id_connect_recv(subreq, state->ctx, &state->ctx->gsh);
+ret = sdap_id_connect_recv(subreq);
talloc_zfree(subreq);
if (ret) {
tevent_req_error(req, ret);
@@ -440,7 +439,7 @@ static struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
/* FIXME: add option to decide if tls should be used
* or SASL/GSSAPI, etc ... */
-subreq = sdap_id_connect_send(state, ev, ctx->opts, false);
+subreq = sdap_id_connect_send(state, ev, ctx, false);
if (!subreq) {
ret = ENOMEM;
goto fail;
@@ -477,7 +476,7 @@ static void groups_get_connect_done(struct tevent_req *subreq)
struct users_get_state);
int ret;
-ret = sdap_id_connect_recv(subreq, state->ctx, &state->ctx->gsh);
+ret = sdap_id_connect_recv(subreq);
talloc_zfree(subreq);
if (ret) {
tevent_req_error(req, ret);
@@ -572,7 +571,7 @@ static struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx,
/* FIXME: add option to decide if tls should be us
Re: [Freeipa-devel] [PATCHES] Infrastructure fixes
On Mon, 2009-07-20 at 13:57 +, Simo Sorce wrote:
> 1/3: Add timestamps when the debug level is set higher then 8
> When doing some tests it may be important to have the timestamp at which
> each operation happened. add automatically a timestamp when the level is
> high enough
Some people complained on IRC about this one, and said they would prefer
to have a separate option to tell when to add a timestamp.
Attached patch that substitute 1/3
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From 47962555e63dbf1d4764eb1e2b4a829e9826bcd0 Mon Sep 17 00:00:00 2001
From: Simo Sorce
Date: Tue, 14 Jul 2009 17:01:26 -0400
Subject: [PATCH] Add option to add timestamps to debug output
use '--debug-timestamps' at the command line
or set 'debug-timestamps = TRUE' in the configuration file.
---
server/monitor/monitor.c | 11 +++
server/util/debug.c |9 -
server/util/server.c | 12
server/util/util.h | 15 ---
4 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/server/monitor/monitor.c b/server/monitor/monitor.c
index 11c35d0..269fcbf 100644
--- a/server/monitor/monitor.c
+++ b/server/monitor/monitor.c
@@ -763,9 +763,11 @@ static int get_service_config(struct mt_ctx *ctx, const char *name,
}
if (!svc->command) {
-svc->command = talloc_asprintf(svc, "%s/sssd_%s -d %d",
- SSSD_LIBEXEC_PATH, svc->name,
- debug_level);
+svc->command = talloc_asprintf(svc, "%s/sssd_%s -d %d%s",
+ SSSD_LIBEXEC_PATH,
+ svc->name, debug_level,
+ (debug_timestamps?
+ " --debug-timestamps":""));
if (!svc->command) {
talloc_free(svc);
return ENOMEM;
@@ -870,8 +872,9 @@ static int get_provider_config(struct mt_ctx *ctx, const char *name,
/* if there are no custom commands, build a default one */
if (!svc->command) {
svc->command = talloc_asprintf(svc,
-"%s/sssd_be -d %d --provider %s --domain %s",
+"%s/sssd_be -d %d%s --provider %s --domain %s",
SSSD_LIBEXEC_PATH, debug_level,
+(debug_timestamps?" --debug-timestamps":""),
svc->provider, svc->name);
if (!svc->command) {
talloc_free(svc);
diff --git a/server/util/debug.c b/server/util/debug.c
index 24dcdb1..814aec1 100644
--- a/server/util/debug.c
+++ b/server/util/debug.c
@@ -7,6 +7,7 @@
const char *debug_prg_name = "sssd";
int debug_level = 0;
+int debug_timestamps = 0;
void debug_fn(const char *format, ...)
{
@@ -58,7 +59,13 @@ void ldb_debug_messages(void *context, enum ldb_debug_level level,
}
if (loglevel <= debug_level) {
-debug_fn("[%s] [ldb] (%d): %s\n", debug_prg_name, loglevel, message);
+if (debug_timestamps) {
+debug_fn("(%010ld) [%s] [ldb] (%d): %s\n",
+ (long)time(NULL), debug_prg_name, loglevel, message);
+} else {
+debug_fn("[%s] [ldb] (%d): %s\n",
+ debug_prg_name, loglevel, message);
+}
}
free(message);
}
diff --git a/server/util/server.c b/server/util/server.c
index 4dfd18e..fd6e4cd 100644
--- a/server/util/server.c
+++ b/server/util/server.c
@@ -253,6 +253,7 @@ int server_setup(const char *name, int flags,
uint16_t stdin_event_flags;
char *conf_db;
int ret = EOK;
+bool dt;
debug_prg_name = strdup(name);
if (!debug_prg_name) {
@@ -324,6 +325,17 @@ int server_setup(const char *name, int flags,
return ret;
}
+/* same for debug timestamps */
+dt = (debug_timestamps != 0);
+ret = confdb_get_bool(ctx->confdb_ctx, ctx, conf_entry,
+ "debug-timestamps", dt, &dt);
+if (ret != EOK) {
+DEBUG(0, ("Error reading from confdb (%d) [%s]\n",
+ ret, strerror(ret)));
+return ret;
+}
+if (dt) debug_timestamps = 1;
+
if (flags & FLAGS_INTERACTIVE) {
/* terminate when stdin goes away */
stdin_event_flags = TEVENT_FD_READ;
diff --git a/server/util/util.h b/server/util/util.h
index fac3ea5..02916c1 100644
--- a/server/util/util.h
+++ b/server/util/util.h
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include "config.h"
#include "talloc.h"
#include "tevent.h"
@@ -15,11 +16,14 @@
extern const char *debug_prg_name;
extern int debug_level;
+extern int debug_timestamps;
void debug_fn(const char *format, ...);
#define SSSD_DEBUG_OPTS \
{"debug-level", 'd', POPT_ARG_INT, &debug_level, 0, \
- "Debug level", NULL},
+ "Debug level", NULL}, \
+ {"debug-timestamps", 0, POPT_ARG_NONE, &debug_timestamps, 0, \
+ "Add debug timestamps", NULL},
/** \def D
