The Samba-Bugzilla – Attachment 17367 Details for
Bug 15090
CTDB child process logging does not work as expected
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for v4-16-test
BZ15090.patch (text/plain), 6.75 KB, created by
Martin Schwenke
on 2022-06-16 22:57:19 UTC
(
hide
)
Description:
Patch for v4-16-test
Filename:
MIME Type:
Creator:
Martin Schwenke
Created:
2022-06-16 22:57:19 UTC
Size:
6.75 KB
patch
obsolete
>From 02e5b4a44c095e0a3fa5fc29369444b6f2b664e5 Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <martin@meltin.net> >Date: Tue, 7 Jun 2022 13:54:20 +1000 >Subject: [PATCH 1/4] util: Add new debug setting debug_no_stderr_redirect > >CTDB doesn't want this redirection of stderr to the log file. It >expects to be able to capture stderr of subprocesses and log them with >a header. This redirection stops that from happening. > >Unfortunately this has to be a negative option (i.e. "no" in the name) >so that the default of 0/false maintains existing behaviour. > >Note that the default behaviour is sub-optimal because it causes raw >data (i.e. debug data without a header) to appear in the log. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15090 > >Signed-off-by: Martin Schwenke <martin@meltin.net> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit a8091bd0c565a3f14542731e642319dbb68b4786) >--- > lib/util/debug.c | 3 ++- > lib/util/debug.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > >diff --git a/lib/util/debug.c b/lib/util/debug.c >index 784357e9370..3cd577dc0eb 100644 >--- a/lib/util/debug.c >+++ b/lib/util/debug.c >@@ -1315,7 +1315,8 @@ bool reopen_logs_internal(void) > * If log file was opened or created successfully, take over stderr to > * catch output into logs. > */ >- if (dbgc_config[DBGC_ALL].fd > 0) { >+ if (!state.settings.debug_no_stderr_redirect && >+ dbgc_config[DBGC_ALL].fd > 0) { > if (dup2(dbgc_config[DBGC_ALL].fd, 2) == -1) { > /* Close stderr too, if dup2 can't point it - > at the logfile. There really isn't much >diff --git a/lib/util/debug.h b/lib/util/debug.h >index 7317c2f43c5..34b63a24478 100644 >--- a/lib/util/debug.h >+++ b/lib/util/debug.h >@@ -304,6 +304,7 @@ struct debug_settings { > bool debug_pid; > bool debug_uid; > bool debug_class; >+ bool debug_no_stderr_redirect; > }; > > void setup_logging(const char *prog_name, enum debug_logtype new_logtype); >-- >2.35.1 > > >From 4d5c2bc8e22722d1111cd73bae143cd3e1204c1c Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <martin@meltin.net> >Date: Tue, 7 Jun 2022 14:00:49 +1000 >Subject: [PATCH 2/4] ctdb-common: Tell file logging not to redirect stderr > >This allows ctdb_set_child_logging() to work. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15090 > >Signed-off-by: Martin Schwenke <martin@meltin.net> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 1596a3e84babb8fdd86af0c4b98906b309be7907) >--- > ctdb/common/logging.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/ctdb/common/logging.c b/ctdb/common/logging.c >index 1b91cdcc92b..3aa5ca996ee 100644 >--- a/ctdb/common/logging.c >+++ b/ctdb/common/logging.c >@@ -148,6 +148,7 @@ static int file_log_setup(TALLOC_CTX *mem_ctx, > struct debug_settings settings = { > .debug_syslog_format = true, > .debug_hires_timestamp = true, >+ .debug_no_stderr_redirect = true, > }; > const char *t = NULL; > >-- >2.35.1 > > >From 119dd560dc268bb54de586cf84a2e50817b7dd47 Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <martin@meltin.net> >Date: Mon, 6 Jun 2022 17:57:51 +1000 >Subject: [PATCH 3/4] ctdb-daemon: Drop unused prefix, logfn, logfn_private > >These aren't set anywhere in the code. > >Drop the log argument because it is also no longer used. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15090 > >Signed-off-by: Martin Schwenke <martin@meltin.net> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 88f35cf86285b7a818282d5f465711de66dfad59) >--- > ctdb/server/ctdb_logging.c | 21 ++++----------------- > 1 file changed, 4 insertions(+), 17 deletions(-) > >diff --git a/ctdb/server/ctdb_logging.c b/ctdb/server/ctdb_logging.c >index 8af787c189f..6fcc0535ddd 100644 >--- a/ctdb/server/ctdb_logging.c >+++ b/ctdb/server/ctdb_logging.c >@@ -38,12 +38,9 @@ > #include "common/logging.h" > > struct ctdb_log_state { >- const char *prefix; > int fd, pfd; > char buf[1024]; > uint16_t buf_used; >- void (*logfn)(const char *, uint16_t, void *); >- void *logfn_private; > }; > > /* Used by ctdb_set_child_logging() */ >@@ -68,20 +65,10 @@ bool ctdb_logging_init(TALLOC_CTX *mem_ctx, const char *logging, > return true; > } > >-/* Note that do_debug always uses the global log state. */ >-static void write_to_log(struct ctdb_log_state *log, >- const char *buf, unsigned int len) >+static void write_to_log(const char *buf, unsigned int len) > { > if (script_log_level <= DEBUGLEVEL) { >- if (log != NULL && log->prefix != NULL) { >- dbgtext("%s: %*.*s\n", log->prefix, len, len, buf); >- } else { >- dbgtext("%*.*s\n", len, len, buf); >- } >- /* log it in the eventsystem as well */ >- if (log && log->logfn) { >- log->logfn(log->buf, len, log->logfn_private); >- } >+ dbgtext("%*.*s\n", len, len, buf); > } > } > >@@ -119,7 +106,7 @@ static void ctdb_child_log_handler(struct tevent_context *ev, > if (n2 > 0 && log->buf[n2-1] == '\r') { > n2--; > } >- write_to_log(log, log->buf, n2); >+ write_to_log(log->buf, n2); > memmove(log->buf, p+1, sizeof(log->buf) - n1); > log->buf_used -= n1; > } >@@ -127,7 +114,7 @@ static void ctdb_child_log_handler(struct tevent_context *ev, > /* the buffer could have completely filled - unfortunately we have > no choice but to dump it out straight away */ > if (log->buf_used == sizeof(log->buf)) { >- write_to_log(log, log->buf, log->buf_used); >+ write_to_log(log->buf, log->buf_used); > log->buf_used = 0; > } > } >-- >2.35.1 > > >From 5efd45266ddbb49f07b5f8766f497441904ba4ce Mon Sep 17 00:00:00 2001 >From: Martin Schwenke <martin@meltin.net> >Date: Mon, 6 Jun 2022 18:02:31 +1000 >Subject: [PATCH 4/4] ctdb-daemon: Use DEBUG() macro for child logging > >Directly using dbgtext() with file logging results in a log entry with >no header, which is wrong. This is a regression, introduced in commit >10d15c9e5dfe4e8595d0b322c96f474fc7078f46. Prior to this, CTDB's >callback for file logging would always add a header. > >Use DEBUG() instead dbgtext(). Note that DEBUG() effectively compares >the passed script_log_level with DEBUGLEVEL, so an explicit check is >no longer necessary. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15090 > >Signed-off-by: Martin Schwenke <martin@meltin.net> >Reviewed-by: Volker Lendecke <vl@samba.org> > >Autobuild-User(master): Volker Lendecke <vl@samba.org> >Autobuild-Date(master): Thu Jun 16 13:33:10 UTC 2022 on sn-devel-184 > >(cherry picked from commit e752f841e682cc571006c09249b03d82aea5f8cd) >--- > ctdb/server/ctdb_logging.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > >diff --git a/ctdb/server/ctdb_logging.c b/ctdb/server/ctdb_logging.c >index 6fcc0535ddd..1da26b5534c 100644 >--- a/ctdb/server/ctdb_logging.c >+++ b/ctdb/server/ctdb_logging.c >@@ -67,9 +67,7 @@ bool ctdb_logging_init(TALLOC_CTX *mem_ctx, const char *logging, > > static void write_to_log(const char *buf, unsigned int len) > { >- if (script_log_level <= DEBUGLEVEL) { >- dbgtext("%*.*s\n", len, len, buf); >- } >+ DEBUG(script_log_level, ("%*.*s\n", len, len, buf)); > } > > /* >-- >2.35.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
vl
:
review+
Actions:
View
Attachments on
bug 15090
: 17367