MINOR: server: change adm_st_chg_cause storage type

Even though it doesn't look like it at first glance, this is more like
a cleanup than an actual code improvement:

Given that srv->adm_st_chg_cause has been used to exclusively store
static strings ever since it was implemented, we make the choice to
store it as an enum instead of a fixed-size string within server
struct.

This will allow to save some space in server struct, and will make
it more easily exportable (ie: event handlers) because of the
reduced memory footprint during handling and the ability to later get
the corresponding human-readable message when it's explicitly needed.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index d465fab58a9cb3ceec3459a177fc674671e62bde..003605427da5523ff4b9218c2e4c0a602e9f6c3d 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -189,6 +189,19 @@ enum srv_log_proto {
         SRV_LOG_PROTO_OCTET_COUNTING, // TCP frames: MSGLEN SP MSG
 };
 
+/* srv administrative change causes */
+enum srv_adm_st_chg_cause {
+       SRV_ADM_STCHGC_NONE = 0,
+       SRV_ADM_STCHGC_DNS_NOENT,     /* entry removed from srv record */
+       SRV_ADM_STCHGC_DNS_NOIP,      /* no server ip in the srv record */
+       SRV_ADM_STCHGC_DNS_NX,        /* resolution spent too much time in NX state */
+       SRV_ADM_STCHGC_DNS_TIMEOUT,   /* resolution timeout */
+       SRV_ADM_STCHGC_DNS_REFUSED,   /* query refused by dns server */
+       SRV_ADM_STCHGC_DNS_UNSPEC,    /* unspecified dns error */
+       SRV_ADM_STCHGC_STATS_DISABLE, /* legacy disable from the stats */
+       SRV_ADM_STCHGC_STATS_STOP     /* legacy stop from the stats */
+};
+
 struct pid_list {
        struct list list;
        pid_t pid;
@@ -403,8 +416,8 @@ struct server {
                long duration;
                short status, code;
                char reason[128];
-       } op_st_chg;                            /* operational status change's reason */
-       char adm_st_chg_cause[48];              /* administrative status change's cause */
+       } op_st_chg;                                    /* operational status change's reason */
+       enum srv_adm_st_chg_cause adm_st_chg_cause;     /* administrative status change's cause */
 
        event_hdl_sub_list e_subs;              /* event_hdl: server's subscribers list (atomically updated) */
 

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index e5866a5528cb8b96380d13cde8a63ff9b2e620b7..f58afd1e87cdc7798f23e9995ad012a48edc6cef 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -64,6 +64,7 @@ void srv_take(struct server *srv);
 struct server *srv_drop(struct server *srv);
 int srv_init_per_thr(struct server *srv);
 void srv_set_ssl(struct server *s, int use_ssl);
+const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause);
 
 /* functions related to server name resolution */
 int srv_prepare_for_resolution(struct server *srv, const char *hostname);
@@ -152,7 +153,7 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check)
  * is done. If <cause> is non-null, it will be displayed at the end of the log
  * lines to justify the state change.
  */
-void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause);
+void srv_set_admin_flag(struct server *s, enum srv_admin mode, enum srv_adm_st_chg_cause cause);
 
 /* Disables admin flag <mode> (among SRV_ADMF_*) on server <s>. This is used to
  * stop enforcing either maint mode or drain mode. It is not allowed to set more
@@ -212,7 +213,7 @@ static inline int server_is_draining(const struct server *s)
  */
 static inline void srv_adm_set_maint(struct server *s)
 {
-       srv_set_admin_flag(s, SRV_ADMF_FMAINT, NULL);
+       srv_set_admin_flag(s, SRV_ADMF_FMAINT, SRV_ADM_STCHGC_NONE);
        srv_clr_admin_flag(s, SRV_ADMF_FDRAIN);
 }
 
@@ -221,7 +222,7 @@ static inline void srv_adm_set_maint(struct server *s)
  */
 static inline void srv_adm_set_drain(struct server *s)
 {
-       srv_set_admin_flag(s, SRV_ADMF_FDRAIN, NULL);
+       srv_set_admin_flag(s, SRV_ADMF_FDRAIN, SRV_ADM_STCHGC_NONE);
        srv_clr_admin_flag(s, SRV_ADMF_FMAINT);
 }
 

