Norton.Zhu
2015-Aug-20 11:50 UTC
[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.zhu at 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
Joseph Qi
2015-Aug-20 12:42 UTC
[Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
On 2015/8/20 19:50, 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.zhu at huawei.com>Reviewed-by: Joseph Qi <joseph.qi at 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, >
Srinivas Eeda
2015-Aug-20 16:56 UTC
[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.zhu at 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,