[sanlock] 03/04: sanlock: use careful release on error paths

[git repository hosting]

This is an automated email from the git hooks/post-receive script.

teigland pushed a commit to branch testing2
in repository sanlock.

commit 516e5ea54a385dfac4b673c238fc4a02d37a1fc4
Author: David Teigland <teigland(a)redhat.com&gt;
Date:   Mon Nov 27 15:13:01 2017 -0600

    sanlock: use careful release on error paths
    
    Extend the previous commit to instances of release
    called on error paths.
---
 src/resource.c         | 45 +++++++++++++++++++++------------------------
 src/sanlock_internal.h |  2 +-
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/src/resource.c b/src/resource.c
index 3339422..81b6d99 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -383,22 +383,22 @@ static int write_host_block(struct task *task, struct token *token,
 	return rv;
 }
 
-static int write_host_block_zero_dblock_release(struct task *task, struct token *token)
+static int write_mblock_zero_dblock_release(struct task *task, struct token *token)
 {
 	struct paxos_dblock dblock;
 
-	memcpy(&dblock, &token->resource->acquire_dblock, sizeof(dblock));
+	memcpy(&dblock, &token->resource->dblock, sizeof(dblock));
 
 	dblock.flags = DBLOCK_FL_RELEASED;
 
 	return write_host_block(task, token, token->host_id, 0, 0, &dblock);
 }
 
