[pve-devel] [RFC cluster 1/3] pmxcfs: switch log domain to enum
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Oct 1 10:53:24 CEST 2020
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 at 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
More information about the pve-devel
mailing list