[linux-lvm] clvmd-corosync should check lksb.sb_status instead of return code ?

[linux-lvm] clvmd-corosync should check lksb.sb_status instead of return code ?

[Xinwei Hu]

[-- Attachment #1: Type: text/plain, Size: 368 bytes --]

Hi all,

  In clvmd-corosync, lock_resource calls dlm_ls_lock_wait with flags
set to LKF_NOQUEUE.
  When LKF_NOQUEUE is set, the return code of dlm_ls_lock_wait is
always 0, while the actual errno is in lksb.sb_status.

  This causes problem when using vgchange -aey or something like.

  I attached a patch to try to fix this issue.

  Please help to review. Thanks.

[-- Attachment #2: clvmd-corosync.diff --]
[-- Type: text/x-patch, Size: 861 bytes --]

Index: LVM2-corosync-patch/daemons/clvmd/clvmd-corosync.c
===================================================================
--- LVM2-corosync-patch.orig/daemons/clvmd/clvmd-corosync.c	2009-02-11 18:13:20.000000000 +0800
+++ LVM2-corosync-patch/daemons/clvmd/clvmd-corosync.c	2009-02-25 18:34:49.000000000 +0800
@@ -478,6 +478,11 @@
 		DEBUGLOG("dlm_ls_lock returned %d\n", errno);
 		return err;
 	}
+	if (lksb.sb_status != 0)
+	{
+		DEBUGLOG("dlm_ls_lock returns lksb.sb_status %d\n", lksb.sb_status);
+		return -lksb.sb_status;
+	}
 
 	DEBUGLOG("lock_resource returning %d, lock_id=%x\n", err, lksb.sb_lkid);
 
@@ -504,6 +509,12 @@
 		DEBUGLOG("Unlock returned %d\n", err);
 		return err;
 	}
+	if (lksb.sb_status != 0)
+	{
+		DEBUGLOG("dlm_ls_unlock_wait returns lksb.sb_status: %d\n", lksb.sb_status);
+		return -lksb.sb_status;
+	}   
+
 
 	return 0;
 }

Re: [linux-lvm] clvmd-corosync should check lksb.sb_status instead of return code ?

[Chrissie Caulfield]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

Xinwei Hu wrote:
> Hi all,
> 
>   In clvmd-corosync, lock_resource calls dlm_ls_lock_wait with flags
> set to LKF_NOQUEUE.
>   When LKF_NOQUEUE is set, the return code of dlm_ls_lock_wait is
> always 0, while the actual errno is in lksb.sb_status.
> 
>   This causes problem when using vgchange -aey or something like.
> 
>   I attached a patch to try to fix this issue.
> 
>   Please help to review. Thanks.

Hi

You are correct, the functions do need to check the LKSB. Your patch
isn't quite right I don't think though - the lksb status should be
returned in errno as shown below:

-- 

Chrissie

[-- Attachment #2: clvmd-corosync2.diff --]
[-- Type: text/x-patch, Size: 968 bytes --]

Index: clvmd-corosync.c
===================================================================
RCS file: /cvs/lvm2/LVM2/daemons/clvmd/clvmd-corosync.c,v
retrieving revision 1.5
diff -u -p -r1.5 clvmd-corosync.c
--- clvmd-corosync.c	11 Feb 2009 10:13:20 -0000	1.5
+++ clvmd-corosync.c	25 Feb 2009 13:28:34 -0000
@@ -478,6 +478,12 @@ static int _lock_resource(const char *re
 		DEBUGLOG("dlm_ls_lock returned %d\n", errno);
 		return err;
 	}
+	if (lksb.sb_status != 0)
+	{
+		DEBUGLOG("dlm_ls_lock returns lksb.sb_status %d\n", lksb.sb_status);
+		errno = -lksb.sb_status;
+		return -1;
+	}
 
 	DEBUGLOG("lock_resource returning %d, lock_id=%x\n", err, lksb.sb_lkid);
 
@@ -504,6 +510,13 @@ static int _unlock_resource(const char *
 		DEBUGLOG("Unlock returned %d\n", err);
 		return err;
 	}
+	if (lksb.sb_status != 0)
+	{
+		DEBUGLOG("dlm_ls_unlock_wait returns lksb.sb_status: %d\n", lksb.sb_status);
+		errno = -lksb.sb_status;
+		return -1;
+	}   
+
 
 	return 0;
 }

Re: [linux-lvm] clvmd-corosync should check lksb.sb_status instead of return code ?

[Xinwei Hu]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

Hi Chrissie,

  You are right. It should be returned in errno. Also, it seems errno
should positive as lksb.sb_status too.

  I modified the patch a little bit further.

  Any comments ?

