* [pve-devel] [PATCH cluster 1/2] pmxcfs: pretty print corosync error codes
@ 2025-06-05 14:17 Fabian Grünbichler
2025-06-05 14:17 ` [pve-devel] [PATCH cluster 2/2] fix #6434: pmxcfs: add context to *_initialize errors Fabian Grünbichler
0 siblings, 1 reply; 2+ messages in thread
From: Fabian Grünbichler @ 2025-06-05 14:17 UTC (permalink / raw)
To: pve-devel
this has always been a bit annoying to manually map via the header file, and
while the helper does not do much more at the moment than translate them back
to the enum keys, it is better than nothing.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/pmxcfs/Makefile | 2 +-
src/pmxcfs/confdb.c | 33 +++++++++++++++++----------------
src/pmxcfs/dfsm.c | 22 +++++++++++-----------
src/pmxcfs/quorum.c | 17 +++++++++--------
src/test/cpgtest.c | 15 ++++++++-------
5 files changed, 46 insertions(+), 43 deletions(-)
diff --git a/src/pmxcfs/Makefile b/src/pmxcfs/Makefile
index 4eb5378..547f258 100644
--- a/src/pmxcfs/Makefile
+++ b/src/pmxcfs/Makefile
@@ -1,5 +1,5 @@
-DEPENDENCIES=libcpg libcmap libquorum libqb glib-2.0 fuse sqlite3 librrd
+DEPENDENCIES=libcpg libcmap libcorosync_common libquorum libqb glib-2.0 fuse sqlite3 librrd
CC = gcc
CFLAGS += -std=gnu99
diff --git a/src/pmxcfs/confdb.c b/src/pmxcfs/confdb.c
index 967809a..e226fed 100644
--- a/src/pmxcfs/confdb.c
+++ b/src/pmxcfs/confdb.c
@@ -35,6 +35,7 @@
#include <unistd.h>
#include <corosync/cmap.h>
+#include <corosync/corotypes.h>
#include "cfs-utils.h"
#include "loop.h"
@@ -53,7 +54,7 @@ static cs_error_t cmap_read_clusternodes(cmap_handle_t handle, cfs_clinfo_t *cli
result = cmap_iter_init(handle, "nodelist.node.", &iter);
if (result != CS_OK) {
- cfs_critical("cmap_iter_init failed %d", result);
+ cfs_critical("cmap_iter_init failed %s", cs_strerror(result));
return result;
}
@@ -87,23 +88,23 @@ static cs_error_t cmap_read_clusternodes(cmap_handle_t handle, cfs_clinfo_t *cli
if (strcmp(subkey, "nodeid") == 0) {
if ((result = cmap_get_uint32(handle, key_name, &nodeid)) != CS_OK) {
- cfs_critical("cmap_get %s failed %d", key_name, result);
+ cfs_critical("cmap_get %s failed %s", key_name, cs_strerror(result));
}
} else if (strcmp(subkey, "quorum_votes") == 0) {
if ((result = cmap_get_uint32(handle, key_name, &votes)) != CS_OK) {
- cfs_critical("cmap_get %s failed %d", key_name, result);
+ cfs_critical("cmap_get %s failed %s", key_name, cs_strerror(result));
}
} else if (strcmp(subkey, "ring0_addr") == 0) {
// prefering the 'name' subkey over 'ring0_addr', needed for RRP
// and when using a IP address for ring0_addr
if (name == NULL && (result = cmap_get_string(handle, key_name, &name)) != CS_OK) {
- cfs_critical("cmap_get %s failed %d", key_name, result);
+ cfs_critical("cmap_get %s failed %s", key_name, cs_strerror(result));
}
} else if (strcmp(subkey, "name") == 0) {
free(name);
name = NULL;
if ((result = cmap_get_string(handle, key_name, &name)) != CS_OK) {
- cfs_critical("cmap_get %s failed %d", key_name, result);
+ cfs_critical("cmap_get %s failed %s", key_name, cs_strerror(result));
}
}
}
@@ -116,7 +117,7 @@ static cs_error_t cmap_read_clusternodes(cmap_handle_t handle, cfs_clinfo_t *cli
result = cmap_iter_finalize(handle, iter);
if (result != CS_OK) {
- cfs_critical("cmap_iter_finalize failed %d", result);
+ cfs_critical("cmap_iter_finalize failed %s", cs_strerror(result));
return result;
}
@@ -130,14 +131,14 @@ static cs_error_t cmap_read_config(cmap_handle_t handle) {
result = cmap_get_uint64(handle, "totem.config_version", &config_version);
if (result != CS_OK) {
- cfs_critical("cmap_get totem.config_version failed %d", result);
+ cfs_critical("cmap_get totem.config_version failed %s", cs_strerror(result));
// optional, do not throw error
}
char *clustername = NULL;
result = cmap_get_string(handle, "totem.cluster_name", &clustername);
if (result != CS_OK) {
- cfs_critical("cmap_get totem.cluster_name failed %d", result);
+ cfs_critical("cmap_get totem.cluster_name failed %s", cs_strerror(result));
return result;
}
@@ -165,7 +166,7 @@ static gboolean service_cmap_finalize(cfs_service_t *service, gpointer context)
if (private->track_nodelist_handle) {
result = cmap_track_delete(handle, private->track_nodelist_handle);
if (result != CS_OK) {
- cfs_critical("cmap_track_delete nodelist failed: %d", result);
+ cfs_critical("cmap_track_delete nodelist failed: %s", cs_strerror(result));
}
private->track_nodelist_handle = 0;
}
@@ -173,7 +174,7 @@ static gboolean service_cmap_finalize(cfs_service_t *service, gpointer context)
if (private->track_version_handle) {
result = cmap_track_delete(handle, private->track_version_handle);
if (result != CS_OK) {
- cfs_critical("cmap_track_delete version failed: %d", result);
+ cfs_critical("cmap_track_delete version failed: %s", cs_strerror(result));
}
private->track_version_handle = 0;
}
@@ -181,7 +182,7 @@ static gboolean service_cmap_finalize(cfs_service_t *service, gpointer context)
result = cmap_finalize(handle);
private->handle = 0;
if (result != CS_OK) {
- cfs_critical("cmap_finalize failed: %d", result);
+ cfs_critical("cmap_finalize failed: %s", cs_strerror(result));
return FALSE;
}
@@ -220,14 +221,14 @@ static int service_cmap_initialize(cfs_service_t *service, gpointer context) {
result = cmap_initialize(&handle);
if (result != CS_OK) {
- cfs_critical("cmap_initialize failed: %d", result);
+ cfs_critical("cmap_initialize failed: %s", cs_strerror(result));
private->handle = 0;
return -1;
}
result = cmap_context_set(handle, private);
if (result != CS_OK) {
- cfs_critical("cmap_context_set failed: %d", result);
+ cfs_critical("cmap_context_set failed: %s", cs_strerror(result));
cmap_finalize(handle);
private->handle = 0;
return -1;
@@ -250,18 +251,18 @@ static int service_cmap_initialize(cfs_service_t *service, gpointer context) {
}
if (result == CS_ERR_LIBRARY || result == CS_ERR_BAD_HANDLE) {
- cfs_critical("cmap_track_changes failed: %d - closing handle", result);
+ cfs_critical("cmap_track_changes failed: %s - closing handle", cs_strerror(result));
cmap_finalize(handle);
private->handle = 0;
return -1;
} else if (result != CS_OK) {
- cfs_critical("cmap_track_changes failed: %d - trying again", result);
+ cfs_critical("cmap_track_changes failed: %s - trying again", cs_strerror(result));
return -1;
}
int cmap_fd = -1;
if ((result = cmap_fd_get(handle, &cmap_fd)) != CS_OK) {
- cfs_critical("confdb_fd_get failed %d - trying again", result);
+ cfs_critical("confdb_fd_get failed %s - trying again", cs_strerror(result));
return -1;
}
diff --git a/src/pmxcfs/dfsm.c b/src/pmxcfs/dfsm.c
index 4c94f18..9228ddc 100644
--- a/src/pmxcfs/dfsm.c
+++ b/src/pmxcfs/dfsm.c
@@ -205,7 +205,7 @@ loop:
}
if (result != CS_OK && (!retry || result != CS_ERR_TRY_AGAIN)) {
- cfs_dom_critical(dfsm->log_domain, "cpg_send_message failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_send_message failed: %s", cs_strerror(result));
}
return result;
@@ -287,7 +287,7 @@ cs_error_t dfsm_send_message_sync(
g_mutex_unlock(&dfsm->sync_mutex);
if (result != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_send_message failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_send_message failed: %s", cs_strerror(result));
if (rp) {
g_mutex_lock(&dfsm->sync_mutex);
@@ -567,7 +567,7 @@ static void dfsm_cpg_deliver_callback(
dfsm_t *dfsm = NULL;
result = cpg_context_get(handle, (gpointer *)&dfsm);
if (result != CS_OK || !dfsm || dfsm->cpg_callbacks != &cpg_callbacks) {
- cfs_critical("cpg_context_get error: %d (%p)", result, (void *)dfsm);
+ cfs_critical("cpg_context_get error: %s (%p)", cs_strerror(result), (void *)dfsm);
return; /* we have no valid dfsm pointer, so we can just ignore this */
}
dfsm_mode_t mode = dfsm_get_mode(dfsm);
@@ -1079,7 +1079,7 @@ static void dfsm_cpg_confchg_callback(
dfsm_t *dfsm = NULL;
result = cpg_context_get(handle, (gpointer *)&dfsm);
if (result != CS_OK || !dfsm || dfsm->cpg_callbacks != &cpg_callbacks) {
- cfs_critical("cpg_context_get error: %d (%p)", result, (void *)dfsm);
+ cfs_critical("cpg_context_get error: %s (%p)", cs_strerror(result), (void *)dfsm);
return; /* we have no valid dfsm pointer, so we can just ignore this */
}
@@ -1328,7 +1328,7 @@ loop:
}
if (!(result == CS_OK || result == CS_ERR_TRY_AGAIN)) {
- cfs_dom_critical(dfsm->log_domain, "cpg_dispatch failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_dispatch failed: %s", cs_strerror(result));
}
return result;
@@ -1351,12 +1351,12 @@ cs_error_t dfsm_initialize(dfsm_t *dfsm, int *fd) {
if (dfsm->cpg_handle == 0) {
if ((result = cpg_initialize(&dfsm->cpg_handle, dfsm->cpg_callbacks)) != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_initialize failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_initialize failed: %s", cs_strerror(result));
goto err_no_finalize;
}
if ((result = cpg_local_get(dfsm->cpg_handle, &dfsm->nodeid)) != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_local_get failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_local_get failed: %s", cs_strerror(result));
goto err_finalize;
}
@@ -1364,14 +1364,14 @@ cs_error_t dfsm_initialize(dfsm_t *dfsm, int *fd) {
result = cpg_context_set(dfsm->cpg_handle, dfsm);
if (result != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_context_set failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_context_set failed: %s", cs_strerror(result));
goto err_finalize;
}
}
result = cpg_fd_get(dfsm->cpg_handle, fd);
if (result != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_fd_get failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_fd_get failed: %s", cs_strerror(result));
goto err_finalize;
}
@@ -1407,7 +1407,7 @@ loop:
}
if (result != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_join failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_join failed: %s", cs_strerror(result));
return result;
}
@@ -1437,7 +1437,7 @@ loop:
}
if (result != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_leave failed: %d", result);
+ cfs_dom_critical(dfsm->log_domain, "cpg_leave failed: %s", cs_strerror(result));
return result;
}
diff --git a/src/pmxcfs/quorum.c b/src/pmxcfs/quorum.c
index cff5e06..07b2a39 100644
--- a/src/pmxcfs/quorum.c
+++ b/src/pmxcfs/quorum.c
@@ -30,6 +30,7 @@
#include <string.h>
#include <unistd.h>
+#include <corosync/corotypes.h>
#include <corosync/quorum.h>
#include "cfs-utils.h"
@@ -59,7 +60,7 @@ static void quorum_notification_fn(
result = quorum_context_get(handle, (gconstpointer *)&private);
if (result != CS_OK || !private) {
- cfs_critical("quorum_context_get error: %d (%p)", result, (void *)private);
+ cfs_critical("quorum_context_get error: %s (%p)", cs_strerror(result), (void *)private);
return;
}
@@ -84,7 +85,7 @@ static gboolean service_quorum_finalize(cfs_service_t *service, gpointer context
result = quorum_finalize(handle);
private->handle = 0;
if (result != CS_OK) {
- cfs_critical("quorum_finalize failed: %d", result);
+ cfs_critical("quorum_finalize failed: %s", cs_strerror(result));
return FALSE;
}
@@ -106,7 +107,7 @@ static int service_quorum_initialize(cfs_service_t *service, gpointer context) {
result = quorum_initialize(&handle, &quorum_callbacks, &quorum_type);
if (result != CS_OK) {
- cfs_critical("quorum_initialize failed: %d", result);
+ cfs_critical("quorum_initialize failed: %s", cs_strerror(result));
goto err_reset_handle;
}
@@ -117,7 +118,7 @@ static int service_quorum_initialize(cfs_service_t *service, gpointer context) {
result = quorum_context_set(handle, private);
if (result != CS_OK) {
- cfs_critical("quorum_context_set failed: %d", result);
+ cfs_critical("quorum_context_set failed: %s", cs_strerror(result));
goto err_finalize;
}
@@ -126,16 +127,16 @@ static int service_quorum_initialize(cfs_service_t *service, gpointer context) {
result = quorum_trackstart(handle, CS_TRACK_CHANGES);
if (result == CS_ERR_LIBRARY || result == CS_ERR_BAD_HANDLE) {
- cfs_critical("quorum_trackstart failed: %d - closing handle", result);
+ cfs_critical("quorum_trackstart failed: %s - closing handle", cs_strerror(result));
goto err_finalize;
} else if (result != CS_OK) {
- cfs_critical("quorum_trackstart failed: %d - trying again", result);
+ cfs_critical("quorum_trackstart failed: %s - trying again", cs_strerror(result));
return -1;
}
int quorum_fd = -1;
if ((result = quorum_fd_get(handle, &quorum_fd)) != CS_OK) {
- cfs_critical("quorum_fd_get failed %d - trying again", result);
+ cfs_critical("quorum_fd_get failed %s - trying again", cs_strerror(result));
return -1;
}
@@ -175,7 +176,7 @@ loop:
return TRUE;
}
- cfs_critical("quorum_dispatch failed: %d", result);
+ cfs_critical("quorum_dispatch failed: %s", cs_strerror(result));
cfs_set_quorate(0, FALSE);
quorum_finalize(handle);
diff --git a/src/test/cpgtest.c b/src/test/cpgtest.c
index 10e42a9..82f577e 100644
--- a/src/test/cpgtest.c
+++ b/src/test/cpgtest.c
@@ -82,8 +82,9 @@ loop:
goto loop;
}
- if (result != CS_OK)
- printf("cpg_send_message failed: %d\n", result);
+ if (result != CS_OK) {
+ printf("cpg_send_message failed: %s\n", cs_strerror(result));
+ }
}
static cpg_callbacks_t callbacks = {
@@ -110,18 +111,18 @@ start:
printf("calling cpg_initialize\n");
result = cpg_initialize(&handle, &callbacks);
if (result != CS_OK) {
- printf("cpg_initialize failed: %d\n", result);
+ printf("cpg_initialize failed: %s\n", cs_strerror(result));
goto retry;
}
printf("calling cpg_join\n");
while ((result = cpg_join(handle, &group_name)) == CS_ERR_TRY_AGAIN) {
- printf("cpg_join returned %d\n", result);
+ printf("cpg_join returned %s\n", cs_strerror(result));
sleep(1);
}
if (result != CS_OK) {
- printf("cpg_join failed: %d\n", result);
+ printf("cpg_join failed: %s\n", cs_strerror(result));
exit(-1);
}
@@ -147,7 +148,7 @@ start:
if (FD_ISSET(cpg_fd, &read_fds)) {
cs_error_t res = cpg_dispatch(handle, CS_DISPATCH_ALL);
if (res != CS_OK) {
- printf("cpg_dispatch failed: %d\n", res);
+ printf("cpg_dispatch failed: %s\n", cs_strerror(res));
break;
}
}
@@ -168,7 +169,7 @@ retry:
result = cpg_finalize(handle);
if (result != CS_OK) {
- printf("cpg_finalize failed: %d\n", result);
+ printf("cpg_finalize failed: %s\n", cs_strerror(result));
exit(-1);
}
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
* [pve-devel] [PATCH cluster 2/2] fix #6434: pmxcfs: add context to *_initialize errors
2025-06-05 14:17 [pve-devel] [PATCH cluster 1/2] pmxcfs: pretty print corosync error codes Fabian Grünbichler
@ 2025-06-05 14:17 ` Fabian Grünbichler
0 siblings, 0 replies; 2+ messages in thread
From: Fabian Grünbichler @ 2025-06-05 14:17 UTC (permalink / raw)
To: pve-devel
in case initializing fails with CS_ERR_LIBRARY, the connection to corosync
failed - add that context to make the error message a bit less cryptic.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/pmxcfs/confdb.c | 9 ++++++++-
src/pmxcfs/dfsm.c | 11 ++++++++++-
src/pmxcfs/quorum.c | 9 ++++++++-
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/src/pmxcfs/confdb.c b/src/pmxcfs/confdb.c
index e226fed..3479e52 100644
--- a/src/pmxcfs/confdb.c
+++ b/src/pmxcfs/confdb.c
@@ -221,7 +221,14 @@ static int service_cmap_initialize(cfs_service_t *service, gpointer context) {
result = cmap_initialize(&handle);
if (result != CS_OK) {
- cfs_critical("cmap_initialize failed: %s", cs_strerror(result));
+ if (result == CS_ERR_LIBRARY) {
+ cfs_critical(
+ "cmap_initialize failed: %s (failed to connect to corosync)",
+ cs_strerror(result)
+ );
+ } else {
+ cfs_critical("cmap_initialize failed: %s", cs_strerror(result));
+ }
private->handle = 0;
return -1;
}
diff --git a/src/pmxcfs/dfsm.c b/src/pmxcfs/dfsm.c
index 9228ddc..c6dfb68 100644
--- a/src/pmxcfs/dfsm.c
+++ b/src/pmxcfs/dfsm.c
@@ -1351,7 +1351,16 @@ cs_error_t dfsm_initialize(dfsm_t *dfsm, int *fd) {
if (dfsm->cpg_handle == 0) {
if ((result = cpg_initialize(&dfsm->cpg_handle, dfsm->cpg_callbacks)) != CS_OK) {
- cfs_dom_critical(dfsm->log_domain, "cpg_initialize failed: %s", cs_strerror(result));
+ if (result == CS_ERR_LIBRARY) {
+ cfs_dom_critical(
+ dfsm->log_domain, "cpg_initialize failed: %s (failed to connect to corosync)",
+ cs_strerror(result)
+ );
+ } else {
+ cfs_dom_critical(
+ dfsm->log_domain, "cpg_initialize failed: %s", cs_strerror(result)
+ );
+ }
goto err_no_finalize;
}
diff --git a/src/pmxcfs/quorum.c b/src/pmxcfs/quorum.c
index 07b2a39..e2ec0a3 100644
--- a/src/pmxcfs/quorum.c
+++ b/src/pmxcfs/quorum.c
@@ -107,7 +107,14 @@ static int service_quorum_initialize(cfs_service_t *service, gpointer context) {
result = quorum_initialize(&handle, &quorum_callbacks, &quorum_type);
if (result != CS_OK) {
- cfs_critical("quorum_initialize failed: %s", cs_strerror(result));
+ if (result == CS_ERR_LIBRARY) {
+ cfs_critical(
+ "quorum_initialize failed: %s (failed to connect to corosync)",
+ cs_strerror(result)
+ );
+ } else {
+ cfs_critical("quorum_initialize failed: %s", cs_strerror(result));
+ }
goto err_reset_handle;
}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-06-05 14:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-05 14:17 [pve-devel] [PATCH cluster 1/2] pmxcfs: pretty print corosync error codes Fabian Grünbichler
2025-06-05 14:17 ` [pve-devel] [PATCH cluster 2/2] fix #6434: pmxcfs: add context to *_initialize errors Fabian Grünbichler
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.