-static int write_host_block_shared_dblock_release(struct task *task, struct token
*token)
+static int write_mblock_shared_dblock_release(struct task *task, struct token *token)
 {
 	struct paxos_dblock dblock;
 
-	memcpy(&dblock, &token->resource->acquire_dblock, sizeof(dblock));
+	memcpy(&dblock, &token->resource->dblock, sizeof(dblock));
 
 	dblock.flags = DBLOCK_FL_RELEASED;
 
@@ -773,6 +773,11 @@ static int release_disk(struct task *task, struct token *token,
  * leader says we own the lease, but our dblock is cleared, then our
  * leader write in release was clobbered, and other hosts will run a
  * ballot to set a new owner.
+ * UPDATE to above: we no longer clear our dblock values because that
+ * can interfere with other hosts running a paxos ballot at the same time,
+ * instead we now set the DBLOCK_FL_RELEASED flag in our dblock, leaving our
+ * other dblock values intact, and other hosts look for this flag to indicate
+ * that we have released.
  *
  * [**] For ERASE_ALL we don't want another host running the ballot to select
  * our dblock values and commit them, making us the owner after we've aborted
@@ -874,9 +879,7 @@ static int _release_token(struct task *task, struct token *token,
 	 */
 
 	if (r_flags & R_ERASE_ALL) {
-		/* FIXME: figure out what to clear to avoid disrupting ongoing paxos */
-
-		rv = write_host_block(task, token, token->host_id, 0, 0, NULL);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0) {
 			log_errot(token, "release_token erase all write_host_block %d", rv);
 			ret = rv;
@@ -906,9 +909,7 @@ static int _release_token(struct task *task, struct token *token,
 			  (unsigned long long)lver, rv);
 
 	} else if (r_flags & R_UNDO_SHARED) {
-		/* FIXME: figure out what to clear to avoid disrupting ongoing paxos */
-
-		rv = write_host_block(task, token, token->host_id, 0, 0, NULL);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0) {
 			log_errot(token, "release_token undo shared write_host_block %d", rv);
 			ret = rv;
@@ -929,7 +930,7 @@ static int _release_token(struct task *task, struct token *token,
 	} else if (r_flags & R_SHARED) {
 		/* normal release of sh lease */
 
-		rv = write_host_block_zero_dblock_release(task, token);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0) {
 			log_errot(token, "release_token shared write_host_block %d", rv);
 			ret = rv;
@@ -959,7 +960,7 @@ static int _release_token(struct task *task, struct token *token,
 		}
 
 		/* Failure here is not a big deal and can be ignored. */
-		rv = write_host_block_zero_dblock_release(task, token);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0)
 			log_errot(token, "release_token write_host_block %d", rv);
 
@@ -1219,6 +1220,7 @@ static int convert_sh2ex_token(struct task *task, struct resource
*r, struct tok
 	}
 
 	memcpy(&r->leader, &leader, sizeof(struct leader_record));
+	memcpy(&r->dblock, &dblock, sizeof(dblock));
 	token->r.lver = leader.lver;
 
 	/* paxos_lease_acquire set token->shared_count to the number of
@@ -1340,7 +1342,7 @@ static int convert_ex2sh_token(struct task *task, struct resource
*r, struct tok
 	if (r->flags & R_LVB_WRITE_RELEASE)
 		write_lvb_block(task, r, token);
 
-	rv = write_host_block_shared_dblock_release(task, token);
+	rv = write_mblock_shared_dblock_release(task, token);
 	if (rv < 0) {
 		log_errot(token, "convert_ex2sh write_host_block error %d", rv);
 		return rv;
@@ -1702,20 +1704,19 @@ int acquire_token(struct task *task, struct token *token, uint32_t
cmd_flags,
 	}
 
 	memcpy(&r->leader, &leader, sizeof(struct leader_record));
+	memcpy(&r->dblock, &dblock, sizeof(dblock));
 
 	/* copy lver into token because inquire looks there for it */
 	if (!(token->acquire_flags & SANLK_RES_SHARED))
 		token->r.lver = leader.lver;
 
-	memcpy(&token->resource->acquire_dblock, &dblock, sizeof(dblock));
-
 	/*
 	 * acquiring shared lease, so we set SHARED in our mode_block
 	 * and release the leader owner.
 	 */
 
 	if (token->acquire_flags & SANLK_RES_SHARED) {
-		rv = write_host_block_shared_dblock_release(task, token);
+		rv = write_mblock_shared_dblock_release(task, token);
 		if (rv < 0) {
 			log_errot(token, "acquire_token sh write_host_block error %d", rv);
 			r->flags &= ~R_SHARED;
@@ -2074,9 +2075,7 @@ static void resource_thread_release(struct task *task, struct
resource *r, struc
 	log_token(token, "release async r_flags %x", r_flags);
 
 	if (r_flags & R_ERASE_ALL) {
-		/* FIXME: figure out what to clear to avoid disrupting ongoing paxos */
-
-		rv = write_host_block(task, token, token->host_id, 0, 0, NULL);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0)
 			log_errot(token, "release async erase all write_host_block %d", rv);
 
@@ -2101,9 +2100,7 @@ static void resource_thread_release(struct task *task, struct
resource *r, struc
 			  (unsigned long long)r->leader.lver, rv);
 
 	} else if (r_flags & R_UNDO_SHARED) {
-		/* FIXME: figure out what to clear to avoid disrupting ongoing paxos */
-
-		rv = write_host_block(task, token, token->host_id, 0, 0, NULL);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0)
 			log_errot(token, "release async undo shared write_host_block %d", rv);
 
@@ -2120,7 +2117,7 @@ static void resource_thread_release(struct task *task, struct
resource *r, struc
 	} else if (r_flags & R_SHARED) {
 		/* normal release of sh lease */
 
-		rv = write_host_block_zero_dblock_release(task, token);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0)
 			log_errot(token, "release async shared write_host_block %d", rv);
 
@@ -2139,7 +2136,7 @@ static void resource_thread_release(struct task *task, struct
resource *r, struc
 		}
 
 		/* Failure here is not a big deal and can be ignored. */
-		rv = write_host_block_zero_dblock_release(task, token);
+		rv = write_mblock_zero_dblock_release(task, token);
 		if (rv < 0)
 			log_errot(token, "release async write_host_block %d", rv);
 
diff --git a/src/sanlock_internal.h b/src/sanlock_internal.h
index 279b56d..5d799b9 100644
--- a/src/sanlock_internal.h
+++ b/src/sanlock_internal.h
@@ -130,7 +130,7 @@ struct resource {
 	char killpath[SANLK_HELPER_PATH_LEN]; /* copied from client */
 	char killargs[SANLK_HELPER_ARGS_LEN]; /* copied from client */
 	struct leader_record leader; /* copy of last leader_record we wrote */
-	struct paxos_dblock acquire_dblock; /* dblock we wrote in acquire */
+	struct paxos_dblock dblock;  /* copy of last paxos_dblock we wrote */
 	struct sanlk_resource r;
 };
 

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.