[lvm-devel] [PATCH 15/17] Change vg_convert code to actually work with recent changes in metadata handling interface.

Peter Rajnoha prajnoha at redhat.com
Mon Jan 24 11:04:06 UTC 2011


Well, the problem with vgconvert was that it was a little bit mixed up
code that used new PVs in new format to write PVs out and used former
VG with the PV list to do a vg_write/vg_commit. So there was no
interconnection among newly created PVs and the former VG.

When we pass the metadata area information in format_instance, this
really couldn't work with current vgconvert code (unless I do an ugly
hack :)). I need that PV<->VG interconnection. Otherwise I end up with
no metadata areas to write to.

So, imho, to clean this up, we should probably replace VG's pv list
with the newly created PVs in target format directly as we're creating
those new PVs.

I defined a vg_convert fn that takes a VG and target format parameter
and does the conversion. The converted VG structure could be used after
that since it has also the right PV list with the new PVs in new format.

Signed-off-by: Peter Rajnoha <prajnoha at redhat.com>
---
 lib/format_text/format-text.c    |    9 ++--
 lib/metadata/metadata-exported.h |    3 +
 lib/metadata/metadata.c          |  103 ++++++++++++++++++++++++++++++++++++++
 tools/vgconvert.c                |   93 ++++++++--------------------------
 4 files changed, 133 insertions(+), 75 deletions(-)

diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index f17bdb1..04abf58 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1656,11 +1656,12 @@ static int _text_pv_setup(const struct format_type *fmt,
 	    (pv_mdac = pv_mda->metadata_locn))
 		size_reduction = pv_mdac->area.size >> SECTOR_SHIFT;
 
-	/* VG format instance will be used from now on. */
-	if (pv->fid->pv_only) {
+	/* Destroy PV specific format instance. */
+	if (pv->fid->pv_only)
 		pv->fmt->ops->destroy_instance(pv->fid);
-		pv->fid = vg->fid;
-	}
+
+	/* VG format instance will be used from now on. */
+	pv->fid = vg->fid;
 
 	/* FIXME Cope with genuine pe_count 0 */
 
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 74086a2..5a06580 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -415,6 +415,9 @@ void vg_remove_pvs(struct volume_group *vg);
 int vg_remove(struct volume_group *vg);
 int vg_rename(struct cmd_context *cmd, struct volume_group *vg,
 	      const char *new_name);
+int vg_convert(struct cmd_context *cmd, struct volume_group *vg,
+	       const struct format_type *target_fmt, int64_t label_sector,
+	       int pvmetadatacopies, uint64_t pvmetadatasize);
 int vg_extend(struct volume_group *vg, int pv_count, char **pv_names,
 	      struct pvcreate_params *pp);
 int vg_reduce(struct volume_group *vg, char *pv_name);
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index de9960d..5332e45 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -496,6 +496,109 @@ int vg_rename(struct cmd_context *cmd, struct volume_group *vg,
 	return 1;
 }
 
