MEDIUM: log/sink: make logsrv postparsing more generic

We previously had postparsing logic but only for logsrv sinks, but now we
need to make this operation on logsrv directly instead of sinks to prepare
for additional postparsing logic that is not sink-specific.

To do this, we migrated post_sink_resolve() and sink_postresolve_logsrvs()
to their postresolve_logsrvs() and postresolve_logsrv_list() equivalents.

Then, we split postresolve_logsrv_list() so that the sink-only logic stays
in sink.c (sink_resolve_logsrv_buffer() function), and the "generic"
target part stays in log.c as resolve_logsrv().

Error messages formatting was preserved as far as possible but some slight
variations are to be expected.
As for the functional aspect, no change should be expected.

diff --git a/include/haproxy/log.h b/include/haproxy/log.h
index 1a4c79071a00f4a9ceda6e3a84fa8463b1111c5c..4c7b455eadc69526192d17f13c9e7085ec726538 100644 (file)
--- a/include/haproxy/log.h
+++ b/include/haproxy/log.h
@@ -87,6 +87,8 @@ int add_to_logformat_list(char *start, char *end, int type, struct list *list_fo
  */
 int parse_logformat_string(const char *str, struct proxy *curproxy, struct list *list_format, int options, int cap, char **err);
 
+int postresolve_logsrv_list(struct list *logsrvs, const char *section, const char *section_name);
+
 struct logsrv *dup_logsrv(struct logsrv *def);
 void free_logsrv(struct logsrv *logsrv);
 

diff --git a/include/haproxy/sink.h b/include/haproxy/sink.h
index 650d8d816cc7158d2bd3640c7ab8e106da73dfa5..50f82b1c5487ff1ff7b7b26b54e08ffc9d9e10d8 100644 (file)
--- a/include/haproxy/sink.h
+++ b/include/haproxy/sink.h
@@ -81,7 +81,7 @@ static inline ssize_t sink_write(struct sink *sink, size_t maxlen,
        return sent;
 }
 
-int sink_postresolve_logsrvs(struct list *logsrvs, const char *section, const char *section_name);
+int sink_resolve_logsrv_buffer(struct logsrv *logsrv, char **msg);
 
 #endif /* _HAPROXY_SINK_H */
 

diff --git a/src/log.c b/src/log.c
index 97b809e2070873853f003bcf6b54808738e4aca6..e1cc448cd50b76c4be46eccfb6b9013eb67adb4e 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -734,6 +734,26 @@ int smp_log_range_cmp(const void *a, const void *b)
        return 0;
 }
 
+/* resolves a single logsrv entry (it is expected to be called
+ * at postparsing stage)
+ *
+ * <logsrv> is parent logsrv used for implicit settings
+ *
+ * Returns err_code which defaults to ERR_NONE and can be set to a combination
+ * of ERR_WARN, ERR_ALERT, ERR_FATAL and ERR_ABORT in case of errors.
+ * <msg> could be set at any time (it will usually be set on error, but
+ * could also be set when no error occured to report a diag warning), thus is
+ * up to the caller to check it and to free it.
+ */
+int resolve_logsrv(struct logsrv *logsrv, char **msg)
+{
+       int err_code = ERR_NONE;
+
+       if (logsrv->type == LOG_TARGET_BUFFER)
+               err_code = sink_resolve_logsrv_buffer(logsrv, msg);
+       return err_code;
+}
+
 /* tries to duplicate <def> logsrv
  *
  * Returns the newly allocated and duplicated logsrv or NULL
@@ -4019,9 +4039,67 @@ out:
        return err_code;
 }
 
+/* function: post-resolve a single list of logsrvs
+ *
+ * Returns err_code which defaults to ERR_NONE and can be set to a combination
+ * of ERR_WARN, ERR_ALERT, ERR_FATAL and ERR_ABORT in case of errors.
+ */
+int postresolve_logsrv_list(struct list *logsrvs, const char *section, const char *section_name)
+{
+       int err_code = ERR_NONE;
+       struct logsrv *logsrv;
+
+       list_for_each_entry(logsrv, logsrvs, list) {
+               int cur_code;
+               char *msg = NULL;
+
+               cur_code = resolve_logsrv(logsrv, &msg);
+               if (msg) {
+                       void (*e_func)(const char *fmt, ...) = NULL;
+
+                       if (cur_code & ERR_ALERT)
+                               e_func = ha_alert;
+                       else if (cur_code & ERR_WARN)
+                               e_func = ha_warning;
+                       else
+                               e_func = ha_diag_warning;
+                       if (!section)
+                               e_func("global log server declared in file %s at line '%d' %s.\n",
+                                      logsrv->conf.file, logsrv->conf.line, msg);
+                       else
+                               e_func("log server declared in %s section '%s' in file '%s' at line %d %s.\n",
+                                      section, section_name, logsrv->conf.file, logsrv->conf.line, msg);
+                       ha_free(&msg);
+               }
+               err_code |= cur_code;
+       }
+       return err_code;
+}
+
+/* resolve default log directives at end of config. Returns 0 on success
+ * otherwise error flags.
+*/
+static int postresolve_logsrvs()
+{
+       struct proxy *px;
+       int err_code = ERR_NONE;
+
+       /* global log directives */
+       err_code |= postresolve_logsrv_list(&global.logsrvs, NULL, NULL);
+       /* proxy log directives */
+       for (px = proxies_list; px; px = px->next)
+               err_code |= postresolve_logsrv_list(&px->logsrvs, "proxy", px->id);
+       /* log-forward log directives */
+       for (px = cfg_log_forward; px; px = px->next)
+               err_code |= postresolve_logsrv_list(&px->logsrvs, "log-forward", px->id);
+
+       return err_code;
+}
+
 
 /* config parsers for this section */
 REGISTER_CONFIG_SECTION("log-forward", cfg_parse_log_forward, NULL);
