Introduce revert_lv for better pvmove cleanup.

(One further fix needed to remove the stray pvmove LVs left behind.)

diff --git a/WHATS_NEW b/WHATS_NEW
index e2eab96e8072790d5826200f590ce9458a7e8c59..93c2512b7257a6fc246fd17453a373b2e06ce4d2 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.89 - 
 ==================================
+  Introduce revert_lv for better pvmove cleanup.
   Replace incomplete pvmove activation failure recovery code with a message.
   Abort if _finish_pvmove suspend_lvs fails instead of cleaning up incompletely.
   Change suspend_lvs to call vg_revert internally.

diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index f16d006c9a468283bcdcd82308cd3b0d4294c278..aa30670916eada9819fe301556de8a02f5560fa0 100644 (file)
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -406,7 +406,7 @@ error:
 /* Resume the LV if it was active */
 static int do_resume_lv(char *resource, unsigned char lock_flags)
 {
-       int oldmode, origin_only, exclusive;
+       int oldmode, origin_only, exclusive, revert;
 
        /* Is it open ? */
        oldmode = get_current_lock(resource);
@@ -416,8 +416,9 @@ static int do_resume_lv(char *resource, unsigned char lock_flags)
        }
        origin_only = (lock_flags & LCK_ORIGIN_ONLY_MODE) ? 1 : 0;
        exclusive = (oldmode == LCK_EXCL) ? 1 : 0;
+       revert = (lock_flags & LCK_REVERT_MODE) ? 1 : 0;
 
-       if (!lv_resume_if_active(cmd, resource, origin_only, exclusive))
+       if (!lv_resume_if_active(cmd, resource, origin_only, exclusive, revert))
                return EIO;
 
        return 0;

diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index 4ae4192648d2f14b1f1f92887259d81192f72112..51862ac3d1a5cc076465205d70d393b76e4f7872 100644 (file)
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -190,7 +190,7 @@ int lv_resume(struct cmd_context *cmd, const char *lvid_s, unsigned origin_only)
        return 1;
 }
 int lv_resume_if_active(struct cmd_context *cmd, const char *lvid_s,
-                       unsigned origin_only, unsigned exclusive)
+                       unsigned origin_only, unsigned exclusive, unsigned revert)
 {
        return 1;
 }
@@ -1356,14 +1356,16 @@ static int _lv_resume(struct cmd_context *cmd, const char *lvid_s,
                laopts->origin_only = 0;
 
        if (test_mode()) {
-               _skip("Resuming %s%s.", lv->name, laopts->origin_only ? " without snapshots" : "");
+               _skip("Resuming %s%s%s.", lv->name, laopts->origin_only ? " without snapshots" : "",
+                     laopts->revert ? " (reverting)" : "");
                r = 1;
                goto out;
        }
 
-       log_debug("Resuming LV %s/%s%s%s.", lv->vg->name, lv->name,
+       log_debug("Resuming LV %s/%s%s%s%s.", lv->vg->name, lv->name,
                  error_if_not_active ? "" : " if active",
-                 laopts->origin_only ? " without snapshots" : "");
+                 laopts->origin_only ? " without snapshots" : "",
+                 laopts->revert ? " (reverting)" : "");
 
        if (!lv_info(cmd, lv, laopts->origin_only, &info, 0, 0))
                goto_out;
@@ -1395,7 +1397,7 @@ out:
 
 /* Returns success if the device is not active */
 int lv_resume_if_active(struct cmd_context *cmd, const char *lvid_s,
-                       unsigned origin_only, unsigned exclusive)
+                       unsigned origin_only, unsigned exclusive, unsigned revert)
 {
        struct lv_activate_opts laopts = {
                .origin_only = origin_only,
@@ -1404,7 +1406,8 @@ int lv_resume_if_active(struct cmd_context *cmd, const char *lvid_s,
                 * non-clustered target should be used.  This only happens
                 * if exclusive is set.
                 */
-               .exclusive = exclusive
+               .exclusive = exclusive,
+               .revert = revert
        };
 
        return _lv_resume(cmd, lvid_s, &laopts, 0);

diff --git a/lib/activate/activate.h b/lib/activate/activate.h
index 5b90420f8c58f66bc3b59f9dc3164a46beb58114..3b3d738a82709e2d61fde33f16aea066a6310f77 100644 (file)
--- a/lib/activate/activate.h
+++ b/lib/activate/activate.h
@@ -34,6 +34,7 @@ struct lv_activate_opts {
        int exclusive;
        int origin_only;
        int no_merging;
+       unsigned revert;
 };
 
 /* target attribute flags */
@@ -63,7 +64,7 @@ void activation_exit(void);
 int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid_s, unsigned origin_only);
 int lv_resume(struct cmd_context *cmd, const char *lvid_s, unsigned origin_only);
 int lv_resume_if_active(struct cmd_context *cmd, const char *lvid_s,
-                       unsigned origin_only, unsigned exclusive);
+                       unsigned origin_only, unsigned exclusive, unsigned revert);
 int lv_activate(struct cmd_context *cmd, const char *lvid_s, int exclusive);
 int lv_activate_with_filter(struct cmd_context *cmd, const char *lvid_s,
                            int exclusive);

diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c
index 0a3c29c14515080037365dd6dd75e35b2193b6e3..b364d9197088bdfb8bec2ceb4cc04fd18b993440 100644 (file)
--- a/lib/activate/dev_manager.c
+++ b/lib/activate/dev_manager.c
@@ -1590,7 +1590,7 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
                                             layer ? UINT32_C(0) : (uint32_t) lv->major,
                                             layer ? UINT32_C(0) : (uint32_t) lv->minor,
                                             _read_only_lv(lv),
-                                            (lv->vg->status & PRECOMMITTED) ? 1 : 0,
+                                            ((lv->vg->status & PRECOMMITTED) | laopts->revert) ? 1 : 0,
                                             lvlayer,
                                             _get_udev_flags(dm, lv, layer))))
                return_0;

diff --git a/lib/locking/cluster_locking.c b/lib/locking/cluster_locking.c
index 4401bc7b7fd286c4020804a9caa860b29317b307..bd5fe43990ee6cb16e2ce24b5e69959e4b71447b 100644 (file)
--- a/lib/locking/cluster_locking.c
+++ b/lib/locking/cluster_locking.c
@@ -327,6 +327,9 @@ static int _lock_for_cluster(struct cmd_context *cmd, unsigned char clvmd_cmd,
        if (flags & LCK_ORIGIN_ONLY)
                args[1] |= LCK_ORIGIN_ONLY_MODE;
 
+       if (flags & LCK_REVERT)
+               args[1] |= LCK_REVERT_MODE;
+
        if (mirror_in_sync())
                args[1] |= LCK_MIRROR_NOSYNC_MODE;
 

diff --git a/lib/locking/file_locking.c b/lib/locking/file_locking.c
index 93c1e1bd706e858692fbebf13149cf22ccc25abe..468c8b500b26975bedd640ebe7272e629bc34a18 100644 (file)
--- a/lib/locking/file_locking.c
+++ b/lib/locking/file_locking.c
@@ -257,6 +257,7 @@ static int _file_lock_resource(struct cmd_context *cmd, const char *resource,
 {
        char lockfile[PATH_MAX];
        unsigned origin_only = (flags & LCK_ORIGIN_ONLY) ? 1 : 0;
+       unsigned revert = (flags & LCK_REVERT) ? 1 : 0;
 
        switch (flags & LCK_SCOPE_MASK) {
        case LCK_VG:
@@ -292,8 +293,8 @@ static int _file_lock_resource(struct cmd_context *cmd, const char *resource,
        case LCK_LV:
                switch (flags & LCK_TYPE_MASK) {
                case LCK_UNLOCK:
-                       log_very_verbose("Unlocking LV %s%s", resource, origin_only ? " without snapshots" : "");
-                       if (!lv_resume_if_active(cmd, resource, origin_only, 0))
+                       log_very_verbose("Unlocking LV %s%s%s", resource, origin_only ? " without snapshots" : "", revert ? " (reverting)" : "");
+                       if (!lv_resume_if_active(cmd, resource, origin_only, 0, revert))
                                return 0;
                        break;
                case LCK_NULL:

diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index 17fa3a8c6661f8a85ef1963f9b43c07a3ee9dd36..7dfd5fca749ea4832ea89897342cddeffc1754de 100644 (file)
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -493,6 +493,20 @@ int resume_lvs(struct cmd_context *cmd, struct dm_list *lvs)
        return r;
 }
 
+/* Unlock and revert list of LVs */
+int revert_lvs(struct cmd_context *cmd, struct dm_list *lvs)
+{
+       struct lv_list *lvl;
+       int r = 1;
+
+       dm_list_iterate_items(lvl, lvs)
+               if (!revert_lv(cmd, lvl->lv)) {
+                       r = 0;
+                       stack;
+               }
+
+       return r;
+}
 /*
  * Lock a list of LVs.
  * On failure to lock any LV, calls vg_revert() if vg_to_revert is set and 
@@ -511,7 +525,7 @@ int suspend_lvs(struct cmd_context *cmd, struct dm_list *lvs,
                                vg_revert(vg_to_revert);
                        dm_list_uniterate(lvh, lvs, &lvl->list) {
                                lvl = dm_list_item(lvh, struct lv_list);
-                               if (!resume_lv(cmd, lvl->lv))
+                               if (!revert_lv(cmd, lvl->lv))
                                        stack;
                        }
 

diff --git a/lib/locking/locking.h b/lib/locking/locking.h
index 69b31ee092993e26f7d5d42801322d869bb4b171..1924440e5a24127fb763fd5a05534d755eed501a 100644 (file)
--- a/lib/locking/locking.h
+++ b/lib/locking/locking.h
@@ -97,6 +97,7 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
 #define LCK_CLUSTER_VG 0x00000080U     /* VG is clustered */
 #define LCK_CACHE      0x00000100U     /* Operation on cache only using P_ lock */
 #define LCK_ORIGIN_ONLY        0x00000200U     /* Operation should bypass any snapshots */
+#define LCK_REVERT     0x00000400U     /* Revert any incomplete change */
 
 /*
  * Additional lock bits for cluster communication via args[1]
@@ -107,6 +108,7 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
 #define LCK_CONVERT                    0x08    /* Convert existing lock */
 #define LCK_ORIGIN_ONLY_MODE           0x20    /* Same as above */
 #define LCK_TEST_MODE                  0x10    /* Test mode: No activation */
+#define LCK_REVERT_MODE                        0x40    /* Remove inactive tables */
 
 /*
  * Special cases of VG locks.
@@ -164,6 +166,7 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
 
 #define resume_lv(cmd, lv)     lock_lv_vol(cmd, lv, LCK_LV_RESUME)
 #define resume_lv_origin(cmd, lv)      lock_lv_vol(cmd, lv, LCK_LV_RESUME | LCK_ORIGIN_ONLY)
+#define revert_lv(cmd, lv)     lock_lv_vol(cmd, lv, LCK_LV_RESUME | LCK_REVERT)
 #define suspend_lv(cmd, lv)    lock_lv_vol(cmd, lv, LCK_LV_SUSPEND | LCK_HOLD)
 #define suspend_lv_origin(cmd, lv)     lock_lv_vol(cmd, lv, LCK_LV_SUSPEND | LCK_HOLD | LCK_ORIGIN_ONLY)
 #define deactivate_lv(cmd, lv) lock_lv_vol(cmd, lv, LCK_LV_DEACTIVATE)
@@ -191,6 +194,7 @@ struct volume_group;
 int suspend_lvs(struct cmd_context *cmd, struct dm_list *lvs,
                struct volume_group *vg_to_revert);
 int resume_lvs(struct cmd_context *cmd, struct dm_list *lvs);
+int revert_lvs(struct cmd_context *cmd, struct dm_list *lvs);
 int activate_lvs(struct cmd_context *cmd, struct dm_list *lvs, unsigned exclusive);
 
 /* Interrupt handling */

diff --git a/lib/locking/no_locking.c b/lib/locking/no_locking.c
index 212eb4dcf1a0d7eba3b6c3e3233245aaed4b8196..b8ee61375b8cb41f684b7e501b0787f2153f7452 100644 (file)
--- a/lib/locking/no_locking.c
+++ b/lib/locking/no_locking.c
@@ -46,7 +46,7 @@ static int _no_lock_resource(struct cmd_context *cmd, const char *resource,
                case LCK_NULL:
                        return lv_deactivate(cmd, resource);
                case LCK_UNLOCK:
-                       return lv_resume_if_active(cmd, resource, (flags & LCK_ORIGIN_ONLY) ? 1: 0, 0);
+                       return lv_resume_if_active(cmd, resource, (flags & LCK_ORIGIN_ONLY) ? 1: 0, 0, (flags & LCK_REVERT) ? 1 : 0);
                case LCK_READ:
                        return lv_activate_with_filter(cmd, resource, 0);
                case LCK_WRITE:

diff --git a/libdm/libdm-deptree.c b/libdm/libdm-deptree.c
index c38f15c87eabbd017a0555898f37f08c68c7f931..5cf98bca338aff75ea42c944c33597b93e41a7c3 100644 (file)
--- a/libdm/libdm-deptree.c
+++ b/libdm/libdm-deptree.c
@@ -610,6 +610,8 @@ static int _node_clear_table(struct dm_tree_node *dnode)
        if (!info->exists || !info->inactive_table)
                return 1;
 
+// FIXME Get inactive deps.  If any dev referenced has 1 opener and no live table, remove it after the clear.
+
        log_verbose("Clearing inactive table %s (%" PRIu32 ":%" PRIu32 ")",
                    name, info->major, info->minor);
 

diff --git a/tools/pvmove.c b/tools/pvmove.c
index e603086f68d3ecb6461bf40d31be80c0065711df..4a67e481e994353e0d00ce8a1c207dc505c8e2aa 100644 (file)
--- a/tools/pvmove.c
+++ b/tools/pvmove.c
@@ -370,13 +370,19 @@ static int _update_metadata(struct cmd_context *cmd, struct volume_group *vg,
 
        if (!_suspend_lvs(cmd, first_time, lv_mirr, lvs_changed, vg)) {
                log_error("ABORTING: Volume group metadata update failed.");
-               goto out;
+               if (!first_time && !revert_lv(cmd, lv_mirr))
+                       stack;
+               return 0;
        }
 
        /* Commit on-disk metadata */
        if (!vg_commit(vg)) {
                log_error("ABORTING: Volume group metadata update failed.");
-               goto out;
+               if (!_resume_lvs(cmd, first_time, lv_mirr, lvs_changed))
+                       stack;
+               if (!first_time && !revert_lv(cmd, lv_mirr))
+                       stack;
+               return 0;
        }
 
        /* Activate the temporary mirror LV */
@@ -403,7 +409,9 @@ out:
        if (!_resume_lvs(cmd, first_time, lv_mirr, lvs_changed))
                r = 0;
 
-       backup(vg);
+       if (r)
+               backup(vg);
+
        return r;
 }
 
@@ -546,6 +554,8 @@ static int _finish_pvmove(struct cmd_context *cmd, struct volume_group *vg,
        /* Suspend LVs changed (implicitly suspends lv_mirr) */
        if (!suspend_lvs(cmd, lvs_changed, vg)) {
                log_error("ABORTING: Locking LVs to remove temporary mirror failed");
+               if (!revert_lv(cmd, lv_mirr))
+                       stack;
                return 0;
        }
 
@@ -553,9 +563,9 @@ static int _finish_pvmove(struct cmd_context *cmd, struct volume_group *vg,
        if (!vg_commit(vg)) {
                log_error("ABORTING: Failed to write new data locations "
                          "to disk.");
-               if (!resume_lv(cmd, lv_mirr))
+               if (!revert_lv(cmd, lv_mirr))
                        stack;
-               if (!resume_lvs(cmd, lvs_changed))
+               if (!revert_lvs(cmd, lvs_changed))
                        stack;
                return 0;
        }