[lvm-devel] master - Clean up repair and result values in vg_read
David Teigland
teigland at sourceware.org
Tue Jun 12 16:42:13 UTC 2018
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=981a3ba98ed6815a8b761261ed5652ec81294938
Commit: 981a3ba98ed6815a8b761261ed5652ec81294938
Parent: 9a8c36b8917edf00ea875e727b0b775ca5a87a43
Author: David Teigland <teigland at redhat.com>
AuthorDate: Tue Jun 12 09:44:37 2018 -0500
Committer: David Teigland <teigland at redhat.com>
CommitterDate: Tue Jun 12 11:08:26 2018 -0500
Clean up repair and result values in vg_read
Fix the confusing mix of input and output values
in the single variable.
---
lib/metadata/metadata-exported.h | 11 ++--
lib/metadata/metadata.c | 104 +++++++++++++++++++-------------------
lib/metadata/vg.c | 3 +-
tools/toollib.c | 3 +-
4 files changed, 60 insertions(+), 61 deletions(-)
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 36a87b5..501d0fa 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -653,8 +653,6 @@ void pvcreate_params_set_defaults(struct pvcreate_params *pp);
int vg_write(struct volume_group *vg);
int vg_commit(struct volume_group *vg);
void vg_revert(struct volume_group *vg);
-struct volume_group *vg_read_internal(struct cmd_context *cmd, const char *vg_name,
- const char *vgid, uint32_t lockd_state, uint32_t warn_flags, int *consistent);
/*
* Add/remove LV to/from volume group
@@ -689,15 +687,18 @@ int lv_resize(struct logical_volume *lv,
/*
* Return a handle to VG metadata.
*/
+struct volume_group *vg_read_internal(struct cmd_context *cmd,
+ const char *vgname, const char *vgid,
+ uint32_t lockd_state, uint32_t warn_flags,
+ int enable_repair,
+ int *mdas_consistent);
struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
const char *vgid, uint32_t read_flags, uint32_t lockd_state);
struct volume_group *vg_read_for_update(struct cmd_context *cmd, const char *vg_name,
const char *vgid, uint32_t read_flags, uint32_t lockd_state);
struct volume_group *vg_read_orphans(struct cmd_context *cmd,
uint32_t warn_flags,
- const char *orphan_vgname,
- int *consistent);
-
+ const char *orphan_vgname);
/*
* Test validity of a VG handle.
*/
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 9dc5627..a4308bc 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3276,8 +3276,7 @@ static int _vg_read_orphan_pv(struct lvmcache_info *info, void *baton)
/* Make orphan PVs look like a VG. */
struct volume_group *vg_read_orphans(struct cmd_context *cmd,
uint32_t warn_flags,
- const char *orphan_vgname,
- int *consistent)
+ const char *orphan_vgname)
{
const struct format_type *fmt;
struct lvmcache_vginfo *vginfo;
@@ -3624,7 +3623,9 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
const char *vgid,
uint32_t lockd_state,
uint32_t warn_flags,
- int *consistent, unsigned precommitted)
+ int enable_repair,
+ int *mdas_consistent,
+ unsigned precommitted)
{
struct format_instance *fid = NULL;
struct format_instance_ctx fic;
@@ -3637,19 +3638,20 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
int inconsistent_pvs = 0;
int inconsistent_mdas = 0;
int inconsistent_mda_count = 0;
- int strip_historical_lvs = *consistent;
- int update_old_pv_ext = *consistent;
+ int strip_historical_lvs = enable_repair;
+ int update_old_pv_ext = enable_repair;
unsigned use_precommitted = precommitted;
struct dm_list *pvids;
struct pv_list *pvl;
struct dm_list all_pvs;
char uuid[64] __attribute__((aligned(8)));
int skipped_rescan = 0;
-
int reappeared = 0;
struct cached_vg_fmtdata *vg_fmtdata = NULL; /* Additional format-specific data about the vg */
unsigned use_previous_vg;
+ *mdas_consistent = 1;
+
if (is_orphan_vg(vgname)) {
log_very_verbose("Reading VG %s", vgname);
@@ -3658,7 +3660,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
"with pre-commit.");
return NULL;
}
- return vg_read_orphans(cmd, warn_flags, vgname, consistent);
+ return vg_read_orphans(cmd, warn_flags, vgname);
}
uuid[0] = '\0';
@@ -3670,11 +3672,11 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
if (lvmetad_used() && !use_precommitted) {
if ((correct_vg = lvmetad_vg_lookup(cmd, vgname, vgid))) {
dm_list_iterate_items(pvl, &correct_vg->pvs)
- reappeared += _check_reappeared_pv(correct_vg, pvl->pv, *consistent);
- if (reappeared && *consistent)
- *consistent = _repair_inconsistent_vg(correct_vg, lockd_state);
+ reappeared += _check_reappeared_pv(correct_vg, pvl->pv, enable_repair);
+ if (reappeared && enable_repair)
+ *mdas_consistent = _repair_inconsistent_vg(correct_vg, lockd_state);
else
- *consistent = !reappeared;
+ *mdas_consistent = !reappeared;
if (_wipe_outdated_pvs(cmd, correct_vg, &correct_vg->pvs_outdated, lockd_state)) {
/* clear the list */
dm_list_init(&correct_vg->pvs_outdated);
@@ -4153,7 +4155,6 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
log_error(INTERNAL_ERROR "Too many inconsistent MDAs.");
if (!inconsistent_mda_count) {
- *consistent = 0;
_free_pv_list(&all_pvs);
return correct_vg;
}
@@ -4162,13 +4163,9 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
return NULL;
}
- if (!*consistent) {
- _free_pv_list(&all_pvs);
- return correct_vg;
- }
-
- if (cmd->is_clvmd) {
+ if (!enable_repair) {
_free_pv_list(&all_pvs);
+ *mdas_consistent = 0;
return correct_vg;
}
@@ -4181,10 +4178,9 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
/* Don't touch if vgids didn't match */
if (inconsistent_vgid) {
- log_warn("WARNING: Inconsistent metadata UUIDs found for "
- "volume group %s.", vgname);
- *consistent = 0;
+ log_warn("WARNING: Inconsistent metadata UUIDs found for volume group %s.", vgname);
_free_pv_list(&all_pvs);
+ *mdas_consistent = 0;
return correct_vg;
}
@@ -4225,16 +4221,13 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
}
/* We have the VG now finally, check if PV ext info is in sync with VG metadata. */
- if (!cmd->is_clvmd && !_check_or_repair_pv_ext(cmd, correct_vg, lockd_state,
- skipped_rescan ? 0 : *consistent,
+ if (!_check_or_repair_pv_ext(cmd, correct_vg, lockd_state, skipped_rescan ? 0 : enable_repair,
&inconsistent_pvs)) {
release_vg(correct_vg);
return_NULL;
}
- *consistent = !inconsistent_pvs;
-
- if (!cmd->is_clvmd && correct_vg && *consistent && !skipped_rescan) {
+ if (correct_vg && enable_repair && !skipped_rescan) {
if (update_old_pv_ext && !_vg_update_old_pv_ext_if_needed(correct_vg)) {
release_vg(correct_vg);
return_NULL;
@@ -4246,6 +4239,9 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
}
}
+ if (inconsistent_pvs)
+ *mdas_consistent = 0;
+
return correct_vg;
}
@@ -4390,12 +4386,14 @@ static int _check_devs_used_correspond_with_vg(struct volume_group *vg)
struct volume_group *vg_read_internal(struct cmd_context *cmd,
const char *vgname, const char *vgid,
uint32_t lockd_state, uint32_t warn_flags,
- int *consistent)
+ int enable_repair,
+ int *mdas_consistent)
{
struct volume_group *vg;
struct lv_list *lvl;
- if (!(vg = _vg_read(cmd, vgname, vgid, lockd_state, warn_flags, consistent, 0)))
+ if (!(vg = _vg_read(cmd, vgname, vgid, lockd_state,
+ warn_flags, enable_repair, mdas_consistent, 0)))
goto_out;
if (!check_pv_dev_sizes(vg))
@@ -4435,7 +4433,7 @@ struct volume_group *vg_read_internal(struct cmd_context *cmd,
(void) _check_devs_used_correspond_with_vg(vg);
out:
- if (!*consistent && (warn_flags & WARN_INCONSISTENT)) {
+ if (!*mdas_consistent && (warn_flags & WARN_INCONSISTENT)) {
if (is_orphan_vg(vgname))
log_warn("WARNING: Found inconsistent standalone Physical Volumes.");
else
@@ -4756,7 +4754,7 @@ static struct volume_group *_recover_vg(struct cmd_context *cmd,
const char *vg_name, const char *vgid,
int is_shared, uint32_t lockd_state)
{
- int consistent = 1;
+ int mdas_consistent = 0;
struct volume_group *vg;
uint32_t state = 0;
@@ -4777,12 +4775,12 @@ static struct volume_group *_recover_vg(struct cmd_context *cmd,
lockd_state |= LDST_EX;
}
- if (!(vg = vg_read_internal(cmd, vg_name, vgid, lockd_state, WARN_PV_READ, &consistent))) {
+ if (!(vg = vg_read_internal(cmd, vg_name, vgid, lockd_state, 0, 1, &mdas_consistent))) {
unlock_vg(cmd, NULL, vg_name);
return_NULL;
}
- if (!consistent) {
+ if (!mdas_consistent) {
release_vg(vg);
unlock_vg(cmd, NULL, vg_name);
return_NULL;
@@ -5015,15 +5013,17 @@ static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const cha
uint32_t lockd_state)
{
struct volume_group *vg = NULL;
- int consistent = 1;
- int consistent_in;
uint32_t failure = 0;
uint32_t warn_flags = 0;
+ int mdas_consistent = 1;
+ int enable_repair = 1;
int is_shared = 0;
int skip_lock = is_orphan_vg(vg_name) && (read_flags & PROCESS_SKIP_ORPHAN_LOCK);
- if ((read_flags & READ_ALLOW_INCONSISTENT) || (lock_flags != LCK_VG_WRITE))
- consistent = 0;
+ if ((read_flags & READ_ALLOW_INCONSISTENT) || (lock_flags != LCK_VG_WRITE)) {
+ enable_repair = 0;
+ warn_flags |= WARN_INCONSISTENT;
+ }
if (!validate_name(vg_name) && !is_orphan_vg(vg_name)) {
log_error("Volume group name \"%s\" has invalid characters.",
@@ -5043,18 +5043,7 @@ static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const cha
if (is_orphan_vg(vg_name))
status_flags &= ~LVM_WRITE;
- consistent_in = consistent;
-
- warn_flags = WARN_PV_READ;
- if (consistent || (read_flags & READ_WARN_INCONSISTENT))
- warn_flags |= WARN_INCONSISTENT;
-
- /* If consistent == 1, we get NULL here if correction fails. */
- if (!(vg = vg_read_internal(cmd, vg_name, vgid, lockd_state, warn_flags, &consistent))) {
- if (consistent_in && !consistent) {
- failure |= FAILED_INCONSISTENT;
- goto bad;
- }
+ if (!(vg = vg_read_internal(cmd, vg_name, vgid, lockd_state, warn_flags, enable_repair, &mdas_consistent))) {
if (!(read_flags & READ_OK_NOTFOUND))
log_error("Volume group \"%s\" not found", vg_name);
failure |= FAILED_NOTFOUND;
@@ -5064,16 +5053,27 @@ static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const cha
if (!_vg_access_permitted(cmd, vg, lockd_state, &failure))
goto bad;
- /* consistent == 0 when VG is not found, but failed == FAILED_NOTFOUND */
- if (!consistent && !failure) {
+ /*
+ * If we called vg_read_internal above without repair enabled,
+ * and the read found inconsistent mdas, then then get a write/ex
+ * lock and call it again with repair enabled so it will fix
+ * the inconsistent mdas.
+ *
+ * FIXME: factor vg repair out of vg_read. The vg_read caller
+ * should get an error about the vg have problems and then call
+ * a repair-specific function if it wants to. (NB there are
+ * other kinds of repairs hidden in _vg_read that should be
+ * pulled out in addition to _recover_vg).
+ */
+ if (!mdas_consistent && !enable_repair) {
is_shared = vg_is_shared(vg);
release_vg(vg);
+
if (!(vg = _recover_vg(cmd, vg_name, vgid, is_shared, lockd_state))) {
if (is_orphan_vg(vg_name))
log_error("Recovery of standalone physical volumes failed.");
else
- log_error("Recovery of volume group \"%s\" failed.",
- vg_name);
+ log_error("Recovery of volume group \"%s\" failed.", vg_name);
failure |= FAILED_RECOVERY;
goto bad_no_unlock;
}
diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c
index d837a55..75a054f 100644
--- a/lib/metadata/vg.c
+++ b/lib/metadata/vg.c
@@ -671,7 +671,6 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
{
struct pv_list *pvl;
struct volume_group *orphan_vg = NULL;
- int consistent;
int r = 0;
const char *name = pv_dev_name(pv);
@@ -715,7 +714,7 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
vg->extent_count -= pv_pe_count(pv);
/* FIXME: we don't need to vg_read the orphan vg here */
- orphan_vg = vg_read_orphans(cmd, 0, vg->fid->fmt->orphan_vg_name, &consistent);
+ orphan_vg = vg_read_orphans(cmd, 0, vg->fid->fmt->orphan_vg_name);
if (vg_read_error(orphan_vg))
goto bad;
diff --git a/tools/toollib.c b/tools/toollib.c
index fb37d07..7caab45 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -5348,7 +5348,6 @@ int pvcreate_each_device(struct cmd_context *cmd,
struct pv_list *vgpvl;
struct device_list *devl;
const char *pv_name;
- int consistent = 0;
int must_use_all = (cmd->cname->flags & MUST_USE_ALL_ARGS);
int found;
unsigned i;
@@ -5639,7 +5638,7 @@ do_command:
if (pp->preserve_existing && pp->orphan_vg_name) {
log_debug("Using existing orphan PVs in %s.", pp->orphan_vg_name);
- if (!(orphan_vg = vg_read_internal(cmd, pp->orphan_vg_name, NULL, 0, 0, &consistent))) {
+ if (!(orphan_vg = vg_read_orphans(cmd, 0, pp->orphan_vg_name))) {
log_error("Cannot read orphans VG %s.", pp->orphan_vg_name);
goto bad;
}
More information about the lvm-devel
mailing list