* [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
@ 2015-08-20 11:50 Norton.Zhu
2015-08-20 12:42 ` Joseph Qi
2015-08-20 16:56 ` Srinivas Eeda
0 siblings, 2 replies; 4+ messages in thread
From: Norton.Zhu @ 2015-08-20 11:50 UTC (permalink / raw)
To: ocfs2-devel
Currently error handling in dlm_request_join is a little obscure.
So optimize it to promote readability.
Signed-off-by: Norton.Zhu <norton.zhu@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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
2015-08-20 11:50 [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join Norton.Zhu
@ 2015-08-20 12:42 ` Joseph Qi
2015-08-20 16:56 ` Srinivas Eeda
1 sibling, 0 replies; 4+ messages in thread
From: Joseph Qi @ 2015-08-20 12:42 UTC (permalink / raw)
To: ocfs2-devel
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@huawei.com>
Reviewed-by: Joseph Qi <joseph.qi@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,
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
2015-08-20 11:50 [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join Norton.Zhu
2015-08-20 12:42 ` Joseph Qi
@ 2015-08-20 16:56 ` Srinivas Eeda
2015-08-21 1:54 ` Norton.Zhu
1 sibling, 1 reply; 4+ messages in thread
From: Srinivas Eeda @ 2015-08-20 16:56 UTC (permalink / raw)
To: ocfs2-devel
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@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,
^ permalink raw reply [flat|nested] 4+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join
2015-08-20 16:56 ` Srinivas Eeda
@ 2015-08-21 1:54 ` Norton.Zhu
0 siblings, 0 replies; 4+ messages in thread
From: Norton.Zhu @ 2015-08-21 1:54 UTC (permalink / raw)
To: ocfs2-devel
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.zhu@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,
>
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-08-21 1:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-20 11:50 [Ocfs2-devel] [PATCH] ocfs2: optimize error handling in dlm_request_join Norton.Zhu
2015-08-20 12:42 ` Joseph Qi
2015-08-20 16:56 ` Srinivas Eeda
2015-08-21 1:54 ` Norton.Zhu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.