Re: [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join

2015-08-20 Thread Srinivas Eeda
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

2015-08-20 Thread Norton.Zhu
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

2015-08-20 Thread Norton.Zhu
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

2015-08-20 Thread Joseph Qi
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

2015-08-20 Thread Norton.Zhu
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

2015-08-20 Thread Joseph Qi
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