Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=57534733b7750f17…
Commit: 57534733b7750f17f5ffa115306dcb650d8015b9
Parent: 1612c570b6412b68349b055ba3a6dab1796b8f35
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Thu Jul 23 10:34:24 2015 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Thu Jul 23 10:39:11 2015 -0500
lvmlockd: improve check for duplicate global locks
When there are duplicate global locks, check if the gl
is still enabled each time a gl or vg lock is acquired
in the lockspace. Once one of the duplicates is disabled,
then other hosts will recognize that the issue is resolved
without needing to restart the lockspaces.
---
daemons/lvmlockd/lvmlockd-core.c | 23 +++++------------------
daemons/lvmlockd/lvmlockd-internal.h | 11 +++++++++++
daemons/lvmlockd/lvmlockd-sanlock.c | 10 ++++++++--
3 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/daemons/lvmlockd/lvmlockd-core.c b/daemons/lvmlockd/lvmlockd-core.c
index a01c8a6..7375d9e 100644
--- a/daemons/lvmlockd/lvmlockd-core.c
+++ b/daemons/lvmlockd/lvmlockd-core.c
@@ -199,17 +199,6 @@ static struct list_head lockspaces;
static struct list_head lockspaces_inactive;
/*
- * This flag is set to 1 if we see multiple vgs with the global
- * lock enabled. While this is set, we return a special flag
- * with the vg lock result indicating to the lvm command that
- * there is a duplicate gl in the vg which should be resolved.
- * While this is set, find_lockspace_name has the side job of
- * counting the number of lockspaces with enabled gl's so that
- * this can be set back to zero when the duplicates are disabled.
- */
-static int sanlock_gl_dup;
-
-/*
* Client thread reads client requests and writes client results.
*/
static pthread_t client_thread;
@@ -1046,6 +1035,9 @@ static int res_lock(struct lockspace *ls, struct resource *r, struct action *act
log_debug("S %s R %s res_lock lm done r_version %u",
ls->name, r->name, r_version);
+ if (sanlock_gl_dup && ls->sanlock_gl_enabled)
+ act->flags |= LD_AF_DUP_GL_LS;
+
/* lm_lock() reads new r_version */
if ((r_version > r->version) || (!r->version && !r->version_zero_valid)) {
@@ -2096,9 +2088,6 @@ static void *lockspace_thread_main(void *arg_in)
act = list_first_entry(&ls->actions, struct action, list);
- if (sanlock_gl_dup && ls->sanlock_gl_enabled)
- act->flags |= LD_AF_DUP_GL_LS;
-
if (act->op == LD_OP_STOP) {
ls->thread_work = 0;
break;
@@ -3855,8 +3844,7 @@ static int print_lockspace(struct lockspace *ls, const char *prefix, int pos, in
"thread_work=%d "
"thread_stop=%d "
"thread_done=%d "
- "sanlock_gl_enabled=%d "
- "sanlock_gl_dup=%d\n",
+ "sanlock_gl_enabled=%d",
prefix,
ls->name,
ls->vg_name,
@@ -3870,8 +3858,7 @@ static int print_lockspace(struct lockspace *ls, const char *prefix, int pos, in
ls->thread_work ? 1 : 0,
ls->thread_stop ? 1 : 0,
ls->thread_done ? 1 : 0,
- ls->sanlock_gl_enabled ? 1 : 0,
- ls->sanlock_gl_dup ? 1 : 0);
+ ls->sanlock_gl_enabled ? 1 : 0);
}
static int print_action(struct action *act, const char *prefix, int pos, int len)
diff --git a/daemons/lvmlockd/lvmlockd-internal.h b/daemons/lvmlockd/lvmlockd-internal.h
index 1fd7125..7bbddb4 100644
--- a/daemons/lvmlockd/lvmlockd-internal.h
+++ b/daemons/lvmlockd/lvmlockd-internal.h
@@ -322,6 +322,17 @@ EXTERN int daemon_host_id;
EXTERN const char *daemon_host_id_file;
EXTERN int sanlock_io_timeout;
+/*
+ * This flag is set to 1 if we see multiple vgs with the global
+ * lock enabled. While this is set, we return a special flag
+ * with the vg lock result indicating to the lvm command that
+ * there is a duplicate gl in the vg which should be resolved.
+ * While this is set, find_lockspace_name has the side job of
+ * counting the number of lockspaces with enabled gl's so that
+ * this can be set back to zero when the duplicates are disabled.
+ */
+EXTERN int sanlock_gl_dup;
+
void log_level(int level, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
#define log_debug(fmt, args...) log_level(LOG_DEBUG, fmt, ##args)
#define log_error(fmt, args...) log_level(LOG_ERR, fmt, ##args)
diff --git a/daemons/lvmlockd/lvmlockd-sanlock.c b/daemons/lvmlockd/lvmlockd-sanlock.c
index 28dac6b..44926da 100644
--- a/daemons/lvmlockd/lvmlockd-sanlock.c
+++ b/daemons/lvmlockd/lvmlockd-sanlock.c
@@ -801,8 +801,6 @@ int lm_able_gl_sanlock(struct lockspace *ls, int enable)
log_debug("S %s able_gl %s", ls->name, gl_name);
ls->sanlock_gl_enabled = enable;
- if (ls->sanlock_gl_dup && !enable)
- ls->sanlock_gl_dup = 0;
if (enable)
strncpy(gl_lsname_sanlock, ls->name, MAX_NAME);
@@ -1254,6 +1252,14 @@ int lm_lock_sanlock(struct lockspace *ls, struct resource *r, int ld_mode,
rs = &rds->rs;
+ /*
+ * While there are duplicate global locks, keep checking
+ * to see if any have been disabled.
+ */
+ if (sanlock_gl_dup && ls->sanlock_gl_enabled &&
+ (r->type == LD_RT_GL || r->type == LD_RT_VG))
+ ls->sanlock_gl_enabled = gl_is_enabled(ls, ls->lm_data);
+
if (r->type == LD_RT_LV) {
/*
* The lv may have been removed and recreated with a new lease
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=b92e5026957c5ca7…
Commit: b92e5026957c5ca72e6e88304feaf7527ca80af1
Parent: 27e6aee3904d195de658505b3bacc65601344d46
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Wed Jul 22 14:53:46 2015 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Wed Jul 22 15:44:20 2015 -0500
pvscan: skip autoactivation for lockd VGs
pvscan autoactivation does not work for lockd VGs because
lock start is needed on a lockd VG before locking can be
done for it. Add a check to skip the attempt at autoactivate
rather than calling it, knowing it will fail.
Add a comment explaining why pvscan --cache works fine for
lockd VGs without locks, and why autoactivate is not done.
---
lib/cache/lvmetad.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index 60f0277..856b30f 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -952,6 +952,51 @@ int lvmetad_pv_found(const struct id *pvid, struct device *dev, const struct for
daemon_reply_int(reply, "seqno_after", -1) != daemon_reply_int(reply, "seqno_before", -1)))
log_warn("WARNING: Inconsistent metadata found for VG %s", vg->name);
+ /*
+ * pvscan --cache does not perform any lvmlockd locking, and
+ * pvscan --cache -aay skips autoactivation in lockd VGs.
+ *
+ * pvscan --cache populates lvmetad with VG metadata from disk.
+ * No lvmlockd locking is needed. It is expected that lockd VG
+ * metadata that is read by pvscan and populated in lvmetad may
+ * be immediately stale due to changes to the VG from other hosts
+ * during or after this pvscan. This is normal and not a problem.
+ * When a subsequent lvm command uses the VG, it will lock the VG
+ * with lvmlockd, read the VG from lvmetad, and update the cached
+ * copy from disk if necessary.
+ *
+ * pvscan --cache -aay does not activate LVs in lockd VGs because
+ * activation requires locking, and a lock-start operation is needed
+ * on a lockd VG before any locking can be performed in it.
+ *
+ * An equivalent of pvscan --cache -aay for lockd VGs is:
+ * 1. pvscan --cache
+ * 2. vgchange --lock-start
+ * 3. vgchange -aay -S 'locktype=sanlock || locktype=dlm'
+ *
+ * [We could eventually add support for autoactivating lockd VGs
+ * using pvscan by incorporating the lock start step (which can
+ * take a long time), but there may be a better option than
+ * continuing to overload pvscan.]
+ *
+ * Stages of starting a lockd VG:
+ *
+ * . pvscan --cache populates lockd VGs in lvmetad without locks,
+ * and this initial cached copy may quickly become stale.
+ *
+ * . vgchange --lock-start VG reads the VG without the VG lock
+ * because no locks are available until the locking is started.
+ * It only uses the VG name and lock_type from the VG metadata,
+ * and then only uses it to start the VG lockspace in lvmlockd.
+ *
+ * . Further lvm commands, e.g. activation, can then lock the VG
+ * with lvmlockd and use current VG metdata.
+ */
+ if (handler && vg && is_lockd_type(vg->lock_type)) {
+ log_debug_lvmetad("Skip pvscan activation for lockd type VG %s", vg->name);
+ handler = NULL;
+ }
+
if (result && handler) {
status = daemon_reply_str(reply, "status", "<missing>");
vgname = daemon_reply_str(reply, "vgname", "<missing>");
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=27e6aee3904d195d…
Commit: 27e6aee3904d195de658505b3bacc65601344d46
Parent: 8bfcefe11a2ce594d1f6e8ef5a1b17e80786ceab
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Wed Jul 22 11:42:57 2015 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Wed Jul 22 12:28:06 2015 -0500
lvconvert: merge polling fixes for lockd
. the poll check will eventually call finish which will
write the VG, so an ex VG lock is needed from lvmlockd.
. fix missing unlock on poll error path
. remove the lockd locking while monitoring the progress
of the command, as suggested by the earlier FIXME comment,
as it's not needed.
---
tools/lvconvert.c | 10 +------
tools/polldaemon.c | 61 +++++++++++++++++++++++++++++-----------------------
2 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index 0c5e2bd..6a0e0ca 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -3410,10 +3410,7 @@ static int lvconvert_single(struct cmd_context *cmd, struct lvconvert_params *lp
cmd->handles_missing_pvs = 1;
}
- /*
- * The VG lock will be released when the command exits.
- * Commands that poll the LV will reacquire the VG lock.
- */
+ /* Unlock on error paths not required, it's automatic when command exits. */
if (!lockd_vg(cmd, lp->vg_name, "ex", 0, &lockd_state))
goto_out;
@@ -3461,10 +3458,7 @@ static int lvconvert_single(struct cmd_context *cmd, struct lvconvert_params *lp
bad:
unlock_vg(cmd, lp->vg_name);
- /*
- * The command may sit and monitor progress for some time,
- * and we do not need or want the VG lock held during that.
- */
+ /* Unlock here so it's not held during polling. */
lockd_vg(cmd, lp->vg_name, "un", 0, &lockd_state);
release_vg(vg);
diff --git a/tools/polldaemon.c b/tools/polldaemon.c
index bbe4641..4527efb 100644
--- a/tools/polldaemon.c
+++ b/tools/polldaemon.c
@@ -136,17 +136,22 @@ static void _sleep_and_rescan_devices(struct daemon_parms *parms)
int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
struct daemon_parms *parms)
{
- struct volume_group *vg;
+ struct volume_group *vg = NULL;
struct logical_volume *lv;
int finished = 0;
uint32_t lockd_state = 0;
+ int ret;
/* Poll for completion */
while (!finished) {
if (parms->wait_before_testing)
_sleep_and_rescan_devices(parms);
- if (!lockd_vg(cmd, id->vg_name, "sh", 0, &lockd_state)) {
+ /*
+ * An ex VG lock is needed because the check can call finish_copy
+ * which writes the VG.
+ */
+ if (!lockd_vg(cmd, id->vg_name, "ex", 0, &lockd_state)) {
log_error("ABORTING: Can't lock VG for %s.", id->display_name);
return 0;
}
@@ -154,10 +159,12 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
/* Locks the (possibly renamed) VG again */
vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state);
if (vg_read_error(vg)) {
- release_vg(vg);
- log_error("ABORTING: Can't reread VG for %s.", id->display_name);
/* What more could we do here? */
- return 0;
+ log_error("ABORTING: Can't reread VG for %s.", id->display_name);
+ release_vg(vg);
+ vg = NULL;
+ ret = 0;
+ goto out;
}
lv = find_lv(vg, id->lv_name);
@@ -174,9 +181,8 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
else
log_print_unless_silent("Can't find LV in %s for %s.",
vg->name, id->display_name);
-
- unlock_and_release_vg(cmd, vg, vg->name);
- return 1;
+ ret = 1;
+ goto out;
}
/*
@@ -185,13 +191,13 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
*/
if (!lv_is_active_locally(lv)) {
log_print_unless_silent("%s: Interrupted: No longer active.", id->display_name);
- unlock_and_release_vg(cmd, vg, vg->name);
- return 1;
+ ret = 1;
+ goto out;
}
if (!_check_lv_status(cmd, vg, lv, id->display_name, parms, &finished)) {
- unlock_and_release_vg(cmd, vg, vg->name);
- return_0;
+ ret = 0;
+ goto_out;
}
unlock_and_release_vg(cmd, vg, vg->name);
@@ -215,6 +221,12 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
}
return 1;
+
+out:
+ if (vg)
+ unlock_and_release_vg(cmd, vg, vg->name);
+ lockd_vg(cmd, id->vg_name, "un", 0, &lockd_state);
+ return ret;
}
struct poll_id_list {
@@ -373,21 +385,17 @@ static int report_progress(struct cmd_context *cmd, struct poll_operation_id *id
int ret;
/*
- * FIXME: we don't really need to take the vg lock here,
- * because we only report the progress on the same host
- * where the pvmove/lvconvert is happening. This means
- * that the local pvmove/lvconvert/lvpoll commands are
- * updating the local lvmetad with the latest info they
- * have, and we just need to read the latest info that
- * they have put into lvmetad about their progress.
- * No VG lock is needed to protect anything here
- * (we're just reading the VG), and no VG lock is
- * needed to force a VG read from disk to get changes
- * from other hosts, because the only change to the VG
- * we're interested in is the change done locally.
+ * It's reasonable to expect a lockd_vg("sh") here, but it should
+ * not actually be needed, because we only report the progress on
+ * the same host where the pvmove/lvconvert is happening. This means
+ * that the local pvmove/lvconvert/lvpoll commands are updating the
+ * local lvmetad with the latest info they have, and we just need to
+ * read the latest info that they have put into lvmetad about their
+ * progress. No VG lock is needed to protect anything here (we're
+ * just reading the VG), and no VG lock is needed to force a VG read
+ * from disk to get changes from other hosts, because the only change
+ * to the VG we're interested in is the change done locally.
*/
- if (!lockd_vg(cmd, id->vg_name, "sh", 0, &lockd_state))
- return 0;
vg = vg_read(cmd, id->vg_name, NULL, 0, lockd_state);
if (vg_read_error(vg)) {
@@ -431,7 +439,6 @@ static int report_progress(struct cmd_context *cmd, struct poll_operation_id *id
out:
unlock_and_release_vg(cmd, vg, vg->name);
out_ret:
- lockd_vg(cmd, id->vg_name, "un", 0, &lockd_state);
return ret;
}
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=8bfcefe11a2ce594…
Commit: 8bfcefe11a2ce594d1f6e8ef5a1b17e80786ceab
Parent: 00d24511bc5e1ae446867f65b5adfe3e7d404398
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Wed Jul 22 14:19:07 2015 +0200
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Wed Jul 22 14:25:42 2015 +0200
config: add CFG_SECTION_NO_CHECK flag
The CFG_SECTION_NO_CHECK flag can be used to mark a section
and its whole subtree as containing settings where checks
won't be made (lvmconfig --validate).
These are setting where we don't know the names and and type
in advance and they're recognized in runtime. As we don't know
the type and name in advance, we can't do any checks here
of course.
Use this flag with great care as it disables config checks
for the whole config subtree found under such section.
This flag is going to be used by subsequent patches from
Zdenek to support some cache settings...
---
lib/config/config.c | 12 ++++++------
lib/config/config.h | 2 ++
lib/config/config_settings.h | 1 +
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/lib/config/config.c b/lib/config/config.c
index 35ee3ac..eb8880e 100644
--- a/lib/config/config.c
+++ b/lib/config/config.c
@@ -1040,9 +1040,14 @@ static int _config_def_check_tree(struct cft_check_handle *handle,
size_t buf_size, struct dm_config_node *root)
{
struct dm_config_node *cn;
+ cfg_def_item_t *def;
int valid, r = 1;
size_t len;
+ def = cfg_def_get_item_p(root->id);
+ if (def->flags & CFG_SECTION_NO_CHECK)
+ return 1;
+
for (cn = root->child; cn; cn = cn->sib) {
if ((valid = _config_def_check_node(handle, vp, pvp, rp, prp,
buf_size, cn)) && !cn->v) {
@@ -1662,14 +1667,9 @@ static int _out_prefix_fn(const struct dm_config_node *cn, const char *line, voi
char path[CFG_PATH_MAX_LEN];
char commentline[MAX_COMMENT_LINE+1];
- if (cn->id < 0)
+ if (cn->id <= 0)
return 1;
- if (!cn->id) {
- log_error(INTERNAL_ERROR "Configuration node %s has invalid id.", cn->key);
- return 0;
- }
-
if (out->tree_spec->type == CFG_DEF_TREE_LIST)
return 1;
diff --git a/lib/config/config.h b/lib/config/config.h
index 03319bf..7520fc0 100644
--- a/lib/config/config.h
+++ b/lib/config/config.h
@@ -121,6 +121,8 @@ typedef union {
#define CFG_DISABLED 0x200
/* whether to print integers in octal form (prefixed by "0") */
#define CFG_FORMAT_INT_OCTAL 0x400
+/* whether to disable checks for the whole config section subtree */
+#define CFG_SECTION_NO_CHECK 0x800
/* configuration definition item structure */
typedef struct cfg_def_item {
diff --git a/lib/config/config_settings.h b/lib/config/config_settings.h
index ce03b36..5209531 100644
--- a/lib/config/config_settings.h
+++ b/lib/config/config_settings.h
@@ -51,6 +51,7 @@
* CFG_DEFAULT_COMMENTED - node's default value is commented out on output
* CFG_DISABLED - configuration is disabled (defaults always used)
* CFG_FORMAT_INT_OCTAL - print integer number in octal form (also prefixed by "0")
+ * CFG_SECTION_NO_CHECK - do not check content of the section at all - use with care!!!
*
* type: Allowed type for the value of simple configuation setting, one of:
* CFG_TYPE_BOOL