* [PATCH 1/4] Rename the polldaemon's mirror-specific names
2009-09-29 15:22 [PATCH 0/4] Clean-up polldaemon to be more generic to allow re-use Mike Snitzer
@ 2009-09-29 15:22 ` Mike Snitzer
2009-09-29 15:22 ` [PATCH 2/4] Eliminate _check_lv_status's 'name' argument Mike Snitzer
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Mike Snitzer @ 2009-09-29 15:22 UTC (permalink / raw)
To: lvm-devel
Just drop "mirror" or "mirr" part of names in polldaemon. The polldaemon will
also be used for polling snapshot merging, so naming the arguments with
"mirror" is misleading.
This patch does nothing but change variable and function names.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
---
tools/polldaemon.c | 57 ++++++++++++++++++++++++++---------------------------
tools/polldaemon.h | 6 ++---
2 files changed, 31 insertions(+), 32 deletions(-)
Index: lvm2/tools/polldaemon.c
===================================================================
--- lvm2.orig/tools/polldaemon.c
+++ lvm2/tools/polldaemon.c
@@ -63,11 +63,11 @@ static int _become_daemon(struct cmd_con
return 1;
}
-static int _check_mirror_status(struct cmd_context *cmd,
- struct volume_group *vg,
- struct logical_volume *lv_mirr,
- const char *name, struct daemon_parms *parms,
- int *finished)
+static int _check_lv_status(struct cmd_context *cmd,
+ struct volume_group *vg,
+ struct logical_volume *lv,
+ const char *name, struct daemon_parms *parms,
+ int *finished)
{
struct dm_list *lvs_changed;
float segment_percent = 0.0, overall_percent = 0.0;
@@ -78,22 +78,22 @@ static int _check_mirror_status(struct c
*finished = 1;
if (parms->aborting) {
- if (!(lvs_changed = lvs_using_lv(cmd, vg, lv_mirr))) {
+ if (!(lvs_changed = lvs_using_lv(cmd, vg, lv))) {
log_error("Failed to generate list of copied LVs: "
"can't abort.");
return 0;
}
- parms->poll_fns->finish_copy(cmd, vg, lv_mirr, lvs_changed);
+ parms->poll_fns->finish_copy(cmd, vg, lv, lvs_changed);
return 0;
}
- if (!(status = lv_mirror_percent(cmd, lv_mirr, !parms->interval,
+ if (!(status = lv_mirror_percent(cmd, lv, !parms->interval,
&segment_percent, &event_nr))) {
log_error("ABORTING: Mirror percentage check failed.");
return 0;
}
- overall_status = copy_percent(lv_mirr, &overall_percent);
+ overall_status = copy_percent(lv, &overall_percent);
if (parms->progress_display)
log_print("%s: %s: %.1f%%", name, parms->progress_title,
overall_percent);
@@ -107,22 +107,20 @@ static int _check_mirror_status(struct c
return 1;
}
- if (!(lvs_changed = lvs_using_lv(cmd, vg, lv_mirr))) {
+ if (!(lvs_changed = lvs_using_lv(cmd, vg, lv))) {
log_error("ABORTING: Failed to generate list of copied LVs");
return 0;
}
/* Finished? Or progress to next segment? */
if (overall_status == TARGET_STATUS_FINISHED) {
- if (!parms->poll_fns->finish_copy(cmd, vg, lv_mirr,
- lvs_changed))
+ if (!parms->poll_fns->finish_copy(cmd, vg, lv, lvs_changed))
return 0;
} else {
- if (!parms->poll_fns->update_metadata(cmd, vg, lv_mirr,
+ if (!parms->poll_fns->update_metadata(cmd, vg, lv,
lvs_changed, 0)) {
log_error("ABORTING: Segment progression failed.");
- parms->poll_fns->finish_copy(cmd, vg, lv_mirr,
- lvs_changed);
+ parms->poll_fns->finish_copy(cmd, vg, lv, lvs_changed);
return 0;
}
*finished = 0; /* Another segment */
@@ -131,11 +129,11 @@ static int _check_mirror_status(struct c
return 1;
}
-static int _wait_for_single_mirror(struct cmd_context *cmd, const char *name, const char *uuid,
+static int _wait_for_single_lv(struct cmd_context *cmd, const char *name, const char *uuid,
struct daemon_parms *parms)
{
struct volume_group *vg;
- struct logical_volume *lv_mirr;
+ struct logical_volume *lv;
int finished = 0;
/* Poll for mirror completion */
@@ -157,16 +155,15 @@ static int _wait_for_single_mirror(struc
return 0;
}
- if (!(lv_mirr = parms->poll_fns->get_copy_lv(cmd, vg, name, uuid,
- parms->lv_type))) {
+ if (!(lv = parms->poll_fns->get_copy_lv(cmd, vg, name, uuid,
+ parms->lv_type))) {
log_error("ABORTING: Can't find mirror LV in %s for %s",
vg->name, name);
unlock_and_release_vg(cmd, vg, vg->name);
return 0;
}
- if (!_check_mirror_status(cmd, vg, lv_mirr, name, parms,
- &finished)) {
+ if (!_check_lv_status(cmd, vg, lv, name, parms, &finished)) {
unlock_and_release_vg(cmd, vg, vg->name);
return 0;
}
@@ -182,20 +179,20 @@ static int _poll_vg(struct cmd_context *
{
struct daemon_parms *parms = (struct daemon_parms *) handle;
struct lv_list *lvl;
- struct logical_volume *lv_mirr;
+ struct logical_volume *lv;
const char *name;
int finished;
dm_list_iterate_items(lvl, &vg->lvs) {
- lv_mirr = lvl->lv;
- if (!(lv_mirr->status & parms->lv_type))
+ lv = lvl->lv;
+ if (!(lv->status & parms->lv_type))
continue;
- if (!(name = parms->poll_fns->get_copy_name_from_lv(lv_mirr)))
+ if (!(name = parms->poll_fns->get_copy_name_from_lv(lv)))
continue;
/* FIXME Need to do the activation from _set_up_pvmove here
* if it's not running and we're not aborting */
- if (_check_mirror_status(cmd, vg, lv_mirr, name,
- parms, &finished) && !finished)
+ if (_check_lv_status(cmd, vg, lv, name,
+ parms, &finished) && !finished)
parms->outstanding_count++;
}
@@ -251,12 +248,14 @@ int poll_daemon(struct cmd_context *cmd,
}
if (name) {
- if (!_wait_for_single_mirror(cmd, name, uuid, &parms)) {
+ if (!_wait_for_single_lv(cmd, name, uuid, &parms)) {
stack;
return ECMD_FAILED;
}
- } else
+ } else {
+ /* Only used for pvmove --abort */
_poll_for_all_vgs(cmd, &parms);
+ }
return ECMD_PROCESSED;
}
Index: lvm2/tools/polldaemon.h
===================================================================
--- lvm2.orig/tools/polldaemon.h
+++ lvm2/tools/polldaemon.h
@@ -19,7 +19,7 @@
#include "metadata-exported.h"
struct poll_functions {
- const char *(*get_copy_name_from_lv) (struct logical_volume *lv_mirr);
+ const char *(*get_copy_name_from_lv) (struct logical_volume *lv);
struct volume_group *(*get_copy_vg) (struct cmd_context *cmd,
const char *name,
const char *uuid);
@@ -30,11 +30,11 @@ struct poll_functions {
uint32_t lv_type);
int (*update_metadata) (struct cmd_context *cmd,
struct volume_group *vg,
- struct logical_volume *lv_mirr,
+ struct logical_volume *lv,
struct dm_list *lvs_changed, unsigned flags);
int (*finish_copy) (struct cmd_context *cmd,
struct volume_group *vg,
- struct logical_volume *lv_mirr,
+ struct logical_volume *lv,
struct dm_list *lvs_changed);
};
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 2/4] Eliminate _check_lv_status's 'name' argument
2009-09-29 15:22 [PATCH 0/4] Clean-up polldaemon to be more generic to allow re-use Mike Snitzer
2009-09-29 15:22 ` [PATCH 1/4] Rename the polldaemon's mirror-specific names Mike Snitzer
@ 2009-09-29 15:22 ` Mike Snitzer
2009-09-29 22:09 ` Mike Snitzer
2009-09-29 15:22 ` [PATCH 3/4] Define new periodic polling method: poll_progress Mike Snitzer
2009-09-29 15:22 ` [PATCH 4/4] Move activation of background polling processes to a common function Mike Snitzer
3 siblings, 1 reply; 8+ messages in thread
From: Mike Snitzer @ 2009-09-29 15:22 UTC (permalink / raw)
To: lvm-devel
_check_lv_status's 'name' argument sometimes contains a name of a VG and
sometimes name of an LV, so using it in generic code would be dangerous.
'name' was never used for anything other than progress_display.
The next patch will properly split out context specific
progress_display.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
tools/polldaemon.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
Index: lvm2/tools/polldaemon.c
===================================================================
--- lvm2.orig/tools/polldaemon.c
+++ lvm2/tools/polldaemon.c
@@ -66,7 +66,7 @@ static int _become_daemon(struct cmd_con
static int _check_lv_status(struct cmd_context *cmd,
struct volume_group *vg,
struct logical_volume *lv,
- const char *name, struct daemon_parms *parms,
+ struct daemon_parms *parms,
int *finished)
{
struct dm_list *lvs_changed;
@@ -94,12 +94,6 @@ static int _check_lv_status(struct cmd_c
}
overall_status = copy_percent(lv, &overall_percent);
- if (parms->progress_display)
- log_print("%s: %s: %.1f%%", name, parms->progress_title,
- overall_percent);
- else
- log_verbose("%s: %s: %.1f%%", name, parms->progress_title,
- overall_percent);
if (status < TARGET_STATUS_FINISHED) {
/* The only case the caller *should* try again later */
@@ -163,7 +157,7 @@ static int _wait_for_single_lv(struct cm
return 0;
}
- if (!_check_lv_status(cmd, vg, lv, name, parms, &finished)) {
+ if (!_check_lv_status(cmd, vg, lv, parms, &finished)) {
unlock_and_release_vg(cmd, vg, vg->name);
return 0;
}
@@ -180,19 +174,19 @@ static int _poll_vg(struct cmd_context *
struct daemon_parms *parms = (struct daemon_parms *) handle;
struct lv_list *lvl;
struct logical_volume *lv;
- const char *name;
int finished;
dm_list_iterate_items(lvl, &vg->lvs) {
lv = lvl->lv;
if (!(lv->status & parms->lv_type))
continue;
- if (!(name = parms->poll_fns->get_copy_name_from_lv(lv)))
+ /* NOTE don't need name but this check has significance
+ * because it skips LVs that aren't mirrored */
+ if (parms->poll_fns->get_copy_name_from_lv(lv) == NULL)
continue;
/* FIXME Need to do the activation from _set_up_pvmove here
* if it's not running and we're not aborting */
- if (_check_lv_status(cmd, vg, lv, name,
- parms, &finished) && !finished)
+ if (_check_lv_status(cmd, vg, lv, parms, &finished) && !finished)
parms->outstanding_count++;
}
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 3/4] Define new periodic polling method: poll_progress
2009-09-29 15:22 [PATCH 0/4] Clean-up polldaemon to be more generic to allow re-use Mike Snitzer
2009-09-29 15:22 ` [PATCH 1/4] Rename the polldaemon's mirror-specific names Mike Snitzer
2009-09-29 15:22 ` [PATCH 2/4] Eliminate _check_lv_status's 'name' argument Mike Snitzer
@ 2009-09-29 15:22 ` Mike Snitzer
2009-09-29 22:09 ` Mike Snitzer
2009-09-29 15:22 ` [PATCH 4/4] Move activation of background polling processes to a common function Mike Snitzer
3 siblings, 1 reply; 8+ messages in thread
From: Mike Snitzer @ 2009-09-29 15:22 UTC (permalink / raw)
To: lvm-devel
Define new polling method poll_progress that is called periodically. The method
can return:
0 --- error
PROGRESS_TRY_AGAIN --- should be called again after specified interval
PROGRESS_UPDATE_METADATA --- should call update_metadata method and continue
polling
PROGRESS_FINISHED --- should call finish_copy and exit polling
The hard-coded mirror-specific code in polldaemon was moved to function
poll_mirror_progress. The rest of the polldaemon is mirror-free so it can be
used for merging snapshots as well.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
---
tools/lvconvert.c | 1 +
tools/polldaemon.c | 49 +++++++++++++++++++++++++++++++++++++------------
tools/polldaemon.h | 31 +++++++++++++++++++++----------
tools/pvmove.c | 1 +
4 files changed, 60 insertions(+), 22 deletions(-)
Index: lvm2/tools/lvconvert.c
===================================================================
--- lvm2.orig/tools/lvconvert.c
+++ lvm2/tools/lvconvert.c
@@ -316,6 +316,7 @@ static struct poll_functions _lvconvert_
.get_copy_vg = _get_lvconvert_vg,
.get_copy_lv = _get_lvconvert_lv,
.update_metadata = _update_lvconvert_mirror,
+ .poll_progress = poll_mirror_progress,
.finish_copy = _finish_lvconvert_mirror,
};
Index: lvm2/tools/polldaemon.c
===================================================================
--- lvm2.orig/tools/polldaemon.c
+++ lvm2/tools/polldaemon.c
@@ -63,6 +63,38 @@ static int _become_daemon(struct cmd_con
return 1;
}
+int poll_mirror_progress(struct cmd_context *cmd, struct volume_group *vg,
+ struct logical_volume *lv, struct daemon_parms *parms)
+{
+ float segment_percent = 0.0, overall_percent = 0.0;
+ int status, overall_status;
+ uint32_t event_nr = 0;
+
+ if (!(status = lv_mirror_percent(cmd, lv, !parms->interval,
+ &segment_percent, &event_nr))) {
+ log_error("ABORTING: Mirror percentage check failed.");
+ return 0;
+ }
+
+ overall_status = copy_percent(lv, &overall_percent);
+ if (parms->progress_display)
+ log_print("%s: %s: %.1f%%", lv->name, parms->progress_title,
+ overall_percent);
+ else
+ log_verbose("%s: %s: %.1f%%", lv->name, parms->progress_title,
+ overall_percent);
+
+ if (status < TARGET_STATUS_FINISHED) {
+ /* The only case the caller *should* try again later */
+ return PROGRESS_TRY_AGAIN;
+ }
+
+ if (overall_status == TARGET_STATUS_FINISHED)
+ return PROGRESS_FINISHED;
+ else
+ return PROGRESS_UPDATE_METADATA;
+}
+
static int _check_lv_status(struct cmd_context *cmd,
struct volume_group *vg,
struct logical_volume *lv,
@@ -70,9 +102,7 @@ static int _check_lv_status(struct cmd_c
int *finished)
{
struct dm_list *lvs_changed;
- float segment_percent = 0.0, overall_percent = 0.0;
- int status, overall_status;
- uint32_t event_nr = 0;
+ int r;
/* By default, caller should not retry */
*finished = 1;
@@ -87,16 +117,11 @@ static int _check_lv_status(struct cmd_c
return 0;
}
- if (!(status = lv_mirror_percent(cmd, lv, !parms->interval,
- &segment_percent, &event_nr))) {
- log_error("ABORTING: Mirror percentage check failed.");
+ r = parms->poll_fns->poll_progress(cmd, vg, lv, parms);
+ if (!r)
return 0;
- }
- overall_status = copy_percent(lv, &overall_percent);
-
- if (status < TARGET_STATUS_FINISHED) {
- /* The only case the caller *should* try again later */
+ if (r == PROGRESS_TRY_AGAIN) {
*finished = 0;
return 1;
}
@@ -107,7 +132,7 @@ static int _check_lv_status(struct cmd_c
}
/* Finished? Or progress to next segment? */
- if (overall_status == TARGET_STATUS_FINISHED) {
+ if (r == PROGRESS_FINISHED) {
if (!parms->poll_fns->finish_copy(cmd, vg, lv, lvs_changed))
return 0;
} else {
Index: lvm2/tools/polldaemon.h
===================================================================
--- lvm2.orig/tools/polldaemon.h
+++ lvm2/tools/polldaemon.h
@@ -18,6 +18,19 @@
#include "metadata-exported.h"
+struct poll_functions;
+
+struct daemon_parms {
+ unsigned interval;
+ unsigned aborting;
+ unsigned background;
+ unsigned outstanding_count;
+ unsigned progress_display;
+ const char *progress_title;
+ uint32_t lv_type;
+ struct poll_functions *poll_fns;
+};
+
struct poll_functions {
const char *(*get_copy_name_from_lv) (struct logical_volume *lv);
struct volume_group *(*get_copy_vg) (struct cmd_context *cmd,
@@ -28,6 +41,8 @@ struct poll_functions {
const char *name,
const char *uuid,
uint32_t lv_type);
+ int (*poll_progress) (struct cmd_context *cmd, struct volume_group *vg,
+ struct logical_volume *lv, struct daemon_parms *parms);
int (*update_metadata) (struct cmd_context *cmd,
struct volume_group *vg,
struct logical_volume *lv,
@@ -38,16 +53,12 @@ struct poll_functions {
struct dm_list *lvs_changed);
};
-struct daemon_parms {
- unsigned interval;
- unsigned aborting;
- unsigned background;
- unsigned outstanding_count;
- unsigned progress_display;
- const char *progress_title;
- uint32_t lv_type;
- struct poll_functions *poll_fns;
-};
+int poll_mirror_progress(struct cmd_context *cmd, struct volume_group *vg,
+ struct logical_volume *lv, struct daemon_parms *parms);
+
+#define PROGRESS_TRY_AGAIN 1
+#define PROGRESS_UPDATE_METADATA 2
+#define PROGRESS_FINISHED 3
int poll_daemon(struct cmd_context *cmd, const char *name, const char *uuid,
unsigned background,
Index: lvm2/tools/pvmove.c
===================================================================
--- lvm2.orig/tools/pvmove.c
+++ lvm2/tools/pvmove.c
@@ -562,6 +562,7 @@ static struct poll_functions _pvmove_fns
.get_copy_name_from_lv = get_pvmove_pvname_from_lv_mirr,
.get_copy_vg = _get_move_vg,
.get_copy_lv = find_pvmove_lv_from_pvname,
+ .poll_progress = poll_mirror_progress,
.update_metadata = _update_metadata,
.finish_copy = _finish_pvmove,
};
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 4/4] Move activation of background polling processes to a common function.
2009-09-29 15:22 [PATCH 0/4] Clean-up polldaemon to be more generic to allow re-use Mike Snitzer
` (2 preceding siblings ...)
2009-09-29 15:22 ` [PATCH 3/4] Define new periodic polling method: poll_progress Mike Snitzer
@ 2009-09-29 15:22 ` Mike Snitzer
2009-09-29 20:44 ` Alasdair G Kergon
3 siblings, 1 reply; 8+ messages in thread
From: Mike Snitzer @ 2009-09-29 15:22 UTC (permalink / raw)
To: lvm-devel
Move activation of background polling processes to a common function.
Avoids code duplication and fixes an associated where vgchange would
only restart pvmove and _not_ lvconvert (lvchange would restart
both).
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Reviewed-by: Mike Snitzer <snitzer@redhat.com>
---
tools/lvchange.c | 33 ++++++++++++++++++++-------------
tools/tools.h | 1 +
tools/vgchange.c | 10 ++--------
3 files changed, 23 insertions(+), 21 deletions(-)
Index: lvm2/tools/lvchange.c
===================================================================
--- lvm2.orig/tools/lvchange.c
+++ lvm2/tools/lvchange.c
@@ -101,11 +101,29 @@ static int lvchange_monitoring(struct cm
return 1;
}
+void lvchange_activate_background_polling(struct cmd_context *cmd,
+ struct logical_volume *lv)
+{
+ const char *pvname;
+
+ if ((lv->status & LOCKED) &&
+ (pvname = get_pvmove_pvname_from_lv(lv))) {
+ log_verbose("Spawning background pvmove process for %s",
+ pvname);
+ pvmove_poll(cmd, pvname, 1);
+ }
+
+ if (lv->status & CONVERTING) {
+ log_verbose("Spawning background lvconvert process for %s",
+ lv->name);
+ lvconvert_poll(cmd, lv, 1);
+ }
+}
+
static int lvchange_availability(struct cmd_context *cmd,
struct logical_volume *lv)
{
int activate;
- const char *pvname;
activate = arg_uint_value(cmd, available_ARG, 0);
@@ -136,18 +154,7 @@ static int lvchange_availability(struct
return_0;
}
- if ((lv->status & LOCKED) &&
- (pvname = get_pvmove_pvname_from_lv(lv))) {
- log_verbose("Spawning background pvmove process for %s",
- pvname);
- pvmove_poll(cmd, pvname, 1);
- }
-
- if (lv->status & CONVERTING) {
- log_verbose("Spawning background lvconvert process for %s",
- lv->name);
- lvconvert_poll(cmd, lv, 1);
- }
+ lvchange_activate_background_polling(cmd, lv);
}
return 1;
Index: lvm2/tools/tools.h
===================================================================
--- lvm2.orig/tools/tools.h
+++ lvm2/tools/tools.h
@@ -169,5 +169,6 @@ const char *command_name(struct cmd_cont
int pvmove_poll(struct cmd_context *cmd, const char *pv, unsigned background);
int lvconvert_poll(struct cmd_context *cmd, struct logical_volume *lv, unsigned background);
+void lvchange_activate_background_polling(struct cmd_context *cmd, struct logical_volume *lv);
#endif
Index: lvm2/tools/vgchange.c
===================================================================
--- lvm2.orig/tools/vgchange.c
+++ lvm2/tools/vgchange.c
@@ -56,7 +56,6 @@ static int _activate_lvs_in_vg(struct cm
{
struct lv_list *lvl;
struct logical_volume *lv;
- const char *pvname;
int count = 0;
dm_list_iterate_items(lvl, &vg->lvs) {
@@ -91,13 +90,8 @@ static int _activate_lvs_in_vg(struct cm
} else if (!activate_lv(cmd, lv))
continue;
- if ((lv->status & PVMOVE) &&
- (pvname = get_pvmove_pvname_from_lv_mirr(lv))) {
- log_verbose("Spawning background process for %s %s",
- lv->name, pvname);
- pvmove_poll(cmd, pvname, 1);
- continue;
- }
+ if (activate != CHANGE_AN && activate != CHANGE_ALN)
+ lvchange_activate_background_polling(cmd, lv);
count++;
}
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 4/4] Move activation of background polling processes to a common function.
2009-09-29 15:22 ` [PATCH 4/4] Move activation of background polling processes to a common function Mike Snitzer
@ 2009-09-29 20:44 ` Alasdair G Kergon
0 siblings, 0 replies; 8+ messages in thread
From: Alasdair G Kergon @ 2009-09-29 20:44 UTC (permalink / raw)
To: lvm-devel
On Tue, Sep 29, 2009 at 11:22:57AM -0400, Mike Snitzer wrote:
> Move activation of background polling processes to a common function.
> Avoids code duplication and fixes an associated where vgchange would
> only restart pvmove and _not_ lvconvert (lvchange would restart
> both).
Applied, but I've retained the more-efficient version for pvmove for
vgchange: when you know you are looping through all the LVs in the VG
you can skip the normal (LOCKED) LVs which are being moved and trigger
from the PVMOVE ones that perform the pvmove. As lvchange might only
be processing a single LV, you can't do that.
Alasdair
^ permalink raw reply [flat|nested] 8+ messages in thread