+REGISTER_POST_CHECK(postresolve_logsrvs);
 
 REGISTER_PER_THREAD_ALLOC(init_log_buffers);
 REGISTER_PER_THREAD_FREE(deinit_log_buffers);

diff --git a/src/sink.c b/src/sink.c
index bbab05944265380e921a3636a9babf9e86dbd03e..3768a44bdeeeca967ffdb1f1c1c07818da6b0ec8 100644 (file)
--- a/src/sink.c
+++ b/src/sink.c
@@ -1240,105 +1240,60 @@ int cfg_post_parse_ring()
        return err_code;
 }
 
-/* helper macro: generate an error message for logsrvs post resolve
+/* function: resolve a single logsrv target of BUFFER type
  *
- * The error function (ha_alert, ha_warning..) should be provided as <e_func>
- * If <section> is non NULL, then it is considered that the section is not
- * global, so section and section_name infos will be added to the final message
- *
- * <fmt> is treated after contextual infos, and leverages varargs to pass
- * additional arguments to <e_func>
- */
-#define _e_sink_postresolve_logsrvs(e_func, name, fmt, section, section_name, file, linenum, ...) \
-       {                                                                                         \
-               if (!section)                                                                     \
-                       e_func("global %s declared in file '%s' at line %d "fmt".\n",             \
-                              name, file, linenum, ## __VA_ARGS__);                              \
-               else                                                                              \
-                       e_func("%s declared in %s section '%s' in file '%s' at line %d "fmt".\n", \
-                              name, section, section_name, file, linenum, ## __VA_ARGS__);       \
-       }
-
-/* function: post-resolve a single logsrvs list
+ * <logsrv> is parent logsrv used for implicit settings
  *
  * Returns err_code which defaults to ERR_NONE and can be set to a combination
  * of ERR_WARN, ERR_ALERT, ERR_FATAL and ERR_ABORT in case of errors.
+ * <msg> could be set at any time (it will usually be set on error, but
+ * could also be set when no error occured to report a diag warning), thus is
+ * up to the caller to check it and to free it.
  */
-int sink_postresolve_logsrvs(struct list *logsrvs, const char *section, const char *section_name)
+int sink_resolve_logsrv_buffer(struct logsrv *target, char **msg)
 {
        int err_code = ERR_NONE;
-       struct logsrv *logsrv, *logb;
        struct sink *sink;
 
-       list_for_each_entry_safe(logsrv, logb, logsrvs, list) {
-               if (logsrv->type == LOG_TARGET_BUFFER) {
-                       sink = sink_find(logsrv->ring_name);
+       BUG_ON(target->type != LOG_TARGET_BUFFER);
+       sink = sink_find(target->ring_name);
+       if (!sink) {
+               /* LOG_TARGET_BUFFER but !AF_UNSPEC
+                * means we must allocate a sink
+                * buffer to send messages to this logsrv
+                */
+               if (target->addr.ss_family != AF_UNSPEC) {
+                       sink = sink_new_from_logsrv(target);
                        if (!sink) {
-                               /* LOG_TARGET_BUFFER but !AF_UNSPEC
-                                * means we must allocate a sink
-                                * buffer to send messages to this logsrv
-                                */
-                               if (logsrv->addr.ss_family != AF_UNSPEC) {
-                                       sink = sink_new_from_logsrv(logsrv);
-                                       if (!sink) {
-                                               _e_sink_postresolve_logsrvs(ha_alert, "stream log server", "cannot be initialized",
-                                                                           section, section_name, logsrv->conf.file, logsrv->conf.line);
-                                               err_code |= ERR_ALERT | ERR_FATAL;
-                                       }
-                               }
-                               else {
-                                       _e_sink_postresolve_logsrvs(ha_alert, "log server", "uses unknown ring named '%s'",
-                                                                   section, section_name,
-                                                                   logsrv->conf.file, logsrv->conf.line, logsrv->ring_name);
-                                       err_code |= ERR_ALERT | ERR_FATAL;
-                               }
-                       }
-                       else if (sink->type != SINK_TYPE_BUFFER) {
-                               _e_sink_postresolve_logsrvs(ha_alert, "log server", "uses incompatible ring '%s'",
-                                                           section, section_name,
-                                                           logsrv->conf.file, logsrv->conf.line, logsrv->ring_name);
+                               memprintf(msg, "cannot be initialized (failed to create implicit ring)");
                                err_code |= ERR_ALERT | ERR_FATAL;
+                               goto end;
                        }
-                       if (sink && logsrv->maxlen > ring_max_payload(sink->ctx.ring)) {
-                               _e_sink_postresolve_logsrvs(ha_diag_warning, "log server", "uses a max length which exceeds ring capacity ('%s' supports %lu bytes at most).",
-                                                           section, section_name,
-                                                           logsrv->conf.file, logsrv->conf.line,
-                                                           logsrv->ring_name, (unsigned long)ring_max_payload(sink->ctx.ring));
-                       }
-                       else if (sink && logsrv->maxlen > sink->maxlen) {
-                               _e_sink_postresolve_logsrvs(ha_diag_warning, "log server", "uses a ring with a smaller maxlen than the one specified on the log directive ('%s' has maxlen = %d), logs will be truncated according to the lowest maxlen between the two.",
-                                                           section, section_name,
-                                                           logsrv->conf.file, logsrv->conf.line,
-                                                           logsrv->ring_name, sink->maxlen);
-                       }
-                       logsrv->sink = sink;
                }
-
+               else {
+                       memprintf(msg, "uses unknown ring named '%s'", target->ring_name);
+                       err_code |= ERR_ALERT | ERR_FATAL;
+                       goto end;
+               }
        }
+       else if (sink->type != SINK_TYPE_BUFFER) {
+               memprintf(msg, "uses incompatible ring '%s'", target->ring_name);
+               err_code |= ERR_ALERT | ERR_FATAL;
+               goto end;
+       }
+       if (sink && target->maxlen > ring_max_payload(sink->ctx.ring)) {
+               memprintf(msg, "uses a max length which exceeds ring capacity ('%s' supports %lu bytes at most)",
+                         target->ring_name, (unsigned long)ring_max_payload(sink->ctx.ring));
+       }
+       else if (sink && target->maxlen > sink->maxlen) {
+               memprintf(msg, "uses a ring with a smaller maxlen than the one specified on the log directive ('%s' has maxlen = %d), logs will be truncated according to the lowest maxlen between the two",
+                         target->ring_name, sink->maxlen);
+       }
+ end:
+       target->sink = sink;
        return err_code;
 }
 
-/* resolve sink names at end of config. Returns 0 on success otherwise error
- * flags.
-*/
-int post_sink_resolve()
-{
-       struct proxy *px;
-       int err_code = ERR_NONE;
-
-       /* global log directives */
-       err_code |= sink_postresolve_logsrvs(&global.logsrvs, NULL, NULL);
-       /* proxy log directives */
-       for (px = proxies_list; px; px = px->next)
-               err_code |= sink_postresolve_logsrvs(&px->logsrvs, "proxy", px->id);
-       /* log-forward log directives */
-       for (px = cfg_log_forward; px; px = px->next)
-               err_code |= sink_postresolve_logsrvs(&px->logsrvs, "log-forward", px->id);
-
-       return err_code;
-}
-
-
 static void sink_init()
 {
        sink_new_fd("stdout", "standard output (fd#1)", LOG_FORMAT_RAW, 1);
@@ -1390,7 +1345,6 @@ INITCALL1(STG_REGISTER, cli_register_kw, &cli_kws);
 
 /* config parsers for this section */
 REGISTER_CONFIG_SECTION("ring", cfg_parse_ring, cfg_post_parse_ring);
-REGISTER_POST_CHECK(post_sink_resolve);
 
 /*
  * Local variables: