Support vgsplit on cached volumes

[mingnus]

I'm trying vgsplit on VG containing cached volumes. The code seems okay, except the following:

1. _move_cache() should not move the volume if the underlying volumes were not moved.
2. To support cached pools, _move_cache() should go before _move_thins()

Suggested patch:

diff --git a/tools/vgsplit.c b/tools/vgsplit.c
index 02f7132a9..e163ee466 100644
--- a/tools/vgsplit.c
+++ b/tools/vgsplit.c
@@ -388,10 +388,6 @@ static int _move_cache(struct volume_group *vg_from,
 		 *        cache LVs.
 		 * Waiting for next release before fixing and enabling.
 		 */
-		log_error("Unable to split VG while it contains cache LVs");
-		return 0;
-
-		/* NOTREACHED */
 
 		if (lv_is_cache(lv)) {
 			orig = seg_lv(seg, 0);
@@ -431,7 +427,7 @@ static int _move_cache(struct volume_group *vg_from,
 				  lv->name, meta->name);
 			return 0;
 		}
-		if (!_move_one_lv(vg_from, vg_to, lvh))
+		if (is_moving && !_move_one_lv(vg_from, vg_to, lvh))
 			return_0;
 	}
 
@@ -666,13 +662,13 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
 	if (!(_move_snapshots(vg_from, vg_to)))
 		goto_bad;
 
+	if (!(_move_cache(vg_from, vg_to)))
+		goto_bad;
+
 	/* Move required pools across */
 	if (!(_move_thins(vg_from, vg_to)))
 		goto_bad;
 
-	if (!(_move_cache(vg_from, vg_to)))
-		goto_bad;
-
 	/* Split metadata areas and check if both vgs have at least one area */
 	if (!(vg_split_mdas(cmd, vg_from, vg_to)) && vg_from->pv_count) {
 		log_error("Cannot split: Nowhere to store metadata for new Volume Group");

[mingnus]

Bugfix for splitting unused cache pool:

1. fix segfault caused by uninitialized pointer
2. fix uninitialized local flag

diff --git a/tools/vgsplit.c b/tools/vgsplit.c
index 16c5877f6..b76d5a68d 100644
--- a/tools/vgsplit.c
+++ b/tools/vgsplit.c
@@ -398,16 +398,16 @@ static int _move_cache(struct volume_group *vg_from,
                        /* Ensure all components are coming along */
                        is_moving = _lv_is_in_vg(vg_to, orig);
                } else {
-                       if (!dm_list_empty(&seg->lv->segs_using_this_lv) &&
-                           !(cache_seg = get_only_segment_using_this_lv(seg->lv)))
-                               return_0;
-                       orig = seg_lv(cache_seg, 0);
+                       if (!dm_list_empty(&seg->lv->segs_using_this_lv)) {
+                               if (!(cache_seg = get_only_segment_using_this_lv(seg->lv)))
+                                       return_0;
+                               orig = seg_lv(cache_seg, 0);
+                       }
                        data = seg_lv(seg, 0);
                        meta = seg->metadata_lv;
 
-                       if (_lv_is_in_vg(vg_to, data) ||
-                           _lv_is_in_vg(vg_to, meta))
-                               is_moving = 1;
+                       is_moving = (_lv_is_in_vg(vg_to, data) ||
+                                    _lv_is_in_vg(vg_to, meta)) ? 1 : 0;
                }
 
                if (orig && (_lv_is_in_vg(vg_to, orig) != is_moving)) {

[zkabelac]

Hi

Thanks, good catch.

[zkabelac]

Committed somewhat different fix:
https://www.redhat.com/archives/lvm-devel/2019-March/msg00035.html

I assume it's worth to back-port to stable branch.

[zkabelac]

Now also in stable branch:
https://www.redhat.com/archives/lvm-devel/2019-April/msg00061.html