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
