4 commits - src/cmd.c src/diskio.c src/leader.h src/lockspace.c src/main.c src/paxos_lease.c src/paxos_lease.h src/resource.c src/sanlock.h src/sanlock_internal.h src/sanlock_rv.h tests/sanlk_load.c

[David Teigland]

 src/cmd.c              |   26 +++++++++++++--------
 src/diskio.c           |    4 +--
 src/leader.h           |    4 ++-
 src/lockspace.c        |    3 ++
 src/main.c             |    2 +
 src/paxos_lease.c      |   21 +++++++++++++++++
 src/paxos_lease.h      |    1 
 src/resource.c         |   59 +++++++++++++++++++++++++++++++++++++++----------
 src/sanlock.h          |    2 -
 src/sanlock_internal.h |    2 +
 src/sanlock_rv.h       |    1 
 tests/sanlk_load.c     |   10 +++++---
 12 files changed, 106 insertions(+), 29 deletions(-)

New commits:
commit 3c40f2a3ff23320340da2cd858ef9b4ed720b1ed
Author: David Teigland <teigland(a)redhat.com&gt;
Date:   Wed Mar 7 15:37:52 2012 -0600

    sanlock: status for all shared tokens
    
    For the status command, output a resource
    once for each token so the specific pid
    and token_id can be included.
    
    Signed-off-by: David Teigland <teigland(a)redhat.com&gt;

diff --git a/src/cmd.c b/src/cmd.c
index 60d36b2..7a44ce2 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -1293,17 +1293,20 @@ static int print_state_lockspace(struct space *sp, char *str,
const char *list_n
 	return strlen(str) + 1;
 }
 
-static int print_state_resource(struct resource *r, char *str, const char *list_name)
+static int print_state_resource(struct resource *r, char *str, const char *list_name,
+				uint32_t token_id)
 {
 	memset(str, 0, SANLK_STATE_MAXSTR);
 
 	snprintf(str, SANLK_STATE_MAXSTR-1,
 		 "list=%s "
 		 "flags=%x "
-		 "lver=%llu",
+		 "lver=%llu "
+		 "token_id=%u",
 		 list_name,
 		 r->flags,
-		 (unsigned long long)r->leader.lver);
+		 (unsigned long long)r->leader.lver,
+		 token_id);
 
 	return strlen(str) + 1;
 }
@@ -1399,9 +1402,11 @@ static void send_state_lockspace(int fd, struct space *sp, const
char *list_name
 	send(fd, &lockspace, sizeof(lockspace), MSG_NOSIGNAL);
 }
 
-void send_state_resource(int fd, struct resource *r, const char *list_name);
+void send_state_resource(int fd, struct resource *r, const char *list_name,
+			 int pid, uint32_t token_id);
 
