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 d465fab..0036054 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -189,6 +189,19 @@
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 @@
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 e5866a5..f58afd1 100644
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -64,6 +64,7 @@
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 @@
* 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 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_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 f62a7e3..9483e89 100644
--- a/src/server.c
+++ b/src/server.c
@@ -104,6 +104,23 @@
.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 @@
*
* 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
"%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 @@
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 @@
}
}
- /* 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 1ba4008..7bb7f81 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -4170,7 +4170,7 @@
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 @@
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++;
}