ocfs2: optimize error handling in dlm_request_join
PATCH 1/1 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 <[email protected]>
--- fs/ocfs2/dlm/dlmdomain.c | 69 ++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 32 deletions(-)
fs/ocfs2/dlm/dlmdomain.c +37 -32
@@ -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,
Signature · 1 line
1.8.4.3
PATCH 1/1 ocfs2: optimize error handling in dlm_request_join
Quoted 4 lines · Norton.Zhu
Currently error handling in
dlm_request_joinis a little obscure. So optimize it to promote readability.Signed-off-by Norton.Zhu <[email protected]>
Reviewed-by Joseph Qi <[email protected]>
Quoted 86 lines
--- fs/ocfs2/dlm/dlmdomain.c | 69 ++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 32 deletions(-)fs/ocfs2/dlm/dlmdomain.c +37
-32@@ -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,
PATCH 1/1 ocfs2: optimize error handling in dlm_request_join
In reply to Norton.ZhuQuoted 40 lines · Norton.Zhu
Currently error handling in
dlm_request_joinis a little obscure. So optimize it to promote readability.Signed-off-by Norton.Zhu <[email protected]>--- fs/ocfs2/dlm/dlmdomain.c | 69 ++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 32 deletions(-)fs/ocfs2/dlm/dlmdomain.c +0
-24@@ -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",
fs/ocfs2/dlm/dlmdomain.c static int dlm_request_join(struct dlm_ctxt *dlm, · line 1469
Quoted 10 lines
fs/ocfs2/dlm/dlmdomain.c +1
-8- 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.
Quoted 39 lines
fs/ocfs2/dlm/dlmdomain.c +36
-0+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,
PATCH 1/1 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.
Quoted 100 lines · Srinivas Eeda
Quoted 40 lines · Norton.Zhu
Currently error handling in
dlm_request_joinis a little obscure. So optimize it to promote readability.Signed-off-by Norton.Zhu <[email protected]>--- fs/ocfs2/dlm/dlmdomain.c | 69 ++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 32 deletions(-)fs/ocfs2/dlm/dlmdomain.c +0
-24@@ -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",fs/ocfs2/dlm/dlmdomain.c static int dlm_request_join(struct dlm_ctxt *dlm, · line 1469
Quoted 10 lines
fs/ocfs2/dlm/dlmdomain.c +1
-8- 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.
Quoted 29 lines
fs/ocfs2/dlm/dlmdomain.c +29
-0+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);fs/ocfs2/dlm/dlmdomain.c static int dlm_request_join(struct dlm_ctxt *dlm, · line 1508
Quoted 10 lines
fs/ocfs2/dlm/dlmdomain.c +7
-0+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,.