[PATCH 0/4] pvmove cluster fixes

[PATCH 0/4] pvmove cluster fixes

[Milan Broz]

Various locking changes in previous releases, mainly commit

commit bd86888d7406cdb8ba20ff3d3b807fabcc7ba0db
    Pass exclusive LV locks to all nodes in the cluster.
    This was the intended behaviour, as described in the lvchange man page, so you
    have complete control through volume_list in lvm.conf, but the code seems to
    have been treating -ae as local-only for a very long time.

caused pvmove to stopped working if an LV is activated exclusively.

Milan

Milan Broz (4):
  Remove unused and wrongly set cluster VG flag from clvmd lock query
    command.
  Use new flag PVMOVE_EXCLUSIVE in update_metatada call.
  Do not allow pvmove if some affected LVs are activated locally     or
    on more nodes while others are activated exclusively.
  Keep exclusive activation in pvmove if LV is already active.

 WHATS_NEW                     |    3 ++
 lib/locking/cluster_locking.c |    2 +-
 lib/locking/locking.c         |    2 +-
 tools/pvmove.c                |   44 +++++++++++++++++++++++++++++++++-------
 4 files changed, 41 insertions(+), 10 deletions(-)

-- 
1.7.9.1

[PATCH 1/4] Remove unused and wrongly set cluster VG flag from clvmd lock query command.

[Milan Broz]

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 WHATS_NEW                     |    1 +
 lib/locking/cluster_locking.c |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/WHATS_NEW b/WHATS_NEW
index e70dc58..93b6440 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.96 - 
 ================================
+  Remove unused and wrongly set cluster VG flag from clvmd lock query command.
   Fix pvmove for exclusively activated LV pvmove in clustered VG. (2.02.86)
   Always free hash table on update_pvid_to_vgid() in lvmetad.
   Update and fix monitoring of thin pool devices.
diff --git a/lib/locking/cluster_locking.c b/lib/locking/cluster_locking.c
index 990e8b1..3e2543b 100644
--- a/lib/locking/cluster_locking.c
+++ b/lib/locking/cluster_locking.c
@@ -538,7 +538,7 @@ int query_resource(const char *resource, int *mode)
 	strcpy(args + 2, resource);
 
 	args[0] = 0;
-	args[1] = LCK_CLUSTER_VG;
+	args[1] = 0;
 
 	status = _cluster_request(CLVMD_CMD_LOCK_QUERY, node, args, len,
 				  &response, &num_responses);
-- 
1.7.9.1

[PATCH 2/4] Use new flag PVMOVE_EXCLUSIVE in update_metatada call.

[Milan Broz]

There is no real functional change in this patch except it
avoids checking cluster cmirror module twice.

(Flag used in following patch.)

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 tools/pvmove.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/pvmove.c b/tools/pvmove.c
index 8d543b0..40ef069 100644
--- a/tools/pvmove.c
+++ b/tools/pvmove.c
@@ -18,6 +18,7 @@
 #include "display.h"
 
 #define PVMOVE_FIRST_TIME   0x00000001      /* Called for first time */