-void send_state_resource(int fd, struct resource *r, const char *list_name)
+void send_state_resource(int fd, struct resource *r, const char *list_name,
+			 int pid, uint32_t token_id)
 {
 	struct sanlk_state st;
 	char str[SANLK_STATE_MAXSTR];
@@ -1411,11 +1416,11 @@ void send_state_resource(int fd, struct resource *r, const char
*list_name)
 	memset(&st, 0, sizeof(st));
 
 	st.type = SANLK_STATE_RESOURCE;
-	st.data32 = r->pid;
+	st.data32 = pid;
 	st.data64 = r->leader.lver;
 	strncpy(st.name, r->r.name, NAME_ID_SIZE);
 
-	str_len = print_state_resource(r, str, list_name);
+	str_len = print_state_resource(r, str, list_name, token_id);
 
 	st.str_len = str_len;
 
diff --git a/src/resource.c b/src/resource.c
index 664d69e..6e6de5d 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -33,7 +33,8 @@
 #include "mode_block.h"
 
 /* from cmd.c */
-void send_state_resource(int fd, struct resource *r, const char *list_name);
+void send_state_resource(int fd, struct resource *r, const char *list_name, int pid,
uint32_t token_id);
+
 /* from main.c */
 int get_rand(int a, int b);
 
@@ -50,14 +51,20 @@ static pthread_cond_t resource_cond;
 void send_state_resources(int fd)
 {
 	struct resource *r;
+	struct token *token;
 
 	pthread_mutex_lock(&resource_mutex);
-	list_for_each_entry(r, &resources_held, list)
-		send_state_resource(fd, r, "resources_held");
-	list_for_each_entry(r, &resources_add, list)
-		send_state_resource(fd, r, "resources_add");
+	list_for_each_entry(r, &resources_held, list) {
+		list_for_each_entry(token, &r->tokens, list)
+			send_state_resource(fd, r, "held", token->pid, token->token_id);
+	}
+
+	list_for_each_entry(r, &resources_add, list) {
+		list_for_each_entry(token, &r->tokens, list)
+			send_state_resource(fd, r, "add", token->pid, token->token_id);
+	}
 	list_for_each_entry(r, &resources_rem, list)
-		send_state_resource(fd, r, "resources_rem");
+		send_state_resource(fd, r, "rem", r->pid, r->release_token_id);
 	pthread_mutex_unlock(&resource_mutex);
 }
 


commit 0d816d9d14990c9f4e5f773a412569d51bab8997
Author: David Teigland <teigland(a)redhat.com&gt;
Date:   Wed Mar 7 14:10:58 2012 -0600

    sanlock: retry transient sh failures
    
    Because acquiring a shared lease includes holding
    the lease in ex for a short period, multiple hosts
    trying to acquire the sh lease in parallel can see
    transient failures.  Detect this situation and
    retry the sh acquire up to eight times.
    
    Signed-off-by: David Teigland <teigland(a)redhat.com&gt;

diff --git a/src/cmd.c b/src/cmd.c
index 00fff98..60d36b2 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -306,11 +306,12 @@ static void cmd_acquire(struct task *task, struct cmd_args *ca)
 
 		/* save a record of what this token_id is for later debugging */
 		log_level(space.space_id, token->token_id, NULL, LOG_WARNING,
-			  "resource %.48s:%.48s:%.256s:%llu for %d,%d,%d",
+			  "resource %.48s:%.48s:%.256s:%llu%s for %d,%d,%d",
 			  token->r.lockspace_name,
 			  token->r.name,
 			  token->r.disks[0].path,
 			  (unsigned long long)token->r.disks[0].offset,
+			  (token->acquire_flags & SANLK_RES_SHARED) ? ":SH" : "",
 			  cl_ci, cl_fd, cl_pid);
 	}
 
diff --git a/src/leader.h b/src/leader.h
index cd0dc56..0575255 100644
--- a/src/leader.h
+++ b/src/leader.h
@@ -42,10 +42,12 @@
 #define LEADER_CHECKSUM_LEN 168
 #define LEASE_FREE 0
 
+#define LFL_SHORT_HOLD 0x00000001
+
 struct leader_record {
 	uint32_t magic;
 	uint32_t version;
-	uint32_t unused0;
+	uint32_t flags;
 	uint32_t sector_size;
 	uint64_t num_hosts;
 	uint64_t max_hosts;
diff --git a/src/main.c b/src/main.c
index af82320..7222e22 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1892,6 +1892,7 @@ static int do_direct(void)
 		log_tool("read_leader done %d", rv);
 		log_tool("magic 0x%0x", leader.magic);
 		log_tool("version 0x%x", leader.version);
+		log_tool("flags 0x%x", leader.flags);
 		log_tool("sector_size %u", leader.sector_size);
 		log_tool("num_hosts %llu",
 			 (unsigned long long)leader.num_hosts);
@@ -2006,6 +2007,7 @@ int main(int argc, char *argv[])
 	com.uid = DEFAULT_SOCKET_UID;
 	com.gid = DEFAULT_SOCKET_GID;
 	com.pid = -1;
+	com.sh_retries = DEFAULT_SH_RETRIES;
 
 	memset(&main_task, 0, sizeof(main_task));
 
diff --git a/src/paxos_lease.c b/src/paxos_lease.c
index e6e1aa4..a52c085 100644
--- a/src/paxos_lease.c
+++ b/src/paxos_lease.c
@@ -1276,6 +1276,7 @@ int paxos_lease_acquire(struct task *task,
 					  (unsigned long long)host_id_leader.owner_generation,
 					  (unsigned long long)host_id_leader.timestamp);
 			}
+			memcpy(leader_ret, &cur_leader, sizeof(struct leader_record));
 			error = SANLK_ACQUIRE_IDLIVE;
 			goto out;
 		}
