misc: make lv_is_on_pv use for_each_sub_lv to walk LV tree

Make lv_is_on_pv use for_each_sub_lv to walk the LV tree.  This
reduces code duplication.

diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index d2830b2a28436dac2a64b7c76cee1ef07f3f6417..972831052c2ee42c56b2054f92635bcf69f69a5f 100644 (file)
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -76,88 +76,76 @@ struct lv_names {
        const char *new;
 };
 
-/*
- * lv_is_on_pv
- * @lv:
- * @pv:
- *
- * If any of the component devices of the LV are on the given PV, 1
- * is returned; otherwise 0.  For example if one of the images of a RAID
- * (or its metadata device) is on the PV, 1 would be returned for the
- * top-level LV.
- * If you wish to check the images themselves, you should pass them.
- * Currently handles:
- *  - linear
- *  - stripe
- *  - RAID
- *  - mirror
- *  - thin-pool
- *  - thin
- *
- * FIXME:  This should be made more generic, possibly use 'for_each_sub_lv'.
- * 'for_each_sub_lv' does not yet allow us to short-circuit execution or
- * pass back the values we need yet though (i.e. it only allows success or
- * error - not true or false).
- *
- * Returns: 1 if LV (or part of LV) is on PV, 0 otherwise
- */
-int lv_is_on_pv(struct logical_volume *lv, struct physical_volume *pv)
+struct pv_and_int {
+       struct physical_volume *pv;
+       int *i;
+};
+static int _lv_is_on_pv(struct cmd_context *cmd,
+                       struct logical_volume *lv, void *data)
 {
+       int *is_on_pv = ((struct pv_and_int *)data)->i;
+       struct physical_volume *pv = ((struct pv_and_int *)data)->pv;
        uint32_t s;
        struct physical_volume *pv2;
        struct lv_segment *seg;
 
-       if (!lv)
-               return 0;
-
-       seg = first_seg(lv);
-       if (!seg)
-               return 0;
-
-       /* Check mirror log */
-       if (lv_is_on_pv(seg->log_lv, pv))
-               return 1;
-
-       /* Check thin-pool metadata area */
-       if (lv_is_on_pv(seg->metadata_lv, pv))
-               return 1;
-
-       /* Check thin volume's pool LV */
-       if (lv_is_on_pv(seg->pool_lv, pv))
-               return 1;
+       if (!lv || !(seg = first_seg(lv)))
+               return_0;
 
        /*
-        * Do _not_ check 'external_lv.  We wouldn't count an origin's
-        * PVs as part of a snapshot, so we don't count external_origin's
-        * PVs as part of a thin-snap/LV
+        * If the LV has already been found to be on the PV, then
+        * we don't need to continue checking - just return.
         */
+       if (*is_on_pv)
+               return 1;
 
-       /* Check stack of LVs */
        dm_list_iterate_items(seg, &lv->segments) {
                for (s = 0; s < seg->area_count; s++) {
-                       if (seg_type(seg, s) == AREA_PV) {
-                               pv2 = seg_pv(seg, s);
-                               if (id_equal(&pv->id, &pv2->id))
-                                       return 1;
-                               if (pv->dev && pv2->dev &&
-                                   (pv->dev->dev == pv2->dev->dev))
-                                       return 1;
-                       }
-
-                       if ((seg_type(seg, s) == AREA_LV) &&
-                           lv_is_on_pv(seg_lv(seg, s), pv))
-                               return 1;
-
-                       if (!seg_is_raid(seg))
+                       if (seg_type(seg, s) != AREA_PV)
                                continue;
 
-                       /* This is RAID, so we know the meta_area is AREA_LV */
-                       if (lv_is_on_pv(seg_metalv(seg, s), pv))
+                       pv2 = seg_pv(seg, s);
+                       if (id_equal(&pv->id, &pv2->id)) {
+                               *is_on_pv = 1;
+                               return 1;
+                       }
+                       if (pv->dev && pv2->dev &&
+                           (pv->dev->dev == pv2->dev->dev)) {
+                               *is_on_pv = 1;
                                return 1;
+                       }
                }
        }
 
-       return 0;
+       return 1;
+}
+
+/*
+ * lv_is_on_pv
+ * @lv:
+ * @pv:
+ *
+ * If any of the component devices of the LV are on the given PV, 1
+ * is returned; otherwise 0.  For example if one of the images of a RAID
+ * (or its metadata device) is on the PV, 1 would be returned for the
+ * top-level LV.
+ * If you wish to check the images themselves, you should pass them.
+ *
+ * Returns: 1 if LV (or part of LV) is on PV, 0 otherwise
+ */
+int lv_is_on_pv(struct logical_volume *lv, struct physical_volume *pv)
+{
+       int is_on_pv = 0;
+       struct pv_and_int context = { pv, &is_on_pv };
+
+       if (!_lv_is_on_pv(lv->vg->cmd, lv, &context) ||
+           !for_each_sub_lv(lv->vg->cmd, lv, _lv_is_on_pv, &context))
+               /* Failure only happens if bad arguments are passed */
+               log_error(INTERNAL_ERROR "for_each_sub_lv failure.");
+
+       log_debug_metadata("%s is %son %s", lv->name,
+                          is_on_pv ? "" : "not ", pv_dev_name(pv));
+       return is_on_pv;
 }
 
 /*
@@ -173,13 +161,9 @@ int lv_is_on_pvs(struct logical_volume *lv, struct dm_list *pvs)
        struct pv_list *pvl;
 
        dm_list_iterate_items(pvl, pvs)
-               if (lv_is_on_pv(lv, pvl->pv)) {
-                       log_debug_metadata("%s is on %s", lv->name,
-                                          pv_dev_name(pvl->pv));
+               if (lv_is_on_pv(lv, pvl->pv))
                        return 1;
-               } else
-                       log_debug_metadata("%s is not on %s", lv->name,
-                                          pv_dev_name(pvl->pv));
+
        return 0;
 }