+int vg_convert(struct cmd_context *cmd, struct volume_group *vg,
+	       const struct format_type *target_fmt, int64_t label_sector,
+	       int pvmetadatacopies, uint64_t pvmetadatasize)
+{
+	struct physical_volume *pv, *existing_pv;
+	uint64_t pe_start = 0, pe_end = 0;
+	struct pv_list *pvl;
+	int change_made = 0;
+	const char *vg_name = vg->name;
+
+	/* Replace an old format instance with a new empty one. */
+	vg->fid->fmt->ops->destroy_instance(vg->fid);
+	if (!(vg->fid = target_fmt->ops->create_instance(target_fmt, NULL,
+						vg_name, NULL, NULL, 0))) {
+		log_error("Couldn't create target format instance "
+			  "for VG %s.", vg_name);
+		return 0;
+	}
+
+	/*
+	 * Create new PVs in target format taking original PVs as coimage.
+	 * Write the new PVs out and replace the old PVs in VG structure
+	 * with the new PVs.
+	 */
+	dm_list_iterate_items(pvl, &vg->pvs) {
+		existing_pv = pvl->pv;
+
+		pe_start = pv_pe_start(existing_pv);
+		pe_end = pv_pe_count(existing_pv) * pv_pe_size(existing_pv)
+		    + pe_start - 1;
+
+		if (!(pv = pv_create(cmd, pv_dev(existing_pv),
+				     &existing_pv->id, 0, 0, 0,
+				     pe_start, 1,
+				     pv_pe_count(existing_pv),
+				     pv_pe_size(existing_pv),
+				     label_sector, pvmetadatacopies,
+				     pvmetadatasize, 0))) {
+			log_error("Failed to setup physical volume \"%s\"",
+				  pv_dev_name(existing_pv));
+			if (change_made)
+				goto revert;
+			return 0;
+		}
+
+		/* Need to revert manually if it fails after this point */
+		change_made = 1;
+
+		log_verbose("Set up physical volume for \"%s\" with %" PRIu64
+			    " available sectors", pv_dev_name(pv), pv_size(pv));
+
+		/* Wipe existing label first */
+		if (!label_remove(pv_dev(pv))) {
+			log_error("Failed to wipe existing label on %s",
+				  pv_dev_name(pv));
+		}
+
+		log_very_verbose("Writing physical volume data to disk \"%s\"",
+				 pv_dev_name(pv));
+		/* FIXME: This pv_write will change the VG assignment for the
+		 *	  PV info in the cache to orphan VG! We should just change
+		 *	  the existing VG format information in the cache or throw
+		 *	  the cache away after this pv_write. */
+		if (!(pv_write(cmd, pv))) {
+			log_error("Failed to write physical volume \"%s\"",
+				  pv_dev_name(pv));
+			goto revert;
+		}
+		log_verbose("Physical volume \"%s\" successfully created",
+			    pv_dev_name(pv));
+
+		if (!vg->fid->fmt->ops->pv_setup(vg->fid->fmt, pv, vg)) {
+			log_error("Failed to setup PV %s in new format for VG %s.",
+				  pv_dev_name(pv), vg_name);
+			goto revert;
+		}
+
+		/* FIXME: Free the mem used by the old PV structure? */
+
+		/* Copy relevant fields from old PV and further initialise new PV. */
+		pv->vg = vg;
+		if (!(pv->vg_name = dm_pool_strdup(vg->vgmem, vg_name))) {
+			log_error("vg->name allocation failed for %s",pv_dev_name(pv));
+			goto revert;
+		}
+		memcpy(&pv->vgid, &vg->id, sizeof(vg->id));
+		if (!alloc_pv_segment_whole_pv(vg->vgmem, pv)) {
+			log_error("pv->segments allocation failed for %s", pv_dev_name(pv));
+			goto revert;
+		}
+
+		pvl->pv = pv;
+	}
+
+
+	return 1;
+
+revert:
+	log_error("Use pvcreate and vgcfgrestore to repair "
+		  "from archived metadata.");
+	return 0;
+}
+
 int remove_lvs_in_vg(struct cmd_context *cmd,
 		     struct volume_group *vg,
 		     force_t force)
diff --git a/tools/vgconvert.c b/tools/vgconvert.c
index 21c4d1a..bb48473 100644
--- a/tools/vgconvert.c
+++ b/tools/vgconvert.c
@@ -19,16 +19,11 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
 			    struct volume_group *vg,
 			    void *handle __attribute__((unused)))
 {
-	struct physical_volume *pv, *existing_pv;
+	const struct format_type *target_fmt = cmd->fmt;
 	struct logical_volume *lv;
 	struct lv_list *lvl;
-	uint64_t size = 0;
-	struct dm_list mdas;
 	int pvmetadatacopies = 0;
 	uint64_t pvmetadatasize = 0;
-	uint64_t pe_end = 0, pe_start = 0;
-	struct pv_list *pvl;
-	int change_made = 0;
 	struct lvinfo info;
 	int active = 0;
 
@@ -37,13 +32,13 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
 		return ECMD_FAILED;
 	}
 
