From d76fc6ab85c1ca15975a99172ecea2c2a7a1f78c Mon Sep 17 00:00:00 2001 From: Jan Friesse Date: Tue, 24 Nov 2020 12:20:25 +0100 Subject: [PATCH] cfg: Improve nodestatusget versioning Patch tries to make nodestatusget really extendable. Following changes are implemented: - corosync_cfg_node_status_version_t is added with (for now) single value CFG_NODE_STATUS_V1 - corosync_knet_node_status renamed to corosync_cfg_node_status_v1 (it isn't really knet because it works as well for udp(u() - struct res_lib_cfg_nodestatusget_version is added which holds only ipc result header and version on same position as for corosync_cfg_node_status_v1 - corosync_cfg_node_status_get requires version and pointer to one of corosync_cfg_node_status_v structures - request is handled in case switches to make adding new version easier Also fix following bugs: - totempg_nodestatus_get error was retyped to cs_error_t without any meaning. - header.error was not checked at all in the library Signed-off-by: Jan Friesse Reviewed-by: Christine Caulfield --- exec/cfg.c | 93 ++++++++++++++++++++++++-------------- include/corosync/cfg.h | 17 ++++--- include/corosync/ipc_cfg.h | 9 +++- lib/cfg.c | 52 ++++++++++++++++----- tools/corosync-cfgtool.c | 8 ++-- 5 files changed, 121 insertions(+), 58 deletions(-) diff --git a/exec/cfg.c b/exec/cfg.c index c300cc8f..991baf22 100644 --- a/exec/cfg.c +++ b/exec/cfg.c @@ -971,53 +971,76 @@ static void message_handler_req_lib_cfg_nodestatusget ( void *conn, const void *msg) { - struct res_lib_cfg_nodestatusget res_lib_cfg_nodestatusget; + struct res_lib_cfg_nodestatusget_version res_lib_cfg_nodestatusget_version; + struct res_lib_cfg_nodestatusget_v1 res_lib_cfg_nodestatusget_v1; + void *res_lib_cfg_nodestatusget_ptr = NULL; + size_t res_lib_cfg_nodestatusget_size; struct req_lib_cfg_nodestatusget *req_lib_cfg_nodestatusget = (struct req_lib_cfg_nodestatusget *)msg; struct totem_node_status node_status; - cs_error_t res = CS_OK; int i; ENTER(); - /* Currently only one structure version supported */ - if (req_lib_cfg_nodestatusget->version == TOTEM_NODE_STATUS_STRUCTURE_VERSION) - { - res_lib_cfg_nodestatusget.header.id = MESSAGE_RES_CFG_NODESTATUSGET; - res_lib_cfg_nodestatusget.header.size = sizeof (struct res_lib_cfg_nodestatusget); + memset(&node_status, 0, sizeof(node_status)); + if (totempg_nodestatus_get(req_lib_cfg_nodestatusget->nodeid, &node_status) != 0) { + res_lib_cfg_nodestatusget_ptr = &res_lib_cfg_nodestatusget_version; + res_lib_cfg_nodestatusget_size = sizeof(res_lib_cfg_nodestatusget_version); - memset(&node_status, 0, sizeof(node_status)); - res = totempg_nodestatus_get(req_lib_cfg_nodestatusget->nodeid, - &node_status); - if (res == 0) { - res_lib_cfg_nodestatusget.node_status.nodeid = req_lib_cfg_nodestatusget->nodeid; - res_lib_cfg_nodestatusget.node_status.version = node_status.version; - res_lib_cfg_nodestatusget.node_status.reachable = node_status.reachable; - res_lib_cfg_nodestatusget.node_status.remote = node_status.remote; - res_lib_cfg_nodestatusget.node_status.external = node_status.external; - res_lib_cfg_nodestatusget.node_status.onwire_min = node_status.onwire_min; - res_lib_cfg_nodestatusget.node_status.onwire_max = node_status.onwire_max; - res_lib_cfg_nodestatusget.node_status.onwire_ver= node_status.onwire_ver; + res_lib_cfg_nodestatusget_version.header.error = CS_ERR_FAILED_OPERATION; + res_lib_cfg_nodestatusget_version.header.id = MESSAGE_RES_CFG_NODESTATUSGET; + res_lib_cfg_nodestatusget_version.header.size = res_lib_cfg_nodestatusget_size; - for (i=0; i < KNET_MAX_LINK; i++) { - res_lib_cfg_nodestatusget.node_status.link_status[i].enabled = node_status.link_status[i].enabled; - res_lib_cfg_nodestatusget.node_status.link_status[i].connected = node_status.link_status[i].connected; - res_lib_cfg_nodestatusget.node_status.link_status[i].dynconnected = node_status.link_status[i].dynconnected; - res_lib_cfg_nodestatusget.node_status.link_status[i].mtu = node_status.link_status[i].mtu; - memcpy(res_lib_cfg_nodestatusget.node_status.link_status[i].src_ipaddr, - node_status.link_status[i].src_ipaddr, CFG_MAX_HOST_LEN); - memcpy(res_lib_cfg_nodestatusget.node_status.link_status[i].dst_ipaddr, - node_status.link_status[i].dst_ipaddr, CFG_MAX_HOST_LEN); - } - } - } else { - res = CS_ERR_NOT_SUPPORTED; + goto ipc_response_send; } - res_lib_cfg_nodestatusget.header.error = res; + /* Currently only one structure version supported */ + switch (req_lib_cfg_nodestatusget->version) { + case CFG_NODE_STATUS_V1: + res_lib_cfg_nodestatusget_ptr = &res_lib_cfg_nodestatusget_v1; + res_lib_cfg_nodestatusget_size = sizeof(res_lib_cfg_nodestatusget_v1); + + res_lib_cfg_nodestatusget_v1.header.error = CS_OK; + res_lib_cfg_nodestatusget_v1.header.id = MESSAGE_RES_CFG_NODESTATUSGET; + res_lib_cfg_nodestatusget_v1.header.size = res_lib_cfg_nodestatusget_size; + + res_lib_cfg_nodestatusget_v1.node_status.version = CFG_NODE_STATUS_V1; + res_lib_cfg_nodestatusget_v1.node_status.nodeid = req_lib_cfg_nodestatusget->nodeid; + res_lib_cfg_nodestatusget_v1.node_status.reachable = node_status.reachable; + res_lib_cfg_nodestatusget_v1.node_status.remote = node_status.remote; + res_lib_cfg_nodestatusget_v1.node_status.external = node_status.external; + res_lib_cfg_nodestatusget_v1.node_status.onwire_min = node_status.onwire_min; + res_lib_cfg_nodestatusget_v1.node_status.onwire_max = node_status.onwire_max; + res_lib_cfg_nodestatusget_v1.node_status.onwire_ver = node_status.onwire_ver; + + for (i=0; i < KNET_MAX_LINK; i++) { + res_lib_cfg_nodestatusget_v1.node_status.link_status[i].enabled = node_status.link_status[i].enabled; + res_lib_cfg_nodestatusget_v1.node_status.link_status[i].connected = node_status.link_status[i].connected; + res_lib_cfg_nodestatusget_v1.node_status.link_status[i].dynconnected = node_status.link_status[i].dynconnected; + res_lib_cfg_nodestatusget_v1.node_status.link_status[i].mtu = node_status.link_status[i].mtu; + memcpy(res_lib_cfg_nodestatusget_v1.node_status.link_status[i].src_ipaddr, + node_status.link_status[i].src_ipaddr, CFG_MAX_HOST_LEN); + memcpy(res_lib_cfg_nodestatusget_v1.node_status.link_status[i].dst_ipaddr, + node_status.link_status[i].dst_ipaddr, CFG_MAX_HOST_LEN); + } + break; + default: + /* + * Unsupported version requested + */ + res_lib_cfg_nodestatusget_ptr = &res_lib_cfg_nodestatusget_version; + res_lib_cfg_nodestatusget_size = sizeof(res_lib_cfg_nodestatusget_version); + + res_lib_cfg_nodestatusget_version.header.error = CS_ERR_NOT_SUPPORTED; + res_lib_cfg_nodestatusget_version.header.id = MESSAGE_RES_CFG_NODESTATUSGET; + res_lib_cfg_nodestatusget_version.header.size = res_lib_cfg_nodestatusget_size; + break; + } + +ipc_response_send: api->ipc_response_send ( conn, - &res_lib_cfg_nodestatusget, - sizeof (struct res_lib_cfg_nodestatusget)); + res_lib_cfg_nodestatusget_ptr, + res_lib_cfg_nodestatusget_size); LEAVE(); } diff --git a/include/corosync/cfg.h b/include/corosync/cfg.h index c9cd06d0..a7babc28 100644 --- a/include/corosync/cfg.h +++ b/include/corosync/cfg.h @@ -162,10 +162,14 @@ corosync_cfg_ring_status_get ( char ***status, unsigned int *interface_count); -#define CFG_NODE_STATUS_STRUCT_VERSION 1 +typedef enum { + CFG_NODE_STATUS_V1 = 1, +} corosync_cfg_node_status_version_t; + #define CFG_MAX_HOST_LEN 256 #define CFG_MAX_LINKS 8 -struct corosync_knet_link_status { + +struct corosync_knet_link_status_v1 { uint8_t enabled; /* link is configured and admin enabled for traffic */ uint8_t connected; /* link is connected for data (local view) */ uint8_t dynconnected; /* link has been activated by remote dynip */ @@ -174,8 +178,8 @@ struct corosync_knet_link_status { char dst_ipaddr[CFG_MAX_HOST_LEN]; }; -struct corosync_knet_node_status { - uint32_t version; +struct corosync_cfg_node_status_v1 { + corosync_cfg_node_status_version_t version; unsigned int nodeid; uint8_t reachable; uint8_t remote; @@ -183,7 +187,7 @@ struct corosync_knet_node_status { uint8_t onwire_min; uint8_t onwire_max; uint8_t onwire_ver; - struct corosync_knet_link_status link_status[CFG_MAX_LINKS]; + struct corosync_knet_link_status_v1 link_status[CFG_MAX_LINKS]; }; /** @@ -197,7 +201,8 @@ cs_error_t corosync_cfg_node_status_get ( corosync_cfg_handle_t cfg_handle, unsigned int nodeid, - struct corosync_knet_node_status *node_status); + corosync_cfg_node_status_version_t version, + void *node_status); /** * @brief corosync_cfg_kill_node diff --git a/include/corosync/ipc_cfg.h b/include/corosync/ipc_cfg.h index b4ac9fc5..65285a68 100644 --- a/include/corosync/ipc_cfg.h +++ b/include/corosync/ipc_cfg.h @@ -112,12 +112,17 @@ struct req_lib_cfg_nodestatusget { mar_uint32_t version __attribute__((aligned(8))); }; +struct res_lib_cfg_nodestatusget_version { + struct qb_ipc_response_header header __attribute__((aligned(8))); + corosync_cfg_node_status_version_t version __attribute__((aligned(8))); +}; + /** * @brief The res_lib_cfg_nodestatusget struct */ -struct res_lib_cfg_nodestatusget { +struct res_lib_cfg_nodestatusget_v1 { struct qb_ipc_response_header header __attribute__((aligned(8))); - struct corosync_knet_node_status node_status __attribute__((aligned(8))); + struct corosync_cfg_node_status_v1 node_status __attribute__((aligned(8))); }; /** diff --git a/lib/cfg.c b/lib/cfg.c index 16ce6be5..4ce3582d 100644 --- a/lib/cfg.c +++ b/lib/cfg.c @@ -371,18 +371,33 @@ cs_error_t corosync_cfg_node_status_get ( corosync_cfg_handle_t cfg_handle, unsigned int nodeid, - struct corosync_knet_node_status *node_status) + corosync_cfg_node_status_version_t version, + void *node_status) { struct cfg_inst *cfg_inst; struct req_lib_cfg_nodestatusget req_lib_cfg_nodestatusget; - struct res_lib_cfg_nodestatusget res_lib_cfg_nodestatusget; cs_error_t error; struct iovec iov; + size_t cfg_node_status_size; + void *res_lib_cfg_nodestatuget_ptr; + struct res_lib_cfg_nodestatusget_v1 res_lib_cfg_nodestatusget_v1; + struct res_lib_cfg_nodestatusget_version *res_lib_cfg_nodestatusget_version; if (!node_status) { return (CS_ERR_INVALID_PARAM); } + switch (version) { + case CFG_NODE_STATUS_V1: + cfg_node_status_size = sizeof(struct res_lib_cfg_nodestatusget_v1); + res_lib_cfg_nodestatuget_ptr = &res_lib_cfg_nodestatusget_v1; + + break; + default: + return (CS_ERR_INVALID_PARAM); + break; + } + error = hdb_error_to_cs(hdb_handle_get (&cfg_hdb, cfg_handle, (void *)&cfg_inst)); if (error != CS_OK) { return (error); @@ -391,7 +406,7 @@ corosync_cfg_node_status_get ( req_lib_cfg_nodestatusget.header.size = sizeof (struct req_lib_cfg_nodestatusget); req_lib_cfg_nodestatusget.header.id = MESSAGE_REQ_CFG_NODESTATUSGET; req_lib_cfg_nodestatusget.nodeid = nodeid; - req_lib_cfg_nodestatusget.version = CFG_NODE_STATUS_STRUCT_VERSION; + req_lib_cfg_nodestatusget.version = version; iov.iov_base = (void *)&req_lib_cfg_nodestatusget, iov.iov_len = sizeof (struct req_lib_cfg_nodestatusget), @@ -399,19 +414,34 @@ corosync_cfg_node_status_get ( error = qb_to_cs_error (qb_ipcc_sendv_recv(cfg_inst->c, &iov, 1, - &res_lib_cfg_nodestatusget, - sizeof (struct res_lib_cfg_nodestatusget), CS_IPC_TIMEOUT_MS)); - - if (error == CS_OK) { - memcpy(node_status, &res_lib_cfg_nodestatusget.node_status, sizeof(struct corosync_knet_node_status)); + res_lib_cfg_nodestatuget_ptr, + cfg_node_status_size, CS_IPC_TIMEOUT_MS)); + if (error != CS_OK) { + goto error_put; } - /* corosync sent us something we don't really understand. - - we might need to revisit this in the case of future structure versions */ - if (res_lib_cfg_nodestatusget.node_status.version != CFG_NODE_STATUS_STRUCT_VERSION) { + res_lib_cfg_nodestatusget_version = res_lib_cfg_nodestatuget_ptr; + error = res_lib_cfg_nodestatusget_version->header.error; + if (error != CS_OK) { + goto error_put; + } + + if (res_lib_cfg_nodestatusget_version->version != version) { + /* + * corosync sent us something we don't really understand. + */ error = CS_ERR_NOT_SUPPORTED; + goto error_put; } + switch (version) { + case CFG_NODE_STATUS_V1: + memcpy(node_status, &res_lib_cfg_nodestatusget_v1.node_status, + sizeof(struct corosync_cfg_node_status_v1)); + break; + } + +error_put: (void)hdb_handle_put (&cfg_hdb, cfg_handle); return (error); diff --git a/tools/corosync-cfgtool.c b/tools/corosync-cfgtool.c index c4f23f79..0de50bd7 100644 --- a/tools/corosync-cfgtool.c +++ b/tools/corosync-cfgtool.c @@ -111,7 +111,7 @@ nodestatusget_do (enum user_action action, int brief) int rc = EXIT_SUCCESS; int transport_number = TOTEM_TRANSPORT_KNET; int i,j; - struct corosync_knet_node_status node_status; + struct corosync_cfg_node_status_v1 node_status; result = corosync_cfg_initialize (&handle, NULL); if (result != CS_OK) { @@ -191,7 +191,7 @@ nodestatusget_do (enum user_action action, int brief) /* If node status requested then do print node-based info */ if (action == ACTION_NODESTATUS_GET) { for (i=0; i