@@ -1391,6 +1392,7 @@ int paxos_lease_acquire(struct task *task,
 				  (unsigned long long)tmp_leader.owner_generation,
 				  (unsigned long long)tmp_leader.timestamp);
 
+			memcpy(leader_ret, &tmp_leader, sizeof(struct leader_record));
 			error = SANLK_ACQUIRE_OWNED;
 		}
 		goto out;
@@ -1456,6 +1458,23 @@ int paxos_lease_acquire(struct task *task,
 
 	if (new_num_hosts)
 		new_leader.num_hosts = new_num_hosts;
+
+	if (new_leader.owner_id == token->host_id) {
+		/*
+		 * The LFL_SHORT_HOLD flag is just a "hint" to help
+		 * other nodes be more intelligent about retrying
+		 * due to transient failures when acquiring shared
+		 * leases.  Only modify SHORT_HOLD if we're commiting
+		 * ourself as the new owner.  If we're commiting another
+		 * host as owner, we don't know if they are acquiring
+		 * shared or not.
+		 */
+		if (flags & PAXOS_ACQUIRE_SHARED)
+			new_leader.flags |= LFL_SHORT_HOLD;
+		else
+			new_leader.flags &= ~LFL_SHORT_HOLD;
+	}
+
 	new_leader.checksum = leader_checksum(&new_leader);
 
 	error = write_new_leader(task, token, &new_leader, "paxos_acquire");
@@ -1471,6 +1490,7 @@ int paxos_lease_acquire(struct task *task,
 			  (unsigned long long)new_leader.owner_generation,
 			  (unsigned long long)new_leader.timestamp);
 
+		memcpy(leader_ret, &new_leader, sizeof(struct leader_record));
 		error = SANLK_ACQUIRE_OTHER;
 		goto out;
 	}