-	if (vg->fid->fmt == cmd->fmt) {
+	if (vg->fid->fmt == target_fmt) {
 		log_error("Volume group \"%s\" already uses format %s",
 			  vg_name, cmd->fmt->name);
 		return ECMD_FAILED;
 	}
 
-	if (cmd->fmt->features & FMT_MDAS) {
+	if (target_fmt->features & FMT_MDAS) {
 		if (arg_sign_value(cmd, metadatasize_ARG, 0) == SIGN_MINUS) {
 			log_error("Metadata size may not be negative");
 			return EINVALID_CMD_LINE;
@@ -72,7 +67,7 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
 
 	/* Set PV/LV limit if converting from unlimited metadata format */
 	if (vg->fid->fmt->features & FMT_UNLIMITED_VOLS &&
-	    !(cmd->fmt->features & FMT_UNLIMITED_VOLS)) {
+	    !(target_fmt->features & FMT_UNLIMITED_VOLS)) {
 		if (!vg->max_lv)
 			vg->max_lv = 255;
 		if (!vg->max_pv)
@@ -81,7 +76,7 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
 
 	/* If converting to restricted lvid, check if lvid is compatible */
 	if (!(vg->fid->fmt->features & FMT_RESTRICTED_LVIDS) &&
-	    cmd->fmt->features & FMT_RESTRICTED_LVIDS)
+	    target_fmt->features & FMT_RESTRICTED_LVIDS)
 		dm_list_iterate_items(lvl, &vg->lvs)
 			if (!lvid_in_restricted_range(&lvl->lv->lvid)) {
 				log_error("Logical volume %s lvid format is"
@@ -91,7 +86,7 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
 			}
 
 	/* Attempt to change any LVIDs that are too big */
-	if (cmd->fmt->features & FMT_RESTRICTED_LVIDS) {
+	if (target_fmt->features & FMT_RESTRICTED_LVIDS) {
 		dm_list_iterate_items(lvl, &vg->lvs) {
 			lv = lvl->lv;
 			if (lv->status & SNAPSHOT)
@@ -115,87 +110,43 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
 		return ECMD_FAILED;
 	}
 
-	dm_list_iterate_items(pvl, &vg->pvs) {
-		existing_pv = pvl->pv;
-
-		pe_start = pv_pe_start(existing_pv);
-		pe_end = pv_pe_count(existing_pv) * pv_pe_size(existing_pv)
-		    + pe_start - 1;
-
-		dm_list_init(&mdas);
-		if (!(pv = pv_create(cmd, pv_dev(existing_pv),
-				     &existing_pv->id, size, 0, 0,
-				     pe_start, 1, pv_pe_count(existing_pv),
-				     pv_pe_size(existing_pv),
-				     arg_int64_value(cmd, labelsector_ARG,
-						     DEFAULT_LABELSECTOR),
-				     pvmetadatacopies, pvmetadatasize, 0))) {
-			log_error("Failed to setup physical volume \"%s\"",
-				  pv_dev_name(existing_pv));
-			if (change_made)
-				log_error("Use pvcreate and vgcfgrestore to "
-					  "repair from archived metadata.");
-			return ECMD_FAILED;
-		}
-
-		/* Need to revert manually if it fails after this point */
-		change_made = 1;
-
-		log_verbose("Set up physical volume for \"%s\" with %" PRIu64
-			    " available sectors", pv_dev_name(pv), pv_size(pv));
-
-		/* Wipe existing label first */
-		if (!label_remove(pv_dev(pv))) {
-			log_error("Failed to wipe existing label on %s",
-				  pv_dev_name(pv));
-			log_error("Use pvcreate and vgcfgrestore to repair "
-				  "from archived metadata.");
-			return ECMD_FAILED;
-		}
-
-		log_very_verbose("Writing physical volume data to disk \"%s\"",
-				 pv_dev_name(pv));
-		if (!(pv_write(cmd, pv))) {
-			log_error("Failed to write physical volume \"%s\"",
-				  pv_dev_name(pv));
-			log_error("Use pvcreate and vgcfgrestore to repair "
-				  "from archived metadata.");
-			return ECMD_FAILED;
-		}
-		log_verbose("Physical volume \"%s\" successfully created",
-			    pv_dev_name(pv));
-
-	}
-
 	log_verbose("Deleting existing metadata for VG %s", vg_name);
 	if (!vg_remove_mdas(vg)) {
 		log_error("Removal of existing metadata for %s failed.",
 			  vg_name);
-		log_error("Use pvcreate and vgcfgrestore to repair "
-			  "from archived metadata.");
 		return ECMD_FAILED;
 	}
 
+	if (!vg_convert(cmd, vg, target_fmt,
+			arg_int64_value(cmd, labelsector_ARG,
+					DEFAULT_LABELSECTOR),
+			pvmetadatacopies, pvmetadatasize))
+		return ECMD_FAILED;
+
 	/* FIXME Cache the label format change so we don't have to skip this */
 	if (test_mode()) {
 		log_verbose("Test mode: Skipping metadata writing for VG %s in"
-			    " format %s", vg_name, cmd->fmt->name);
+			    " format %s", vg_name, target_fmt->name);
 		return ECMD_PROCESSED;
 	}
 
 	log_verbose("Writing metadata for VG %s using format %s", vg_name,
-		    cmd->fmt->name);
-	if (!backup_restore_vg(cmd, vg)) {
+		    target_fmt->name);
+	if (!vg_write(vg) || !vg_commit(vg)) {
 		log_error("Conversion failed for volume group %s.", vg_name);
-		log_error("Use pvcreate and vgcfgrestore to repair from "
-			  "archived metadata.");
-		return ECMD_FAILED;
+		goto revert;
 	}
 	log_print("Volume group %s successfully converted", vg_name);
 
 	backup(vg);
 
 	return ECMD_PROCESSED;
+
+revert:
+	log_error("Use pvcreate and vgcfgrestore to repair "
+		  "from archived metadata.");
+	return ECMD_FAILED;
+
 }
 
 int vgconvert(struct cmd_context *cmd, int argc, char **argv)
-- 
1.7.3.4




More information about the lvm-devel mailing list