[lvm-devel] [PATCH 11/17] Refactor pv_setup to not include the PV initialisation code.
Peter Rajnoha
prajnoha at redhat.com
Mon Jan 24 11:04:02 UTC 2011
PV initialisation is done using the new pv_initialise fn. Now, the
pv_setup is supposed to be used only when we're trying to put the
existing PV together with an existing VG.
Also, since the PV is now becoming the part of the VG, we move
any metadata areas from PV-based format instance to VG-based
format_instance. Then we destroy the PV-based one and pv->fid
now references vg->fid.
(...we can still use the same functions to set/get metadata areas
as with the PV-based format_instance, there's no change in use. The
fid handling interface will take care of this internally...)
Signed-off-by: Peter Rajnoha <prajnoha at redhat.com>
---
lib/format1/format1.c | 8 --
lib/format_pool/format_pool.c | 9 --
lib/format_text/archiver.c | 5 +-
lib/format_text/format-text.c | 183 +++++++++++---------------------------
lib/metadata/metadata-exported.h | 7 +-
lib/metadata/metadata.c | 53 +++++------
lib/metadata/metadata.h | 8 +--
tools/vgconvert.c | 6 +-
8 files changed, 85 insertions(+), 194 deletions(-)
diff --git a/lib/format1/format1.c b/lib/format1/format1.c
index cb17ef5..cbb9e76 100644
--- a/lib/format1/format1.c
+++ b/lib/format1/format1.c
@@ -400,14 +400,6 @@ static int _format1_pv_initialise(const struct format_type * fmt,
}
static int _format1_pv_setup(const struct format_type *fmt,
- uint64_t pe_start, uint32_t extent_count,
- uint32_t extent_size,
- unsigned long data_alignment __attribute__((unused)),
- unsigned long data_alignment_offset __attribute__((unused)),
- int pvmetadatacopies __attribute__((unused)),
- uint64_t pvmetadatasize __attribute__((unused)),
- unsigned metadataignore __attribute__((unused)),
- struct dm_list *mdas __attribute__((unused)),
struct physical_volume *pv,
struct volume_group *vg __attribute__((unused)))
{
diff --git a/lib/format_pool/format_pool.c b/lib/format_pool/format_pool.c
index 83f474a..8b1472c 100644
--- a/lib/format_pool/format_pool.c
+++ b/lib/format_pool/format_pool.c
@@ -202,15 +202,6 @@ static int _pool_pv_initialise(const struct format_type *fmt __attribute__((unus
}
static int _pool_pv_setup(const struct format_type *fmt __attribute__((unused)),
- uint64_t pe_start __attribute__((unused)),
- uint32_t extent_count __attribute__((unused)),
- uint32_t extent_size __attribute__((unused)),
- unsigned long data_alignment __attribute__((unused)),
- unsigned long data_alignment_offset __attribute__((unused)),
- int pvmetadatacopies __attribute__((unused)),
- uint64_t pvmetadatasize __attribute__((unused)),
- unsigned metadataignore __attribute__((unused)),
- struct dm_list *mdas __attribute__((unused)),
struct physical_volume *pv __attribute__((unused)),
struct volume_group *vg __attribute__((unused)))
{
diff --git a/lib/format_text/archiver.c b/lib/format_text/archiver.c
index ec65794..5b888fc 100644
--- a/lib/format_text/archiver.c
+++ b/lib/format_text/archiver.c
@@ -333,10 +333,7 @@ int backup_restore_vg(struct cmd_context *cmd, struct volume_group *vg)
pv_dev_name(pv), info->fmt->name);
return 0;
}
- if (!vg->fid->fmt->ops->
- pv_setup(vg->fid->fmt, UINT64_C(0), 0, 0, 0, 0, 0UL,
- UINT64_C(0), 0,
- &vg->fid->metadata_areas_in_use, pv, vg)) {
+ if (!vg->fid->fmt->ops->pv_setup(vg->fid->fmt, pv, vg)) {
log_error("Format-specific setup for %s failed",
pv_dev_name(pv));
return 0;
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index c6adedc..2671adb 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1826,151 +1826,70 @@ static struct metadata_area_ops _metadata_text_raw_ops = {
.mda_locns_match = _mda_locns_match_raw
};
-/* pvmetadatasize in sectors */
-/*
- * pe_start goal: FIXME -- reality of .pv_write complexity undermines this goal
- * - In cases where a pre-existing pe_start is provided (pvcreate --restorefile
- * and vgconvert): pe_start must not be changed (so pv->pe_start = pe_start).
- * - In cases where pe_start is 0: leave pv->pe_start as 0 and defer the
- * setting of pv->pe_start to .pv_write
- */
static int _text_pv_setup(const struct format_type *fmt,
- uint64_t pe_start, uint32_t extent_count,
- uint32_t extent_size, unsigned long data_alignment,
- unsigned long data_alignment_offset,
- int pvmetadatacopies, uint64_t pvmetadatasize,
- unsigned metadataignore, struct dm_list *mdas,
- struct physical_volume *pv, struct volume_group *vg)
+ struct physical_volume *pv,
+ struct volume_group *vg)
{
- struct metadata_area *mda, *mda_new, *mda2;
- struct mda_context *mdac, *mdac2;
- struct dm_list *pvmdas;
- struct lvmcache_info *info;
- int found;
- uint64_t pe_end = 0;
- unsigned mda_count = 0;
- uint64_t mda_size2 = 0;
+ struct format_instance *fid = pv->fid;
+ const char *pvid = (const char *) &pv->id;
+ unsigned mda_index;
+ struct metadata_area *pv_mda;
+ struct mda_context *pv_mdac;
uint64_t pe_count;
+ uint64_t size_reduction = 0;
- /* FIXME Cope with pvchange */
- /* FIXME Merge code with _text_create_text_instance */
-
- /* If new vg, add any further mdas on this PV to the fid's mda list */
- if (vg) {
- /* Iterate through all mdas on this PV */
- if ((info = info_from_pvid(pv->dev->pvid, 0))) {
- pvmdas = &info->mdas;
- dm_list_iterate_items(mda, pvmdas) {
- mda_count++;
- mdac =
- (struct mda_context *) mda->metadata_locn;
-
- /* FIXME Check it isn't already in use */
-
- /* Reduce usable device size */
- if (mda_count > 1)
- mda_size2 = mdac->area.size >> SECTOR_SHIFT;
-
- /* Ensure it isn't already on list */
- found = 0;
- dm_list_iterate_items(mda2, mdas) {
- if (mda2->ops !=
- &_metadata_text_raw_ops) continue;
- mdac2 =
- (struct mda_context *)
- mda2->metadata_locn;
- if (!memcmp
- (&mdac2->area, &mdac->area,
- sizeof(mdac->area))) {
- found = 1;
- break;
- }
- }
- if (found)
- continue;
+ /* FIXME Cope with pvchange
+ * FIXME: What's wrong with pvchange here exactly???
+ * ...despite the fact that pvchange itself is
+ * a hack with respect to pv_write... :)
+ */
- mda_new = mda_copy(fmt->cmd->mem, mda);
- if (!mda_new)
- return_0;
- dm_list_add(mdas, &mda_new->list);
- /* FIXME multiple dev_areas inside area */
- }
- }
+ /* Add any further mdas on this PV to VG's format instance. */
+ for (mda_index = 0; mda_index < FMT_TEXT_MAX_MDAS_PER_PV; mda_index++) {
+ if (!(pv_mda = fid_get_mda_indexed(fid, pvid, ID_LEN, mda_index)))
+ continue;
- /* FIXME Cope with genuine pe_count 0 */
-
- /* If missing, estimate pv->size from file-based metadata */
- if (!pv->size && pv->pe_count)
- pv->size = pv->pe_count * (uint64_t) vg->extent_size +
- pv->pe_start + mda_size2;
-
- /* Recalculate number of extents that will fit */
- if (!pv->pe_count) {
- pe_count = (pv->size - pv->pe_start - mda_size2) /
- vg->extent_size;
- if (pe_count > UINT32_MAX) {
- log_error("PV %s too large for extent size %s.",
- pv_dev_name(pv),
- display_size(vg->cmd, (uint64_t) vg->extent_size));
- return 0;
- }
- pv->pe_count = (uint32_t) pe_count;
- }
+ /* Be sure it's not already in VG's format instance! */
+ if (!fid_get_mda_indexed(vg->fid, pvid, ID_LEN, mda_index))
+ fid_add_mda(vg->fid, pv_mda, pvid, ID_LEN, mda_index);
+ }
- /* Unlike LVM1, we don't store this outside a VG */
- /* FIXME Default from config file? vgextend cmdline flag? */
- pv->status |= ALLOCATABLE_PV;
- } else {
- if (pe_start)
- pv->pe_start = pe_start;
-
- if (!data_alignment)
- data_alignment = find_config_tree_int(pv->fmt->cmd,
- "devices/data_alignment",
- 0) * 2;
-
- if (set_pe_align(pv, data_alignment) != data_alignment &&
- data_alignment) {
- log_error("%s: invalid data alignment of "
- "%lu sectors (requested %lu sectors)",
- pv_dev_name(pv), pv->pe_align, data_alignment);
- return 0;
- }
+ /* If there's the 2nd mda, we need to reduce
+ * usable size for further pe_count calculation! */
+ if ((pv_mda = fid_get_mda_indexed(fid, pvid, ID_LEN, 1)) &&
+ (pv_mdac = pv_mda->metadata_locn))
+ size_reduction = pv_mdac->area.size >> SECTOR_SHIFT;
- if (set_pe_align_offset(pv, data_alignment_offset) != data_alignment_offset &&
- data_alignment_offset) {
- log_error("%s: invalid data alignment offset of "
- "%lu sectors (requested %lu sectors)",
- pv_dev_name(pv), pv->pe_align_offset, data_alignment_offset);
- return 0;
- }
+ /* VG format instance will be used from now on. */
+ if (pv->fid->pv_only) {
+ pv->fmt->ops->destroy_instance(pv->fid);
+ pv->fid = vg->fid;
+ }
- if (pv->pe_align < pv->pe_align_offset) {
- log_error("%s: pe_align (%lu sectors) must not be less "
- "than pe_align_offset (%lu sectors)",
- pv_dev_name(pv), pv->pe_align, pv->pe_align_offset);
- return 0;
- }
+ /* FIXME Cope with genuine pe_count 0 */
- /*
- * This initialization has a side-effect of allowing
- * orphaned PVs to be created with the proper alignment.
- * Setting pv->pe_start here circumvents .pv_write's
- * "pvcreate on PV without prior pvremove" retreival of
- * the PV's previous pe_start.
- * - Without this you get actual != expected pe_start
- * failures in the testsuite.
- */
- if (!pe_start && pv->pe_start < pv->pe_align)
- pv->pe_start = pv->pe_align;
+ /* If missing, estimate pv->size from file-based metadata */
+ if (!pv->size && pv->pe_count)
+ pv->size = pv->pe_count * (uint64_t) vg->extent_size +
+ pv->pe_start + size_reduction;
- if (extent_count)
- pe_end = pe_start + extent_count * extent_size - 1;
- if (!_mda_setup(fmt, pe_start, pe_end, pvmetadatacopies,
- pvmetadatasize, metadataignore, mdas, pv, vg))
- return_0;
+ /* Recalculate number of extents that will fit */
+ if (!pv->pe_count) {
+ pe_count = (pv->size - pv->pe_start - size_reduction) /
+ vg->extent_size;
+ if (pe_count > UINT32_MAX) {
+ log_error("PV %s too large for extent size %s.",
+ pv_dev_name(pv),
+ display_size(vg->cmd, (uint64_t) vg->extent_size));
+ return 0;
+ }
+ pv->pe_count = (uint32_t) pe_count;
}
+ /* Unlike LVM1, we don't store this outside a VG */
+ /* FIXME Default from config file? vgextend cmdline flag? */
+ pv->status |= ALLOCATABLE_PV;
+
return 1;
}
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 627d098..398f245 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -397,9 +397,10 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
int pe_start_locked,
uint32_t existing_extent_count,
uint32_t existing_extent_size,
- int pvmetadatacopies, uint64_t pvmetadatasize,
- unsigned metadataignore,
- struct dm_list *mdas);
+ uint64_t label_sector,
+ int pvmetadatacopies,
+ uint64_t pvmetadatasize,
+ unsigned metadataignore);
int pv_resize(struct physical_volume *pv, struct volume_group *vg,
uint32_t new_pe_count);
int pv_analyze(struct cmd_context *cmd, const char *pv_name,
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 419dcfc..11dc205 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -195,7 +195,6 @@ int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
struct format_instance *fid = vg->fid;
struct dm_pool *mem = vg->vgmem;
char uuid[64] __attribute__((aligned(8)));
- struct dm_list *mdas;
log_verbose("Adding physical volume '%s' to volume group '%s'",
pv_name, vg->name);
@@ -239,24 +238,7 @@ int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
*/
pv->pe_alloc_count = 0;
- /*
- * FIXME: this does not work entirely correctly in the case where a PV
- * has 2 mdas and only one is ignored; ideally all non-ignored mdas
- * should be placed on metadata_areas list and ignored on the
- * metadata_areas_ignored list; however this requires another
- * fairly complex refactoring to remove the 'mdas' parameter from both
- * pv_setup and pv_write. For now, we only put ignored mdas on the
- * metadata_areas_ignored list if all mdas in the PV are ignored;
- * otherwise, we use the non-ignored list.
- */
- if (!pv_mda_used_count(pv))
- mdas = &fid->metadata_areas_ignored;
- else
- mdas = &fid->metadata_areas_in_use;
-
- if (!fid->fmt->ops->pv_setup(fid->fmt, UINT64_C(0), 0,
- vg->extent_size, 0, 0, 0UL, UINT64_C(0),
- 0, mdas, pv, vg)) {
+ if (!fid->fmt->ops->pv_setup(fid->fmt, pv, vg)) {
log_error("Format-specific setup of physical volume '%s' "
"failed.", pv_name);
return 0;
@@ -1488,8 +1470,8 @@ struct physical_volume * pvcreate_single(struct cmd_context *cmd,
pp->data_alignment, pp->data_alignment_offset,
pp->pe_start, pp->pe_start ? 1 : 0,
pp->extent_count, pp->extent_size,
- pp->pvmetadatacopies, pp->pvmetadatasize,
- pp->metadataignore, &mdas))) {
+ pp->labelsector, pp->pvmetadatacopies,
+ pp->pvmetadatasize, pp->metadataignore))) {
log_error("Failed to setup physical volume \"%s\"", pv_name);
goto error;
}
@@ -1595,12 +1577,15 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
int pe_start_locked,
uint32_t existing_extent_count,
uint32_t existing_extent_size,
- int pvmetadatacopies, uint64_t pvmetadatasize,
- unsigned metadataignore, struct dm_list *mdas)
+ uint64_t label_sector,
+ int pvmetadatacopies,
+ uint64_t pvmetadatasize,
+ unsigned metadataignore)
{
const struct format_type *fmt = cmd->fmt;
struct dm_pool *mem = fmt->cmd->mem;
struct physical_volume *pv = _alloc_pv(mem, dev);
+ unsigned mda_index;
if (!pv)
return NULL;
@@ -1648,16 +1633,24 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
pv->fmt = fmt;
pv->vg_name = fmt->orphan_vg_name;
- if (!fmt->ops->pv_setup(fmt, pe_start, existing_extent_count,
- existing_extent_size, data_alignment,
- data_alignment_offset,
- pvmetadatacopies, pvmetadatasize,
- metadataignore, mdas, pv, NULL)) {
- log_error("%s: Format-specific setup of physical volume "
- "failed.", pv_dev_name(pv));
+ if (!fmt->ops->pv_initialise(fmt, label_sector, pe_start, pe_start_locked,
+ existing_extent_count, existing_extent_size,
+ data_alignment, data_alignment_offset, pv)) {
+ log_error("Format-specific initialisation of physical "
+ "volume %s failed.", pv_dev_name(pv));
goto bad;
}
+ for (mda_index = 0; mda_index < pvmetadatacopies; mda_index++) {
+ if (pv->fmt->ops->pv_add_metadata_area &&
+ !pv->fmt->ops->pv_add_metadata_area(pv->fmt, pv, pe_start_locked,
+ mda_index, pvmetadatasize, metadataignore)) {
+ log_error("Failed to add metadata area for "
+ "new physical volume %s", pv_dev_name(pv));
+ goto bad;
+ }
+ }
+
return pv;
bad:
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index ae8dd9a..a4f1fc1 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -261,12 +261,8 @@ struct format_handler {
* vg. eg. pe_count is format specific.
*/
int (*pv_setup) (const struct format_type * fmt,
- uint64_t pe_start, uint32_t extent_count,
- uint32_t extent_size, unsigned long data_alignment,
- unsigned long data_alignment_offset,
- int pvmetadatacopies, uint64_t pvmetadatasize,
- unsigned metadataignore, struct dm_list * mdas,
- struct physical_volume * pv, struct volume_group * vg);
+ struct physical_volume * pv,
+ struct volume_group * vg);
/*
* Add metadata area to a PV. Changes will take effect on pv_write.
diff --git a/tools/vgconvert.c b/tools/vgconvert.c
index 81ff616..e747b0e 100644
--- a/tools/vgconvert.c
+++ b/tools/vgconvert.c
@@ -126,8 +126,10 @@ static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
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), pvmetadatacopies,
- pvmetadatasize, 0, &mdas))) {
+ 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)
--
1.7.3.4
More information about the lvm-devel
mailing list