pvmove cluster fixes
cover pvmove cluster fixes
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(-)
Signature · 1 line
1.7.9.1
PATCH 1/4 Remove unused and wrongly set cluster VG flag from clvmd lock query command.
--- WHATS_NEW | 1 + lib/locking/cluster_locking.c | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-)
WHATS_NEW +1 -0
@@ -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.
lib/locking/cluster_locking.c +1 -1
@@ -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);
Signature · 1 line
1.7.9.1
PATCH 2/4 Use new flag PVMOVE_EXCLUSIVE in update_metatada call.
In reply to Milan BrozThere is no real functional change in this patch except it avoids checking cluster cmirror module twice.
(Flag used in following patch.)
--- tools/pvmove.c | 16 +++++++++++----- 1 files changed, 11 insertions(+), 5 deletions(-)
tools/pvmove.c +11 -5
@@ -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; }
Signature · 1 line
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.
In reply to Milan BrozCurrent 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
--- WHATS_NEW | 1 + tools/pvmove.c | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-)
WHATS_NEW +1 -0
@@ -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.
tools/pvmove.c +24 -2
@@ -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; }
Signature · 1 line
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.
Quoted 13 lines · 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.
Quoted 8 lines
(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.
PATCH 4/4 Keep exclusive activation in pvmove if LV is already active.
In reply to Milan BrozPvmove should never try to downgrade exclusive lock for LVs.
This allows pvmove to work again for exclusive activated LVs.
--- WHATS_NEW | 1 + lib/locking/locking.c | 2 +- tools/pvmove.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-)
WHATS_NEW +1 -0
@@ -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)
lib/locking/locking.c +1 -1
@@ -574,7 +574,7 @@ int activate_lvs(struct cmd_context *cmd, struct dm_list *lvs, unsigned exclusivstruct 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 ;
tools/pvmove.c +1 -1
@@ -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);
Signature · 1 line
1.7.9.1