From d52617ca1c217327bcb8dc53fa67e329f550a667 Mon Sep 17 00:00:00 2001 From: Martin Nagy Date: Dec 15 2009 12:23:57 +0000 Subject: Don't consider one address with different port numbers as the same There were two problems with the code. We were using fo_set_server_status() instead of fo_set_port_status() when we failed to connect to a service. This is a problem because if two services use the same server, or we want to use one server with two different ports, marking the whole server as bad is incorrect. The other problem was that be_resolve_server_done() was comparing the hostent structures -- these are, however, equal across multiple server:port pairs with the same server addresses. Fixes: #321 --- diff --git a/server/providers/data_provider_fo.c b/server/providers/data_provider_fo.c index bc274ce..23c857a 100644 --- a/server/providers/data_provider_fo.c +++ b/server/providers/data_provider_fo.c @@ -41,7 +41,7 @@ struct be_svc_data { const char *name; struct fo_service *fo_service; - struct hostent *last_good_srvaddr; + struct fo_server *last_good_srv; struct be_svc_callback *callbacks; }; @@ -267,7 +267,6 @@ static void be_resolve_server_done(struct tevent_req *subreq) struct be_resolve_server_state *state = tevent_req_data(req, struct be_resolve_server_state); struct be_svc_callback *callback; - struct hostent *srvaddr; int ret; ret = fo_resolve_service_recv(subreq, &state->srv); @@ -295,9 +294,6 @@ static void be_resolve_server_done(struct tevent_req *subreq) DEBUG(6, ("Couldn't resolve server (%s), resolver returned (%d)\n", fo_get_server_name(state->srv), ret)); - /* mark as bad server */ - fo_set_server_status(state->srv, SERVER_NOT_WORKING); - state->attempts++; if (state->attempts >= 10) { DEBUG(2, ("Failed to find a server after 10 attempts\n")); @@ -320,10 +316,11 @@ static void be_resolve_server_done(struct tevent_req *subreq) } /* all fine we got the server */ - srvaddr = fo_get_server_hostent(state->srv); if (debug_level >= 4) { + struct hostent *srvaddr; char ipaddr[128]; + srvaddr = fo_get_server_hostent(state->srv); inet_ntop(srvaddr->h_addrtype, srvaddr->h_addr_list[0], ipaddr, 128); @@ -332,8 +329,8 @@ static void be_resolve_server_done(struct tevent_req *subreq) } /* now call all svc callbacks if server changed */ - if (srvaddr != state->svc->last_good_srvaddr) { - state->svc->last_good_srvaddr = srvaddr; + if (state->srv != state->svc->last_good_srv) { + state->svc->last_good_srv = state->srv; DLIST_FOR_EACH(callback, state->svc->callbacks) { callback->fn(callback->private_data, state->srv); diff --git a/server/providers/fail_over.c b/server/providers/fail_over.c index 8288171..5c91bfc 100644 --- a/server/providers/fail_over.c +++ b/server/providers/fail_over.c @@ -341,8 +341,8 @@ fo_add_server(struct fo_service *service, const char *name, int port, struct fo_server *server; int ret; - DEBUG(3, ("Adding new server '%s', to service '%s'\n", name, - service->name)); + DEBUG(3, ("Adding new server '%s', to service '%s'\n", + name ? name : "(no name)", service->name)); DLIST_FOR_EACH(server, service->server_list) { if (server->port != port || server->user_data != user_data) continue; @@ -446,7 +446,7 @@ static void fo_resolve_service_done(struct tevent_req *subreq); struct tevent_req * fo_resolve_service_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, - struct resolv_ctx *resolv, struct fo_service *service) + struct resolv_ctx *resolv, struct fo_service *service) { int ret; struct fo_server *server; @@ -484,6 +484,7 @@ fo_resolve_service_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, goto done; } tevent_req_set_callback(subreq, fo_resolve_service_done, server->common); + fo_set_server_status(server, SERVER_RESOLVING_NAME); /* FALLTHROUGH */ case SERVER_RESOLVING_NAME: /* Name resolution is already under way. Just add ourselves into the @@ -506,6 +507,9 @@ done: return req; } +static void set_server_common_status(struct server_common *common, + enum server_status status); + static void fo_resolve_service_done(struct tevent_req *subreq) { @@ -526,6 +530,9 @@ fo_resolve_service_done(struct tevent_req *subreq) if (ret != EOK) { DEBUG(1, ("Failed to resolve server '%s': %s\n", common->name, resolv_strerror(resolv_status))); + set_server_common_status(common, SERVER_NOT_WORKING); + } else { + set_server_common_status(common, SERVER_NAME_RESOLVED); } /* Take care of all requests for this server. */ @@ -560,6 +567,17 @@ fo_resolve_service_recv(struct tevent_req *req, struct fo_server **server) return EOK; } +static void +set_server_common_status(struct server_common *common, + enum server_status status) +{ + DEBUG(4, ("Marking server '%s' as '%s'\n", common->name, + str_server_status(status))); + + common->server_status = status; + gettimeofday(&common->last_status_change, NULL); +} + void fo_set_server_status(struct fo_server *server, enum server_status status) { @@ -568,11 +586,7 @@ fo_set_server_status(struct fo_server *server, enum server_status status) return; } - DEBUG(4, ("Marking server '%s' as '%s'\n", SERVER_NAME(server), - str_server_status(status))); - - server->common->server_status = status; - gettimeofday(&server->common->last_status_change, NULL); + set_server_common_status(server->common, status); } void diff --git a/server/providers/fail_over.h b/server/providers/fail_over.h index 5fa9ff0..e581fba 100644 --- a/server/providers/fail_over.h +++ b/server/providers/fail_over.h @@ -88,7 +88,7 @@ void fo_set_server_status(struct fo_server *server, /* * Set feedback about the port status. This function should be used when * the server itself is working but the service is not. When status is set - * to PORT_WORKING, 'server' is also marked as an "active server" for it's + * to PORT_WORKING, 'server' is also marked as an "active server" for its * service. When the next fo_resolve_service_send() function is called, this * server will be preferred. This will hold as long as it is not marked as * not-working. diff --git a/server/providers/krb5/krb5_auth.c b/server/providers/krb5/krb5_auth.c index 0321ce8..4581278 100644 --- a/server/providers/krb5/krb5_auth.c +++ b/server/providers/krb5/krb5_auth.c @@ -975,10 +975,12 @@ static void krb5_child_done(struct tevent_req *req) if (*msg_status == PAM_AUTHINFO_UNAVAIL) { if (kr->srv != NULL) { - fo_set_server_status(kr->srv, SERVER_NOT_WORKING); + fo_set_port_status(kr->srv, PORT_NOT_WORKING); } be_mark_offline(be_req->be_ctx); kr->is_offline = true; + } else if (kr->srv != NULL) { + fo_set_port_status(kr->srv, PORT_WORKING); } struct sysdb_attrs *attrs; diff --git a/server/providers/ldap/ldap_auth.c b/server/providers/ldap/ldap_auth.c index 81c9666..88e637b 100644 --- a/server/providers/ldap/ldap_auth.c +++ b/server/providers/ldap/ldap_auth.c @@ -513,12 +513,14 @@ static void auth_connect_done(struct tevent_req *subreq) talloc_zfree(subreq); if (ret) { if (state->srv) { - /* mark the server as bad if connection failed */ - fo_set_server_status(state->srv, SERVER_NOT_WORKING); + /* mark this server as bad if connection failed */ + fo_set_port_status(state->srv, PORT_NOT_WORKING); } tevent_req_error(req, ret); return; + } else if (state->srv) { + fo_set_port_status(state->srv, PORT_WORKING); } subreq = get_user_dn_send(state, state->ev, diff --git a/server/providers/ldap/sdap_async_connection.c b/server/providers/ldap/sdap_async_connection.c index b742471..1349790 100644 --- a/server/providers/ldap/sdap_async_connection.c +++ b/server/providers/ldap/sdap_async_connection.c @@ -1051,13 +1051,15 @@ int sdap_cli_connect_recv(struct tevent_req *req, if (tevent_req_is_error(req, &tstate, &err)) { /* mark the server as bad if connection failed */ if (state->srv) { - fo_set_server_status(state->srv, SERVER_NOT_WORKING); + fo_set_port_status(state->srv, PORT_NOT_WORKING); } if (tstate == TEVENT_REQ_USER_ERROR) { return err; } return EIO; + } else if (state->srv) { + fo_set_port_status(state->srv, PORT_WORKING); } if (gsh) {