[PATCH 1/1] Fix udev synchronization for no-locking mode

[PATCH 1/1] Fix udev synchronization for no-locking mode

[Zdenek Kabelac]

Instead of implicitly syncing udev operation in  clustered and
file locking code -  call  syncing directly in lock_vol when
the operation unlocks VG

Currently problem is missing implicit fs_unlock() in the no_locking code.
This is used with --sysinit on read-only filesystem.  In this case
vgchange -ay could exit before all udev nodes are properly synchronised
and may cause problems with accessing such node right after
vgchange --sysinint command is finished.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 daemons/clvmd/clvmd-command.c |    1 -
 lib/locking/file_locking.c    |    1 -
 lib/locking/locking.c         |    4 ++++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/daemons/clvmd/clvmd-command.c b/daemons/clvmd/clvmd-command.c
index 3872702..6d0dee4 100644
--- a/daemons/clvmd/clvmd-command.c
+++ b/daemons/clvmd/clvmd-command.c
@@ -217,7 +217,6 @@ static int lock_vg(struct local_client *client)
 	if (lkid == 0)
 	    return EINVAL;
 
-	lvm_do_fs_unlock(); /* Wait for devices */
 	status = sync_unlock(lockname, lkid);
 	if (status)
 	    status = errno;
diff --git a/lib/locking/file_locking.c b/lib/locking/file_locking.c
index 0c8bbed..68b0420 100644
--- a/lib/locking/file_locking.c
+++ b/lib/locking/file_locking.c
@@ -215,7 +215,6 @@ static int _lock_file(const char *file, uint32_t flags)
 		state = 'W';
 		break;
 	case LCK_UNLOCK:
-		fs_unlock(); /* Wait until devices are available */
 		return _release_lock(file, 1);
 	default:
 		log_error("Unrecognised lock type: %d", flags & LCK_TYPE_MASK);
diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index 645f25d..c923610 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -442,6 +442,10 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
 		/* If LVM1 driver knows about the VG, it can't be accessed. */
 		if (!check_lvm1_vg_inactive(cmd, vol))
 			return_0;
+
+		/* Before unlocking VG wait until devices are available. */
+		if ((flags & LCK_TYPE_MASK) == LCK_UNLOCK)
+			sync_local_dev_names(cmd);
 		break;
 	case LCK_LV:
 		/* All LV locks are non-blocking. */
-- 
1.7.3.5

[PATCH 1/1] Fix udev synchronization for no-locking mode

[Petr Rockai]

Hi,

looks OK, although one thing is definitely puzzling: the udev wait is
mentioned in 3 places (2 removals and 1 addition) in the patch:
nevertheless, no two places agree on which function to use for
that. It'd be good to know why is that? (I can sort of understand that
clvmd has a wrapper, but the removed functions still don't agree with
the added one.)

Other than that, ack.

On a second thought: are we sure this is not going to break clvmd?
Please double-check that. There was trouble with that recently, IIRC.

Zdenek Kabelac <zkabelac@redhat.com> writes:

> Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
Reviewed-by: Petr Rockai <prockai@redhat.com>

> diff --git a/daemons/clvmd/clvmd-command.c b/daemons/clvmd/clvmd-command.c
> index 3872702..6d0dee4 100644
> --- a/daemons/clvmd/clvmd-command.c
> +++ b/daemons/clvmd/clvmd-command.c
> @@ -217,7 +217,6 @@ static int lock_vg(struct local_client *client)
>  	if (lkid == 0)
>  	    return EINVAL;
>  
> -	lvm_do_fs_unlock(); /* Wait for devices */
>  	status = sync_unlock(lockname, lkid);
>  	if (status)
>  	    status = errno;
> diff --git a/lib/locking/file_locking.c b/lib/locking/file_locking.c
> index 0c8bbed..68b0420 100644
> --- a/lib/locking/file_locking.c
> +++ b/lib/locking/file_locking.c
> @@ -215,7 +215,6 @@ static int _lock_file(const char *file, uint32_t flags)
>  		state = 'W';
>  		break;
>  	case LCK_UNLOCK:
> -		fs_unlock(); /* Wait until devices are available */
>  		return _release_lock(file, 1);
>  	default:
>  		log_error("Unrecognised lock type: %d", flags & LCK_TYPE_MASK);
> diff --git a/lib/locking/locking.c b/lib/locking/locking.c
> index 645f25d..c923610 100644
> --- a/lib/locking/locking.c
> +++ b/lib/locking/locking.c
> @@ -442,6 +442,10 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
>  		/* If LVM1 driver knows about the VG, it can't be accessed. */
>  		if (!check_lvm1_vg_inactive(cmd, vol))
>  			return_0;
> +
> +		/* Before unlocking VG wait until devices are available. */
> +		if ((flags & LCK_TYPE_MASK) == LCK_UNLOCK)
> +			sync_local_dev_names(cmd);
>  		break;
>  	case LCK_LV:
>  		/* All LV locks are non-blocking. */

[PATCH 1/1] Fix udev synchronization for no-locking mode

[Peter Rajnoha]

On 01/31/2011 04:12 PM +0100, Petr Rockai wrote:
> On a second thought: are we sure this is not going to break clvmd?
> Please double-check that. There was trouble with that recently, IIRC.

...seems it's working fine (no locking/file locking/cluster locking).
The nodes are always synced now.

>> Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
> Reviewed-by: Petr Rockai <prockai@redhat.com>
Tested-by: Peter Rajnoha <prajnoha@redhat.com>

[PATCH 1/1] Fix udev synchronization for no-locking mode

[Alasdair G Kergon]

On Mon, Jan 31, 2011 at 04:12:54PM +0100, Peter Rockai wrote:
> nevertheless, no two places agree on which function to use for
> that. 

Indeed - can this be fixed without a recursive lock_vol call?

Alasdair