Keep a global (per-format) orphan_vg and keep any and all orphan PVs linked to

it. Avoids the need for FMT_INSTANCE_PV and enables further simplifications. No
functional change, internal refactor only.

7 files changed, 110 insertions, 75 deletions

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 791a1d26..ffc61771 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -1322,8 +1322,6 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info,
 				  const char *vgname, const char *vgid,
 				  uint32_t vgstatus, const char *creation_host)
 {
-        log_error("lvmcache_update_vgname_and_id: %s -> %s", dev_name(info->dev), vgname);
-
 	if (!vgname && !info->vginfo) {
 		log_error(INTERNAL_ERROR "NULL vgname handed to cache");
 		/* FIXME Remove this */
diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index 6b5bdb95..ee3d7251 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -546,6 +546,13 @@ static void _format1_destroy_instance(struct format_instance *fid)
 
 static void _format1_destroy(struct format_type *fmt)
 {
+	/* FIXME out of place, but the main (cmd) pool has been already
+	 * destroyed and touching the fid (also via release_vg) will crash the
+	 * program */
+	dm_hash_destroy(fmt->orphan_vg->hostnames);
+	dm_pool_destroy(fmt->orphan_vg->fid->mem);
+	dm_pool_destroy(fmt->orphan_vg->vgmem);
+
 	dm_free(fmt);
 }
 
@@ -570,6 +577,8 @@ struct format_type *init_format(struct cmd_context *cmd)
 #endif
 {
 	struct format_type *fmt = dm_malloc(sizeof(*fmt));
+	struct format_instance_ctx fic;
+	struct format_instance *fid;
 
 	if (!fmt)
 		return_NULL;
@@ -596,6 +605,19 @@ struct format_type *init_format(struct cmd_context *cmd)
 		return NULL;
 	}
 
+	if (!(fmt->orphan_vg = alloc_vg("text_orphan", cmd, fmt->orphan_vg_name))) {
+		log_error("Couldn't create lvm1 orphan VG.");
+		return NULL;
+	}
+	fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_AUX_MDAS;
+	fic.context.vg_ref.vg_name = fmt->orphan_vg_name;
+	fic.context.vg_ref.vg_id = NULL;
+	if (!(fid = _format1_create_instance(fmt, &fic))) {
+		log_error("Couldn't create lvm1 orphan VG format instance.");
+		return NULL;
+	}
+	vg_set_fid(fmt->orphan_vg, fid);
+
 	log_very_verbose("Initialised format: %s", fmt->name);
 
 	return fmt;
diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c
index 5f0c3b6b..ec0abd1d 100644
--- a/lib/format_pool/format_pool.c
+++ b/lib/format_pool/format_pool.c
@@ -259,6 +259,13 @@ static void _pool_destroy_instance(struct format_instance *fid)
 
 static void _pool_destroy(struct format_type *fmt)
 {
+	/* FIXME out of place, but the main (cmd) pool has been already
+	 * destroyed and touching the fid (also via release_vg) will crash the
+	 * program */
+	dm_hash_destroy(fmt->orphan_vg->hostnames);
+	dm_pool_destroy(fmt->orphan_vg->fid->mem);
+	dm_pool_destroy(fmt->orphan_vg->vgmem);
+
 	dm_free(fmt);
 }
 
@@ -281,6 +288,8 @@ struct format_type *init_format(struct cmd_context *cmd)
 #endif
 {
 	struct format_type *fmt = dm_malloc(sizeof(*fmt));
+	struct format_instance_ctx fic;
+	struct format_instance *fid;
 
 	if (!fmt) {
 		log_error("Unable to allocate format type structure for pool "
@@ -309,6 +318,19 @@ struct format_type *init_format(struct cmd_context *cmd)
 		return NULL;
 	}
 
+	if (!(fmt->orphan_vg = alloc_vg("text_orphan", cmd, fmt->orphan_vg_name))) {
+		log_error("Couldn't create lvm1 orphan VG.");
+		return NULL;
+	}
+	fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_AUX_MDAS;
+	fic.context.vg_ref.vg_name = fmt->orphan_vg_name;
+	fic.context.vg_ref.vg_id = NULL;
+	if (!(fid = _pool_create_instance(fmt, &fic))) {
+		log_error("Couldn't create lvm1 orphan VG format instance.");
+		return NULL;
+	}
+	vg_set_fid(fmt->orphan_vg, fid);
+
 	log_very_verbose("Initialised format: %s", fmt->name);
 
 	return fmt;
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 56c6e2e8..94e63b3b 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1513,8 +1513,8 @@ static int _text_pv_initialise(const struct format_type *fmt,
 static void _text_destroy_instance(struct format_instance *fid)
 {
 	if (--fid->ref_count <= 1) {
-		if (fid->type & FMT_INSTANCE_VG && fid->metadata_areas_index.hash)
-			dm_hash_destroy(fid->metadata_areas_index.hash);
+		if (fid->type & FMT_INSTANCE_VG && fid->metadata_areas_index)
+			dm_hash_destroy(fid->metadata_areas_index);
 		dm_pool_destroy(fid->mem);
 	}
 }
@@ -1541,6 +1541,14 @@ static void _free_raws(struct dm_list *raw_list)
 
 static void _text_destroy(struct format_type *fmt)
 {
+	/* FIXME out of place, but the main (cmd) pool has been already
+	 * destroyed and touching the fid (also via release_vg) will crash the
+	 * program */
+	dm_hash_destroy(fmt->orphan_vg->fid->metadata_areas_index);
+	dm_hash_destroy(fmt->orphan_vg->hostnames);
+	dm_pool_destroy(fmt->orphan_vg->fid->mem);
+	dm_pool_destroy(fmt->orphan_vg->vgmem);
+
 	if (fmt->private) {
 		_free_dirs(&((struct mda_lists *) fmt->private)->dirs);
 		_free_raws(&((struct mda_lists *) fmt->private)->raws);
@@ -1666,22 +1674,7 @@ static int _text_pv_setup(const struct format_type *fmt,
 static int _create_pv_text_instance(struct format_instance *fid,
                                     const struct format_instance_ctx *fic)
 {
-	struct lvmcache_info *info;
-
-	fid->private = NULL;
-
-	if (!(fid->metadata_areas_index.array = dm_pool_zalloc(fid->mem,
-					FMT_TEXT_MAX_MDAS_PER_PV *
-					sizeof(struct metadata_area *)))) {
-		log_error("Couldn't allocate format instance metadata index.");
-		return 0;
-	}
-
-	if (fic->type & FMT_INSTANCE_MDAS &&
-	    (info = lvmcache_info_from_pvid(fic->context.pv_id, 0)))
-		lvmcache_fid_add_mdas_pv(info, fid);
-
-	return 1;
+	return 0; /* just fail */
 }
 
 static void *_create_text_context(struct dm_pool *mem, struct text_context *tc)
@@ -1760,13 +1753,13 @@ static int _create_vg_text_instance(struct format_instance *fid,
 		mda->ops = &_metadata_text_file_backup_ops;
 		mda->metadata_locn = _create_text_context(fid->mem, fic->context.private);
 		mda->status = 0;
-		fid->metadata_areas_index.hash = NULL;
+		fid->metadata_areas_index = NULL;
 		fid_add_mda(fid, mda, NULL, 0, 0);
 	} else {
 		vg_name = fic->context.vg_ref.vg_name;
 		vg_id = fic->context.vg_ref.vg_id;
 
-		if (!(fid->metadata_areas_index.hash = dm_hash_create(128))) {
+		if (!(fid->metadata_areas_index = dm_hash_create(128))) {
 			log_error("Couldn't create metadata index for format "
 				  "instance of VG %s.", vg_name);
 			return 0;
@@ -1872,6 +1865,10 @@ static int _add_metadata_area_to_pv(struct physical_volume *pv,
 	return 1;
 }
 
+static int _text_pv_remove_metadata_area(const struct format_type *fmt,
+					 struct physical_volume *pv,
+					 unsigned mda_index);
+
 static int _text_pv_add_metadata_area(const struct format_type *fmt,
 				      struct physical_volume *pv,
 				      int pe_start_locked,
@@ -1908,9 +1905,12 @@ static int _text_pv_add_metadata_area(const struct format_type *fmt,
 	mda_size = mda_size << SECTOR_SHIFT;
 
 	if (fid_get_mda_indexed(fid, pvid, ID_LEN, mda_index)) {
-		log_error(INTERNAL_ERROR "metadata area with index %u already "
-			"exists on PV %s.", mda_index, pv_dev_name(pv));
-		return 0;
+		if (!_text_pv_remove_metadata_area(fmt, pv, mda_index)) {
+			log_error(INTERNAL_ERROR "metadata area with index %u already "
+				  "exists on PV %s and removal failed.",
+				  mda_index, pv_dev_name(pv));
+			return 0;
+		}
 	}
 
 	/* First metadata area at the start of the device. */
@@ -2270,6 +2270,8 @@ static int _get_config_disk_area(struct cmd_context *cmd,
 
 struct format_type *create_text_format(struct cmd_context *cmd)
 {
+	struct format_instance_ctx fic;
+	struct format_instance *fid;
 	struct format_type *fmt;
 	const struct dm_config_node *cn;
 	const struct dm_config_value *cv;
@@ -2335,6 +2337,21 @@ struct format_type *create_text_format(struct cmd_context *cmd)
 		}
 	}
 
+	if (!(fmt->orphan_vg = alloc_vg("text_orphan", cmd, fmt->orphan_vg_name))) {
+		dm_free(fmt);
+		return NULL;
+	}
+
+	fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_AUX_MDAS;
+	fic.context.vg_ref.vg_name = fmt->orphan_vg_name;
+	fic.context.vg_ref.vg_id = NULL;
+	if (!(fid = _text_create_text_instance(fmt, &fic))) {
+		log_error("Failed to create format instance");
+		release_vg(fmt->orphan_vg);
+		goto err;
+	}
+	vg_set_fid(fmt->orphan_vg, fid);
+
 	log_very_verbose("Initialised format: %s", fmt->name);
 
 	return fmt;
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 9cfe5c7c..16cce296 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -178,6 +178,7 @@ struct format_type {
 	const char *name;
 	const char *alias;
 	const char *orphan_vg_name;
+	struct volume_group *orphan_vg; /* Only one ever exists. */
 	uint32_t features;
 	void *library;
 	void *private;
@@ -237,10 +238,7 @@ struct format_instance {
 	/* FIXME: Try to use the index only. Remove these lists. */
 	struct dm_list metadata_areas_in_use;
 	struct dm_list metadata_areas_ignored;
-	union {
-		struct metadata_area **array;
-		struct dm_hash_table *hash;
-	} metadata_areas_index;
+	struct dm_hash_table *metadata_areas_index;
 
 	void *private;
 };
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index b487d6ad..85d8d39b 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -170,22 +170,18 @@ void add_pvl_to_vgs(struct volume_group *vg, struct pv_list *pvl)
 
 void del_pvl_from_vgs(struct volume_group *vg, struct pv_list *pvl)
 {
-	struct format_instance_ctx fic;
-	struct format_instance *fid;
+	struct lvmcache_info *info;
 
 	vg->pv_count--;
 	dm_list_del(&pvl->list);
-	pvl->pv->vg = NULL; /* orphan */
-
-	/* Use a new PV-based format instance since the PV is orphan now. */
-	fic.type = FMT_INSTANCE_PV | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
-	fic.context.pv_id = (const char *) &pvl->pv->id;
-	fid = pvl->pv->fid->fmt->ops->create_instance(pvl->pv->fid->fmt, &fic);
 
-	pv_set_fid(pvl->pv, fid);
+	pvl->pv->vg = vg->fid->fmt->orphan_vg; /* orphan */
+	if ((info = lvmcache_info_from_pvid((const char *) &pvl->pv->id, 0)))
+		lvmcache_fid_add_mdas(info, vg->fid->fmt->orphan_vg->fid,
+				      (const char *) &pvl->pv->id, ID_LEN);
+	pv_set_fid(pvl->pv, vg->fid->fmt->orphan_vg->fid);
 }
 
-
 /**
  * add_pv_to_vg - Add a physical volume to a volume group
  * @vg - volume group to add to
@@ -1663,13 +1659,16 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
 		goto bad;
 	}
 
-	fic.type = FMT_INSTANCE_PV;
-	fic.context.pv_id = (const char *) &pv->id;
-	if (!(fid = fmt->ops->create_instance(fmt, &fic))) {
-		log_error("Couldn't create format instance for PV %s.", pv_dev_name(pv));
+	struct pv_list *pvl;
+	if (!(pvl = dm_pool_zalloc(mem, sizeof(*pvl)))) {
+		log_error("pv_list allocation in pv_create failed");
 		goto bad;
 	}
-	pv_set_fid(pv, fid);
+
+	pvl->pv = pv;
+	add_pvl_to_vgs(fmt->orphan_vg, pvl);
+	fmt->orphan_vg->extent_count += pv->pe_count;
+	fmt->orphan_vg->free_count += pv->pe_count;
 
 	pv->fmt = fmt;
 	pv->vg_name = fmt->orphan_vg_name;
@@ -2762,18 +2761,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
 	if (!(fmt = lvmcache_fmt_from_vgname(orphan_vgname, NULL, 0)))
 		return_NULL;
 
-	if (!(vg = alloc_vg("vg_read_orphans", cmd, orphan_vgname)))
-		return_NULL;
-
-	/* create format instance with appropriate metadata area */
-	fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_AUX_MDAS;
-	fic.context.vg_ref.vg_name = orphan_vgname;
-	fic.context.vg_ref.vg_id = NULL;
-	if (!(fid = fmt->ops->create_instance(fmt, &fic))) {
-		log_error("Failed to create format instance");
-		goto bad;
-	}
-	vg_set_fid(vg, fid);
+	vg = fmt->orphan_vg;
 
 	baton.warnings = warnings;
 	baton.vg = vg;
@@ -2781,7 +2769,6 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
 
 	return vg;
 bad:
-	release_vg(vg);
 	return NULL;
 }
 
@@ -3574,6 +3561,7 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
 	struct label *label;
 	struct lvmcache_info *info;
 	struct device *dev;
+	const struct format_type *fmt;
 
 	if (!(dev = dev_cache_get(pv_name, cmd->filter)))
 		return_NULL;
@@ -3586,6 +3574,7 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
 	}
 
 	info = (struct lvmcache_info *) label->info;
+	fmt = lvmcache_fmt(info);
 
 	pv = _alloc_pv(pvmem, dev);
 	if (!pv) {
@@ -3611,14 +3600,8 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
 	if (fid)
 		lvmcache_fid_add_mdas(info, fid, (const char *) &pv->id, ID_LEN);
 	else {
-		fic.type = FMT_INSTANCE_PV | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
-		fic.context.pv_id = (const char *) &pv->id;
-		if (!(fid = pv->fmt->ops->create_instance(pv->fmt, &fic))) {
-			log_error("_pv_read: Couldn't create format instance "
-				  "for PV %s", pv_name);
-			goto bad;
-		}
-		pv_set_fid(pv, fid);
+		lvmcache_fid_add_mdas(info, fmt->orphan_vg->fid, (const char *) &pv->id, ID_LEN);
+		pv_set_fid(pv, fmt->orphan_vg->fid);
 	}
 
 	return pv;
@@ -4236,11 +4219,9 @@ int fid_add_mda(struct format_instance *fid, struct metadata_area *mda,
 					    full_key, sizeof(full_key)))
 		return_0;
 
-		dm_hash_insert(fid->metadata_areas_index.hash,
+		dm_hash_insert(fid->metadata_areas_index,
 			       full_key, mda);
 	}
-	else
-		fid->metadata_areas_index.array[sub_key] = mda;
 
 	return 1;
 }
@@ -4275,11 +4256,9 @@ struct metadata_area *fid_get_mda_indexed(struct format_instance *fid,
 		if (!_convert_key_to_string(key, key_len, sub_key,
 					    full_key, sizeof(full_key)))
 			return_NULL;
-		mda = (struct metadata_area *) dm_hash_lookup(fid->metadata_areas_index.hash,
+		mda = (struct metadata_area *) dm_hash_lookup(fid->metadata_areas_index,
 							      full_key);
 	}
-	else
-		mda = fid->metadata_areas_index.array[sub_key];
 
 	return mda;
 }
@@ -4311,9 +4290,8 @@ int fid_remove_mda(struct format_instance *fid, struct metadata_area *mda,
 					    full_key, sizeof(full_key)))
 				return_0;
 
-			dm_hash_remove(fid->metadata_areas_index.hash, full_key);
-		} else
-			fid->metadata_areas_index.array[sub_key] = NULL;
+			dm_hash_remove(fid->metadata_areas_index, full_key);
+		}
 	}
 
 	dm_list_del(&mda->list);
diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c
index 3159e3e9..42cb48cf 100644
--- a/lib/metadata/vg.c
+++ b/lib/metadata/vg.c
@@ -79,7 +79,7 @@ static void _free_vg(struct volume_group *vg)
 
 void release_vg(struct volume_group *vg)
 {
-	if (!vg)
+	if (!vg || (vg->fid && vg == vg->fid->fmt->orphan_vg))
 		return;
 
 	/* Check if there are any vginfo holders */