[sheepdog] [PATCH] remove sd_*_req and sd_*_rsp

MORITA Kazutaka morita.kazutaka at lab.ntt.co.jp
Tue Apr 30 18:58:40 CEST 2013


This patch removes all the sd_*_req and sd_*_rsp in internal_proto.h,
and adds the necessary fields to sd_req and sd_rsp.  It is not good to
include internal protocols in sheepdog_proto.h, but the change can
decrease many casting, improve type safety, and simplify codes a lot.

This also checks the size of sd_req and sd_rsp so that we don't break
them.

Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
---
 collie/cluster.c          | 18 +++++------
 collie/collie.c           | 15 +++++-----
 collie/collie.h           |  2 +-
 collie/common.c           |  2 +-
 collie/node.c             | 39 ++++++++++++------------
 include/internal_proto.h  | 76 -----------------------------------------------
 include/sheepdog_proto.h  | 30 +++++++++++++++++++
 sheep/group.c             | 13 ++++----
 sheep/object_list_cache.c |  2 +-
 sheep/ops.c               | 14 ++++-----
 sheep/recovery.c          |  9 +++---
 sheep/sheep_priv.h        |  2 +-
 12 files changed, 84 insertions(+), 138 deletions(-)

diff --git a/collie/cluster.c b/collie/cluster.c
index 715d658..7cdbba0 100644
--- a/collie/cluster.c
+++ b/collie/cluster.c
@@ -80,17 +80,16 @@ static bool no_vdi(const unsigned long *vdis)
 static int cluster_format(int argc, char **argv)
 {
 	int ret;
-	struct sd_so_req hdr;
-	struct sd_so_rsp *rsp = (struct sd_so_rsp *)&hdr;
+	struct sd_req hdr;
+	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
 	struct timeval tv;
 	char store_name[STORE_LEN];
 	static DECLARE_BITMAP(vdi_inuse, SD_NR_VDIS);
 
-	sd_init_req((struct sd_req *)&hdr, SD_OP_READ_VDIS);
+	sd_init_req(&hdr, SD_OP_READ_VDIS);
 	hdr.data_length = sizeof(vdi_inuse);
 
-	ret = collie_exec_req(sdhost, sdport, (struct sd_req *)&hdr,
-			      &vdi_inuse);
+	ret = collie_exec_req(sdhost, sdport, &hdr, &vdi_inuse);
 	if (ret < 0)
 		return EXIT_SYSFAIL;
 
@@ -99,14 +98,14 @@ static int cluster_format(int argc, char **argv)
 
 	gettimeofday(&tv, NULL);
 
-	sd_init_req((struct sd_req *)&hdr, SD_OP_MAKE_FS);
-	hdr.copies = cluster_cmd_data.copies;
+	sd_init_req(&hdr, SD_OP_MAKE_FS);
+	hdr.cluster.copies = cluster_cmd_data.copies;
 	if (cluster_cmd_data.nohalt)
 		hdr.flags |= SD_FLAG_NOHALT;
 	if (cluster_cmd_data.quorum)
 		hdr.flags |= SD_FLAG_QUORUM;
 
-	hdr.ctime = (uint64_t) tv.tv_sec << 32 | tv.tv_usec * 1000;
+	hdr.cluster.ctime = (uint64_t) tv.tv_sec << 32 | tv.tv_usec * 1000;
 
 	if (strlen(cluster_cmd_data.name))
 		pstrcpy(store_name, STORE_LEN, cluster_cmd_data.name);
@@ -116,8 +115,7 @@ static int cluster_format(int argc, char **argv)
 	hdr.flags |= SD_FLAG_CMD_WRITE;
 
 	printf("using backend %s store\n", store_name);
-	ret = collie_exec_req(sdhost, sdport, (struct sd_req *)&hdr,
-			      store_name);
+	ret = collie_exec_req(sdhost, sdport, &hdr, store_name);
 	if (ret < 0)
 		return EXIT_SYSFAIL;
 
diff --git a/collie/collie.c b/collie/collie.c
index e798207..6d54829 100644
--- a/collie/collie.c
+++ b/collie/collie.c
@@ -45,23 +45,22 @@ struct sd_vnode sd_vnodes[SD_MAX_VNODES];
 int sd_nodes_nr, sd_vnodes_nr;
 unsigned master_idx;
 
-int update_node_list(int max_nodes, uint32_t epoch)
+int update_node_list(int max_nodes)
 {
 	int ret;
 	unsigned int size;
 	char *buf = NULL;
 	struct sd_node *ent;
-	struct sd_node_req hdr;
-	struct sd_node_rsp *rsp = (struct sd_node_rsp *)&hdr;
+	struct sd_req hdr;
+	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
 
 	size = sizeof(*ent) * max_nodes;
 	buf = xzalloc(size);
-	sd_init_req((struct sd_req *)&hdr, SD_OP_GET_NODE_LIST);
-	hdr.request_ver = epoch;
+	sd_init_req(&hdr, SD_OP_GET_NODE_LIST);
 
 	hdr.data_length = size;
 
-	ret = collie_exec_req(sdhost, sdport, (struct sd_req *)&hdr, buf);
+	ret = collie_exec_req(sdhost, sdport, &hdr, buf);
 	if (ret < 0)
 		goto out;
 
@@ -88,7 +87,7 @@ int update_node_list(int max_nodes, uint32_t epoch)
 	memcpy(sd_nodes, buf, size);
 	sd_vnodes_nr = nodes_to_vnodes(sd_nodes, sd_nodes_nr, sd_vnodes);
 	sd_epoch = hdr.epoch;
-	master_idx = rsp->master_idx;
+	master_idx = rsp->node.master_idx;
 out:
 	if (buf)
 		free(buf);
@@ -384,7 +383,7 @@ int main(int argc, char **argv)
 		highlight = false;
 
 	if (flags & SUBCMD_FLAG_NEED_NODELIST) {
-		ret = update_node_list(SD_MAX_NODES, 0);
+		ret = update_node_list(SD_MAX_NODES);
 		if (ret < 0) {
 			fprintf(stderr, "Failed to get node list\n");
 			exit(EXIT_SYSFAIL);
diff --git a/collie/collie.h b/collie/collie.h
index b4c7117..1f5de61 100644
--- a/collie/collie.h
+++ b/collie/collie.h
@@ -71,7 +71,7 @@ int sd_write_object(uint64_t oid, uint64_t cow_oid, void *data,
 int collie_exec_req(const char *host, int port, struct sd_req *hdr, void *data);
 int send_light_req(struct sd_req *hdr, const char *host, int port);
 int do_generic_subcommand(struct subcommand *sub, int argc, char **argv);
-int update_node_list(int max_nodes, uint32_t epoch);
+int update_node_list(int max_nodes);
 void confirm(const char *message);
 
 extern struct command vdi_command;
diff --git a/collie/common.c b/collie/common.c
index 742981d..063a932 100644
--- a/collie/common.c
+++ b/collie/common.c
@@ -221,7 +221,7 @@ int do_generic_subcommand(struct subcommand *sub, int argc, char **argv)
 			unsigned long flags = sub[i].flags;
 
 			if (flags & SUBCMD_FLAG_NEED_NODELIST) {
-				ret = update_node_list(SD_MAX_NODES, 0);
+				ret = update_node_list(SD_MAX_NODES);
 				if (ret < 0) {
 					fprintf(stderr,
 						"Failed to get node list\n");
diff --git a/collie/node.c b/collie/node.c
index 237500e..1f89482 100644
--- a/collie/node.c
+++ b/collie/node.c
@@ -65,30 +65,31 @@ static int node_info(int argc, char **argv)
 
 	for (i = 0; i < sd_nodes_nr; i++) {
 		char host[128];
-		struct sd_node_req req;
-		struct sd_node_rsp *rsp = (struct sd_node_rsp *)&req;
+		struct sd_req req;
+		struct sd_rsp *rsp = (struct sd_rsp *)&req;
 		char store_str[UINT64_DECIMAL_SIZE], free_str[UINT64_DECIMAL_SIZE];
 
 		addr_to_str(host, sizeof(host), sd_nodes[i].nid.addr, 0);
 
-		sd_init_req((struct sd_req *)&req, SD_OP_STAT_SHEEP);
+		sd_init_req(&req, SD_OP_STAT_SHEEP);
 
-		ret = send_light_req((struct sd_req *)&req, host,
-				     sd_nodes[i].nid.port);
+		ret = send_light_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));
+		size_to_str(rsp->node.store_size, store_str, sizeof(store_str));
+		size_to_str(rsp->node.store_size - rsp->node.store_free,
+			    free_str, sizeof(free_str));
 		if (!ret) {
+			int ratio = (int)(((double)(rsp->node.store_size -
+						    rsp->node.store_free) /
+					   rsp->node.store_size) * 100);
 			printf(raw_output ? "%d %s %s %d%%\n" : "%2d\t%s\t%s\t%3d%%\n",
 			       i, store_str, free_str,
-			       rsp->store_size == 0 ? 0 :
-			       (int)(((double)(rsp->store_size - rsp->store_free) / rsp->store_size) * 100));
+			       rsp->node.store_size == 0 ? 0 : ratio);
 			success++;
 		}
 
-		total_size += rsp->store_size;
-		total_avail += rsp->store_free;
+		total_size += rsp->node.store_size;
+		total_avail += rsp->node.store_free;
 	}
 
 	if (success == 0) {
@@ -123,14 +124,13 @@ static int node_recovery(int argc, char **argv)
 
 	for (i = 0; i < sd_nodes_nr; i++) {
 		char host[128];
-		struct sd_node_req req;
+		struct sd_req req;
 
 		addr_to_str(host, sizeof(host), sd_nodes[i].nid.addr, 0);
 
-		sd_init_req((struct sd_req *)&req, SD_OP_STAT_RECOVERY);
+		sd_init_req(&req, SD_OP_STAT_RECOVERY);
 
-		ret = collie_exec_req(host, sd_nodes[i].nid.port,
-				      (struct sd_req *)&req, NULL);
+		ret = collie_exec_req(host, sd_nodes[i].nid.port, &req, NULL);
 		if (ret == SD_RES_NODE_IN_RECOVERY) {
 			addr_to_str(host, sizeof(host),
 					sd_nodes[i].nid.addr, sd_nodes[i].nid.port);
@@ -180,7 +180,7 @@ static int node_kill(int argc, char **argv)
 {
 	char host[128];
 	int node_id, ret;
-	struct sd_node_req req;
+	struct sd_req req;
 	const char *p = argv[optind++];
 
 	if (!is_numeric(p)) {
@@ -197,10 +197,9 @@ static int node_kill(int argc, char **argv)
 
 	addr_to_str(host, sizeof(host), sd_nodes[node_id].nid.addr, 0);
 
-	sd_init_req((struct sd_req *)&req, SD_OP_KILL_NODE);
+	sd_init_req(&req, SD_OP_KILL_NODE);
 
-	ret = send_light_req((struct sd_req *)&req, host,
-			     sd_nodes[node_id].nid.port);
+	ret = send_light_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/internal_proto.h b/include/internal_proto.h
index 08958db..f124b9e 100644
--- a/include/internal_proto.h
+++ b/include/internal_proto.h
@@ -108,82 +108,6 @@
 #define SD_STATUS_HALT              0x00000020
 #define SD_STATUS_KILLED            0x00000040
 
-struct sd_so_req {
-	uint8_t		proto_ver;
-	uint8_t		opcode;
-	uint16_t	flags;
-	uint32_t	epoch;
-	uint32_t        id;
-	uint32_t        data_length;
-	uint64_t	oid;
-	uint64_t	ctime;
-	uint32_t	copies;
-	uint32_t	tag;
-	uint32_t	opcode_specific[2];
-};
-
-struct sd_so_rsp {
-	uint8_t		proto_ver;
-	uint8_t		opcode;
-	uint16_t	flags;
-	uint32_t	epoch;
-	uint32_t        id;
-	uint32_t        data_length;
-	uint32_t        result;
-	uint32_t	copies;
-	uint64_t	ctime;
-	uint64_t	oid;
-	uint32_t	opcode_specific[2];
-};
-
-struct sd_list_req {
-	uint8_t		proto_ver;
-	uint8_t		opcode;
-	uint16_t	flags;
-	uint32_t	epoch;
-	uint32_t        id;
-	uint32_t        data_length;
-	uint32_t        tgt_epoch;
-	uint32_t        pad[7];
-};
-
-struct sd_list_rsp {
-	uint8_t		proto_ver;
-	uint8_t		opcode;
-	uint16_t	flags;
-	uint32_t	epoch;
-	uint32_t        id;
-	uint32_t        data_length;
-	uint32_t        result;
-	uint32_t        pad[7];
-};
-
-struct sd_node_req {
-	uint8_t		proto_ver;
-	uint8_t		opcode;
-	uint16_t	flags;
-	uint32_t	epoch;
-	uint32_t        id;
-	uint32_t        data_length;
-	uint32_t	request_ver;
-	uint32_t	pad[7];
-};
-
-struct sd_node_rsp {
-	uint8_t		proto_ver;
-	uint8_t		opcode;
-	uint16_t	flags;
-	uint32_t	epoch;
-	uint32_t        id;
-	uint32_t        data_length;
-	uint32_t        result;
-	uint32_t	nr_nodes;
-	uint32_t	local_idx;
-	uint32_t	master_idx;
-	uint64_t	store_size;
-	uint64_t	store_free;
-};
-
 struct node_id {
 	uint8_t addr[16];
 	uint16_t port;
diff --git a/include/sheepdog_proto.h b/include/sheepdog_proto.h
index 96e1de4..89ded0d 100644
--- a/include/sheepdog_proto.h
+++ b/include/sheepdog_proto.h
@@ -105,6 +105,9 @@
 
 #define STORE_LEN 16
 
+#define SD_REQ_SIZE 48
+#define SD_RSP_SIZE 48
+
 struct sd_req {
 	uint8_t		proto_ver;
 	uint8_t		opcode;
@@ -126,6 +129,15 @@ struct sd_req {
 			uint32_t	copies;
 			uint32_t	snapid;
 		} vdi;
+
+		/* sheepdog-internal */
+		struct {
+			uint64_t	oid;
+			uint64_t	ctime;
+			uint32_t	copies;
+			uint32_t	tag;
+		} cluster;
+
 		uint32_t		__pad[8];
 	};
 };
@@ -151,6 +163,17 @@ struct sd_rsp {
 			uint32_t	attr_id;
 			uint32_t	copies;
 		} vdi;
+
+		/* sheepdog-internal */
+		struct {
+			uint32_t	__pad;
+			uint32_t	nr_nodes;
+			uint32_t	local_idx;
+			uint32_t	master_idx;
+			uint64_t	store_size;
+			uint64_t	store_free;
+		} node;
+
 		uint32_t		__pad[8];
 	};
 };
@@ -282,4 +305,11 @@ static inline uint32_t attr_oid_to_vid(uint64_t oid)
 	return (~VDI_ATTR_BIT & oid) >> VDI_SPACE_SHIFT;
 }
 
+static inline __attribute__((used)) void __sd_proto_build_bug_ons(void)
+{
+        /* never called, only for checking BUILD_BUG_ON()s */
+        BUILD_BUG_ON(sizeof(struct sd_req) != SD_REQ_SIZE);
+        BUILD_BUG_ON(sizeof(struct sd_rsp) != SD_RSP_SIZE);
+}
+
 #endif
diff --git a/sheep/group.c b/sheep/group.c
index 3017164..4044d12 100644
--- a/sheep/group.c
+++ b/sheep/group.c
@@ -206,7 +206,6 @@ struct vnode_info *get_vnode_info_epoch(uint32_t epoch,
 int local_get_node_list(const struct sd_req *req, struct sd_rsp *rsp,
 			       void *data)
 {
-	struct sd_node_rsp *node_rsp = (struct sd_node_rsp *)rsp;
 	int nr_nodes;
 	struct vnode_info *cur_vinfo = thread_unsafe_get(current_vnode_info);
 
@@ -214,15 +213,15 @@ int local_get_node_list(const struct sd_req *req, struct sd_rsp *rsp,
 		nr_nodes = cur_vinfo->nr_nodes;
 		memcpy(data, cur_vinfo->nodes,
 			sizeof(struct sd_node) * nr_nodes);
-		node_rsp->data_length = nr_nodes * sizeof(struct sd_node);
-		node_rsp->nr_nodes = nr_nodes;
-		node_rsp->local_idx = get_node_idx(cur_vinfo, &sys->this_node);
+		rsp->data_length = nr_nodes * sizeof(struct sd_node);
+		rsp->node.nr_nodes = nr_nodes;
+		rsp->node.local_idx = get_node_idx(cur_vinfo, &sys->this_node);
 	} else {
-		node_rsp->nr_nodes = 0;
-		node_rsp->local_idx = 0;
+		rsp->node.nr_nodes = 0;
+		rsp->node.local_idx = 0;
 	}
 
-	node_rsp->master_idx = -1;
+	rsp->node.master_idx = -1;
 	return SD_RES_SUCCESS;
 }
 
diff --git a/sheep/object_list_cache.c b/sheep/object_list_cache.c
index 28a75ee..554b60a 100644
--- a/sheep/object_list_cache.c
+++ b/sheep/object_list_cache.c
@@ -130,7 +130,7 @@ int objlist_cache_insert(uint64_t oid)
 	return 0;
 }
 
-int get_obj_list(const struct sd_list_req *hdr, struct sd_list_rsp *rsp, void *data)
+int get_obj_list(const struct sd_req *hdr, struct sd_rsp *rsp, void *data)
 {
 	int nr = 0;
 	struct objlist_cache_entry *entry;
diff --git a/sheep/ops.c b/sheep/ops.c
index 97e1443..48dd08c 100644
--- a/sheep/ops.c
+++ b/sheep/ops.c
@@ -241,7 +241,6 @@ static int remove_epoch(uint32_t epoch)
 static int cluster_make_fs(const struct sd_req *req, struct sd_rsp *rsp,
 			   void *data)
 {
-	const struct sd_so_req *hdr = (const struct sd_so_req *)req;
 	int i, ret;
 	uint32_t latest_epoch;
 	uint64_t created_time;
@@ -265,12 +264,12 @@ static int cluster_make_fs(const struct sd_req *req, struct sd_rsp *rsp,
 	if (ret != SD_RES_SUCCESS)
 		return ret;
 
-	sys->nr_copies = hdr->copies;
-	sys->flags = hdr->flags;
+	sys->nr_copies = req->cluster.copies;
+	sys->flags = req->flags;
 	if (!sys->nr_copies)
 		sys->nr_copies = SD_DEFAULT_COPIES;
 
-	created_time = hdr->ctime;
+	created_time = req->cluster.ctime;
 	set_cluster_ctime(created_time);
 	set_cluster_copies(sys->nr_copies);
 	set_cluster_flags(sys->flags);
@@ -398,10 +397,10 @@ static int local_get_vdi_copies(const struct sd_req *req, struct sd_rsp *rsp,
 
 static int local_stat_sheep(struct request *req)
 {
-	struct sd_node_rsp *node_rsp = (struct sd_node_rsp *)&req->rp;
+	struct sd_rsp *rsp = &req->rp;
 	uint32_t epoch = req->rq.epoch;
 
-	return stat_sheep(&node_rsp->store_size, &node_rsp->store_free, epoch);
+	return stat_sheep(&rsp->node.store_size, &rsp->node.store_free, epoch);
 }
 
 static int local_stat_recovery(const struct sd_req *req, struct sd_rsp *rsp,
@@ -468,8 +467,7 @@ out:
 
 static int local_get_obj_list(struct request *req)
 {
-	return get_obj_list((const struct sd_list_req *)&req->rq,
-			    (struct sd_list_rsp *)&req->rp, req->data);
+	return get_obj_list(&req->rq, &req->rp, req->data);
 }
 
 static int local_get_epoch(struct request *req)
diff --git a/sheep/recovery.c b/sheep/recovery.c
index 3cae911..2ee1470 100644
--- a/sheep/recovery.c
+++ b/sheep/recovery.c
@@ -503,8 +503,8 @@ static uint64_t *fetch_object_list(struct sd_node *e, uint32_t epoch,
 				   size_t *nr_oids)
 {
 	char name[128];
-	struct sd_list_req hdr;
-	struct sd_list_rsp *rsp = (struct sd_list_rsp *)&hdr;
+	struct sd_req hdr;
+	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
 	size_t buf_size = list_buffer_size;
 	uint64_t *buf = xmalloc(buf_size);
 	int ret;
@@ -513,11 +513,10 @@ static uint64_t *fetch_object_list(struct sd_node *e, uint32_t epoch,
 	sd_dprintf("%s %"PRIu32, name, e->nid.port);
 
 retry:
-	sd_init_req((struct sd_req *)&hdr, SD_OP_GET_OBJ_LIST);
-	hdr.tgt_epoch = epoch - 1;
+	sd_init_req(&hdr, SD_OP_GET_OBJ_LIST);
 	hdr.data_length = buf_size;
 	hdr.epoch = sys_epoch();
-	ret = sheep_exec_req(&e->nid, (struct sd_req *)&hdr, buf);
+	ret = sheep_exec_req(&e->nid, &hdr, buf);
 
 	switch (ret) {
 	case SD_RES_SUCCESS:
diff --git a/sheep/sheep_priv.h b/sheep/sheep_priv.h
index f0ba3fe..1fc291c 100644
--- a/sheep/sheep_priv.h
+++ b/sheep/sheep_priv.h
@@ -321,7 +321,7 @@ void init_config_path(const char *base_path);
 int init_config_file(void);
 int set_cluster_ctime(uint64_t ctime);
 uint64_t get_cluster_ctime(void);
-int get_obj_list(const struct sd_list_req *, struct sd_list_rsp *, void *);
+int get_obj_list(const struct sd_req *, struct sd_rsp *, void *);
 int objlist_cache_cleanup(uint32_t vid);
 
 int start_recovery(struct vnode_info *cur_vinfo, struct vnode_info *old_vinfo);
-- 
1.8.1.3.566.gaa39828




More information about the sheepdog mailing list