2009/2/25 Chrissie Caulfield <ccaulfie@redhat.com>:
> Xinwei Hu wrote:
>> Hi all,
>>
>>   In clvmd-corosync, lock_resource calls dlm_ls_lock_wait with flags
>> set to LKF_NOQUEUE.
>>   When LKF_NOQUEUE is set, the return code of dlm_ls_lock_wait is
>> always 0, while the actual errno is in lksb.sb_status.
>>
>>   This causes problem when using vgchange -aey or something like.
>>
>>   I attached a patch to try to fix this issue.
>>
>>   Please help to review. Thanks.
>
> Hi
>
> You are correct, the functions do need to check the LKSB. Your patch
> isn't quite right I don't think though - the lksb status should be
> returned in errno as shown below:
>
> --
>
> Chrissie
>
> _______________________________________________
> linux-lvm mailing list
> linux-lvm@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-lvm
> read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
>

[-- Attachment #2: clvmd-corosync2.diff --]
[-- Type: text/x-patch, Size: 966 bytes --]

Index: clvmd-corosync.c
===================================================================
RCS file: /cvs/lvm2/LVM2/daemons/clvmd/clvmd-corosync.c,v
retrieving revision 1.5
diff -u -p -r1.5 clvmd-corosync.c
--- clvmd-corosync.c	11 Feb 2009 10:13:20 -0000	1.5
+++ clvmd-corosync.c	25 Feb 2009 13:28:34 -0000
@@ -478,6 +478,12 @@ static int _lock_resource(const char *re
 		DEBUGLOG("dlm_ls_lock returned %d\n", errno);
 		return err;
 	}
+	if (lksb.sb_status != 0)
+	{
+		DEBUGLOG("dlm_ls_lock returns lksb.sb_status %d\n", lksb.sb_status);
+		errno = lksb.sb_status;
+		return -1;
+	}
 
 	DEBUGLOG("lock_resource returning %d, lock_id=%x\n", err, lksb.sb_lkid);
 
@@ -504,6 +510,13 @@ static int _unlock_resource(const char *
 		DEBUGLOG("Unlock returned %d\n", err);
 		return err;
 	}
+	if (lksb.sb_status != 0)
+	{
+		DEBUGLOG("dlm_ls_unlock_wait returns lksb.sb_status: %d\n", lksb.sb_status);
+		errno = lksb.sb_status;
+		return -1;
+	}   
+
 
 	return 0;
 }

Re: [linux-lvm] clvmd-corosync should check lksb.sb_status instead of return code ?

[Chrissie Caulfield]

Xinwei Hu wrote:
> Hi Chrissie,
> 
>   You are right. It should be returned in errno. Also, it seems errno
> should positive as lksb.sb_status too.
> 
>   I modified the patch a little bit further.
> 
>   Any comments ?


Hi

You're right ... this kernel-itis is catching! lksb status are indeed
positive and correct for returning as errno.

I'll commit this patch

Thank you.

Chrissie

> 2009/2/25 Chrissie Caulfield <ccaulfie@redhat.com>:
>> Xinwei Hu wrote:
>>> Hi all,
>>>
>>>   In clvmd-corosync, lock_resource calls dlm_ls_lock_wait with flags
>>> set to LKF_NOQUEUE.
>>>   When LKF_NOQUEUE is set, the return code of dlm_ls_lock_wait is
>>> always 0, while the actual errno is in lksb.sb_status.
>>>
>>>   This causes problem when using vgchange -aey or something like.
>>>
>>>   I attached a patch to try to fix this issue.
>>>
>>>   Please help to review. Thanks.
>> Hi
>>
>> You are correct, the functions do need to check the LKSB. Your patch
>> isn't quite right I don't think though - the lksb status should be
>> returned in errno as shown below:
>>
>> --
>>
>> Chrissie

Re: [linux-lvm] clvmd-corosync should check lksb.sb_status instead of return code ?

[Xinwei Hu]

2009/2/25 Chrissie Caulfield <ccaulfie@redhat.com>:
> Xinwei Hu wrote:
>> Hi Chrissie,
>>
>>   You are right. It should be returned in errno. Also, it seems errno
>> should positive as lksb.sb_status too.
>>
>>   I modified the patch a little bit further.
>>
>>   Any comments ?
>
>
> Hi
>
> You're right ... this kernel-itis is catching! lksb status are indeed
> positive and correct for returning as errno.
>
> I'll commit this patch

Good. Thanks for taking care of this.

> Thank you.
>
> Chrissie
>
>> 2009/2/25 Chrissie Caulfield <ccaulfie@redhat.com>:
>>> Xinwei Hu wrote:
>>>> Hi all,
>>>>
>>>>   In clvmd-corosync, lock_resource calls dlm_ls_lock_wait with flags
>>>> set to LKF_NOQUEUE.
>>>>   When LKF_NOQUEUE is set, the return code of dlm_ls_lock_wait is
>>>> always 0, while the actual errno is in lksb.sb_status.
>>>>
>>>>   This causes problem when using vgchange -aey or something like.
>>>>
>>>>   I attached a patch to try to fix this issue.
>>>>
>>>>   Please help to review. Thanks.
>>> Hi
>>>
>>> You are correct, the functions do need to check the LKSB. Your patch
>>> isn't quite right I don't think though - the lksb status should be
>>> returned in errno as shown below:
>>>
>>> --
>>>
>>> Chrissie
>
> _______________________________________________
> linux-lvm mailing list
> linux-lvm@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-lvm
> read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
>