@@ -1590,6 +1610,7 @@ int paxos_lease_release(struct task *task,
 	leader.write_id = token->host_id;
 	leader.write_generation = token->host_generation;
 	leader.write_timestamp = monotime();
+	leader.flags &= ~LFL_SHORT_HOLD;
 	leader.checksum = leader_checksum(&leader);
 
 	error = write_new_leader(task, token, &leader, "paxos_release");
diff --git a/src/paxos_lease.h b/src/paxos_lease.h
index 5f71ba4..6e7e833 100644
--- a/src/paxos_lease.h
+++ b/src/paxos_lease.h
@@ -11,6 +11,7 @@
 
 #define PAXOS_ACQUIRE_FORCE		0x00000001
 #define PAXOS_ACQUIRE_QUIET_FAIL	0x00000002
+#define PAXOS_ACQUIRE_SHARED		0x00000004
 
 uint32_t leader_checksum(struct leader_record *lr);
 
diff --git a/src/resource.c b/src/resource.c
index bb4f787..664d69e 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -34,6 +34,8 @@
 
 /* from cmd.c */
 void send_state_resource(int fd, struct resource *r, const char *list_name);
+/* from main.c */
+int get_rand(int a, int b);
 
 static pthread_t resource_pt;
 static int resource_thread_stop;
@@ -292,6 +294,9 @@ static int acquire_disk(struct task *task, struct token *token,
 	if (com.quiet_fail)
 		flags |= PAXOS_ACQUIRE_QUIET_FAIL;
 
+	if (token->acquire_flags & SANLK_RES_SHARED)
+		flags |= PAXOS_ACQUIRE_SHARED;
+
 	memset(&leader_tmp, 0, sizeof(leader_tmp));
 
 	rv = paxos_lease_acquire(task, token, flags, &leader_tmp, acquire_lver,
@@ -301,11 +306,9 @@ static int acquire_disk(struct task *task, struct token *token,
 		  (unsigned long long)leader_tmp.lver,
 		  (unsigned long long)leader_tmp.timestamp);
 
-	if (rv < 0)
-		return rv;
-
 	memcpy(leader, &leader_tmp, sizeof(struct leader_record));
-	return rv; /* SANLK_OK */
+
+	return rv; /* SANLK_RV */
 }
 
 /* return < 0 on error, 1 on success */
@@ -526,7 +529,9 @@ int acquire_token(struct task *task, struct token *token)
 	struct resource *r;
 	uint64_t acquire_lver = 0;
 	uint32_t new_num_hosts = 0;
-	int rv, live_count = 0;
+	int sh_retries = 0;
+	int live_count = 0;
+	int rv;
 
 	if (token->acquire_flags & SANLK_RES_LVER)
 		acquire_lver = token->acquire_lver;
@@ -589,8 +594,29 @@ int acquire_token(struct task *task, struct token *token)
 
 	copy_disks(&r->r.disks, &token->r.disks, token->r.num_disks);
 
+ retry:
+	memset(&leader, 0, sizeof(struct leader_record));
+
 	rv = acquire_disk(task, token, acquire_lver, new_num_hosts, &leader);
 	if (rv < 0) {
+		if ((token->acquire_flags & SANLK_RES_SHARED) &&
+		    (leader.flags & LFL_SHORT_HOLD)) {
+			/*
+			 * Multiple parallel sh requests can fail because
+			 * the lease is briefly held in ex mode.  The ex
+			 * holder sets SHORT_HOLD in the leader record to
+			 * indicate that it's only held for a short time
+			 * while acquiring a shared lease.  A retry will
+			 * probably succeed.
+			 */
+			if (sh_retries++ < com.sh_retries) {
+				int us = get_rand(0, 1000000);
+				log_token(token, "acquire_token sh_retry %d %d", rv, us);
+				usleep(us);
+				goto retry;
+			}
+			rv = SANLK_ACQUIRE_SHRETRY;
+		}
 		release_token_opened(task, token);
 		return rv;
 	}
diff --git a/src/sanlock.h b/src/sanlock.h
index e65215d..9267c2b 100644
--- a/src/sanlock.h
+++ b/src/sanlock.h
@@ -62,7 +62,7 @@ struct sanlk_resource {
 	uint64_t data64;   /* per-resource command-specific data */
 	uint32_t data32;   /* per-resource command-specific data */
 	uint32_t unused;
-	uint32_t flags;
+	uint32_t flags;    /* SANLK_RES_ */
 	uint32_t num_disks;
 	/* followed by num_disks sanlk_disk structs */
 	struct sanlk_disk disks[0];
diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h
index 1ed5d98..4565855 100644
--- a/src/sanlock_internal.h
+++ b/src/sanlock_internal.h
@@ -518,6 +518,7 @@ EXTERN struct client *client;
 #define DEFAULT_SOCKET_MODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP)
 #define DEFAULT_MIN_WORKER_THREADS 2
 #define DEFAULT_MAX_WORKER_THREADS 8
+#define DEFAULT_SH_RETRIES 8
 
 struct command_line {
 	int type;				/* COM_ */
@@ -539,6 +540,7 @@ struct command_line {
 	int num_hosts;				/* -n */
 	int max_hosts;				/* -m */
 	int res_count;
+	int sh_retries;
 	uint32_t force_mode;
 	char our_host_name[SANLK_NAME_LEN+1];
 	char *dump_path;
diff --git a/src/sanlock_rv.h b/src/sanlock_rv.h
index 60f90c8..95234c7 100644
--- a/src/sanlock_rv.h
+++ b/src/sanlock_rv.h
@@ -45,6 +45,7 @@
 #define SANLK_ACQUIRE_IDLIVE	-243
 #define SANLK_ACQUIRE_OWNED	-244
 #define SANLK_ACQUIRE_OTHER	-245
+#define SANLK_ACQUIRE_SHRETRY	-246
 
 #define SANLK_RELEASE_LVER	-250
 #define SANLK_RELEASE_OWNER	-251


commit f5d100e46d5c97ac2ed5632c11e89df30039d5fc
Author: David Teigland <teigland(a)redhat.com&gt;
Date:   Mon Mar 5 14:24:17 2012 -0600

    sanlock: fix inquire state string
    
    If the first entry of the token array was null
    (due to a release), the inquire state string
    would have a leading space.
    
    Signed-off-by: David Teigland <teigland(a)redhat.com&gt;

diff --git a/src/cmd.c b/src/cmd.c
index b4f356d..00fff98 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -654,7 +654,7 @@ static void cmd_inquire(struct task *task, struct cmd_args *ca)
 			goto done;
 		}
 
-		if (i)
+		if (cat_count)
 			strcat(state, " ");
 		strcat(state, str);
 		cat_count++;
diff --git a/tests/sanlk_load.c b/tests/sanlk_load.c
index 7f4da37..cc511f2 100644
--- a/tests/sanlk_load.c
+++ b/tests/sanlk_load.c
@@ -165,7 +165,10 @@ static void dump_inquire_state(int pid, char *state)
 	for (i = 0; i < len; i++) {
 		if (state[i] == ' ') {
 			state[i] = '\0';
-			log_debug("%d %s", pid, p);
+			if (!i)
+				log_debug("%d leading space", pid);
+			else
+				log_debug("%d %s", pid, p);
 			p = state + i + 1;
 		}
 	}
@@ -227,8 +230,6 @@ static int check_lock_state(int pid, int result, int count, char
*res_state)
 	if ((found_count != count) || bad_count)
 		goto fail;
 
-	if (res_state)
-		free(res_state);
 	return 0;
 
  fail:
@@ -472,6 +473,9 @@ static void inquire_all(int pid, int fd)
 
 	if (count && debug_verbose)
 		dump_inquire_state(pid, state);
+
+	if (state)
+		free(state);
 }
 
 int do_rand_child(void)


commit 2dffb14a90e9e9fee4fc91a85d7ddc795e0228d3
Author: David Teigland <teigland(a)redhat.com&gt;
Date:   Mon Mar 5 13:54:41 2012 -0600

    sanlock: clean up warnings
    
    Signed-off-by: David Teigland <teigland(a)redhat.com&gt;

diff --git a/src/cmd.c b/src/cmd.c
index 8845260..b4f356d 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -1061,7 +1061,7 @@ static void cmd_init_lockspace(struct task *task, struct cmd_args
*ca)
 
 static void cmd_init_resource(struct task *task, struct cmd_args *ca)
 {
-	struct token *token;
+	struct token *token = NULL;
 	struct sanlk_resource res;
 	int token_len, disks_len;
 	int j, fd, rv, result;
diff --git a/src/diskio.c b/src/diskio.c
index 32f1b86..ec702f9 100644
--- a/src/diskio.c
+++ b/src/diskio.c
@@ -112,7 +112,7 @@ int open_disks_fd(struct sync_disk *disks, int num_disks)
 {
 	struct sync_disk *disk;
 	int num_opens = 0;
-	int d, fd, rv;
+	int d, fd, rv = -1;
 
 	for (d = 0; d < num_disks; d++) {
 		disk = &disks[d];
@@ -217,7 +217,7 @@ int open_disks(struct sync_disk *disks, int num_disks)
 {
 	struct sync_disk *disk;
 	int num_opens = 0;
-	int d, err, rv;
+	int d, err, rv = -1;
 	uint32_t ss = 0;
 
 	for (d = 0; d < num_disks; d++) {
diff --git a/src/lockspace.c b/src/lockspace.c
index 812264b..3432555 100644
--- a/src/lockspace.c
+++ b/src/lockspace.c
@@ -396,6 +396,7 @@ static void *lockspace_thread(void *arg_in)
 	if (rv < 0) {
 		log_erros(sp, "open_disk %s error %d", sp->host_id_disk.path, rv);
 		acquire_result = -ENODEV;
+		delta_result = -1;
 		goto set_status;
 	}
 	opened = 1;
@@ -404,12 +405,14 @@ static void *lockspace_thread(void *arg_in)
 	if (sp->align_size < 0) {
 		log_erros(sp, "direct_align error");
 		acquire_result = sp->align_size;
+		delta_result = -1;
 		goto set_status;
 	}
 
 	sp->lease_status.renewal_read_buf = malloc(sp->align_size);
 	if (!sp->lease_status.renewal_read_buf) {
 		acquire_result = -ENOMEM;
+		delta_result = -1;
 		goto set_status;
 	}
 
diff --git a/src/resource.c b/src/resource.c
index ba2dcb6..bb4f787 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -370,8 +370,10 @@ static int _release_token(struct task *task, struct token *token, int
opened, in
 		goto out;
 	}
 
-	if (nodisk)
+	if (nodisk) {
+		rv = SANLK_OK;
 		goto out;
+	}
 
 	if (!opened) {
 		rv = open_disks_fd(token->disks, token->r.num_disks);