From af8d20048fbb0941cb81042fa8c11f8d6b6766db Mon Sep 17 00:00:00 2001 From: David Teigland Date: Fri, 15 Apr 2016 14:19:46 -0500 Subject: lvmetad: disable if device scan fails If a command begins repopulating the lvmetad cache, and fails part way through, it should set the disabled state in lvmetad so other commands don't use bad data. If a subsequent scan succeeds, the disabled state is cleared. --- daemons/lvmetad/lvmetad-core.c | 10 +++++++--- lib/cache/lvmetad.c | 36 ++++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c index 4ad4b274f..6c3a912c3 100644 --- a/daemons/lvmetad/lvmetad-core.c +++ b/daemons/lvmetad/lvmetad-core.c @@ -201,8 +201,9 @@ struct vg_info { #define GLFL_DISABLE_REASON_DIRECT 0x00000004 #define GLFL_DISABLE_REASON_LVM1 0x00000008 #define GLFL_DISABLE_REASON_DUPLICATES 0x00000010 +#define GLFL_DISABLE_REASON_SCANERROR 0x00000020 -#define GLFL_DISABLE_REASON_ALL (GLFL_DISABLE_REASON_DIRECT | GLFL_DISABLE_REASON_LVM1 | GLFL_DISABLE_REASON_DUPLICATES) +#define GLFL_DISABLE_REASON_ALL (GLFL_DISABLE_REASON_DIRECT | GLFL_DISABLE_REASON_LVM1 | GLFL_DISABLE_REASON_DUPLICATES | GLFL_DISABLE_REASON_SCANERROR) #define VGFL_INVALID 0x00000001 @@ -2511,6 +2512,8 @@ static response set_global_info(lvmetad_state *s, request r) reason_flags |= GLFL_DISABLE_REASON_LVM1; if (strstr(reason, "DUPLICATES")) reason_flags |= GLFL_DISABLE_REASON_DUPLICATES; + if (strstr(reason, "SCANERROR")) + reason_flags |= GLFL_DISABLE_REASON_SCANERROR; } if (global_invalid != -1) { @@ -2564,10 +2567,11 @@ static response get_global_info(lvmetad_state *s, request r) memset(reason, 0, sizeof(reason)); if (s->flags & GLFL_DISABLE) { - snprintf(reason, REASON_BUF_SIZE - 1, "%s%s%s", + snprintf(reason, REASON_BUF_SIZE - 1, "%s%s%s%s", (s->flags & GLFL_DISABLE_REASON_DIRECT) ? "DIRECT," : "", (s->flags & GLFL_DISABLE_REASON_LVM1) ? "LVM1," : "", - (s->flags & GLFL_DISABLE_REASON_DUPLICATES) ? "DUPLICATES," : ""); + (s->flags & GLFL_DISABLE_REASON_DUPLICATES) ? "DUPLICATES," : "", + (s->flags & GLFL_DISABLE_REASON_SCANERROR) ? "SCANERROR," : ""); } if (!reason[0]) diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c index f3df96855..cabdd21b6 100644 --- a/lib/cache/lvmetad.c +++ b/lib/cache/lvmetad.c @@ -1706,11 +1706,6 @@ int lvmetad_pvscan_single(struct cmd_context *cmd, struct device *dev, if (!baton.vg) lvmcache_fmt(info)->ops->destroy_instance(baton.fid); - /* - * NB. If this command failed and we are relying on lvmetad to have an - * *exact* image of the system, the lvmetad instance that went out of - * sync needs to be killed. - */ if (!lvmetad_pv_found((const struct id *) &dev->pvid, dev, lvmcache_fmt(info), label->sector, baton.vg, handler)) { release_vg(baton.vg); @@ -1755,13 +1750,13 @@ static int _lvmetad_pvscan_all_devs(struct cmd_context *cmd, activation_handler struct dev_iter *iter; struct device *dev; daemon_reply reply; - int r = 1; char *future_token; const char *reason; int was_silent; int replacing_other_update = 0; int replaced_update = 0; int retries = 0; + int ret = 1; if (!lvmetad_used()) { log_error("Cannot proceed since lvmetad is not active."); @@ -1819,7 +1814,7 @@ static int _lvmetad_pvscan_all_devs(struct cmd_context *cmd, activation_handler log_debug_lvmetad("Telling lvmetad to clear its cache"); reply = _lvmetad_send(cmd, "pv_clear_all", NULL); if (!_lvmetad_handle_reply(reply, "pv_clear_all", "", NULL)) - r = 0; + ret = 0; daemon_reply_destroy(reply); was_silent = silent_mode(); @@ -1827,33 +1822,38 @@ static int _lvmetad_pvscan_all_devs(struct cmd_context *cmd, activation_handler while ((dev = dev_iter_get(iter))) { if (sigint_caught()) { - r = 0; + ret = 0; stack; break; } if (!lvmetad_pvscan_single(cmd, dev, handler, ignore_obsolete)) - r = 0; + ret = 0; } init_silent(was_silent); dev_iter_destroy(iter); + if (!ret) + lvmetad_set_disabled(cmd, "SCANERROR"); + _lvmetad_token = future_token; - if (!_token_update(NULL)) + if (!_token_update(NULL)) { + log_error("Failed to update lvmetad token after device scan."); return 0; + } /* * If lvmetad is disabled, and no lvm1 metadata was seen and no * duplicate PVs were seen, then re-enable lvmetad. */ - if (lvmetad_is_disabled(cmd, &reason) && + if (ret && lvmetad_is_disabled(cmd, &reason) && !lvmcache_found_duplicate_pvs() && !_found_lvm1_metadata) { log_debug_lvmetad("Enabling lvmetad which was previously disabled."); lvmetad_clear_disabled(cmd); } - return r; + return ret; } int lvmetad_pvscan_all_devs(struct cmd_context *cmd, activation_handler handler, int do_wait) @@ -2235,6 +2235,15 @@ int lvmetad_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const cha return ret; } +/* + * FIXME: if we fail to disable lvmetad, then other commands could + * potentially use incorrect cache data from lvmetad. Should we + * do something more severe if the disable messages fails, like + * sending SIGKILL to the lvmetad pid? + * + * FIXME: log something in syslog any time we disable lvmetad? + * At a minimum if we fail to disable lvmetad. + */ void lvmetad_set_disabled(struct cmd_context *cmd, const char *reason) { daemon_reply reply; @@ -2319,6 +2328,9 @@ int lvmetad_is_disabled(struct cmd_context *cmd, const char **reason) } else if (strstr(reply_reason, "DUPLICATES")) { *reason = "duplicate PVs were found"; + } else if (strstr(reply_reason, "SCANERROR")) { + *reason = "scanning devices failed"; + } else { *reason = ""; } -- cgit v1.2.1