pv_write: clean up non-orphan format1 PV write

...to not pollute the common and format-independent code in the
abstraction layer above.

The format1 pv_write has common code for writing metadata and
PV header by calling the "write_disks" fn and when rewriting
the header itself only (e.g. just for the purpose of changing
the PV UUID) during the pvchange operation, we had to tweak
this functionality for the format1 case and we had to assign
the PV the orphan state temporarily.

This patch removes the need for this format1 tweak and it calls
the write_disks with appropriate flag indicating whether this is
a PV write call or a VG write call, allowing for metatada update
for the latter one.

Also, a side effect of the former tweak was that it effectively
invalidated the cache (even for the non-format1 PVs) as we
assigned it the orphan state temporarily just for the format1
PV write to pass.

Also, that tweak made it difficult to directly detect whether
a PV was part of a VG or not because the state was incorrect.

Also, it's not necessary to backup and restore some PV fields
when doing a PV write:

  orig_pe_size = pv_pe_size(pv);
  orig_pe_start = pv_pe_start(pv);
  orig_pe_count = pv_pe_count(pv);
  ...
  pv_write(pv)
  ...
  pv->pe_size = orig_pe_size;
  pv->pe_start = orig_pe_start;
  pv->pe_count = orig_pe_count;

...this is already done by the layer below itself (the _format1_pv_write fn).

So let's have this cleaned up so we don't need to be bothered
about any 'format1 special case for pv_write' anymore.

WHATS_NEW

@@ -1,5 +1,6 @@
 Version 2.02.99 - 
 ===================================
+  Clean up format1 PV write to remove a need for an orphan VG for it to pass.
   Fix vgextend to not allow a PV with 0 MDAs to be used while already in a VG.
   Move update_pool_params() from /tools to /lib for better reuse.
   Give precedence to EMC power2 devices with duplicate PVIDs.

lib/format1/disk-rep.c

@@ -674,7 +674,7 @@ static int _write_pvd(struct disk_list *data)
  * assumes the device has been opened.
  */
 static int __write_all_pvd(const struct format_type *fmt __attribute__((unused)),
-			   struct disk_list *data)
+			   struct disk_list *data, int write_vg_metadata)
 {
 	const char *pv_name = dev_name(data->dev);
 
@@ -685,9 +685,9 @@ static int __write_all_pvd(const struct format_type *fmt __attribute__((unused))
 
 	/* vgcache_add(data->pvd.vg_name, data->vgd.vg_uuid, data->dev, fmt); */
 	/*
-	 * Stop here for orphan pv's.
+	 * Stop here for orphan PVs or if VG metadata write not requested.
 	 */
-	if (data->pvd.vg_name[0] == '\0') {
+	if ((data->pvd.vg_name[0] == '\0') || !write_vg_metadata) {
 		/* if (!test_mode())
 		   vgcache_add(data->pvd.vg_name, NULL, data->dev, fmt); */
 		return 1;
@@ -723,14 +723,14 @@ static int __write_all_pvd(const struct format_type *fmt __attribute__((unused))
 /*
  * opens the device and hands to the above fn.
  */
-static int _write_all_pvd(const struct format_type *fmt, struct disk_list *data)
+static int _write_all_pvd(const struct format_type *fmt, struct disk_list *data, int write_vg_metadata)
 {
 	int r;
 
 	if (!dev_open(data->dev))
 		return_0;
 
-	r = __write_all_pvd(fmt, data);
+	r = __write_all_pvd(fmt, data, write_vg_metadata);
 
 	if (!dev_close(data->dev))
 		stack;
@@ -743,12 +743,12 @@ static int _write_all_pvd(const struct format_type *fmt, struct disk_list *data)
  * little sanity checking, so make sure correct
  * data is passed to here.
  */
-int write_disks(const struct format_type *fmt, struct dm_list *pvs)
+int write_disks(const struct format_type *fmt, struct dm_list *pvs, int write_vg_metadata)
 {
 	struct disk_list *dl;
 
 	dm_list_iterate_items(dl, pvs) {
-		if (!(_write_all_pvd(fmt, dl)))
+		if (!(_write_all_pvd(fmt, dl, write_vg_metadata)))
 			return_0;
 
 		log_very_verbose("Successfully wrote data to %s",

lib/format1/disk-rep.h

@@ -197,7 +197,8 @@ int read_pvs_in_vg(const struct format_type *fmt, const char *vg_name,
 		   struct dev_filter *filter,
 		   struct dm_pool *mem, struct dm_list *results);
 
-int write_disks(const struct format_type *fmt, struct dm_list *pvds);
+int write_disks(const struct format_type *fmt, struct dm_list *pvds,
+		int write_vg_metadata);
 
 /*
  * Functions to translate to between disk and in

lib/format1/format1.c

@@ -299,7 +299,7 @@ static int _format1_vg_write(struct format_instance *fid, struct volume_group *v
 
 	r = (_flatten_vg(fid, mem, vg, &pvds, fid->fmt->cmd->dev_dir,
 			 fid->fmt->cmd->filter) &&
-	     write_disks(fid->fmt, &pvds));
+	     write_disks(fid->fmt, &pvds, 1));
 
 	lvmcache_update_vg(vg, 0);
 	dm_pool_destroy(mem);
@@ -458,7 +458,7 @@ static int _format1_pv_write(const struct format_type *fmt, struct physical_volu
 	dl->pvd.pe_on_disk.base = LVM1_PE_ALIGN << SECTOR_SHIFT;
 
 	dm_list_add(&pvs, &dl->list);
-	if (!write_disks(fmt, &pvs))
+	if (!write_disks(fmt, &pvs, 0))
 		goto_bad;
 
 	goto out;

tools/pvchange.c

@@ -19,13 +19,7 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,
 			    struct physical_volume *pv,
 			    void *handle __attribute__((unused)))
 {
-	uint32_t orig_pe_alloc_count;
-	/* FIXME Next three only required for format1. */
-	uint32_t orig_pe_count, orig_pe_size;
-	uint64_t orig_pe_start;
-
 	const char *pv_name = pv_dev_name(pv);
-	const char *orig_vg_name;
 	char uuid[64] __attribute__((aligned(8)));
 
 	int allocatable = 0;
@@ -129,29 +123,11 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,
 		if (!id_write_format(&pv->id, uuid, sizeof(uuid)))
 			return 0;
 		log_verbose("Changing uuid of %s to %s.", pv_name, uuid);
-		if (!is_orphan(pv)) {
+		if (!is_orphan(pv) && (!pv_write(cmd, pv, 1))) {
-			orig_vg_name = pv_vg_name(pv);
-			orig_pe_alloc_count = pv_pe_alloc_count(pv);
-
-			/* FIXME format1 pv_write doesn't preserve these. */
-			orig_pe_size = pv_pe_size(pv);
-			orig_pe_start = pv_pe_start(pv);
-			orig_pe_count = pv_pe_count(pv);
-
-			pv->vg_name = pv->fmt->orphan_vg_name;
-			pv->pe_alloc_count = 0;
-			if (!(pv_write(cmd, pv, 0))) {
 			log_error("pv_write with new uuid failed "
 				  "for %s.", pv_name);
 			return 0;
 		}
-			pv->vg_name = orig_vg_name;
-			pv->pe_alloc_count = orig_pe_alloc_count;
-
-			pv->pe_size = orig_pe_size;
-			pv->pe_start = orig_pe_start;
-			pv->pe_count = orig_pe_count;
-		}
 	}
 
 	log_verbose("Updating physical volume \"%s\"", pv_name);