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

6 files changed, 38 insertions, 21 deletions

diff --git a/server/providers/data_provider_fo.c b/server/providers/data_provider_fo.c
index bc274cef8..23c857aa0 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 8288171f1..5c91bfc6d 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 5fa9ff0cc..e581fbaf5 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 0321ce820..4581278de 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 81c966662..88e637b09 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 b742471c6..134979039 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) {