[sheepdog] [PATCH V3] collie: cleanup callbacks of collie command when they send header only requests

Yunkai Zhang yunkai.me at gmail.com
Fri Jul 27 08:38:17 CEST 2012


From: Yunkai Zhang <qiushu.zyk at taobao.com>

V3:
- make send_light_req() only return -1 or 0, and update related upper code

V2:
- rename send_empty_req() to send_light_req(), and move it to net.c
- cleanup tow more places: collie/node.c collie/debug.c
-------------------------------------------------------------------- >8

There are several callbacks of collie command send requests which only contain
header. So let's abstract the common part of the them into a new function:
send_light_req() which could make code more compactness.

Signed-off-by: Yunkai Zhang <qiushu.zyk at taobao.com>
---
 collie/cluster.c | 118 +++++++------------------------------------------------
 collie/debug.c   |  15 +------
 collie/node.c    |  51 ++++++------------------
 include/net.h    |   1 +
 lib/net.c        |  29 ++++++++++++++
 5 files changed, 58 insertions(+), 156 deletions(-)

diff --git a/collie/cluster.c b/collie/cluster.c
index 35bfa23..45d39a4 100644
--- a/collie/cluster.c
+++ b/collie/cluster.c
@@ -204,66 +204,30 @@ error:
 
 static int cluster_shutdown(int argc, char **argv)
 {
-	int fd, ret;
+	int ret;
 	struct sd_req hdr;
-	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
-	unsigned rlen, wlen;
-
-	fd = connect_to(sdhost, sdport);
-	if (fd < 0)
-		return EXIT_SYSFAIL;
 
 	sd_init_req(&hdr, SD_OP_SHUTDOWN);
 	hdr.epoch = sd_epoch;
 
-	rlen = 0;
-	wlen = 0;
-	ret = exec_req(fd, &hdr, NULL, &wlen, &rlen);
-	close(fd);
-
-	if (ret) {
-		fprintf(stderr, "Failed to connect\n");
-		return EXIT_SYSFAIL;
-	}
-
-	if (rsp->result != SD_RES_SUCCESS) {
-		fprintf(stderr, "Shutdown failed: %s\n",
-				sd_strerror(rsp->result));
+	ret = send_light_req(&hdr, sdhost, sdport);
+	if (ret)
 		return EXIT_FAILURE;
-	}
 
 	return EXIT_SUCCESS;
 }
 
 static int restore_snap(uint32_t epoch)
 {
-	int fd, ret;
+	int ret;
 	struct sd_req hdr;
-	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
-	unsigned rlen, wlen;
-
-	fd = connect_to(sdhost, sdport);
-	if (fd < 0)
-		return EXIT_SYSFAIL;
 
 	sd_init_req(&hdr, SD_OP_RESTORE);
 	hdr.obj.tgt_epoch = epoch;
 
-	rlen = 0;
-	wlen = 0;
-	ret = exec_req(fd, &hdr, NULL, &wlen, &rlen);
-	close(fd);
-
-	if (ret) {
-		fprintf(stderr, "Failed to connect\n");
-		return EXIT_SYSFAIL;
-	}
-
-	if (rsp->result != SD_RES_SUCCESS) {
-		fprintf(stderr, "Restore failed: %s\n",
-				sd_strerror(rsp->result));
+	ret = send_light_req(&hdr, sdhost, sdport);
+	if (ret)
 		return EXIT_FAILURE;
-	}
 
 	printf("Cluster restore to the snapshot %d\n", epoch);
 	return EXIT_SUCCESS;
@@ -327,32 +291,14 @@ out:
 
 static int do_snapshot(void)
 {
-	int fd, ret;
+	int ret;
 	struct sd_req hdr;
-	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
-	unsigned rlen, wlen;
-
-	fd = connect_to(sdhost, sdport);
-	if (fd < 0)
-		return EXIT_SYSFAIL;
 
 	sd_init_req(&hdr, SD_OP_SNAPSHOT);
 
-	rlen = 0;
-	wlen = 0;
-	ret = exec_req(fd, &hdr, NULL, &wlen, &rlen);
-	close(fd);
-
-	if (ret) {
-		fprintf(stderr, "Failed to connect\n");
-		return EXIT_SYSFAIL;
-	}
-
-	if (rsp->result != SD_RES_SUCCESS) {
-		fprintf(stderr, "Snapshot failed: %s\n",
-				sd_strerror(rsp->result));
+	ret = send_light_req(&hdr, sdhost, sdport);
+	if (ret)
 		return EXIT_FAILURE;
-	}
 
 	return EXIT_SUCCESS;
 }
@@ -371,32 +317,14 @@ static int cluster_snapshot(int argc, char **argv)
 
 static int cluster_cleanup(int argc, char **argv)
 {
-	int fd, ret;
+	int ret;
 	struct sd_req hdr;
-	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
-	unsigned rlen, wlen;
-
-	fd = connect_to(sdhost, sdport);
-	if (fd < 0)
-		return EXIT_SYSFAIL;
 
 	sd_init_req(&hdr, SD_OP_CLEANUP);
 
-	rlen = 0;
-	wlen = 0;
-	ret = exec_req(fd, &hdr, NULL, &wlen, &rlen);
-	close(fd);
-
-	if (ret) {
-		fprintf(stderr, "Failed to connect\n");
-		return EXIT_SYSFAIL;
-	}
-
-	if (rsp->result != SD_RES_SUCCESS) {
-		fprintf(stderr, "Cleanup failed: %s\n",
-				sd_strerror(rsp->result));
+	ret = send_light_req(&hdr, sdhost, sdport);
+	if (ret)
 		return EXIT_FAILURE;
-	}
 
 	return EXIT_SUCCESS;
 }
@@ -411,16 +339,10 @@ Are you sure you want to continue? [yes/no]: "
 
 static int cluster_recover(int argc, char **argv)
 {
-	int fd, ret;
+	int ret;
 	struct sd_req hdr;
-	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
-	unsigned rlen, wlen;
 	char str[123] = {'\0'};
 
-	fd = connect_to(sdhost, sdport);
-	if (fd < 0)
-		return EXIT_SYSFAIL;
-
 	if (!cluster_cmd_data.force) {
 		int i, l;
 		printf(RECOVER_PRINT);
@@ -437,19 +359,9 @@ static int cluster_recover(int argc, char **argv)
 	sd_init_req(&hdr, SD_OP_RECOVER);
 	hdr.epoch = sd_epoch;
 
-	rlen = 0;
-	wlen = 0;
-	ret = exec_req(fd, &hdr, NULL, &wlen, &rlen);
-	close(fd);
-
+	ret = send_light_req(&hdr, sdhost, sdport);
 	if (ret) {
-		fprintf(stderr, "Failed to connect\n");
-		return EXIT_SYSFAIL;
-	}
-
-	if (rsp->result != SD_RES_SUCCESS) {
-		fprintf(stderr, "Recovery failed: %s\n",
-				sd_strerror(rsp->result));
+		fprintf(stderr, "Recovery failed\n");
 		return EXIT_FAILURE;
 	}
 
diff --git a/collie/debug.c b/collie/debug.c
index 549e2e6..d016e85 100644
--- a/collie/debug.c
+++ b/collie/debug.c
@@ -97,25 +97,12 @@ static int debug_trace(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	fd = connect_to(sdhost, sdport);
-	if (fd < 0)
-		return EXIT_SYSFAIL;
-
 	sd_init_req(&hdr, SD_OP_TRACE);
 	hdr.epoch = sd_epoch;
 	hdr.data_length = enabled;
 
-	rlen = 0;
-	wlen = 0;
-	ret = exec_req(fd, &hdr, NULL, &wlen, &rlen);
-	close(fd);
-
+	ret = send_light_req(&hdr, sdhost, sdport);
 	if (ret) {
-		fprintf(stderr, "Failed to connect\n");
-		return EXIT_SYSFAIL;
-	}
-
-	if (rsp->result != SD_RES_SUCCESS) {
 		fprintf(stderr, "Trace failed: %s\n",
 				sd_strerror(rsp->result));
 		return EXIT_FAILURE;
diff --git a/collie/node.c b/collie/node.c
index 87cf97b..df841a0 100644
--- a/collie/node.c
+++ b/collie/node.c
@@ -60,31 +60,23 @@ static int node_info(int argc, char **argv)
 		printf("Id\tSize\tUsed\tUse%%\n");
 
 	for (i = 0; i < sd_nodes_nr; i++) {
-		char name[128];
-		int fd;
-		unsigned wlen, rlen;
+		char host[128];
 		struct sd_node_req req;
 		struct sd_node_rsp *rsp = (struct sd_node_rsp *)&req;
 		char store_str[UINT64_DECIMAL_SIZE], free_str[UINT64_DECIMAL_SIZE];
 
-		addr_to_str(name, sizeof(name), sd_nodes[i].nid.addr, 0);
-
-		fd = connect_to(name, sd_nodes[i].nid.port);
-		if (fd < 0)
-			return 1;
+		addr_to_str(host, sizeof(host), sd_nodes[i].nid.addr, 0);
 
 		sd_init_req((struct sd_req *)&req, SD_OP_STAT_SHEEP);
 		req.epoch = sd_epoch;
 
-		wlen = 0;
-		rlen = 0;
-		ret = exec_req(fd, (struct sd_req *)&req, NULL, &wlen, &rlen);
-		close(fd);
+		ret = send_light_req((struct sd_req *)&req, host,
+				     sd_nodes[i].nid.port);
 
 		size_to_str(rsp->store_size, store_str, sizeof(store_str));
 		size_to_str(rsp->store_size - rsp->store_free, free_str,
 			    sizeof(free_str));
-		if (!ret && rsp->result == SD_RES_SUCCESS) {
+		if (!ret) {
 			printf(raw_output ? "%d %s %s %d%%\n" : "%2d\t%s\t%s\t%3d%%\n",
 			       i, store_str, free_str,
 			       (int)(((double)(rsp->store_size - rsp->store_free) / rsp->store_size) * 100));
@@ -127,25 +119,15 @@ static int node_recovery(int argc, char **argv)
 
 	for (i = 0; i < sd_nodes_nr; i++) {
 		char host[128];
-		int fd;
-		unsigned wlen, rlen;
 		struct sd_node_req req;
-		struct sd_node_rsp *rsp = (struct sd_node_rsp *)&req;
 
 		addr_to_str(host, sizeof(host), sd_nodes[i].nid.addr, 0);
 
-		fd = connect_to(host, sd_nodes[i].nid.port);
-		if (fd < 0)
-			return EXIT_FAILURE;
-
 		sd_init_req((struct sd_req *)&req, SD_OP_STAT_RECOVERY);
 
-		wlen = 0;
-		rlen = 0;
-		ret = exec_req(fd, (struct sd_req *)&req, NULL, &wlen, &rlen);
-		close(fd);
-
-		if (!ret && rsp->result == SD_RES_SUCCESS) {
+		ret = send_light_req((struct sd_req *)&req, host,
+				     sd_nodes[i].nid.port);
+		if (!ret) {
 			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",
@@ -160,10 +142,8 @@ static int node_recovery(int argc, char **argv)
 static int node_kill(int argc, char **argv)
 {
 	char host[128];
-	int fd, node_id, ret;
-	unsigned wlen, rlen;
+	int node_id, ret;
 	struct sd_node_req req;
-	struct sd_node_rsp *rsp = (struct sd_node_rsp *)&req;
 
 	node_id = strtol(argv[optind++], NULL, 10);
 	if (node_id < 0 || node_id >= sd_nodes_nr) {
@@ -173,18 +153,11 @@ static int node_kill(int argc, char **argv)
 
 	addr_to_str(host, sizeof(host), sd_nodes[node_id].nid.addr, 0);
 
-	fd = connect_to(host, sd_nodes[node_id].nid.port);
-	if (fd < 0)
-		return EXIT_FAILURE;
-
 	sd_init_req((struct sd_req *)&req, SD_OP_KILL_NODE);
 
-	wlen = 0;
-	rlen = 0;
-	ret = exec_req(fd, (struct sd_req *)&req, NULL, &wlen, &rlen);
-	close(fd);
-
-	if (ret || rsp->result != SD_RES_SUCCESS) {
+	ret = send_light_req((struct sd_req *)&req, host,
+			     sd_nodes[node_id].nid.port);
+	if (ret) {
 		fprintf(stderr, "Failed to execute request\n");
 		exit(EXIT_FAILURE);
 	}
diff --git a/include/net.h b/include/net.h
index 4ee0eb6..42c3620 100644
--- a/include/net.h
+++ b/include/net.h
@@ -41,6 +41,7 @@ int do_read(int sockfd, void *buf, int len);
 int rx(struct connection *conn, enum conn_state next_state);
 int tx(struct connection *conn, enum conn_state next_state, int flags);
 int connect_to(const char *name, int port);
+int send_light_req(struct sd_req *hdr, const char *host, int port);
 int send_req(int sockfd, struct sd_req *hdr, void *data, unsigned int *wlen);
 int exec_req(int sockfd, struct sd_req *hdr, void *data,
 	     unsigned int *wlen, unsigned int *rlen);
diff --git a/lib/net.c b/lib/net.c
index 3527884..a832244 100644
--- a/lib/net.c
+++ b/lib/net.c
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 
 #include "sheepdog_proto.h"
+#include "sheep.h"
 #include "util.h"
 #include "event.h"
 #include "net.h"
@@ -370,6 +371,34 @@ int exec_req(int sockfd, struct sd_req *hdr, 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 fd, ret;
+	struct sd_rsp *rsp = (struct sd_rsp *)hdr;
+	unsigned rlen, wlen;
+
+	fd = connect_to(host, port);
+	if (fd < 0)
+		return -1;
+
+	rlen = 0;
+	wlen = 0;
+	ret = exec_req(fd, hdr, NULL, &wlen, &rlen);
+	close(fd);
+	if (ret)
+		return -1;
+
+	if (rsp->result != SD_RES_SUCCESS) {
+		eprintf("Response's result: %s\n", sd_strerror(rsp->result));
+		return -1;
+	}
+
+	return 0;
+}
+
 char *addr_to_str(char *str, int size, uint8_t *addr, uint16_t port)
 {
 	int  af = AF_INET6;
-- 
1.7.11.2




More information about the sheepdog mailing list