From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC cluster 1/3] pmxcfs: switch log domain to enum
Date: Thu, 1 Oct 2020 10:53:24 +0200 [thread overview]
Message-ID: <20201001085326.3815261-2-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20201001085326.3815261-1-f.gruenbichler@proxmox.com>
GQuarks does not work correctly across threads, which can cause
misattribution to log domains, e.g.
[status] notice: members: 1/2116, 2/2454111
[status] notice: starting data syncronisation
[status] notice: members: 1/2116, 2/2454111
[status] notice: starting data syncronisation
[dcdb] notice: received sync request (epoch 1/2116/00000036)
[status] notice: received sync request (epoch 1/2116/00000036)
[dcdb] notice: received all states
[dcdb] notice: leader is 1/2116
[dcdb] notice: synced members: 1/2116, 2/2454111
[dcdb] notice: all data is up to date
[status] notice: received all states
[status] notice: all data is up to date
gets logged instead of
[dcdb] notice: members: 1/2116, 2/2454111, 3/1254
[dcdb] notice: starting data syncronisation
[status] notice: members: 1/2116, 2/2454111, 3/1254
[status] notice: starting data syncronisation
[dcdb] notice: received sync request (epoch 1/2116/00000035)
[status] notice: received sync request (epoch 1/2116/00000035)
[dcdb] notice: received all states
[dcdb] notice: leader is 1/2116
[dcdb] notice: synced members: 1/2116, 2/2454111
[dcdb] notice: all data is up to date
[status] notice: received all states
[status] notice: all data is up to date
this only happens with threaded logging (not yet enabled).
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
not entirely happy with this approach - open for more elegant
suggestions ;)
data/src/cfs-utils.h | 36 ++++++++++++++++++++++++++++++++----
data/src/dfsm.h | 6 +++---
data/src/loop.h | 2 +-
data/src/cfs-utils.c | 4 ++--
data/src/confdb.c | 2 +-
data/src/dcdb.c | 2 +-
data/src/dfsm.c | 4 ++--
data/src/loop.c | 10 +++++-----
data/src/pmxcfs.c | 14 +++++---------
data/src/quorum.c | 2 +-
data/src/status.c | 2 +-
11 files changed, 54 insertions(+), 30 deletions(-)
diff --git a/data/src/cfs-utils.h b/data/src/cfs-utils.h
index eb86379..51cfef0 100644
--- a/data/src/cfs-utils.h
+++ b/data/src/cfs-utils.h
@@ -37,6 +37,34 @@
#define CFS_MAX(a, b) (((a) > (b)) ? (a) : (b))
#define CFS_MIN(a, b) (((a) < (b)) ? (a) : (b))
+typedef enum {
+ CFS_LOG_MAIN = 0,
+ CFS_LOG_LOOP = 1,
+ CFS_LOG_IPCS = 2,
+ CFS_LOG_GLIB = 3,
+ CFS_LOG_CONFDB = 4,
+ CFS_LOG_QUORUM = 5,
+ CFS_LOG_DB = 6,
+ CFS_LOG_STATUS = 7,
+ CFS_LOG_DCDB = 8,
+ CFS_LOG_UNKNOWN = 9,
+ CFS_LOG_LAST_VALUE = 10,
+} cfs_log_domain_t;
+
+static const char * const cfs_log_domain_strings[] = {
+ "main",
+ "loop",
+ "ipcs",
+ "glib",
+ "confdb",
+ "quorum",
+ "database",
+ "status",
+ "dcdb",
+ "unknown",
+ "invalid",
+};
+
typedef struct {
char *nodename;
char *ip;
@@ -55,7 +83,7 @@ utf8_to_ascii(
void
cfs_log(
- const gchar *log_domain,
+ cfs_log_domain_t log_domain,
GLogLevelFlags log_level,
const char *file,
int line,
@@ -72,7 +100,7 @@ void ipc_log_fn(
#define cfs_debug(...) G_STMT_START { \
if (cfs.debug) \
- cfs_log(G_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__); \
+ cfs_log(CFS_LOG_UNKNOWN, G_LOG_LEVEL_DEBUG, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__); \
} G_STMT_END
#define cfs_dom_debug(domain, ...) G_STMT_START { \
@@ -80,9 +108,9 @@ void ipc_log_fn(
cfs_log(domain, G_LOG_LEVEL_DEBUG, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__); \
} G_STMT_END
-#define cfs_critical(...) cfs_log(G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__)
+#define cfs_critical(...) cfs_log(CFS_LOG_UNKNOWN, G_LOG_LEVEL_CRITICAL, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__)
#define cfs_dom_critical(domain, ...) cfs_log(domain, G_LOG_LEVEL_CRITICAL, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__)
-#define cfs_message(...) cfs_log(G_LOG_DOMAIN, G_LOG_LEVEL_MESSAGE, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__)
+#define cfs_message(...) cfs_log(CFS_LOG_UNKNOWN, G_LOG_LEVEL_MESSAGE, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__)
#define cfs_dom_message(domain, ...) cfs_log(domain, G_LOG_LEVEL_MESSAGE, __FILE__, __LINE__, G_STRFUNC, __VA_ARGS__)
guint64
diff --git a/data/src/dfsm.h b/data/src/dfsm.h
index 858127f..79d20a4 100644
--- a/data/src/dfsm.h
+++ b/data/src/dfsm.h
@@ -120,9 +120,9 @@ typedef struct {
dfsm_t *
dfsm_new(
gpointer data,
- const char *group_name,
- const char *log_domain,
- guint32 protocol_version,
+ const char *group_name,
+ cfs_log_domain_t log_domain,
+ guint32 protocol_version,
dfsm_callbacks_t *callbacks);
void
diff --git a/data/src/loop.h b/data/src/loop.h
index 68b783c..b1e39e2 100644
--- a/data/src/loop.h
+++ b/data/src/loop.h
@@ -58,7 +58,7 @@ typedef struct {
cfs_service_t *cfs_service_new(
cfs_service_callbacks_t *callbacks,
- const char *log_domain,
+ cfs_log_domain_t log_domain,
gpointer context);
gpointer cfs_service_get_context(
diff --git a/data/src/cfs-utils.c b/data/src/cfs-utils.c
index 5770833..134c1cb 100644
--- a/data/src/cfs-utils.c
+++ b/data/src/cfs-utils.c
@@ -108,7 +108,7 @@ utf8_to_ascii(
void
cfs_log(
- const gchar *log_domain,
+ cfs_log_domain_t log_domain,
GLogLevelFlags log_level,
const char *file,
int line,
@@ -152,7 +152,7 @@ cfs_log(
char msg[8192];
utf8_to_ascii(msg, sizeof(msg), orgmsg, FALSE);
- uint32_t tag = g_quark_from_string(log_domain);
+ uint32_t tag = (uint32_t) log_domain;
qb_log_from_external_source(func, file, "%s", level, line, tag, msg);
diff --git a/data/src/confdb.c b/data/src/confdb.c
index 839f576..7c4c992 100644
--- a/data/src/confdb.c
+++ b/data/src/confdb.c
@@ -344,7 +344,7 @@ service_confdb_new(void)
if (!private)
return NULL;
- service = cfs_service_new(&cfs_confdb_callbacks, G_LOG_DOMAIN, private);
+ service = cfs_service_new(&cfs_confdb_callbacks, CFS_LOG_CONFDB, private);
return service;
}
diff --git a/data/src/dcdb.c b/data/src/dcdb.c
index b690355..b4f4359 100644
--- a/data/src/dcdb.c
+++ b/data/src/dcdb.c
@@ -949,6 +949,6 @@ dfsm_t *dcdb_new(memdb_t *memdb)
{
g_return_val_if_fail(memdb != NULL, NULL);
- return dfsm_new(memdb, DCDB_CPG_GROUP_NAME, G_LOG_DOMAIN,
+ return dfsm_new(memdb, DCDB_CPG_GROUP_NAME, CFS_LOG_DCDB,
DCDB_PROTOCOL_VERSION, &dcdb_dfsm_callbacks);
}
diff --git a/data/src/dfsm.c b/data/src/dfsm.c
index ddfcc23..ab761e1 100644
--- a/data/src/dfsm.c
+++ b/data/src/dfsm.c
@@ -103,7 +103,7 @@ typedef struct {
} dfsm_queued_message_t;
struct dfsm {
- const char *log_domain;
+ cfs_log_domain_t log_domain;
cpg_callbacks_t *cpg_callbacks;
dfsm_callbacks_t *dfsm_callbacks;
cpg_handle_t cpg_handle;
@@ -1223,7 +1223,7 @@ dfsm_t *
dfsm_new(
gpointer data,
const char *group_name,
- const char *log_domain,
+ cfs_log_domain_t log_domain,
guint32 protocol_version,
dfsm_callbacks_t *callbacks)
{
diff --git a/data/src/loop.c b/data/src/loop.c
index a80fc66..09c8590 100644
--- a/data/src/loop.c
+++ b/data/src/loop.c
@@ -40,8 +40,8 @@
#include "loop.h"
struct cfs_service {
- qb_loop_t *qbloop;
- const char *log_domain;
+ qb_loop_t *qbloop;
+ cfs_log_domain_t log_domain;
cfs_service_callbacks_t *callbacks;
gboolean restartable;
gpointer context;
@@ -98,7 +98,7 @@ cfs_service_set_restartable(
cfs_service_t *
cfs_service_new(
cfs_service_callbacks_t *callbacks,
- const char *log_domain,
+ cfs_log_domain_t log_domain,
gpointer context)
{
g_return_val_if_fail(callbacks != NULL, NULL);
@@ -113,7 +113,7 @@ cfs_service_new(
if (log_domain)
service->log_domain = log_domain;
else
- service->log_domain = G_LOG_DOMAIN;
+ service->log_domain = CFS_LOG_LOOP;
service->callbacks = callbacks;
@@ -170,7 +170,7 @@ cfs_loop_add_service(
{
g_return_val_if_fail(loop != NULL, FALSE);
g_return_val_if_fail(service != NULL, FALSE);
- g_return_val_if_fail(service->log_domain != NULL, FALSE);
+ g_return_val_if_fail(service->log_domain < CFS_LOG_LAST_VALUE, FALSE);
service->priority = priority;
service->qbloop = loop->qbloop;
diff --git a/data/src/pmxcfs.c b/data/src/pmxcfs.c
index b6d6576..d5a3aac 100644
--- a/data/src/pmxcfs.c
+++ b/data/src/pmxcfs.c
@@ -81,7 +81,7 @@ static void glib_log_handler(const gchar *log_domain,
gpointer user_data)
{
- cfs_log(log_domain, log_level, NULL, 0, NULL, "%s", message);
+ cfs_log(CFS_LOG_GLIB, log_level, NULL, 0, NULL, "%s - %s", log_domain, message);
}
static gboolean write_pidfile(pid_t pid)
@@ -760,15 +760,11 @@ static const char*
log_tags_stringify(uint32_t tags) {
if (qb_bit_is_set(tags, QB_LOG_TAG_LIBQB_MSG_BIT)) {
return "libqb";
+ } else if (tags >= CFS_LOG_LAST_VALUE) {
+ return cfs_log_domain_strings[CFS_LOG_MAIN];
} else {
- GQuark quark = tags;
- const char *domain = g_quark_to_string(quark);
- if (domain != NULL) {
- return domain;
- } else {
- return "main";
- }
- }
+ return cfs_log_domain_strings[tags];
+ }
}
int main(int argc, char *argv[])
diff --git a/data/src/quorum.c b/data/src/quorum.c
index 9df0e90..0a75f7f 100644
--- a/data/src/quorum.c
+++ b/data/src/quorum.c
@@ -202,7 +202,7 @@ cfs_service_t *service_quorum_new(void)
if (!private)
return NULL;
- service = cfs_service_new(&cfs_quorum_callbacks, G_LOG_DOMAIN, private);
+ service = cfs_service_new(&cfs_quorum_callbacks, CFS_LOG_QUORUM, private);
return service;
}
diff --git a/data/src/status.c b/data/src/status.c
index e15a257..004f02e 100644
--- a/data/src/status.c
+++ b/data/src/status.c
@@ -1834,7 +1834,7 @@ cfs_status_dfsm_new(void)
{
g_mutex_lock (&mutex);
- cfs_status.kvstore = dfsm_new(NULL, KVSTORE_CPG_GROUP_NAME, G_LOG_DOMAIN,
+ cfs_status.kvstore = dfsm_new(NULL, KVSTORE_CPG_GROUP_NAME, CFS_LOG_STATUS,
0, &kvstore_dfsm_callbacks);
g_mutex_unlock (&mutex);
--
2.20.1
next prev parent reply other threads:[~2020-10-01 8:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 8:53 [pve-devel] [PATCH cluster 0/3] logging improvements Fabian Grünbichler
2020-10-01 8:53 ` Fabian Grünbichler [this message]
2020-10-01 8:53 ` [pve-devel] [PATCH cluster 2/3] pmxcfs: set log domain in more places Fabian Grünbichler
2020-10-01 8:53 ` [pve-devel] [PATCH cluster 3/3] pmxcfs: enable QB log thread Fabian Grünbichler
2021-04-22 19:45 ` [pve-devel] [PATCH cluster 0/3] logging improvements Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201001085326.3815261-2-f.gruenbichler@proxmox.com \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.