[linux-lvm] Clarifications wanted about locking process

[linux-lvm] Clarifications wanted about locking process

[Eric Bélanger]

Hi,

Since I updated from lvm2 2.02.48 to 2.02.51, I'm getting this message
on boot-up:

Write locks are prohibited with --ignorelockingfailure.
Unable to obtain global lock.

I found that it comes from the: "/sbin/lvm vgscan
--ignorelockingfailure --mknodes" command in /etc/rc.sysinit.  I was
getting the same message from "/sbin/lvm vgchange
--ignorelockingfailure -a y" when using the 2.02.49 (or 2.02.50?)
versions.

Is this just an harmless informational message?  I don't understand
why it would want a write lock when scanning or activating volume
groups. Wouldn't a read lock be sufficient?  Also, shouldn't the use
of the --ignorelockingfailure option disable all locking-related
warnings?  Any clarifications would be appreciated.

Thanks,
Eric

Re: [linux-lvm] Clarifications wanted about locking process

[Alasdair G Kergon]

On Thu, Aug 13, 2009 at 08:13:39PM -0400, Eric B�langer wrote:
> Since I updated from lvm2 2.02.48 to 2.02.51, I'm getting this message
> on boot-up:
> Write locks are prohibited with --ignorelockingfailure.
> Unable to obtain global lock.
 
Sounds like a bug.

> I found that it comes from the: "/sbin/lvm vgscan
> --ignorelockingfailure --mknodes" command in /etc/rc.sysinit.  I was
> getting the same message from "/sbin/lvm vgchange
> --ignorelockingfailure -a y" when using the 2.02.49 (or 2.02.50?)
> versions.

Alasdair

[PATCH] Re: [linux-lvm] Clarifications wanted about locking process

[Petr Rockai]

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

Alasdair G Kergon <agk@redhat.com> writes:
> On Thu, Aug 13, 2009 at 08:13:39PM -0400, Eric Bélanger wrote:
>> Since I updated from lvm2 2.02.48 to 2.02.51, I'm getting this message
>> on boot-up:
>> Write locks are prohibited with --ignorelockingfailure.
>> Unable to obtain global lock.
>  
> Sounds like a bug.
The attached patch should fix the issue. I believe that taking a global write
lock while scanning is a mistake (I consulted with Milan, he says it only
serves to enable metadata caching).

Yours,
   Petr.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: scanning-read-locks.diff --]
[-- Type: text/x-diff, Size: 1011 bytes --]

diff -rN -u -p old-upstream/tools/pvscan.c new-upstream/tools/pvscan.c
--- old-upstream/tools/pvscan.c	2009-08-14 13:44:39.000000000 +0200
+++ new-upstream/tools/pvscan.c	2009-08-14 13:44:40.000000000 +0200
@@ -124,7 +124,7 @@ int pvscan(struct cmd_context *cmd, int 
 			  arg_count(cmd, exported_ARG) ?
 			  "of exported volume group(s)" : "in no volume group");
 
-	if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_WRITE)) {
+	if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_READ)) {
 		log_error("Unable to obtain global lock.");
 		return ECMD_FAILED;
 	}
diff -rN -u -p old-upstream/tools/vgscan.c new-upstream/tools/vgscan.c
--- old-upstream/tools/vgscan.c	2009-08-14 13:44:39.000000000 +0200
+++ new-upstream/tools/vgscan.c	2009-08-14 13:44:40.000000000 +0200
@@ -40,7 +40,7 @@ int vgscan(struct cmd_context *cmd, int 
 		return EINVALID_CMD_LINE;
 	}
 
-	if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_WRITE)) {
+	if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_READ)) {
 		log_error("Unable to obtain global lock.");
 		return ECMD_FAILED;
 	}

Re: [PATCH] Re: [linux-lvm] Clarifications wanted about locking process

[Alasdair G Kergon]

On Fri, Aug 14, 2009 at 01:49:13PM +0200, Peter Rockai wrote:
> The attached patch should fix the issue. I believe that taking a global write
> lock while scanning is a mistake (I consulted with Milan, he says it only
> serves to enable metadata caching).
 
Well only one instance of pvscan and/or vgscan is allowed to run at once,
and IIRC VG_GLOBAL with LCK_VG_WRITE is what enforces that.

IOW The write lock is correct.  

See the comment in toollib:

/*
 * If the lock_type is LCK_VG_READ (used only in reporting commands),
 * we lock VG_GLOBAL to enable use of metadata cache.
 * This can pause alongide pvscan or vgscan process for a while.
 */

I don't want to touch the VG_GLOBAL logic here at this stage.

Are there any problems with just excluding an already-held VG_GLOBAL from the
test that generates this error message?

Alasdair