diff --git a/src/server.c b/src/server.c
index f62a7e3962ec16d88b410807a07c75646cc2e1ef..9483e8972fa1c13e3b7c196b1a7ee1465823f5f8 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -104,6 +104,23 @@ struct dict server_key_dict = {
        .values = EB_ROOT_UNIQUE,
 };
 
+static const char *srv_adm_st_chg_cause_str[] = {
+       [SRV_ADM_STCHGC_NONE] = "",
+       [SRV_ADM_STCHGC_DNS_NOENT] = "entry removed from SRV record",
+       [SRV_ADM_STCHGC_DNS_NOIP] = "No IP for server ",
+       [SRV_ADM_STCHGC_DNS_NX] = "DNS NX status",
+       [SRV_ADM_STCHGC_DNS_TIMEOUT] = "DNS timeout status",
+       [SRV_ADM_STCHGC_DNS_REFUSED] = "DNS refused status",
+       [SRV_ADM_STCHGC_DNS_UNSPEC] = "unspecified DNS error",
+       [SRV_ADM_STCHGC_STATS_DISABLE] = "'disable' on stats page",
+       [SRV_ADM_STCHGC_STATS_STOP] = "'stop' on stats page"
+};
+
+const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause)
+{
+       return srv_adm_st_chg_cause_str[cause];
+}
+
 int srv_downtime(const struct server *s)
 {
        if ((s->cur_state != SRV_ST_STOPPED) || s->last_change >= now.tv_sec)           // ignore negative time
@@ -1702,7 +1719,7 @@ void srv_set_stopping(struct server *s, const char *reason, struct check *check)
  *
  * Must be called with the server lock held.
  */
-void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause)
+void srv_set_admin_flag(struct server *s, enum srv_admin mode, enum srv_adm_st_chg_cause cause)
 {
        struct server *srv;
 
@@ -1714,8 +1731,7 @@ void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause
                return;
 
        s->next_admin |= mode;
-       if (cause)
-               strlcpy2(s->adm_st_chg_cause, cause, sizeof(s->adm_st_chg_cause));
+       s->adm_st_chg_cause = cause;
 
        /* propagate changes */
        srv_update_status(s);
@@ -1792,10 +1808,10 @@ static void srv_propagate_admin_state(struct server *srv)
        for (srv2 = srv->trackers; srv2; srv2 = srv2->tracknext) {
                HA_SPIN_LOCK(SERVER_LOCK, &srv2->lock);
                if (srv->next_admin & (SRV_ADMF_MAINT | SRV_ADMF_CMAINT))
-                       srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, NULL);
+                       srv_set_admin_flag(srv2, SRV_ADMF_IMAINT, SRV_ADM_STCHGC_NONE);
 
                if (srv->next_admin & SRV_ADMF_DRAIN)
-                       srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, NULL);
+                       srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN, SRV_ADM_STCHGC_NONE);
                HA_SPIN_UNLOCK(SERVER_LOCK, &srv2->lock);
        }
 }
@@ -3510,7 +3526,7 @@ int srvrq_update_srv_status(struct server *s, int has_no_ip)
        if (s->next_admin & SRV_ADMF_RMAINT)
                return 1;
 
-       srv_set_admin_flag(s, SRV_ADMF_RMAINT, "entry removed from SRV record");
+       srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOENT);
        return 0;
 }
 
@@ -3546,8 +3562,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
                        if (has_no_ip) {
                                if (s->next_admin & SRV_ADMF_RMAINT)
                                        return 1;
-                               srv_set_admin_flag(s, SRV_ADMF_RMAINT,
-                                   "No IP for server ");
+                               srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NOIP);
                                return 0;
                        }
 
@@ -3569,7 +3584,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
 
                        if (s->next_admin & SRV_ADMF_RMAINT)
                                return 1;
-                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS NX status");
+                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_NX);
                        return 0;
 
                case RSLV_STATUS_TIMEOUT:
@@ -3580,7 +3595,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
 
                        if (s->next_admin & SRV_ADMF_RMAINT)
                                return 1;
-                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS timeout status");
+                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_TIMEOUT);
                        return 0;
 
                case RSLV_STATUS_REFUSED:
@@ -3591,7 +3606,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
 
                        if (s->next_admin & SRV_ADMF_RMAINT)
                                return 1;
-                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, "DNS refused status");
+                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_REFUSED);
                        return 0;
 
                default:
@@ -3602,7 +3617,7 @@ int snr_update_srv_status(struct server *s, int has_no_ip)
 
                        if (s->next_admin & SRV_ADMF_RMAINT)
                                return 1;
