From 8dc351e8d43074a68becee8e6210aab14ce3151a Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Thu, 14 Jan 2010 14:39:57 +0000 Subject: [PATCH] Note some problems still to be addressed. --- lib/activate/dev_manager.c | 8 +++++++- lib/metadata/lv_manip.c | 1 + lib/snapshot/snapshot.c | 6 +++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index de47786a0..6f2a45293 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -327,6 +327,7 @@ static int _status(const char *name, const char *uuid, return 0; } +/* FIXME Is there anything simpler to check for instead? */ static int _lv_has_target_type(struct dev_manager *dm, struct logical_volume *lv, const char *layer, @@ -362,6 +363,7 @@ static int _lv_has_target_type(struct dev_manager *dm, &type, ¶ms); if (type && strncmp(type, target_type, strlen(target_type)) == 0) { + /* FIXME Why the inactive test? */ if (info.live_table && !info.inactive_table) r = 1; break; @@ -447,6 +449,7 @@ static int _percent_run(struct dev_manager *dm, const char *name, * - allows the situation when 'type' is "snapshot-merge" and * 'target_type' is "snapshot" */ + /* FIXME Do this properly - relying on target prefixes is incorrect. (E.g. snapshot-origin)*/ if (!type || !params || strncmp(type, target_type, strlen(target_type))) continue; @@ -1051,7 +1054,7 @@ static int _add_segment_to_dtree(struct dev_manager *dm, return_0; /* If this is a snapshot origin, add real LV */ - /* If this is a snapshot origin w/ merging snapshot, add cow and real LV */ + /* If this is a snapshot origin + merging snapshot, add cow + real LV */ if (lv_is_origin(seg->lv) && !layer) { if (vg_is_clustered(seg->lv->vg)) { log_error("Clustered snapshots are not yet supported"); @@ -1117,6 +1120,8 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, uint32_t read_ahead_flags = UINT32_C(0); uint16_t udev_flags = 0; + /* FIXME Seek a simpler way to lay out the snapshot-merge tree. */ + if (lv_is_origin(lv) && lv_is_merging_origin(lv) && !layer) { /* * Clear merge attributes if merge isn't currently possible: @@ -1125,6 +1130,7 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, * existing nodes' info isn't an option * - but use "snapshot-merge" if it is already being used */ + /* FIXME Avoid this - open_count is always returned by kernel now. */ if ((dev_manager_info(dm->mem, NULL, lv, 0, 1, 0, &dinfo, NULL) && dinfo.open_count) || (dev_manager_info(dm->mem, NULL, find_merging_cow(lv)->cow, diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index 583cc3fd1..d8431f901 100644 --- a/lib/metadata/lv_manip.c +++ b/lib/metadata/lv_manip.c @@ -2142,6 +2142,7 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv, if (!vg_write(vg)) return_0; + /* FIXME Seek a simpler way of dealing with this within the tree. */ /* If no snapshots left or if we stopped merging, reload */ if (origin && (!lv_is_origin(origin) || was_merging)) { if (!suspend_lv(cmd, origin)) { diff --git a/lib/snapshot/snapshot.c b/lib/snapshot/snapshot.c index 8e1f89262..3f59e8109 100644 --- a/lib/snapshot/snapshot.c +++ b/lib/snapshot/snapshot.c @@ -46,10 +46,10 @@ static int _snap_text_import(struct lv_segment *seg, const struct config_node *s old_suppress = log_suppress(1); - cow_name = find_config_str(sn, "merging_store", NULL); - if (cow_name) { + /* FIXME Detect case of both merging_store and cow_store supplied */ + if ((cow_name = find_config_str(sn, "merging_store", NULL))) merge = 1; - } else if (!(cow_name = find_config_str(sn, "cow_store", NULL))) { + else if (!(cow_name = find_config_str(sn, "cow_store", NULL))) { log_suppress(old_suppress); log_error("Snapshot cow storage not specified."); return 0; -- 2.43.5