Default to checking LV's progress before waiting in _wait_for_single_lv.

Support "wait before testing" using '+' in pvmove and lvconvert
interval.  Doing so overrides the new default of sleeping after checking
the LV's progress.

Sleeping before checking progress can lead to extraneous polldaemons
being left running.  These polldaemons would have otherwise exited had
they checked before sleeping.  Checking progress before sleeping helps
workaround the subtly unreliable nature of "finished" state checking
in _percent_run.

Update test/t-mirror-names.sh to use '+' when providing its lvconvert
interval.

diff --git a/WHATS_NEW b/WHATS_NEW
index ad8c1915a327bf0f16af79d99bb713c850e2b85e..a7c211d11821264927a9bf644e2717ed96c652e2 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,7 @@
 Version 2.02.60
 ===================================
+  Support "wait before testing" using '+' in pvmove and lvconvert interval.
+  Default to checking LV's progress before waiting in _wait_for_single_lv.
   Fix syntax error in cmirror init script.
   Eliminate extra ioctls just to check open_count in _add_new_lv_to_dtree.
   Disable not thread-safe memory debugging if dmeventd is configured.

diff --git a/test/t-mirror-names.sh b/test/t-mirror-names.sh
index b73fd2530d396b4cdebb2f18afc3e4ad09adcf52..edf9fd7ad49a328804f5ebf06e7d9f4131a9a847 100644 (file)
--- a/test/t-mirror-names.sh
+++ b/test/t-mirror-names.sh
@@ -122,7 +122,7 @@ prepare_lvs_
 
 #COMM "converting mirror names is ${lv1}_mimagetmp_2"
 lvcreate -l2 -m1 -n $lv1 $vg 
-lvconvert -m+1 -i1000 -b $vg/$lv1 
+lvconvert -m+1 -i+1000 -b $vg/$lv1
 convlv=$(lv_convert_lv_ "$vg/$lv1") 
 test "$convlv" = "$lv1"_mimagetmp_2 
 lv_devices_ $vg/$lv1 "$convlv" "$lv1"_mimage_2 

diff --git a/tools/polldaemon.c b/tools/polldaemon.c
index 7fdc502e5d0b07dc81b87e78162e8695846bcb1a..427a9c6b0296b88311f2c85544f8a36927934003 100644 (file)
--- a/tools/polldaemon.c
+++ b/tools/polldaemon.c
@@ -156,8 +156,18 @@ static int _check_lv_status(struct cmd_context *cmd,
        return 1;
 }
 
+static void _sleep_and_rescan_devices(struct daemon_parms *parms)
+{
+       /* FIXME Use alarm for regular intervals instead */
+       if (parms->interval && !parms->aborting) {
+               sleep(parms->interval);
+               /* Devices might have changed while we slept */
+               init_full_scan_done(0);
+       }
+}
+
 static int _wait_for_single_lv(struct cmd_context *cmd, const char *name, const char *uuid,
-                                  struct daemon_parms *parms)
+                              struct daemon_parms *parms)
 {
        struct volume_group *vg;
        struct logical_volume *lv;
@@ -165,13 +175,8 @@ static int _wait_for_single_lv(struct cmd_context *cmd, const char *name, const
 
        /* Poll for completion */
        while (!finished) {
-               /* FIXME Also needed in vg/lvchange -ay? */
-               /* FIXME Use alarm for regular intervals instead */
-               if (parms->interval && !parms->aborting) {
-                       sleep(parms->interval);
-                       /* Devices might have changed while we slept */
-                       init_full_scan_done(0);
-               }
+               if (parms->wait_before_testing)
+                       _sleep_and_rescan_devices(parms);
 
                /* Locks the (possibly renamed) VG again */
                vg = parms->poll_fns->get_copy_vg(cmd, name, uuid);
@@ -196,6 +201,21 @@ static int _wait_for_single_lv(struct cmd_context *cmd, const char *name, const
                }
 
                unlock_and_release_vg(cmd, vg, vg->name);
+
+               /*
+                * FIXME Sleeping after testing, while preferred, also works around
+                * unreliable "finished" state checking in _percent_run.  If the
+                * above _check_lv_status is deferred until after the first sleep it
+                * may be that a polldaemon will run without ever completing.
+                *
+                * This happens when one snapshot-merge polldaemon is racing with
+                * another (polling the same LV).  The first to see the LV status
+                * reach the "finished" state will alter the LV that the other
+                * polldaemon(s) are polling.  These other polldaemon(s) can then
+                * continue polling an LV that doesn't have a "status".
+                */
+               if (!parms->wait_before_testing)
+                       _sleep_and_rescan_devices(parms);
        }
 
        return 1;
@@ -253,17 +273,23 @@ int poll_daemon(struct cmd_context *cmd, const char *name, const char *uuid,
        struct daemon_parms parms;
        int daemon_mode = 0;
        int ret = ECMD_PROCESSED;
+       sign_t interval_sign;
 
        parms.aborting = arg_is_set(cmd, abort_ARG);
        parms.background = background;
+       interval_sign = arg_sign_value(cmd, interval_ARG, 0);
+       if (interval_sign == SIGN_MINUS)
+               log_error("Argument to --interval cannot be negative");
        parms.interval = arg_uint_value(cmd, interval_ARG, DEFAULT_INTERVAL);
+       parms.wait_before_testing = (interval_sign == SIGN_PLUS);
        parms.progress_display = 1;
        parms.progress_title = progress_title;
        parms.lv_type = lv_type;
        parms.poll_fns = poll_fns;
 
        if (parms.interval && !parms.aborting)
-               log_verbose("Checking progress every %u seconds",
+               log_verbose("Checking progress %s waiting every %u seconds",
+                           (parms.wait_before_testing ? "after" : "before"),
                            parms.interval);
 
        if (!parms.interval) {

diff --git a/tools/polldaemon.h b/tools/polldaemon.h
index f8afbbe0822a9ae2f8d96a9e7086be9224ce9900..8ebbb2568dd2250d643e83fc2180105a5965bf2a 100644 (file)
--- a/tools/polldaemon.h
+++ b/tools/polldaemon.h
@@ -53,6 +53,7 @@ struct poll_functions {
 
 struct daemon_parms {
        unsigned interval;
+       unsigned wait_before_testing;
        unsigned aborting;
        unsigned background;
        unsigned outstanding_count;