+#define PVMOVE_EXCLUSIVE    0x00000002      /* Require exclusive LV */
 
 static int _pvmove_target_present(struct cmd_context *cmd, int clustered)
 {
@@ -358,7 +359,7 @@ static int _update_metadata(struct cmd_context *cmd, struct volume_group *vg,
 			    struct logical_volume *lv_mirr,
 			    struct dm_list *lvs_changed, unsigned flags)
 {
-	unsigned exclusive = _pvmove_is_exclusive(cmd, vg);
+	unsigned exclusive = (flags & PVMOVE_EXCLUSIVE) ? 1 : 0;
 	unsigned first_time = (flags & PVMOVE_FIRST_TIME) ? 1 : 0;
 	int r = 0;
 
@@ -390,6 +391,9 @@ static int _update_metadata(struct cmd_context *cmd, struct volume_group *vg,
 	/* Only the first mirror segment gets activated as a mirror */
 	/* FIXME: Add option to use a log */
 	if (first_time) {
+		if (!exclusive && _pvmove_is_exclusive(cmd, vg))
+			exclusive = 1;
+
 		if (!_activate_lv(cmd, lv_mirr, exclusive)) {
 			if (test_mode()) {
 				r = 1;
@@ -428,7 +432,7 @@ static int _set_up_pvmove(struct cmd_context *cmd, const char *pv_name,
 	struct dm_list *lvs_changed;
 	struct physical_volume *pv;
 	struct logical_volume *lv_mirr;
-	unsigned first_time = 1;
+	unsigned flags = PVMOVE_FIRST_TIME;
 	unsigned exclusive;
 	int r = ECMD_FAILED;
 
@@ -485,7 +489,7 @@ static int _set_up_pvmove(struct cmd_context *cmd, const char *pv_name,
 			goto out;
 		}
 
-		first_time = 0;
+		flags &= ~PVMOVE_FIRST_TIME;
 	} else {
 		/* Determine PE ranges to be moved */
 		if (!(source_pvl = create_pv_list(cmd->mem, vg, 1,
@@ -518,9 +522,11 @@ static int _set_up_pvmove(struct cmd_context *cmd, const char *pv_name,
 	/* init_pvmove(1); */
 	/* vg->status |= PVMOVE; */
 
-	if (first_time) {
+	if (flags & PVMOVE_FIRST_TIME) {
+		if (exclusive)
+			flags |= PVMOVE_EXCLUSIVE;
 		if (!_update_metadata
-		    (cmd, vg, lv_mirr, lvs_changed, PVMOVE_FIRST_TIME))
+		    (cmd, vg, lv_mirr, lvs_changed, flags))
 			goto_out;
 	}
 
-- 
1.7.9.1

[PATCH 3/4] Do not allow pvmove if some affected LVs are activated locally or on more nodes while others are activated exclusively.

[Milan Broz]

Current pvmove code can either use local mirror (for exclusive
activation) or cmirror (for clustered LVs).

Because the whole intenal pvmove LV is just segmented LV containing
segments of several top-level LVs, code cannot properly handle
situation if some segment need to be activated exclusively.

Previously, it wrongly activated exclusive LV on all nodes
(locing code allowed it) but now this is no lnger possible.

If there is exclusively activated LV, pvmove is only
possible if all affected LVs are aslo activated exclusively.

(Note that in non-exclusive mode pvmove still activates LVs
on other nodes during move.)

Example - lv1 and lv2 are on sdc:
  Error locking on node bar-01: Device or resource busy
  Error locking on node bar-03: Volume is busy on another node
...
  Failed to activate lv2

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 WHATS_NEW      |    1 +
 tools/pvmove.c |   26 ++++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/WHATS_NEW b/WHATS_NEW
index 93b6440..13cb12d 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.96 - 
 ================================
+  Disallow pvmove for exclusive LV if some affected LVs are not exclusively activated.
   Remove unused and wrongly set cluster VG flag from clvmd lock query command.
   Fix pvmove for exclusively activated LV pvmove in clustered VG. (2.02.86)
   Always free hash table on update_pvid_to_vgid() in lvmetad.
diff --git a/tools/pvmove.c b/tools/pvmove.c
index 40ef069..a6bcfc3 100644
--- a/tools/pvmove.c
+++ b/tools/pvmove.c
@@ -175,13 +175,16 @@ static struct logical_volume *_set_up_pvmove_lv(struct cmd_context *cmd,
 						const char *lv_name,
 						struct dm_list *allocatable_pvs,
 						alloc_policy_t alloc,
-						struct dm_list **lvs_changed)
+						struct dm_list **lvs_changed,
+						unsigned *exclusive)
 {
 	struct logical_volume *lv_mirr, *lv;
 	struct lv_list *lvl;
 	uint32_t log_count = 0;
 	int lv_found = 0;
 	int lv_skipped = 0;
+	int lv_active_count = 0;
+	int lv_exclusive_count = 0;
 
 	/* FIXME Cope with non-contiguous => splitting existing segments */
 	if (!(lv_mirr = lv_create_empty("pvmove%d", NULL,
@@ -235,6 +238,14 @@ static struct logical_volume *_set_up_pvmove_lv(struct cmd_context *cmd,
 			log_print("Skipping locked LV %s", lv->name);
 			continue;
 		}
+
+		if (vg_is_clustered(vg)) {
+			if (lv_is_active_exclusive(lv))
+				lv_exclusive_count++;
+			else if (lv_is_active(lv))
+				lv_active_count++;
+		}
+
 		if (!_insert_pvmove_mirrors(cmd, lv_mirr, source_pvl, lv,
 					    *lvs_changed))
 			return_NULL;
@@ -255,6 +266,17 @@ static struct logical_volume *_set_up_pvmove_lv(struct cmd_context *cmd,
 		return NULL;
 	}
 
+	if (lv_exclusive_count) {
+		if (lv_active_count) {
+			log_error("Cannot move in clustered VG %s "
+				  "if some LVs are activated "
+				  "exclusively while others don't.",
+				  vg->name);
+			return NULL;
+		}
+		*exclusive = 1;
+	}
+
 	if (!lv_add_mirrors(cmd, lv_mirr, 1, 1, 0, 0, log_count,
 			    allocatable_pvs, alloc, MIRROR_BY_SEG)) {
 		log_error("Failed to convert pvmove LV to mirrored");
@@ -510,7 +532,7 @@ static int _set_up_pvmove(struct cmd_context *cmd, const char *pv_name,
 
 		if (!(lv_mirr = _set_up_pvmove_lv(cmd, vg, source_pvl, lv_name,
 						  allocatable_pvs, alloc,
-						  &lvs_changed)))
+						  &lvs_changed, &exclusive)))
 			goto_out;
 	}
 
-- 
1.7.9.1

[PATCH 4/4] Keep exclusive activation in pvmove if LV is already active.

[Milan Broz]

Pvmove should never try to downgrade exclusive lock
for LVs.

This allows pvmove to work again for exclusive activated LVs.

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 WHATS_NEW             |    1 +
 lib/locking/locking.c |    2 +-
 tools/pvmove.c        |    2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/WHATS_NEW b/WHATS_NEW
index 13cb12d..eb5f53b 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.96 - 
 ================================
+  Keep exclusive activation in pvmove if LV is already active.
   Disallow pvmove for exclusive LV if some affected LVs are not exclusively activated.
   Remove unused and wrongly set cluster VG flag from clvmd lock query command.
   Fix pvmove for exclusively activated LV pvmove in clustered VG. (2.02.86)
diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index 1b4e0ac..228078a 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -574,7 +574,7 @@ int activate_lvs(struct cmd_context *cmd, struct dm_list *lvs, unsigned exclusiv
 	struct lv_list *lvl;
 
 	dm_list_iterate_items(lvl, lvs) {
-		if (!exclusive) {
+		if (!exclusive && !lv_is_active_exclusive(lvl->lv)) {
 			if (!activate_lv(cmd, lvl->lv)) {
 				log_error("Failed to activate %s", lvl->lv->name);
 				return 0;
diff --git a/tools/pvmove.c b/tools/pvmove.c
index a6bcfc3..4573dbd 100644
--- a/tools/pvmove.c
+++ b/tools/pvmove.c
@@ -296,7 +296,7 @@ static int _activate_lv(struct cmd_context *cmd, struct logical_volume *lv_mirr,
 {
 	int r = 0;
 
-	if (exclusive)
+	if (exclusive || lv_is_active_exclusive(lv_mirr))
 		r = activate_lv_excl(cmd, lv_mirr);
 	else
 		r = activate_lv(cmd, lv_mirr);
-- 
1.7.9.1

[PATCH 3/4] Do not allow pvmove if some affected LVs are activated locally or on more nodes while others are activated exclusively.

[Milan Broz]

On 03/25/2012 10:56 PM, Milan Broz wrote:
> Current pvmove code can either use local mirror (for exclusive
> activation) or cmirror (for clustered LVs).
>
> Because the whole intenal pvmove LV is just segmented LV containing
> segments of several top-level LVs, code cannot properly handle
> situation if some segment need to be activated exclusively.
>
> Previously, it wrongly activated exclusive LV on all nodes
> (locing code allowed it) but now this is no lnger possible.
>
> If there is exclusively activated LV, pvmove is only
> possible if all affected LVs are aslo activated exclusively.
>
> (Note that in non-exclusive mode pvmove still activates LVs
> on other nodes during move.)
>
> Example - lv1 and lv2 are on sdc:
>    Error locking on node bar-01: Device or resource busy
>    Error locking on node bar-03: Volume is busy on another node
> ...
>    Failed to activate lv2

Ehm. git commented out all lines starting # :-)

It should be read:

Example - lv1 and lv2 are on sdc:

# lvchange -aly vg_test/lv1
# lvchange -aey vg_test/lv2
# pvmove -i 1 /dev/sdc
   Error locking on node bar-01: Device or resource busy
   Error locking on node bar-03: Volume is busy on another node
...
   Failed to activate lv2

m.