shaba/lvm2
1
0
Fork 0
mirror of git://sourceware.org/git/lvm2.git synced 2024-12-21 13:34:40 +03:00
Code Releases Activity

clvmd: fix test mode race

Browse Source 
When TEST_MODE flag is passed around the cluster,
it's been use in thread unprotected way, so it may have
influenced behaviour of other running parallel lvm commands
(activation/deactivation/suspend/resume).

Fix it by set/query function only under lvm mutex.
For hold_un/lock function calls check lock_flags bits directly.
This commit is contained in:
Zdenek Kabelac 2014-04-09 07:58:40 +02:00
parent bd3e44643d
commit 91f4e09b48
4 changed files with 19 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
WHATS_NEW
View File

@ -1,5 +1,6 @@
Version 2.02.107 - Version 2.02.107 -
================================== ==================================
Fix usage of --test option in clvmd.
Skip more libraries to be mlocked in memory. Skip more libraries to be mlocked in memory.
Remove LOCKED flag for pvmove replaced with error target. Remove LOCKED flag for pvmove replaced with error target.
Return invalid command when specifying negative polling interval. Return invalid command when specifying negative polling interval.

13
daemons/clvmd/clvmd-command.c
View File

@ -78,9 +78,6 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
unsigned char lock_cmd; unsigned char lock_cmd;
unsigned char lock_flags; unsigned char lock_flags;
/* Reset test mode before we start */
init_test(0);
/* Do the command */ /* Do the command */
switch (msg->cmd) { switch (msg->cmd) {
/* Just a test message */ /* Just a test message */
@ -112,8 +109,6 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
lockname = &args[2]; lockname = &args[2];
/* Check to see if the VG is in use by LVM1 */ /* Check to see if the VG is in use by LVM1 */
status = do_check_lvm1(lockname); status = do_check_lvm1(lockname);
if (lock_flags & LCK_TEST_MODE)
init_test(1);
do_lock_vg(lock_cmd, lock_flags, lockname); do_lock_vg(lock_cmd, lock_flags, lockname);
break; break;
@ -122,8 +117,6 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
lock_cmd = args[0]; lock_cmd = args[0];
lock_flags = args[1]; lock_flags = args[1];
lockname = &args[2]; lockname = &args[2];
if (lock_flags & LCK_TEST_MODE)
init_test(1);
status = do_lock_lv(lock_cmd, lock_flags, lockname); status = do_lock_lv(lock_cmd, lock_flags, lockname);
/* Replace EIO with something less scary */ /* Replace EIO with something less scary */
if (status == EIO) { if (status == EIO) {
@ -252,7 +245,6 @@ int do_pre_command(struct local_client *client)
int status = 0; int status = 0;
char *lockname; char *lockname;
init_test(0);
switch (header->cmd) { switch (header->cmd) {
case CLVMD_CMD_TEST: case CLVMD_CMD_TEST:
status = sync_lock("CLVMD_TEST", LCK_EXCL, 0, &lockid); status = sync_lock("CLVMD_TEST", LCK_EXCL, 0, &lockid);
@ -271,8 +263,6 @@ int do_pre_command(struct local_client *client)
lock_cmd = args[0]; lock_cmd = args[0];
lock_flags = args[1]; lock_flags = args[1];
lockname = &args[2]; lockname = &args[2];
if (lock_flags & LCK_TEST_MODE)
init_test(1);
status = pre_lock_lv(lock_cmd, lock_flags, lockname); status = pre_lock_lv(lock_cmd, lock_flags, lockname);
break; break;
@ -304,7 +294,6 @@ int do_post_command(struct local_client *client)
char *args = header->node + strlen(header->node) + 1; char *args = header->node + strlen(header->node) + 1;
char *lockname; char *lockname;
init_test(0);
switch (header->cmd) { switch (header->cmd) {
case CLVMD_CMD_TEST: case CLVMD_CMD_TEST:
status = sync_unlock("CLVMD_TEST", (int) (long) client->bits.localsock.private); status = sync_unlock("CLVMD_TEST", (int) (long) client->bits.localsock.private);
@ -315,8 +304,6 @@ int do_post_command(struct local_client *client)
lock_cmd = args[0]; lock_cmd = args[0];
lock_flags = args[1]; lock_flags = args[1];
lockname = &args[2]; lockname = &args[2];
if (lock_flags & LCK_TEST_MODE)
init_test(1);
status = post_lock_lv(lock_cmd, lock_flags, lockname); status = post_lock_lv(lock_cmd, lock_flags, lockname);
break; break;

1
daemons/clvmd/clvmd.c
View File

@ -1762,7 +1762,6 @@ static int process_local_command(struct clvm_header *msg, int msglen,
if (msg->flags & CLVMD_FLAG_REMOTE) if (msg->flags & CLVMD_FLAG_REMOTE)
status = 0; status = 0;
else else
/* FIXME: usage of init_test() is unprotected */
status = do_command(client, msg, msglen, &replybuf, buflen, &replylen); status = do_command(client, msg, msglen, &replybuf, buflen, &replylen);
if (status) if (status)

32
daemons/clvmd/lvm-functions.c
View File

@ -251,9 +251,6 @@ static int hold_lock(char *resource, int mode, int flags)
int saved_errno; int saved_errno;
struct lv_info *lvi; struct lv_info *lvi;
if (test_mode())
return 0;
/* Mask off invalid options */ /* Mask off invalid options */
flags &= LCKF_NOQUEUE | LCKF_CONVERT; flags &= LCKF_NOQUEUE | LCKF_CONVERT;
@ -319,9 +316,6 @@ static int hold_unlock(char *resource)
int status; int status;
int saved_errno; int saved_errno;
if (test_mode())
return 0;
if (!(lvi = lookup_info(resource))) { if (!(lvi = lookup_info(resource))) {
DEBUGLOG("hold_unlock, lock not already held\n"); DEBUGLOG("hold_unlock, lock not already held\n");
return 0; return 0;
@ -381,7 +375,7 @@ static int do_activate_lv(char *resource, unsigned char command, unsigned char l
* Use lock conversion only if requested, to prevent implicit conversion * Use lock conversion only if requested, to prevent implicit conversion
* of exclusive lock to shared one during activation. * of exclusive lock to shared one during activation.
*/ */
if (command & LCK_CLUSTER_VG) { if (!test_mode() && command & LCK_CLUSTER_VG) {
status = hold_lock(resource, mode, LCKF_NOQUEUE | (lock_flags & LCK_CONVERT ? LCKF_CONVERT:0)); status = hold_lock(resource, mode, LCKF_NOQUEUE | (lock_flags & LCK_CONVERT ? LCKF_CONVERT:0));
if (status) { if (status) {
/* Return an LVM-sensible error for this. /* Return an LVM-sensible error for this.
@ -415,7 +409,7 @@ static int do_activate_lv(char *resource, unsigned char command, unsigned char l
return 0; return 0;
error: error:
if (oldmode == -1 || oldmode != mode) if (!test_mode() && (oldmode == -1 || oldmode != mode))
(void)hold_unlock(resource); (void)hold_unlock(resource);
return EIO; return EIO;
} }
@ -479,7 +473,7 @@ static int do_deactivate_lv(char *resource, unsigned char command, unsigned char
if (!lv_deactivate(cmd, resource, NULL)) if (!lv_deactivate(cmd, resource, NULL))
return EIO; return EIO;
if (command & LCK_CLUSTER_VG) { if (!test_mode() && command & LCK_CLUSTER_VG) {
status = hold_unlock(resource); status = hold_unlock(resource);
if (status) if (status)
return errno; return errno;
@ -526,6 +520,8 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
} }
pthread_mutex_lock(&lvm_lock); pthread_mutex_lock(&lvm_lock);
init_test((lock_flags & LCK_TEST_MODE) ? 1 : 0);
if (lock_flags & LCK_MIRROR_NOSYNC_MODE) if (lock_flags & LCK_MIRROR_NOSYNC_MODE)
init_mirror_in_sync(1); init_mirror_in_sync(1);
@ -578,6 +574,7 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
/* clean the pool for another command */ /* clean the pool for another command */
dm_pool_empty(cmd->mem); dm_pool_empty(cmd->mem);
init_test(0);
pthread_mutex_unlock(&lvm_lock); pthread_mutex_unlock(&lvm_lock);
DEBUGLOG("Command return is %d, critical_section is %d\n", status, critical_section()); DEBUGLOG("Command return is %d, critical_section is %d\n", status, critical_section());
@ -597,7 +594,8 @@ int pre_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
DEBUGLOG("pre_lock_lv: resource '%s', cmd = %s, flags = %s\n", DEBUGLOG("pre_lock_lv: resource '%s', cmd = %s, flags = %s\n",
resource, decode_locking_cmd(command), decode_flags(lock_flags)); resource, decode_locking_cmd(command), decode_flags(lock_flags));
if (hold_lock(resource, LCK_WRITE, LCKF_NOQUEUE | LCKF_CONVERT)) if (!(lock_flags & LCK_TEST_MODE) &&
hold_lock(resource, LCK_WRITE, LCKF_NOQUEUE | LCKF_CONVERT))
return errno; return errno;
} }
return 0; return 0;
@ -629,11 +627,13 @@ int post_lock_lv(unsigned char command, unsigned char lock_flags,
if (!status) if (!status)
return EIO; return EIO;
if (lvi.exists) { if (!(lock_flags & LCK_TEST_MODE)) {
if (hold_lock(resource, LCK_READ, LCKF_CONVERT)) if (lvi.exists) {
if (hold_lock(resource, LCK_READ, LCKF_CONVERT))
return errno;
} else if (hold_unlock(resource))
return errno; return errno;
} else if (hold_unlock(resource)) }
return errno;
} }
} }
return 0; return 0;
@ -697,6 +697,8 @@ void do_lock_vg(unsigned char command, unsigned char lock_flags, char *resource)
} }
pthread_mutex_lock(&lvm_lock); pthread_mutex_lock(&lvm_lock);
init_test((lock_flags & LCK_TEST_MODE) ? 1 : 0);
switch (lock_cmd) { switch (lock_cmd) {
case LCK_VG_COMMIT: case LCK_VG_COMMIT:
DEBUGLOG("vg_commit notification for VG %s\n", vgname); DEBUGLOG("vg_commit notification for VG %s\n", vgname);
@ -711,6 +713,8 @@ void do_lock_vg(unsigned char command, unsigned char lock_flags, char *resource)
DEBUGLOG("Invalidating cached metadata for VG %s\n", vgname); DEBUGLOG("Invalidating cached metadata for VG %s\n", vgname);
lvmcache_drop_metadata(vgname, 0); lvmcache_drop_metadata(vgname, 0);
} }
init_test(0);
pthread_mutex_unlock(&lvm_lock); pthread_mutex_unlock(&lvm_lock);
} }