[sheepdog] [PATCH] collie: fix a bug that 'collie node recovery' gives invalid output

[Liu Yuan]

On 08/05/2012 09:50 PM, levin li wrote:
> +		if (rsp->result == SD_RES_SUCCESS) {

We don't need to fall back to the old code, return SD_RES_SUCCESS for node in recovery looks revery weird to me. I have cooked up with another patch.

>From e23276ad3ce11c2e3c73f347f9568a6ddc236e94 Mon Sep 17 00:00:00 2001
From: Liu Yuan <tailai.ly at taobao.com>
Date: Sun, 5 Aug 2012 22:10:38 +0800
Subject: [PATCH] collie: fix collie node recovery

 -add a helper to get the light request's response

Signed-off-by: Liu Yuan <tailai.ly at taobao.com>
---
 collie/collie.h          |    1 +
 collie/common.c          |   25 +++++++++++++++++++------
 collie/node.c            |    6 +++---
 include/sheep.h          |    2 +-
 include/sheepdog_proto.h |    2 +-
 sheep/ops.c              |    8 +++-----
 6 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/collie/collie.h b/collie/collie.h
index e6874c0..48fcece 100644
--- a/collie/collie.h
+++ b/collie/collie.h
@@ -74,6 +74,7 @@ int sd_read_object(uint64_t oid, void *data, unsigned int datalen,
 int sd_write_object(uint64_t oid, uint64_t cow_oid, void *data, unsigned int datalen,
 		    uint64_t offset, uint32_t flags, int copies, int create);
 int send_light_req(struct sd_req *hdr, const char *host, int port);
+int send_light_req_response(struct sd_req *hdr, const char *host, int port);
 
 extern struct command vdi_command;
 extern struct command node_command;
diff --git a/collie/common.c b/collie/common.c
index b93bbd4..af27da3 100644
--- a/collie/common.c
+++ b/collie/common.c
@@ -192,10 +192,7 @@ int parse_vdi(vdi_parser_func_t func, size_t size, void *data)
 	return 0;
 }
 
-/*
- * Light request only contains header, without body content.
- */
-int send_light_req(struct sd_req *hdr, const char *host, int port)
+int send_light_req_response(struct sd_req *hdr, const char *host, int port)
 {
 	int fd, ret;
 	struct sd_rsp *rsp = (struct sd_rsp *)hdr;
@@ -215,9 +212,25 @@ int send_light_req(struct sd_req *hdr, const char *host, int port)
 		return -1;
 	}
 
-	if (rsp->result != SD_RES_SUCCESS) {
+	if (rsp->result != SD_RES_SUCCESS)
+		return rsp->result;
+
+	return SD_RES_SUCCESS;
+}
+
+/*
+ * Light request only contains header, without body content.
+ */
+int send_light_req(struct sd_req *hdr, const char *host, int port)
+{
+	int ret = send_light_req_response(hdr, host, port);
+
+	if (ret == -1)
+		return -1;
+
+	if (ret != SD_RES_SUCCESS) {
 		fprintf(stderr, "Response's result: %s\n",
-			sd_strerror(rsp->result));
+			sd_strerror(ret));
 		return -1;
 	}
 
diff --git a/collie/node.c b/collie/node.c
index 2af4eb3..48d777e 100644
--- a/collie/node.c
+++ b/collie/node.c
@@ -125,9 +125,9 @@ static int node_recovery(int argc, char **argv)
 
 		sd_init_req((struct sd_req *)&req, SD_OP_STAT_RECOVERY);
 
-		ret = send_light_req((struct sd_req *)&req, host,
-				     sd_nodes[i].nid.port);
-		if (!ret) {
+		ret = send_light_req_response((struct sd_req *)&req, host,
+					      sd_nodes[i].nid.port);
+		if (ret == SD_RES_NODE_IN_RECOVERY) {
 			addr_to_str(host, sizeof(host),
 					sd_nodes[i].nid.addr, sd_nodes[i].nid.port);
 			printf(raw_output ? "%d %s %d %d\n" : "%4d   %-20s%5d%11d\n",
diff --git a/include/sheep.h b/include/sheep.h
index 337762e..7efcbf4 100644
--- a/include/sheep.h
+++ b/include/sheep.h
@@ -194,7 +194,7 @@ static inline const char *sd_strerror(int err)
 		{SD_RES_FORCE_RECOVER, "Cluster is running/halted and cannot be force recovered"},
 		{SD_RES_NO_STORE, "Targeted backend store is not found"},
 		{SD_RES_NO_SUPPORT, "Operation is not supported"},
-		{SD_RES_CLUSTER_RECOVERING, "Cluster is recovering"},
+		{SD_RES_NODE_IN_RECOVERY, "Targeted node is in recovery"},
 
 		{SD_RES_OLD_NODE_VER, "Remote node has an old epoch"},
 		{SD_RES_NEW_NODE_VER, "Remote node has a new epoch"},
diff --git a/include/sheepdog_proto.h b/include/sheepdog_proto.h
index 4f124b7..f119f9c 100644
--- a/include/sheepdog_proto.h
+++ b/include/sheepdog_proto.h
@@ -65,7 +65,7 @@
 #define SD_RES_FORCE_RECOVER    0x1A /* Users should not force recover this cluster */
 #define SD_RES_NO_STORE         0x20 /* No targeted backend store */
 #define SD_RES_NO_SUPPORT       0x21 /* Operation is not supported by backend store */
-#define SD_RES_CLUSTER_RECOVERING 0x22 /* Cluster is recovering. */
+#define SD_RES_NODE_IN_RECOVERY 0x22 /*	Targeted node is in recovery */
 #define SD_RES_OBJ_RECOVERING     0x23 /* Object is recovering */
 #define SD_RES_KILLED           0x24 /* Node is killed */
 #define SD_RES_OID_EXIST        0x25 /* Object ID exists already */
diff --git a/sheep/ops.c b/sheep/ops.c
index 945ebff..23bece0 100644
--- a/sheep/ops.c
+++ b/sheep/ops.c
@@ -378,11 +378,9 @@ static int local_stat_recovery(const struct sd_req *req, struct sd_rsp *rsp,
 					void *data)
 {
 	if (node_in_recovery())
-		return SD_RES_SUCCESS;
-	else
-		return SD_RES_UNKNOWN;
+		return SD_RES_NODE_IN_RECOVERY;
 
-	return SD_RES_UNKNOWN;
+	return SD_RES_SUCCESS;
 }
 
 static int local_stat_cluster(struct request *req)
@@ -530,7 +528,7 @@ static int cluster_cleanup(const struct sd_req *req, struct sd_rsp *rsp,
 	iocb.epoch = sys->epoch;
 
 	if (node_in_recovery())
-		return SD_RES_CLUSTER_RECOVERING;
+		return SD_RES_NODE_IN_RECOVERY;
 
 	if (sd_store->cleanup)
 		ret = sd_store->cleanup(&iocb);
-- 
1.7.10.2