From 6e41729eb89cde4b4ad132a8bfbb207f684a6ff4 Mon Sep 17 00:00:00 2001 From: Petr Rockai Date: Fri, 10 Feb 2012 02:53:03 +0000 Subject: [PATCH] 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. --- lib/cache/lvmcache.c | 2 - lib/format1/format1.c | 22 +++++++++++ lib/format_pool/format_pool.c | 22 +++++++++++ lib/format_text/format-text.c | 63 ++++++++++++++++++----------- lib/metadata/metadata-exported.h | 6 +-- lib/metadata/metadata.c | 68 +++++++++++--------------------- lib/metadata/vg.c | 2 +- 7 files changed, 110 insertions(+), 75 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 791a1d26a..ffc617719 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 6b5bdb95d..ee3d72515 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 5f0c3b6bf..ec0abd1d2 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 56c6e2e83..94e63b3b7 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 9cfe5c7c1..16cce2960 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 b487d6ad2..85d8d39bc 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 3159e3e98..42cb48cfb 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 */