WHATS_NEW | 5 +++ daemons/lvmetad/lvmetad-core.c | 74 +++++++++++++++++++++++------------------- lib/cache/lvmetad.c | 9 +++-- lib/cache/lvmetad.h | 3 +- lib/locking/file_locking.c | 2 +- tools/pvscan.c | 32 ++++++++++-------- 6 files changed, 74 insertions(+), 51 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index a3d48f3..b5afb19 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,10 @@ Version 2.02.106 - ==================================== + Use VG read lock during 'pvscan --cache -aay' autoactivation. + Issue a VG refresh before autoactivation only if the PV has changed/is new. + Add flag to lvmetad protocol to indicate the PV scanned has changed/is new. + Also add vgname to lvmetad protocol when referencing VGs for PVs scanned. + Use correct PATH_MAX for locking dir path. Update API for internal function build_dm_uuid(). Do not try to check empty pool with scheduled messages. Fix return value in pool_has_message() when quering for any message. diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c index f35db89..1858ee2 100644 --- a/daemons/lvmetad/lvmetad-core.c +++ b/daemons/lvmetad/lvmetad-core.c @@ -344,8 +344,8 @@ static response pv_lookup(lvmetad_state *s, request r) pvid = dm_hash_lookup_binary(s->device_to_pvid, &devt, sizeof(devt)); if (!pvid) { - WARN(s, "pv_lookup: could not find device %" PRIu64, devt); unlock_pvid_to_pvmeta(s); + WARN(s, "pv_lookup: could not find device %" PRIu64, devt); dm_config_destroy(res.cft); return reply_unknown("device not found"); } @@ -809,30 +809,28 @@ static response pv_gone(lvmetad_state *s, request r) pvid_old = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device)); vgid = dm_hash_lookup(s->pvid_to_vgid, pvid); - if (vgid && !(vgid = dm_strdup(vgid))) { - unlock_pvid_to_pvmeta(s); - return reply_fail("out of memory"); - } - dm_hash_remove_binary(s->device_to_pvid, &device, sizeof(device)); dm_hash_remove(s->pvid_to_pvmeta, pvid); unlock_pvid_to_pvmeta(s); + dm_free(pvid_old); + if (vgid) { + if (!(vgid = dm_strdup(vgid))) + return reply_fail("out of memory"); + lock_vg(s, vgid); vg_remove_if_missing(s, vgid, 1); unlock_vg(s, vgid); dm_free(vgid); } - if (pvid_old) - dm_free(pvid_old); - - if (pvmeta) { - dm_config_destroy(pvmeta); - return daemon_reply_simple("OK", NULL); - } else + if (!pvmeta) return reply_unknown("PVID does not exist"); + + dm_config_destroy(pvmeta); + + return daemon_reply_simple("OK", NULL); } static response pv_clear_all(lvmetad_state *s, request r) @@ -866,7 +864,7 @@ static response pv_found(lvmetad_state *s, request r) char *old; char *pvid_dup; int complete = 0, orphan = 0; - int64_t seqno = -1, seqno_old = -1; + int64_t seqno = -1, seqno_old = -1, changed = 0; if (!pvid) return reply_fail("need PV UUID"); @@ -876,58 +874,60 @@ static response pv_found(lvmetad_state *s, request r) if (!dm_config_get_uint64(pvmeta, "pvmeta/device", &device)) return reply_fail("need PV device number"); + if (!(cft = dm_config_create()) || + (!(pvid_dup = dm_strdup(pvid)))) { + if (cft) + dm_config_destroy(cft); + return reply_fail("out of memory"); + } + lock_pvid_to_pvmeta(s); + if ((pvmeta_old_pvid = dm_hash_lookup(s->pvid_to_pvmeta, pvid))) + dm_config_get_uint64(pvmeta_old_pvid->root, "pvmeta/device", &device_old_pvid); + if ((old = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device)))) { pvmeta_old_dev = dm_hash_lookup(s->pvid_to_pvmeta, old); dm_hash_remove(s->pvid_to_pvmeta, old); vgid_old = dm_hash_lookup(s->pvid_to_vgid, old); } - if ((pvmeta_old_pvid = dm_hash_lookup(s->pvid_to_pvmeta, pvid))) - dm_config_get_uint64(pvmeta_old_pvid->root, "pvmeta/device", &device_old_pvid); - DEBUGLOG(s, "pv_found %s, vgid = %s, device = %" PRIu64 " (previously %" PRIu64 "), old = %s", pvid, vgid, device, device_old_pvid, old); - dm_free(old); - - if (!(cft = dm_config_create()) || - !(cft->root = dm_config_clone_node(cft, pvmeta, 0))) { - unlock_pvid_to_pvmeta(s); - if (cft) - dm_config_destroy(cft); - return reply_fail("out of memory"); - } + if (!(cft->root = dm_config_clone_node(cft, pvmeta, 0))) + goto out_of_mem; - if (!(pvid_dup = dm_strdup(pvid))) { - unlock_pvid_to_pvmeta(s); - dm_config_destroy(cft); - return reply_fail("out of memory"); - } + if (!pvmeta_old_pvid || compare_config(pvmeta_old_pvid->root, cft->root)) + changed |= 1; if (pvmeta_old_pvid && device != device_old_pvid) { DEBUGLOG(s, "pv %s no longer on device %" PRIu64, pvid, device_old_pvid); dm_free(dm_hash_lookup_binary(s->device_to_pvid, &device_old_pvid, sizeof(device_old_pvid))); dm_hash_remove_binary(s->device_to_pvid, &device_old_pvid, sizeof(device_old_pvid)); + changed |= 1; } if (!dm_hash_insert(s->pvid_to_pvmeta, pvid, cft) || !dm_hash_insert_binary(s->device_to_pvid, &device, sizeof(device), (void*)pvid_dup)) { dm_hash_remove(s->pvid_to_pvmeta, pvid); +out_of_mem: unlock_pvid_to_pvmeta(s); dm_config_destroy(cft); dm_free(pvid_dup); + dm_free(old); return reply_fail("out of memory"); } + unlock_pvid_to_pvmeta(s); + + dm_free(old); + if (pvmeta_old_pvid) dm_config_destroy(pvmeta_old_pvid); if (pvmeta_old_dev && pvmeta_old_dev != pvmeta_old_pvid) dm_config_destroy(pvmeta_old_dev); - unlock_pvid_to_pvmeta(s); - if (metadata) { if (!vgid) return reply_fail("need VG UUID"); @@ -939,6 +939,7 @@ static response pv_found(lvmetad_state *s, request r) if (!update_metadata(s, vgname, vgid, metadata, &seqno_old)) return reply_fail("metadata update failed"); + changed |= (seqno_old != dm_config_find_int(metadata, "metadata/seqno", -1)); } else { lock_pvid_to_vgid(s); vgid = dm_hash_lookup(s->pvid_to_vgid, pvid); @@ -956,6 +957,11 @@ static response pv_found(lvmetad_state *s, request r) return reply_fail("non-orphan VG without metadata encountered"); } unlock_vg(s, vgid); + + // TODO: separate vgid->vgname lock + lock_vgid_to_metadata(s); + vgname = dm_hash_lookup(s->vgid_to_vgname, vgid); + unlock_vgid_to_metadata(s); } if (vgid_old && (!vgid || strcmp(vgid, vgid_old))) { @@ -971,7 +977,9 @@ static response pv_found(lvmetad_state *s, request r) return daemon_reply_simple("OK", "status = %s", orphan ? "orphan" : (complete ? "complete" : "partial"), + "changed = %d", changed, "vgid = %s", vgid ? vgid : "#orphan", + "vgname = %s", vgname ? vgname : "#orphan", "seqno_before = %"PRId64, seqno_old, "seqno_after = %"PRId64, seqno, NULL); diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c index c994874..38d9042 100644 --- a/lib/cache/lvmetad.c +++ b/lib/cache/lvmetad.c @@ -749,7 +749,8 @@ int lvmetad_pv_found(const struct id *pvid, struct device *dev, const struct for daemon_reply reply; struct lvmcache_info *info; struct dm_config_tree *pvmeta, *vgmeta; - const char *status, *vgid; + const char *status, *vgname, *vgid; + int64_t changed; int result; if (!lvmetad_active() || test_mode()) @@ -818,11 +819,13 @@ int lvmetad_pv_found(const struct id *pvid, struct device *dev, const struct for if (result && handler) { status = daemon_reply_str(reply, "status", ""); + vgname = daemon_reply_str(reply, "vgname", ""); vgid = daemon_reply_str(reply, "vgid", ""); + changed = daemon_reply_int(reply, "changed", 0); if (!strcmp(status, "partial")) - handler(_lvmetad_cmd, vgid, 1, CHANGE_AAY); + handler(_lvmetad_cmd, vgname, vgid, 1, changed, CHANGE_AAY); else if (!strcmp(status, "complete")) - handler(_lvmetad_cmd, vgid, 0, CHANGE_AAY); + handler(_lvmetad_cmd, vgname, vgid, 0, changed, CHANGE_AAY); else if (!strcmp(status, "orphan")) ; else diff --git a/lib/cache/lvmetad.h b/lib/cache/lvmetad.h index 85b71c2..d9aa77f 100644 --- a/lib/cache/lvmetad.h +++ b/lib/cache/lvmetad.h @@ -23,7 +23,8 @@ struct dm_config_tree; enum activation_change; typedef int (*activation_handler) (struct cmd_context *cmd, - const char *vgid, int partial, + const char *vgname, const char *vgid, + int partial, int changed, enum activation_change activate); #ifdef LVMETAD_SUPPORT diff --git a/lib/locking/file_locking.c b/lib/locking/file_locking.c index fb84c5b..734e0b4 100644 --- a/lib/locking/file_locking.c +++ b/lib/locking/file_locking.c @@ -37,7 +37,7 @@ struct lock_list { }; static struct dm_list _lock_list; -static char _lock_dir[NAME_LEN]; +static char _lock_dir[PATH_MAX]; static int _prioritise_write_locks; static sig_t _oldhandler; diff --git a/tools/pvscan.c b/tools/pvscan.c index 4f99f45..5db627a 100644 --- a/tools/pvscan.c +++ b/tools/pvscan.c @@ -95,13 +95,13 @@ static void _pvscan_display_single(struct cmd_context *cmd, #define REFRESH_BEFORE_AUTOACTIVATION_RETRY_USLEEP_DELAY 100000 static int _auto_activation_handler(struct cmd_context *cmd, - const char *vgid, int partial, + const char *vgname, const char *vgid, + int partial, int changed, activation_change_t activate) { unsigned int refresh_retries = REFRESH_BEFORE_AUTOACTIVATION_RETRIES; int refresh_done = 0; struct volume_group *vg; - int consistent = 0; struct id vgid_raw; int r = 0; @@ -113,8 +113,12 @@ static int _auto_activation_handler(struct cmd_context *cmd, return_0; /* NB. This is safe because we know lvmetad is running and we won't hit disk. */ - if (!(vg = vg_read_internal(cmd, NULL, (const char *) &vgid_raw, 0, &consistent))) - return 1; + vg = vg_read(cmd, vgname, (const char *)&vgid_raw, 0); + if (vg_read_error(vg)) { + log_error("Failed to read Volume Group \"%s\" (%s) during autoactivation.", vgname, vgid); + release_vg(vg); + return 0; + } if (vg_is_clustered(vg)) { r = 1; goto out; @@ -139,16 +143,18 @@ static int _auto_activation_handler(struct cmd_context *cmd, * * Remove this workaround with "refresh_retries" once we have proper locking in! */ - while (refresh_retries--) { - if (vg_refresh_visible(vg->cmd, vg)) { - refresh_done = 1; - break; + if (changed) { + while (refresh_retries--) { + if (vg_refresh_visible(vg->cmd, vg)) { + refresh_done = 1; + break; + } + usleep(REFRESH_BEFORE_AUTOACTIVATION_RETRY_USLEEP_DELAY); } - usleep(REFRESH_BEFORE_AUTOACTIVATION_RETRY_USLEEP_DELAY); - } - if (!refresh_done) - log_warn("%s: refresh before autoactivation failed.", vg->name); + if (!refresh_done) + log_warn("%s: refresh before autoactivation failed.", vg->name); + } if (!vgchange_activate(vg->cmd, vg, activate)) { log_error("%s: autoactivation failed.", vg->name); @@ -158,7 +164,7 @@ static int _auto_activation_handler(struct cmd_context *cmd, r = 1; out: - release_vg(vg); + unlock_and_release_vg(cmd, vg, vgname); return r; }