Chrissie Caulfield wrote:
> The IPC patch broke CFG shutdown in several places, this patches fixes
> all of them.
>
> In particular, cfg_try_shutdown asks all applications that are
> registered for callbacks if they approve the shutdown. This caused a bit
> of a re-entrancy problem because it also asked the process that called
> for the shutdown! The patch causes cfg to only ask OTHER applications in
> the assumption that any application that calls
> corosync_cfg_tryshutdown() will approve of the action :-)
>
> In addition it adds the response to cfg_replyto_shutdown which was
> missing (it couldn't be used with the old system but it mandatory now),
> and removes a double-free in the library finalise code.
Hmm sorry, that was the wrong patch ... good job nobody applied it!
The correct one is attached
Chrissie
Index: trunk/include/corosync/ipc_cfg.h
===================================================================
--- trunk/include/corosync/ipc_cfg.h (revision 1784)
+++ trunk/include/corosync/ipc_cfg.h (working copy)
@@ -69,7 +69,8 @@
MESSAGE_RES_CFG_TRYSHUTDOWN = 9,
MESSAGE_RES_CFG_TESTSHUTDOWN = 10,
MESSAGE_RES_CFG_GET_NODE_ADDRS = 11,
- MESSAGE_RES_CFG_LOCAL_GET = 12
+ MESSAGE_RES_CFG_LOCAL_GET = 12,
+ MESSAGE_RES_CFG_REPLYTOSHUTDOWN = 13
};
struct req_lib_cfg_statetrack {
Index: trunk/services/cfg.c
===================================================================
--- trunk/services/cfg.c (revision 1784)
+++ trunk/services/cfg.c (working copy)
@@ -237,8 +237,8 @@
},
{ /* 10 */
.lib_handler_fn = message_handler_req_lib_cfg_replytoshutdown,
- .response_size = 0,
- .response_id = 0,
+ .response_size = sizeof (struct res_lib_cfg_replytoshutdown),
+ .response_id = MESSAGE_RES_CFG_REPLYTOSHUTDOWN,
.flow_control = CS_LIB_FLOW_CONTROL_NOT_REQUIRED
},
{ /* 11 */
@@ -385,7 +385,7 @@
return 0;
}
-static void send_test_shutdown(void * conn, int status)
+static void send_test_shutdown(void *only_conn, void *exclude_conn, int status)
{
struct res_lib_cfg_testshutdown res_lib_cfg_testshutdown;
struct list_head *iter;
@@ -396,17 +396,19 @@
res_lib_cfg_testshutdown.header.error = status;
res_lib_cfg_testshutdown.flags = shutdown_flags;
- if (conn) {
- TRACE1("sending testshutdown to %p", conn);
- api->ipc_response_send(conn, &res_lib_cfg_testshutdown,
- sizeof(res_lib_cfg_testshutdown));
+ if (only_conn) {
+ TRACE1("sending testshutdown to only %p", only_conn);
+ api->ipc_dispatch_send(only_conn, &res_lib_cfg_testshutdown,
+ sizeof(res_lib_cfg_testshutdown));
} else {
for (iter = trackers_list.next; iter != &trackers_list; iter = iter->next) {
struct cfg_info *ci = list_entry(iter, struct cfg_info, list);
- TRACE1("sending testshutdown to %p", ci->tracker_conn);
- api->ipc_dispatch_send(ci->tracker_conn, &res_lib_cfg_testshutdown,
- sizeof(res_lib_cfg_testshutdown));
+ if (ci->conn != exclude_conn) {
+ TRACE1("sending testshutdown to %p", ci->tracker_conn);
+ api->ipc_dispatch_send(ci->tracker_conn, &res_lib_cfg_testshutdown,
+ sizeof(res_lib_cfg_testshutdown));
+ }
}
}
LEAVE();
@@ -436,11 +438,6 @@
shutdown_flags == CFG_SHUTDOWN_FLAG_REGARDLESS) {
TRACE1("shutdown confirmed");
- /*
- * Tell other nodes we are going down
- */
- send_shutdown();
-
res_lib_cfg_tryshutdown.header.size = sizeof(struct res_lib_cfg_tryshutdown);
res_lib_cfg_tryshutdown.header.id = MESSAGE_RES_CFG_TRYSHUTDOWN;
res_lib_cfg_tryshutdown.header.error = CS_OK;
@@ -451,6 +448,12 @@
api->ipc_response_send(shutdown_con->conn, &res_lib_cfg_tryshutdown,
sizeof(res_lib_cfg_tryshutdown));
shutdown_con = NULL;
+
+ /*
+ * Tell other nodes we are going down
+ */
+ send_shutdown();
+
}
else {
@@ -486,7 +489,7 @@
shutdown_no = shutdown_expected;
check_shutdown_status();
- send_test_shutdown(NULL, CS_ERR_TIMEOUT);
+ send_test_shutdown(NULL, NULL, CS_ERR_TIMEOUT);
LEAVE();
}
@@ -695,7 +698,6 @@
void *msg)
{
struct cfg_info *ci = (struct cfg_info *)api->ipc_private_data_get (conn);
-// struct req_lib_cfg_statetrack *req_lib_cfg_statetrack = (struct req_lib_cfg_statetrack *)message;
struct res_lib_cfg_statetrack res_lib_cfg_statetrack;
ENTER();
@@ -713,7 +715,7 @@
*/
ci->shutdown_reply = SHUTDOWN_REPLY_UNKNOWN;
shutdown_expected++;
- send_test_shutdown(conn, CS_OK);
+ send_test_shutdown(conn, NULL, CS_OK);
}
}
@@ -898,15 +900,32 @@
shutdown_expected = 0;
for (iter = trackers_list.next; iter != &trackers_list; iter = iter->next) {
- struct cfg_info *ci = list_entry(iter, struct cfg_info, list);
- ci->shutdown_reply = SHUTDOWN_REPLY_UNKNOWN;
- shutdown_expected++;
+ struct cfg_info *testci = list_entry(iter, struct cfg_info, list);
+ /*
+ * It is assumed that we will allow shutdown
+ */
+ if (testci != ci) {
+ testci->shutdown_reply = SHUTDOWN_REPLY_UNKNOWN;
+ shutdown_expected++;
+ }
}
/*
* If no-one is listening for events then we can just go down now
*/
if (shutdown_expected == 0) {
+ struct res_lib_cfg_tryshutdown res_lib_cfg_tryshutdown;
+
+ res_lib_cfg_tryshutdown.header.size = sizeof(struct res_lib_cfg_tryshutdown);
+ res_lib_cfg_tryshutdown.header.id = MESSAGE_RES_CFG_TRYSHUTDOWN;
+ res_lib_cfg_tryshutdown.header.error = CS_OK;
+
+ /*
+ * Tell originator that shutdown was confirmed
+ */
+ api->ipc_response_send(conn, &res_lib_cfg_tryshutdown,
+ sizeof(res_lib_cfg_tryshutdown));
+
send_shutdown();
LEAVE();
return;
@@ -944,7 +963,7 @@
/*
* Tell the users we would like to shut down
*/
- send_test_shutdown(NULL, CS_OK);
+ send_test_shutdown(NULL, conn, CS_OK);
}
/*
@@ -961,11 +980,13 @@
{
struct cfg_info *ci = (struct cfg_info *)api->ipc_private_data_get (conn);
struct req_lib_cfg_replytoshutdown *req_lib_cfg_replytoshutdown = (struct req_lib_cfg_replytoshutdown *)msg;
+ struct res_lib_cfg_replytoshutdown res_lib_cfg_replytoshutdown;
+ int status = CS_OK;
ENTER();
if (!shutdown_con) {
- LEAVE();
- return;
+ status = CS_ERR_ACCESS;
+ goto exit_fn;
}
if (req_lib_cfg_replytoshutdown->response) {
@@ -977,6 +998,15 @@
ci->shutdown_reply = SHUTDOWN_REPLY_NO;
}
check_shutdown_status();
+
+exit_fn:
+ res_lib_cfg_replytoshutdown.header.error = status;
+ res_lib_cfg_replytoshutdown.header.id = MESSAGE_RES_CFG_REPLYTOSHUTDOWN;
+ res_lib_cfg_replytoshutdown.header.size = sizeof(res_lib_cfg_replytoshutdown);
+
+ api->ipc_response_send(conn, &res_lib_cfg_replytoshutdown,
+ sizeof(res_lib_cfg_replytoshutdown));
+
LEAVE();
}
Index: trunk/tools/corosync-cfgtool.c
===================================================================
--- trunk/tools/corosync-cfgtool.c (revision 1784)
+++ trunk/tools/corosync-cfgtool.c (working copy)
@@ -147,36 +147,13 @@
(void)corosync_cfg_finalize (handle);
}
-void shutdown_callback (corosync_cfg_handle_t cfg_handle, corosync_cfg_shutdown_flags_t flags)
-{
- printf("shutdown callback called, flags = %d\n",flags);
-
- (void)corosync_cfg_replyto_shutdown (cfg_handle, COROSYNC_CFG_SHUTDOWN_FLAG_YES);
-}
-
-void *shutdown_dispatch_thread(void *arg)
-{
- int res = CS_OK;
- corosync_cfg_handle_t *handle = arg;
-
- while (res == CS_OK) {
- res = corosync_cfg_dispatch(*handle, CS_DISPATCH_ALL);
- if (res != CS_OK)
- printf ("Could not dispatch cfg messages: %d\n", res);
- }
- return NULL;
-}
-
void shutdown_do()
{
cs_error_t result;
corosync_cfg_handle_t handle;
corosync_cfg_callbacks_t callbacks;
- corosync_cfg_state_notification_t notification_buffer;
- pthread_t dispatch_thread;
- printf ("Shutting down corosync\n");
- callbacks.corosync_cfg_shutdown_callback = shutdown_callback;
+ callbacks.corosync_cfg_shutdown_callback = NULL;
result = corosync_cfg_initialize (&handle, &callbacks);
if (result != CS_OK) {
@@ -184,16 +161,7 @@
exit (1);
}
- pthread_create(&dispatch_thread, NULL, shutdown_dispatch_thread, &handle);
-
- result = corosync_cfg_state_track (handle,
- 0,
- ¬ification_buffer);
- if (result != CS_OK) {
- printf ("Could not start corosync cfg tracking error %d\n", result);
- exit (1);
- }
-
+ printf ("Shutting down corosync\n");
result = corosync_cfg_try_shutdown (handle, COROSYNC_CFG_SHUTDOWN_FLAG_REQUEST);
if (result != CS_OK) {
printf ("Could not shutdown (error = %d)\n", result);
@@ -262,7 +230,7 @@
void usage_do (void)
{
- printf ("corosync-cfgtool [-s] [-r] [-l] [-u] [service_name] [-v] [version] [-k] [nodeid] [-a] [nodeid]\n\n");
+ printf ("corosync-cfgtool [-s] [-r] [-l] [-u] [-H] [service_name] [-v] [version] [-k] [nodeid] [-a] [nodeid]\n\n");
printf ("A tool for displaying and configuring active parameters within corosync.\n");
printf ("options:\n");
printf ("\t-s\tDisplays the status of the current rings on this node.\n");
@@ -272,11 +240,11 @@
printf ("\t-u\tUnload a service identified by name.\n");
printf ("\t-a\tDisplay the IP address(es) of a node\n");
printf ("\t-k\tKill a node identified by node id.\n");
- printf ("\t-h\tShutdown corosync cleanly on this node.\n");
+ printf ("\t-H\tShutdown corosync cleanly on this node.\n");
}
int main (int argc, char *argv[]) {
- const char *options = "srl:u:v:k:a:h";
+ const char *options = "srl:u:v:k:a:hH";
int opt;
int service_load = 0;
unsigned int nodeid;
@@ -307,7 +275,7 @@
nodeid = atoi (optarg);
killnode_do(nodeid);
break;
- case 'h':
+ case 'H':
shutdown_do();
break;
case 'a':
@@ -316,6 +284,9 @@
case 'v':
version = atoi (optarg);
break;
+ case 'h':
+ usage_do();
+ break;
}
}
Index: trunk/lib/cfg.c
===================================================================
--- trunk/lib/cfg.c (revision 1784)
+++ trunk/lib/cfg.c (working copy)
@@ -295,8 +295,6 @@
pthread_mutex_destroy (&cfg_instance->dispatch_mutex);
- cslib_service_disconnect (&cfg_instance->ipc_ctx);
-
(void)saHandleDestroy (&cfg_hdb, cfg_handle);
(void)saHandleInstancePut (&cfg_hdb, cfg_handle);
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