-                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, "unspecified DNS error");
+                       srv_set_admin_flag(s, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_DNS_UNSPEC);
                        return 0;
        }
 
@@ -4021,7 +4036,7 @@ static int srv_iterate_initaddr(struct server *srv)
                        break;
 
                case SRV_IADDR_NONE:
-                       srv_set_admin_flag(srv, SRV_ADMF_RMAINT, NULL);
+                       srv_set_admin_flag(srv, SRV_ADMF_RMAINT, SRV_ADM_STCHGC_NONE);
                        if (return_code) {
                                ha_warning("could not resolve address '%s', disabling server.\n",
                                           name);
@@ -5447,7 +5462,9 @@ static void srv_update_status(struct server *s)
                                chunk_printf(tmptrash,
                                    "%sServer %s/%s was DOWN and now enters maintenance%s%s%s",
                                    s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id,
-                                   *(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : "");
+                                   (s->adm_st_chg_cause) ? " (" : "",
+                                   srv_adm_st_chg_cause(s->adm_st_chg_cause),
+                                   (s->adm_st_chg_cause) ? ")" : "");
 
                                srv_append_status(tmptrash, s, NULL, -1, (s->next_admin & SRV_ADMF_FMAINT));
 
@@ -5486,7 +5503,9 @@ static void srv_update_status(struct server *s)
                                             "%sServer %s/%s is going DOWN for maintenance%s%s%s",
                                             s->flags & SRV_F_BACKUP ? "Backup " : "",
                                             s->proxy->id, s->id,
-                                            *(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : "");
+                                            (s->adm_st_chg_cause) ? " (" : "",
+                                            srv_adm_st_chg_cause(s->adm_st_chg_cause),
+                                            (s->adm_st_chg_cause) ? ")" : "");
 
                                srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FMAINT));
 
@@ -5669,7 +5688,9 @@ static void srv_update_status(struct server *s)
                        if (tmptrash) {
                                chunk_printf(tmptrash, "%sServer %s/%s enters drain state%s%s%s",
                                             s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id,
-                                            *(s->adm_st_chg_cause) ? " (" : "", s->adm_st_chg_cause, *(s->adm_st_chg_cause) ? ")" : "");
+                                            (s->adm_st_chg_cause) ? " (" : "",
+                                            srv_adm_st_chg_cause(s->adm_st_chg_cause),
+                                            (s->adm_st_chg_cause) ? ")" : "");
 
                                srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FDRAIN));
 
@@ -5748,8 +5769,8 @@ static void srv_update_status(struct server *s)
                }
        }
 
-       /* Re-set log strings to empty */
-       *s->adm_st_chg_cause = 0;
+       /* Re-set adm st change to none */
+       s->adm_st_chg_cause = SRV_ADM_STCHGC_NONE;
 
        /* explicitly commit state changes (even if it was already applied implicitly
         * by some lb state change function), so we don't miss anything

diff --git a/src/stats.c b/src/stats.c
index 1ba4008daa5b53b94e01bc44755eaf77bcfbecca..7bb7f819008f1ba80171ded2ec490a681f3c83d7 100644 (file)
--- a/src/stats.c
+++ b/src/stats.c
@@ -4170,7 +4170,7 @@ static int stats_process_http_post(struct stconn *sc)
                                                if (!(sv->cur_admin & SRV_ADMF_FMAINT)) {
                                                        altered_servers++;
                                                        total_servers++;
-                                                       srv_set_admin_flag(sv, SRV_ADMF_FMAINT, "'disable' on stats page");
+                                                       srv_set_admin_flag(sv, SRV_ADMF_FMAINT, SRV_ADM_STCHGC_STATS_DISABLE);
                                                }
                                                break;
                                        case ST_ADM_ACTION_ENABLE:
@@ -4182,7 +4182,7 @@ static int stats_process_http_post(struct stconn *sc)
                                                break;
                                        case ST_ADM_ACTION_STOP:
                                                if (!(sv->cur_admin & SRV_ADMF_FDRAIN)) {
-                                                       srv_set_admin_flag(sv, SRV_ADMF_FDRAIN, "'stop' on stats page");
+                                                       srv_set_admin_flag(sv, SRV_ADMF_FDRAIN, SRV_ADM_STCHGC_STATS_STOP);
                                                        altered_servers++;
                                                        total_servers++;
                                                }