[lvm-devel] [PATCH] Refactor lib/ code to allow deferred PV labelling

[Petr Rockai <prockai redhat com>]

Hi,

this patch is a major part of a 623808 fix, making it possible to create
the PV structures in memory in an lvmlib app, and only writing the
labels upon a vg_write. Among other useful properties introduced by this
option, it should make it possible to get a pe_start out of the library
without actually creating a VG on disks, which was the original use case
for this change.

When this is in, I will follow up with the lvmlib side of the necessary
changes, which should be relatively simple in comparison.

Yours,
   Petr

Tue Apr 19 17:36:56 CEST 2011  Petr Rockai <me mornfall net>
  * Refactor lib/ code to allow deferred PV labelling when creating VGs.
diff -rN -up old-lvmlib-pvcreate/lib/format1/format1.c new-lvmlib-pvcreate/lib/format1/format1.c
--- old-lvmlib-pvcreate/lib/format1/format1.c	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/format1/format1.c	2011-04-27 14:02:11.000000000 +0200
@@ -406,6 +406,8 @@ static int _format1_pv_write(const struc
 	struct disk_list *dl;
 	struct dm_list pvs;
 	struct lvmcache_info *info;
+	int pe_count, pe_size, pe_start;
+	int r = 1;
 
 	if (!(info = lvmcache_add(fmt->labeller, (char *) &pv->id, pv->dev,
 				  pv->vg_name, NULL, 0)))
@@ -418,6 +420,10 @@ static int _format1_pv_write(const struc
 
 	dm_list_init(&pvs);
 
+	pe_count = pv->pe_count;
+	pe_size = pv->pe_size;
+	pe_start = pv->pe_start;
+
 	/* Ensure any residual PE structure is gone */
 	pv->pe_size = pv->pe_count = 0;
 	pv->pe_start = LVM1_PE_ALIGN;
@@ -430,6 +436,8 @@ static int _format1_pv_write(const struc
 
 	dl->mem = mem;
 	dl->dev = pv->dev;
+	dm_list_init(&dl->uuids);
+	dm_list_init(&dl->lvds);
 
 	if (!export_pv(fmt->cmd, mem, NULL, &dl->pvd, pv))
 		goto_bad;
@@ -444,12 +452,18 @@ static int _format1_pv_write(const struc
 	if (!write_disks(fmt, &pvs))
 		goto_bad;
 
-	dm_pool_destroy(mem);
-	return 1;
+	goto out;
 
       bad:
+	r = 0;
+
+      out:
+	pv->pe_size = pe_size;
+	pv->pe_count = pe_count;
+	pv->pe_start = pe_start;
+
 	dm_pool_destroy(mem);
-	return 0;
+	return r;
 }
 
 static int _format1_vg_setup(struct format_instance *fid, struct volume_group *vg)
diff -rN -up old-lvmlib-pvcreate/lib/format1/import-export.c new-lvmlib-pvcreate/lib/format1/import-export.c
--- old-lvmlib-pvcreate/lib/format1/import-export.c	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/format1/import-export.c	2011-04-27 14:02:11.000000000 +0200
@@ -149,7 +149,7 @@ int export_pv(struct cmd_context *cmd, s
 
 	memcpy(pvd->pv_uuid, pv->id.uuid, ID_LEN);
 
-	if (pv->vg_name && !is_orphan(pv)) {
+	if (pv->vg_name && !is_orphan(pv) && !(pv->status & UNLABELLED_PV)) {
 		if (!_check_vg_name(pv->vg_name))
 			return_0;
 		strncpy((char *)pvd->vg_name, pv->vg_name, sizeof(pvd->vg_name));
diff -rN -up old-lvmlib-pvcreate/lib/format_text/format-text.c new-lvmlib-pvcreate/lib/format_text/format-text.c
--- old-lvmlib-pvcreate/lib/format_text/format-text.c	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/format_text/format-text.c	2011-04-27 14:02:11.000000000 +0200
@@ -1281,6 +1281,9 @@ static int _text_pv_write(const struct f
 	    ((label_sector = fid_pv_tc->label_sector) != -1))
 		label->sector = label_sector;
 
+	if (pv->status & UNLABELLED_PV)
+		label->sector = pv->label_sector;
+
 	info->device_size = pv->size << SECTOR_SHIFT;
 	info->fmt = fmt;
 
@@ -1572,6 +1575,7 @@ static int _text_pv_initialise(const str
 	if (label_sector != -1) {
 		fid_pv_tc = (struct text_fid_pv_context *) pv->fid->private;
 		fid_pv_tc->label_sector = label_sector;
+                pv->label_sector = label_sector;
 	}
 
 	return 1;
diff -rN -up old-lvmlib-pvcreate/lib/metadata/metadata.c new-lvmlib-pvcreate/lib/metadata/metadata.c
--- old-lvmlib-pvcreate/lib/metadata/metadata.c	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/metadata/metadata.c	2011-04-27 14:02:11.000000000 +0200
@@ -192,6 +192,7 @@ void del_pvl_from_vgs(struct volume_grou
  * @vg - volume group to add to
  * @pv_name - name of the pv (to be removed)
  * @pv - physical volume to add to volume group
+ * @pp - physical volume creation params (OPTIONAL)
  *
  * Returns:
  *  0 - failure
@@ -199,8 +200,9 @@ void del_pvl_from_vgs(struct volume_grou
  * FIXME: remove pv_name - obtain safely from pv
  */
 int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
-		 struct physical_volume *pv)
+		 struct physical_volume *pv, struct pvcreate_params *pp)
 {
+	struct pv_to_create *pvc;
 	struct pv_list *pvl;
 	struct format_instance *fid = vg->fid;
 	struct dm_pool *mem = vg->vgmem;
@@ -289,6 +291,16 @@ int add_pv_to_vg(struct volume_group *vg
 	vg->extent_count += pv->pe_count;
 	vg->free_count += pv->pe_count;
 
+	if (pv->status & UNLABELLED_PV) {
+		if (!(pvc = dm_pool_zalloc(mem, sizeof(*pvc)))) {
+			log_error("pv_to_create allocation for '%s' failed", pv_name);
+			return 0;
+		}
+		pvc->pv = pv;
+		pvc->pp = pp;
+		dm_list_add(&vg->pvs_to_create, &pvc->list);
+	}
+
 	return 1;
 }
 
@@ -640,11 +652,11 @@ static int vg_extend_single_pv(struct vo
 			  "physical volume", pv_name);
 		return 0;
 	} else if (!pv && pp) {
-		pv = pvcreate_single(vg->cmd, pv_name, pp);
+		pv = pvcreate_single(vg->cmd, pv_name, pp, 0);
 		if (!pv)
 			return 0;
 	}
-	if (!add_pv_to_vg(vg, pv_name, pv)) {
+	if (!add_pv_to_vg(vg, pv_name, pv, pp)) {
 		free_pv_fid(pv);
 		return 0;
 	}
@@ -1425,6 +1437,47 @@ void pvcreate_params_set_defaults(struct
 	pp->metadataignore = DEFAULT_PVMETADATAIGNORE;
 }
 
+
+static int _pvcreate_write(struct cmd_context *cmd, struct pv_to_create *pvc)
+{
+	int zero = pvc->pp->zero;
+	struct physical_volume *pv = pvc->pv;
+	struct device *dev = pv->dev;
+	const char *pv_name = dev_name(dev);
+
+	/* Wipe existing label first */
+	if (!label_remove(pv_dev(pv))) {
+		log_error("Failed to wipe existing label on %s", pv_name);
+		return 0;
+	}
+
+	if (zero) {
+		log_verbose("Zeroing start of device %s", pv_name);
+		if (!dev_open_quiet(dev)) {
+			log_error("%s not opened: device not zeroed", pv_name);
+			return 0;
+		}
+
+		if (!dev_set(dev, UINT64_C(0), (size_t) 2048, 0)) {
+			log_error("%s not wiped: aborting", pv_name);
+			dev_close(dev);
+			return 0;
+		}
+		dev_close(dev);
+	}
+
+	log_error("Writing physical volume data to disk \"%s\"",
+			 pv_name);
+
+	if (!(pv_write(cmd, pv, 1))) {
+		log_error("Failed to write physical volume \"%s\"", pv_name);
+		return 0;
+	}
+
+	log_print("Physical volume \"%s\" successfully created", pv_name);
+	return 1;
+}
+
 /*
  * pvcreate_single() - initialize a device with PV label and metadata area
  *
@@ -1438,7 +1491,8 @@ void pvcreate_params_set_defaults(struct
  */
 struct physical_volume * pvcreate_single(struct cmd_context *cmd,
 					 const char *pv_name,
-					 struct pvcreate_params *pp)
+					 struct pvcreate_params *pp,
+					 int write_now)
 {
 	struct physical_volume *pv = NULL;
 	struct device *dev;
@@ -1475,6 +1529,7 @@ struct physical_volume * pvcreate_single
 	}
 
 	dm_list_init(&mdas);
+
 	if (!(pv = pv_create(cmd, dev, pp->idp, pp->size,
 			     pp->data_alignment, pp->data_alignment_offset,
 			     pp->pe_start ? pp->pe_start : PV_PE_START_CALC,
@@ -1488,37 +1543,16 @@ struct physical_volume * pvcreate_single
 	log_verbose("Set up physical volume for \"%s\" with %" PRIu64
 		    " available sectors", pv_name, pv_size(pv));
 
-	/* Wipe existing label first */
-	if (!label_remove(pv_dev(pv))) {
-		log_error("Failed to wipe existing label on %s", pv_name);
-		goto bad;
-	}
-
-	if (pp->zero) {
-		log_verbose("Zeroing start of device %s", pv_name);
-		if (!dev_open_quiet(dev)) {
-			log_error("%s not opened: device not zeroed", pv_name);
+	if (write_now) {
+		struct pv_to_create pvc;
+		pvc.pp = pp;
+		pvc.pv = pv;
+		if (!_pvcreate_write(cmd, &pvc))
 			goto bad;
-		}
-
-		if (!dev_set(dev, UINT64_C(0), (size_t) 2048, 0)) {
-			log_error("%s not wiped: aborting", pv_name);
-			dev_close(dev);
-			goto bad;
-		}
-		dev_close(dev);
-	}
-
-	log_very_verbose("Writing physical volume data to disk \"%s\"",
-			 pv_name);
-
-	if (!(pv_write(cmd, pv, 0))) {
-		log_error("Failed to write physical volume \"%s\"", pv_name);
-		goto bad;
+	} else {
+		pv->status |= UNLABELLED_PV;
 	}
 
-	log_print("Physical volume \"%s\" successfully created", pv_name);
-
 	return pv;
 
 bad:
@@ -2465,6 +2499,7 @@ out:
 int vg_write(struct volume_group *vg)
 {
 	struct dm_list *mdah;
+        struct pv_to_create *pv_to_create;
 	struct metadata_area *mda;
 
 	if (!vg_validate(vg))
@@ -2502,6 +2537,12 @@ int vg_write(struct volume_group *vg)
 
 	vg->seqno++;
 
+        dm_list_iterate_items(pv_to_create, &vg->pvs_to_create) {
+		if (!_pvcreate_write(vg->cmd, pv_to_create))
+			return 0;
+		pv_to_create->pv->status &= ~UNLABELLED_PV;
+        }
+
 	/* Write to each copy of the metadata area */
 	dm_list_iterate_items(mda, &vg->fid->metadata_areas_in_use) {
 		if (!mda->ops->vg_write) {
diff -rN -up old-lvmlib-pvcreate/lib/metadata/metadata-exported.h new-lvmlib-pvcreate/lib/metadata/metadata-exported.h
--- old-lvmlib-pvcreate/lib/metadata/metadata-exported.h	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/metadata/metadata-exported.h	2011-04-27 14:02:11.000000000 +0200
@@ -78,6 +78,7 @@
 
 #define REPLICATOR		0x20000000U	/* LV -internal use only for replicator */
 #define REPLICATOR_LOG		0x40000000U	/* LV -internal use only for replicator-dev */
+#define UNLABELLED_PV           0x80000000U     /* PV -this PV had no label written yet */
 
 #define LVM_READ              	0x00000100U	/* LV VG */
 #define LVM_WRITE             	0x00000200U	/* LV VG */
@@ -360,7 +361,8 @@ struct pvcreate_params {
 
 struct physical_volume *pvcreate_single(struct cmd_context *cmd,
 					const char *pv_name,
-					struct pvcreate_params *pp);
+					struct pvcreate_params *pp,
+					int write_now);
 void pvcreate_params_set_defaults(struct pvcreate_params *pp);
 
 /*
diff -rN -up old-lvmlib-pvcreate/lib/metadata/metadata.h new-lvmlib-pvcreate/lib/metadata/metadata.h
--- old-lvmlib-pvcreate/lib/metadata/metadata.h	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/metadata/metadata.h	2011-04-27 14:02:11.000000000 +0200
@@ -470,7 +470,7 @@ struct id pv_vgid(const struct physical_
 
 struct physical_volume *pv_by_path(struct cmd_context *cmd, const char *pv_name);
 int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
-		 struct physical_volume *pv);
+		 struct physical_volume *pv, struct pvcreate_params *pp);
 int vg_mark_partial_lvs(struct volume_group *vg);
 int is_mirror_image_removable(struct logical_volume *mimage_lv, void *baton);
 
diff -rN -up old-lvmlib-pvcreate/lib/metadata/pv.h new-lvmlib-pvcreate/lib/metadata/pv.h
--- old-lvmlib-pvcreate/lib/metadata/pv.h	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/metadata/pv.h	2011-04-27 14:02:11.000000000 +0200
@@ -51,6 +51,9 @@ struct physical_volume {
 	unsigned long pe_align;
 	unsigned long pe_align_offset;
 
+	/* NB. Only useful/used when status & UNLABELLED_PV! */
+	int64_t label_sector;
+
 	struct dm_list segments;	/* Ordered pv_segments covering complete PV */
 	struct dm_list tags;
 };
diff -rN -up old-lvmlib-pvcreate/lib/metadata/vg.c new-lvmlib-pvcreate/lib/metadata/vg.c
--- old-lvmlib-pvcreate/lib/metadata/vg.c	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/metadata/vg.c	2011-04-27 14:02:11.000000000 +0200
@@ -43,6 +43,7 @@ struct volume_group *alloc_vg(const char
 	vg->alloc = ALLOC_NORMAL;
 
 	dm_list_init(&vg->pvs);
+	dm_list_init(&vg->pvs_to_create);
 	dm_list_init(&vg->lvs);
 	dm_list_init(&vg->tags);
 	dm_list_init(&vg->removed_pvs);
diff -rN -up old-lvmlib-pvcreate/lib/metadata/vg.h new-lvmlib-pvcreate/lib/metadata/vg.h
--- old-lvmlib-pvcreate/lib/metadata/vg.h	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/lib/metadata/vg.h	2011-04-27 14:02:11.000000000 +0200
@@ -31,6 +31,12 @@ typedef enum {
 	ALLOC_INHERIT
 } alloc_policy_t;
 
+struct pv_to_create {
+	struct dm_list list;
+	struct physical_volume *pv;
+	struct pvcreate_params *pp;
+};
+
 struct volume_group {
 	struct cmd_context *cmd;
 	struct dm_pool *vgmem;
@@ -59,6 +65,13 @@ struct volume_group {
 	struct dm_list pvs;
 
 	/*
+	 * List of physical volumes that were used in vgextend but do not carry
+	 * a PV label yet. They need to be pvcreate'd at vg_write time.
+	 */
+
+	struct dm_list pvs_to_create;
+
+	/*
 	 * logical volumes
 	 * The following relationship should always hold:
 	 * dm_list_size(lvs) = user visible lv_count + snapshot_count + other invisible LVs
diff -rN -up old-lvmlib-pvcreate/tools/pvcreate.c new-lvmlib-pvcreate/tools/pvcreate.c
--- old-lvmlib-pvcreate/tools/pvcreate.c	2011-04-27 14:02:11.000000000 +0200
+++ new-lvmlib-pvcreate/tools/pvcreate.c	2011-04-27 14:02:11.000000000 +0200
@@ -112,7 +112,7 @@ int pvcreate(struct cmd_context *cmd, in
 
 		unescape_colons_and_at_signs(argv[i], NULL, NULL);
 
-		if (!(pv = pvcreate_single(cmd, argv[i], &pp))) {
+		if (!(pv = pvcreate_single(cmd, argv[i], &pp, 1))) {
 			stack;
 			ret = ECMD_FAILED;
 		}

-- 
id' Ash = Ash; id' Dust = Dust; id' _ = undefined