Re: [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
On 08/20/2015 04:50 AM, Norton.Zhu wrote: Currently error handling in dlm_request_join is a little obscure. So optimize it to promote readability. Signed-off-by: Norton.Zhu norton@huawei.com --- fs/ocfs2/dlm/dlmdomain.c | 69 ++-- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 7df88a6..af4f7aa 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1465,39 +1465,44 @@ static int dlm_request_join(struct dlm_ctxt *dlm, if (status == -ENOPROTOOPT) { status = 0; *response = JOIN_OK_NO_MAP; - } else if (packet.code == JOIN_DISALLOW || -packet.code == JOIN_OK_NO_MAP) { - *response = packet.code; - } else if (packet.code == JOIN_PROTOCOL_MISMATCH) { - mlog(ML_NOTICE, - This node requested DLM locking protocol %u.%u and - filesystem locking protocol %u.%u. At least one of - the protocol versions on node %d is not compatible, - disconnecting\n, - dlm-dlm_locking_proto.pv_major, - dlm-dlm_locking_proto.pv_minor, - dlm-fs_locking_proto.pv_major, - dlm-fs_locking_proto.pv_minor, - node); - status = -EPROTO; - *response = packet.code; - } else if (packet.code == JOIN_OK) { - *response = packet.code; - /* Use the same locking protocol as the remote node */ - dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; - dlm-fs_locking_proto.pv_minor = packet.fs_minor; - mlog(0, - Node %d responds JOIN_OK with DLM locking protocol - %u.%u and fs locking protocol %u.%u\n, - node, - dlm-dlm_locking_proto.pv_major, - dlm-dlm_locking_proto.pv_minor, - dlm-fs_locking_proto.pv_major, - dlm-fs_locking_proto.pv_minor); } else { - status = -EINVAL; - mlog(ML_ERROR, invalid response %d from node %u\n, - packet.code, node); + *response = packet.code; Norton, it looks much better :) one minor comment. we don't want to reset *response with packet.code if it's unrecognized. We should leave the value to JOIN_DISALLOW; rest looks good. + switch (packet.code) { + case JOIN_DISALLOW: + case JOIN_OK_NO_MAP: + break; + case JOIN_PROTOCOL_MISMATCH: + mlog(ML_NOTICE, + This node requested DLM locking protocol %u.%u and + filesystem locking protocol %u.%u. At least one of + the protocol versions on node %d is not compatible, + disconnecting\n, + dlm-dlm_locking_proto.pv_major, + dlm-dlm_locking_proto.pv_minor, + dlm-fs_locking_proto.pv_major, + dlm-fs_locking_proto.pv_minor, + node); + status = -EPROTO; + break; + case JOIN_OK: + /* Use the same locking protocol as the remote node */ + dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; + dlm-fs_locking_proto.pv_minor = packet.fs_minor; + mlog(0, + Node %d responds JOIN_OK with DLM locking protocol + %u.%u and fs locking protocol %u.%u\n, + node, + dlm-dlm_locking_proto.pv_major, + dlm-dlm_locking_proto.pv_minor, + dlm-fs_locking_proto.pv_major, + dlm-fs_locking_proto.pv_minor); + break; + default: + status = -EINVAL; + mlog(ML_ERROR, invalid response %d from node %u\n, + packet.code, node); + break; + } } mlog(0, status %d, node %d response is %d\n, status, node, ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel
[Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
Currently error handling in dlm_request_join is a little obscure. So optimize it to promote readability. Signed-off-by: Norton.Zhu norton@huawei.com --- fs/ocfs2/dlm/dlmdomain.c | 69 ++-- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 7df88a6..af4f7aa 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1465,39 +1465,44 @@ static int dlm_request_join(struct dlm_ctxt *dlm, if (status == -ENOPROTOOPT) { status = 0; *response = JOIN_OK_NO_MAP; - } else if (packet.code == JOIN_DISALLOW || - packet.code == JOIN_OK_NO_MAP) { - *response = packet.code; - } else if (packet.code == JOIN_PROTOCOL_MISMATCH) { - mlog(ML_NOTICE, -This node requested DLM locking protocol %u.%u and -filesystem locking protocol %u.%u. At least one of -the protocol versions on node %d is not compatible, -disconnecting\n, -dlm-dlm_locking_proto.pv_major, -dlm-dlm_locking_proto.pv_minor, -dlm-fs_locking_proto.pv_major, -dlm-fs_locking_proto.pv_minor, -node); - status = -EPROTO; - *response = packet.code; - } else if (packet.code == JOIN_OK) { - *response = packet.code; - /* Use the same locking protocol as the remote node */ - dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; - dlm-fs_locking_proto.pv_minor = packet.fs_minor; - mlog(0, -Node %d responds JOIN_OK with DLM locking protocol -%u.%u and fs locking protocol %u.%u\n, -node, -dlm-dlm_locking_proto.pv_major, -dlm-dlm_locking_proto.pv_minor, -dlm-fs_locking_proto.pv_major, -dlm-fs_locking_proto.pv_minor); } else { - status = -EINVAL; - mlog(ML_ERROR, invalid response %d from node %u\n, -packet.code, node); + *response = packet.code; + switch (packet.code) { + case JOIN_DISALLOW: + case JOIN_OK_NO_MAP: + break; + case JOIN_PROTOCOL_MISMATCH: + mlog(ML_NOTICE, +This node requested DLM locking protocol %u.%u and +filesystem locking protocol %u.%u. At least one of +the protocol versions on node %d is not compatible, +disconnecting\n, +dlm-dlm_locking_proto.pv_major, +dlm-dlm_locking_proto.pv_minor, +dlm-fs_locking_proto.pv_major, +dlm-fs_locking_proto.pv_minor, +node); + status = -EPROTO; + break; + case JOIN_OK: + /* Use the same locking protocol as the remote node */ + dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; + dlm-fs_locking_proto.pv_minor = packet.fs_minor; + mlog(0, +Node %d responds JOIN_OK with DLM locking protocol +%u.%u and fs locking protocol %u.%u\n, +node, +dlm-dlm_locking_proto.pv_major, +dlm-dlm_locking_proto.pv_minor, +dlm-fs_locking_proto.pv_major, +dlm-fs_locking_proto.pv_minor); + break; + default: + status = -EINVAL; + mlog(ML_ERROR, invalid response %d from node %u\n, +packet.code, node); + break; + } } mlog(0, status %d, node %d response is %d\n, status, node, -- 1.8.4.3 ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel
Re: [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
Hi Srinivas, Thanks for your advice, we should leave *response as JOIN_DISALLOW if packet.code is invalid, I will resend the patch. On 2015/8/21 0:56, Srinivas Eeda wrote: On 08/20/2015 04:50 AM, Norton.Zhu wrote: Currently error handling in dlm_request_join is a little obscure. So optimize it to promote readability. Signed-off-by: Norton.Zhu norton@huawei.com --- fs/ocfs2/dlm/dlmdomain.c | 69 ++-- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 7df88a6..af4f7aa 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1465,39 +1465,44 @@ static int dlm_request_join(struct dlm_ctxt *dlm, if (status == -ENOPROTOOPT) { status = 0; *response = JOIN_OK_NO_MAP; -} else if (packet.code == JOIN_DISALLOW || - packet.code == JOIN_OK_NO_MAP) { -*response = packet.code; -} else if (packet.code == JOIN_PROTOCOL_MISMATCH) { -mlog(ML_NOTICE, - This node requested DLM locking protocol %u.%u and - filesystem locking protocol %u.%u. At least one of - the protocol versions on node %d is not compatible, - disconnecting\n, - dlm-dlm_locking_proto.pv_major, - dlm-dlm_locking_proto.pv_minor, - dlm-fs_locking_proto.pv_major, - dlm-fs_locking_proto.pv_minor, - node); -status = -EPROTO; -*response = packet.code; -} else if (packet.code == JOIN_OK) { -*response = packet.code; -/* Use the same locking protocol as the remote node */ -dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; -dlm-fs_locking_proto.pv_minor = packet.fs_minor; -mlog(0, - Node %d responds JOIN_OK with DLM locking protocol - %u.%u and fs locking protocol %u.%u\n, - node, - dlm-dlm_locking_proto.pv_major, - dlm-dlm_locking_proto.pv_minor, - dlm-fs_locking_proto.pv_major, - dlm-fs_locking_proto.pv_minor); } else { -status = -EINVAL; -mlog(ML_ERROR, invalid response %d from node %u\n, - packet.code, node); +*response = packet.code; Norton, it looks much better :) one minor comment. we don't want to reset *response with packet.code if it's unrecognized. We should leave the value to JOIN_DISALLOW; rest looks good. +switch (packet.code) { +case JOIN_DISALLOW: +case JOIN_OK_NO_MAP: +break; +case JOIN_PROTOCOL_MISMATCH: +mlog(ML_NOTICE, + This node requested DLM locking protocol %u.%u and + filesystem locking protocol %u.%u. At least one of + the protocol versions on node %d is not compatible, + disconnecting\n, + dlm-dlm_locking_proto.pv_major, + dlm-dlm_locking_proto.pv_minor, + dlm-fs_locking_proto.pv_major, + dlm-fs_locking_proto.pv_minor, + node); +status = -EPROTO; +break; +case JOIN_OK: +/* Use the same locking protocol as the remote node */ +dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; +dlm-fs_locking_proto.pv_minor = packet.fs_minor; +mlog(0, + Node %d responds JOIN_OK with DLM locking protocol + %u.%u and fs locking protocol %u.%u\n, + node, + dlm-dlm_locking_proto.pv_major, + dlm-dlm_locking_proto.pv_minor, + dlm-fs_locking_proto.pv_major, + dlm-fs_locking_proto.pv_minor); +break; +default: +status = -EINVAL; +mlog(ML_ERROR, invalid response %d from node %u\n, + packet.code, node); +break; +} } mlog(0, status %d, node %d response is %d\n, status, node, . ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel
[Ocfs2-devel] [PATCH 2/2] ocfs2: remove unneeded code in dlm_register_domain_handlers
The last goto statement is unneeded, so remove it. Signed-off-by: Joseph Qi joseph...@huawei.com --- fs/ocfs2/dlm/dlmdomain.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 4f75070..019459b 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1846,8 +1846,6 @@ static int dlm_register_domain_handlers(struct dlm_ctxt *dlm) sizeof(struct dlm_exit_domain), dlm_begin_exit_domain_handler, dlm, NULL, dlm-dlm_domain_handlers); - if (status) - goto bail; bail: if (status) -- 1.8.4.3 ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel
[Ocfs2-devel] [PATCH v2] ocfs2: optimize error handling in dlm_request_join
Currently error handling in dlm_request_join is a little obscure. So optimize it to promote readability. changelog: If packet.code is invalid, reset it to JOIN_DISALLOW, keep it meaningful. It just make influence on the log printing. Signed-off-by: Norton.Zhu norton@huawei.com --- fs/ocfs2/dlm/dlmdomain.c | 71 ++-- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 7df88a6..312a237 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1465,39 +1465,46 @@ static int dlm_request_join(struct dlm_ctxt *dlm, if (status == -ENOPROTOOPT) { status = 0; *response = JOIN_OK_NO_MAP; - } else if (packet.code == JOIN_DISALLOW || - packet.code == JOIN_OK_NO_MAP) { - *response = packet.code; - } else if (packet.code == JOIN_PROTOCOL_MISMATCH) { - mlog(ML_NOTICE, -This node requested DLM locking protocol %u.%u and -filesystem locking protocol %u.%u. At least one of -the protocol versions on node %d is not compatible, -disconnecting\n, -dlm-dlm_locking_proto.pv_major, -dlm-dlm_locking_proto.pv_minor, -dlm-fs_locking_proto.pv_major, -dlm-fs_locking_proto.pv_minor, -node); - status = -EPROTO; - *response = packet.code; - } else if (packet.code == JOIN_OK) { - *response = packet.code; - /* Use the same locking protocol as the remote node */ - dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; - dlm-fs_locking_proto.pv_minor = packet.fs_minor; - mlog(0, -Node %d responds JOIN_OK with DLM locking protocol -%u.%u and fs locking protocol %u.%u\n, -node, -dlm-dlm_locking_proto.pv_major, -dlm-dlm_locking_proto.pv_minor, -dlm-fs_locking_proto.pv_major, -dlm-fs_locking_proto.pv_minor); } else { - status = -EINVAL; - mlog(ML_ERROR, invalid response %d from node %u\n, -packet.code, node); + *response = packet.code; + switch (packet.code) { + case JOIN_DISALLOW: + case JOIN_OK_NO_MAP: + break; + case JOIN_PROTOCOL_MISMATCH: + mlog(ML_NOTICE, +This node requested DLM locking protocol %u.%u and +filesystem locking protocol %u.%u. At least one of +the protocol versions on node %d is not compatible, +disconnecting\n, +dlm-dlm_locking_proto.pv_major, +dlm-dlm_locking_proto.pv_minor, +dlm-fs_locking_proto.pv_major, +dlm-fs_locking_proto.pv_minor, +node); + status = -EPROTO; + break; + case JOIN_OK: + /* Use the same locking protocol as the remote node */ + dlm-dlm_locking_proto.pv_minor = packet.dlm_minor; + dlm-fs_locking_proto.pv_minor = packet.fs_minor; + mlog(0, +Node %d responds JOIN_OK with DLM locking protocol +%u.%u and fs locking protocol %u.%u\n, +node, +dlm-dlm_locking_proto.pv_major, +dlm-dlm_locking_proto.pv_minor, +dlm-fs_locking_proto.pv_major, +dlm-fs_locking_proto.pv_minor); + break; + default: + status = -EINVAL; + mlog(ML_ERROR, invalid response %d from node %u\n, +packet.code, node); + /* Reset response to JOIN_DISALLOW */ + *response = JOIN_DISALLOW; + break; + } } mlog(0, status %d, node %d response is %d\n, status, node, -- 1.8.4.3 ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel
[Ocfs2-devel] [PATCH 1/2] ocfs2: fix BUG when o2hb_register_callback fails
In dlm_register_domain_handlers, if o2hb_register_callback fails, it will call dlm_unregister_domain_handlers to unregister. This will trigger the BUG_ON in o2hb_unregister_callback because hc_magic is 0. So we should call o2hb_setup_callback to initialize hc first. Signed-off-by: Joseph Qi joseph...@huawei.com --- fs/ocfs2/dlm/dlmdomain.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 7df88a6..4f75070 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1725,12 +1725,13 @@ static int dlm_register_domain_handlers(struct dlm_ctxt *dlm) o2hb_setup_callback(dlm-dlm_hb_down, O2HB_NODE_DOWN_CB, dlm_hb_node_down_cb, dlm, DLM_HB_NODE_DOWN_PRI); + o2hb_setup_callback(dlm-dlm_hb_up, O2HB_NODE_UP_CB, + dlm_hb_node_up_cb, dlm, DLM_HB_NODE_UP_PRI); + status = o2hb_register_callback(dlm-name, dlm-dlm_hb_down); if (status) goto bail; - o2hb_setup_callback(dlm-dlm_hb_up, O2HB_NODE_UP_CB, - dlm_hb_node_up_cb, dlm, DLM_HB_NODE_UP_PRI); status = o2hb_register_callback(dlm-name, dlm-dlm_hb_up); if (status) goto bail; -- 1.8.4.3 ___ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel