[lvm-devel] [PATCH 1/5] Fix clustered and no-lock locking with HOTFIX
Zdenek Kabelac
zkabelac at redhat.com
Wed Feb 2 13:07:57 UTC 2011
Patch is fixing clustered locking - but currently cluster wide
message passing does not work with sync_dev_names().
To avoid any udev synchronizaiton problems with leaving unrelased
udev cookies, revert to the old slow behavior - and check for udev
cookie with after every activate and deactivate operation inside
do_lock_vol().
Once the new message will pass around the cluster - these extra fs_unlock()
shall be removed.
The problem I have now:
Cluster does not pass any message around cluster related to unlock_vg.
So the initial idea to use this point for fs_unlock() works only on
the local/single node.
For the real multinode cluster we need to pass extra message around
- but currently I've not managed to pass this new message around.
We cannot reuse any other message like DROP_CACHE or VG_BACKUP
as this are send when MDA is modifed - however activation does not
modify anything - yet it needs to flush udev cookie.
sync_local_dev_names() - serves for local cluster synchronization
sync_dev_names() - is supposed to be send to all nodes in cluster to
sychronizes its dev nodes. This should happen when vg is unlocked.
For this - macro 'unlock_vg()' is modifed to call sync_dev_names for
clustered locking before it actually releases VG lock - it's called
before lock_vol code to avoid recusion.
Mornfal expressed concerns about possibility to unlock VG by plain
call of lock_vol() with right parameters - this would miss the
sync call - however I'm currently not seeing better way, how
to achieve message passing around - unless we find the way to
to pass send this message within clvmd unlock_vg processing - the
code is currently not very clear in this area.
So once we figure out, how to pass such parameter - we may add
implicit sync_dev_names() into unlock clvmd code and no-locking
code.
For filesystem and external locking implicit fs_unlock() is called
with lock_vol as there is no recursive rick in this case.
For sync names - new LCK is defined:
LCK_VG_SYNC - which should differ from LCK_VG_BACKUP by LCK_HOLD bit.
But somehow I fail to see the reason why this message is not send
around the cluster and only local node is processing this message.
Difference between local & global sync is LCL_LOCAL and usage of
VG_SYNC_NAMES for local message.
As a potential solution could be - to overload DROP_CACHE
and with special paramater use this for as VG_SYNC_NAMES
So the cache would be dropped in other cases.
But my head already spins quite a lot around....
So any help would be appreciated.
Signed-off-by: Zdenek Kabelac <zkabelac at redhat.com>
---
daemons/clvmd/clvmd-command.c | 5 ++++-
daemons/clvmd/clvmd.c | 3 +++
daemons/clvmd/lvm-functions.c | 9 +++++++--
daemons/clvmd/lvm-functions.h | 2 +-
lib/locking/cluster_locking.c | 6 +++---
lib/locking/locking.c | 7 ++++---
lib/locking/locking.h | 22 ++++++++++++++++++++--
7 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/daemons/clvmd/clvmd-command.c b/daemons/clvmd/clvmd-command.c
index 6d0dee4..c2bddc2 100644
--- a/daemons/clvmd/clvmd-command.c
+++ b/daemons/clvmd/clvmd-command.c
@@ -139,7 +139,10 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
break;
case CLVMD_CMD_SYNC_NAMES:
- lvm_do_fs_unlock();
+ lock_cmd = args[0];
+ lock_flags = args[1];
+ lockname = &args[2];
+ lvm_do_fs_unlock(lock_cmd, lock_flags, lockname, client ? 1 : 0);
break;
case CLVMD_CMD_SET_DEBUG:
diff --git a/daemons/clvmd/clvmd.c b/daemons/clvmd/clvmd.c
index 8e21732..f38894c 100644
--- a/daemons/clvmd/clvmd.c
+++ b/daemons/clvmd/clvmd.c
@@ -296,6 +296,9 @@ static const char *decode_cmd(unsigned char cmdl)
case CLVMD_CMD_RESTART:
command = "RESTART";
break;
+ case CLVMD_CMD_SYNC_NAMES:
+ command = "SYNC_NAMES";
+ break;
default:
command = "unknown";
break;
diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index f06657d..8dc200e 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -507,6 +507,7 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
switch (command & LCK_MASK) {
case LCK_LV_EXCLUSIVE:
status = do_activate_lv(resource, lock_flags, LCK_EXCL);
+ fs_unlock();
break;
case LCK_LV_SUSPEND:
@@ -520,10 +521,12 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
case LCK_LV_ACTIVATE:
status = do_activate_lv(resource, lock_flags, LCK_READ);
+ fs_unlock();
break;
case LCK_LV_DEACTIVATE:
status = do_deactivate_lv(resource, lock_flags);
+ fs_unlock();
break;
default:
@@ -894,11 +897,13 @@ struct dm_hash_node *get_next_excl_lock(struct dm_hash_node *v, char **name)
return v;
}
-void lvm_do_fs_unlock(void)
+void lvm_do_fs_unlock(int lock_cmd, int lock_flags, char *lockname, int is_client)
{
pthread_mutex_lock(&lvm_lock);
- DEBUGLOG("Syncing device names\n");
+ DEBUGLOG("Syncing device names cmd=%d flags=%d name=%s client=%d\n",
+ lock_cmd, lock_flags, lockname, is_client);
fs_unlock();
+
pthread_mutex_unlock(&lvm_lock);
}
diff --git a/daemons/clvmd/lvm-functions.h b/daemons/clvmd/lvm-functions.h
index a132f4a..1b60c4f 100644
--- a/daemons/clvmd/lvm-functions.h
+++ b/daemons/clvmd/lvm-functions.h
@@ -36,6 +36,6 @@ extern char *get_last_lvm_error(void);
extern void do_lock_vg(unsigned char command, unsigned char lock_flags,
char *resource);
extern struct dm_hash_node *get_next_excl_lock(struct dm_hash_node *v, char **name);
-void lvm_do_fs_unlock(void);
+void lvm_do_fs_unlock(int lock_cmd, int lock_flags, char *lockname, int is_client);
#endif
diff --git a/lib/locking/cluster_locking.c b/lib/locking/cluster_locking.c
index 83b1cd2..cecd52d 100644
--- a/lib/locking/cluster_locking.c
+++ b/lib/locking/cluster_locking.c
@@ -401,10 +401,10 @@ int lock_resource(struct cmd_context *cmd, const char *resource, uint32_t flags)
switch (flags & LCK_SCOPE_MASK) {
case LCK_VG:
- if (!strcmp(resource, VG_SYNC_NAMES)) {
- log_very_verbose("Requesting sync names.");
+ if ((flags & ~LCK_LOCAL) == LCK_VG_SYNC) {
+ log_very_verbose("Requesting sync names for %s.", resource);
return _lock_for_cluster(cmd, CLVMD_CMD_SYNC_NAMES,
- flags & ~LCK_HOLD, resource);
+ flags, resource);
}
if (flags == LCK_VG_BACKUP) {
log_very_verbose("Requesting backup of VG metadata for %s",
diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index c923610..a12db52 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -443,9 +443,10 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
if (!check_lvm1_vg_inactive(cmd, vol))
return_0;
- /* Before unlocking VG wait until devices are available. */
- if ((flags & LCK_TYPE_MASK) == LCK_UNLOCK)
- sync_local_dev_names(cmd);
+ /* For non clustered unlock - implicit fs_unlock() */
+ if ((lck_type == LCK_UNLOCK) && !(flags & LCK_CACHE) &&
+ is_real_vg(vol) && !locking_is_clustered())
+ fs_unlock();
break;
case LCK_LV:
/* All LV locks are non-blocking. */
diff --git a/lib/locking/locking.h b/lib/locking/locking.h
index 1b5e1fc..122d2fc 100644
--- a/lib/locking/locking.h
+++ b/lib/locking/locking.h
@@ -126,6 +126,7 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
#define LCK_VG_REVERT (LCK_VG | LCK_READ | LCK_CACHE | LCK_HOLD)
#define LCK_VG_BACKUP (LCK_VG | LCK_CACHE)
+#define LCK_VG_SYNC (LCK_VG | LCK_CACHE | LCK_HOLD)
#define LCK_LV_EXCLUSIVE (LCK_LV | LCK_EXCL)
#define LCK_LV_SUSPEND (LCK_LV | LCK_WRITE)
@@ -143,7 +144,20 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
lock_vol(cmd, (lv)->lvid.s, flags | LCK_LV_CLUSTERED(lv)) : \
0)
-#define unlock_vg(cmd, vol) lock_vol(cmd, vol, LCK_VG_UNLOCK)
+/*
+ * For clustered locking (cluster, no_lock) explicit call
+ * of cluster wide sync_dev_names() is made before unlocking vg.
+ * For non-clustered locking (filesystem, external)
+ * use of fs_unlock() is implicit in lock_vol().
+ */
+#define unlock_vg(cmd, vol) \
+ do { \
+ if (is_real_vg(vol) && \
+ locking_is_clustered()) \
+ sync_dev_names(cmd, vol); \
+ lock_vol(cmd, vol, LCK_VG_UNLOCK); \
+ } while (0)
+
#define unlock_and_free_vg(cmd, vg, vol) \
do { \
unlock_vg(cmd, vol); \
@@ -170,8 +184,12 @@ int check_lvm1_vg_inactive(struct cmd_context *cmd, const char *vgname);
lock_vol((vg)->cmd, (vg)->name, LCK_VG_REVERT)
#define remote_backup_metadata(vg) \
lock_vol((vg)->cmd, (vg)->name, LCK_VG_BACKUP)
+/* Synchronize local device names on cluster node */
#define sync_local_dev_names(cmd) \
- lock_vol(cmd, VG_SYNC_NAMES, LCK_NONE | LCK_CACHE | LCK_LOCAL)
+ lock_vol(cmd, VG_SYNC_NAMES, LCK_VG_SYNC | LCK_LOCAL)
+/* Synchronize global device names on all cluster nodes */
+#define sync_dev_names(cmd, vol) \
+ lock_vol(cmd, vol, LCK_VG_SYNC)
/* Process list of LVs */
int suspend_lvs(struct cmd_context *cmd, struct dm_list *lvs);
--
1.7.3.5
More information about the lvm-devel
mailing list