[sheepdog] [PATCH V4 1/2] sheep: refactor gateway_forward_request

[Liu Yuan]

On 09/02/2012 08:59 PM, Yunkai Zhang wrote:
> +static int forward_request_concurrently(struct sd_req *hdr,
> +					void *data, unsigned int *wlen,
> +					struct sd_vnode *target_vnodes[],
> +					struct sd_rsp target_rsps[],
> +					void *target_data[],
> +					int nr_targets)

This interface looks unnecessarily complex to me. I'd suggest changes like following
untested patch , it is more generic and simpler, simply adding one more caller provided
buffer to read response data if any.

Thanks,
Yuan

=====================================================
diff --git a/sheep/gateway.c b/sheep/gateway.c
index 41d712b..690f0ac 100644
--- a/sheep/gateway.c
+++ b/sheep/gateway.c
@@ -150,10 +150,12 @@ static inline void pfd_info_init(struct write_info *wi, struct pfd_info *pi)
  *
  * Return error code if any one request fails.
  */
-static int wait_forward_request(struct write_info *wi, struct sd_rsp *rsp)
+static int wait_forward_request(struct write_info *wi, void *read_buffer)
 {
 	int nr_sent, err_ret = SD_RES_SUCCESS, ret, pollret, i;
-	struct pfd_info pi;;
+	struct pfd_info pi;
+	struct sd_rsp rsp;
+	char *rb_pos = (char *)read_buffer;
 again:
 	pfd_info_init(wi, &pi);
 	pollret = poll(pi.pfds, pi.nr, -1);
@@ -174,23 +176,37 @@ again:
 		if (re & (POLLERR | POLLHUP | POLLNVAL)) {
 			err_ret = SD_RES_NETWORK_ERROR;
 			finish_one_write_err(wi, i);
-		} else if (re & POLLIN) {
-			if (do_read(pi.pfds[i].fd, rsp, sizeof(*rsp))) {
+			goto finish_write;
+		}
+		if (!(re & POLLIN)) {
+			eprintf("unhandled poll event\n");
+			goto finish_write;
+		}
+		if (do_read(pi.pfds[i].fd, &rsp, sizeof(rsp))) {
+			eprintf("remote node might have gone away\n");
+			err_ret = SD_RES_NETWORK_ERROR;
+			finish_one_write_err(wi, i);
+			goto finish_write;
+		}
+
+		if (rsp.data_length && read_buffer) {
+			memcpy(rb_pos, wi->ent[i].nid, sizeof(wi->ent[i].nid));
+			rb_pos += sizeof(wi->ent[i].nid);
+			if (do_read(pi.pfds[i].fd, &read_buffer,
+				    rsp.data_length)) {
 				eprintf("remote node might have gone away\n");
 				err_ret = SD_RES_NETWORK_ERROR;
 				finish_one_write_err(wi, i);
 				goto finish_write;
 			}
-
-			ret = rsp->result;
-			if (ret != SD_RES_SUCCESS) {
-				eprintf("fail %"PRIx32"\n", ret);
-				err_ret = ret;
-			}
-			finish_one_write(wi, i);
-		} else {
-			eprintf("unhandled poll event\n");
+			rb_pos += rsp.data_length;
 		}
+		ret = rsp.result;
+		if (ret != SD_RES_SUCCESS) {
+			eprintf("fail %"PRIx32"\n", ret);
+			err_ret = ret;
+		}
+		finish_one_write(wi, i);
 	}
 finish_write:
 	if (wi->nr_sent > 0)
@@ -225,11 +241,10 @@ static inline void gateway_init_fwd_hdr(struct sd_req *fwd, struct sd_req *hdr)
 	fwd->proto_ver = SD_SHEEP_PROTO_VER;
 }
 
-static int gateway_forward_request(struct request *req)
+static int gateway_forward_request(struct request *req, void *read_buffer)
 {
-	int i, err_ret = SD_RES_SUCCESS, ret, local = -1;
+	int i, err_ret = SD_RES_SUCCESS, ret;
 	unsigned wlen;
-	struct sd_rsp *rsp = (struct sd_rsp *)&req->rp;
 	struct sd_vnode *v;
 	struct sd_vnode *obj_vnodes[SD_MAX_COPIES];
 	uint64_t oid = req->rq.obj.oid;
@@ -253,10 +268,6 @@ static int gateway_forward_request(struct request *req)
 		struct sockfd *sfd;
 
 		v = obj_vnodes[i];
-		if (vnode_is_local(v)) {
-			local = i;
-			continue;
-		}
 
 		sfd = sheep_get_sockfd(&v->nid);
 		if (!sfd) {
@@ -274,21 +285,9 @@ static int gateway_forward_request(struct request *req)
 		write_info_advance(&wi, v, sfd);
 	}
 
-	if (local != -1 && err_ret == SD_RES_SUCCESS) {
-		v = obj_vnodes[local];
-
-		assert(op);
-		ret = sheep_do_op_work(op, req);
-
-		if (ret != SD_RES_SUCCESS) {
-			eprintf("fail to write local %"PRIx32"\n", ret);
-			err_ret = ret;
-		}
-	}
-
 	dprintf("nr_sent %d, err %x\n", wi.nr_sent, err_ret);
 	if (wi.nr_sent > 0) {
-		ret = wait_forward_request(&wi, rsp);
+		ret = wait_forward_request(&wi, read_buffer);
 		if (ret != SD_RES_SUCCESS)
 			err_ret = ret;
 	}
@@ -301,7 +300,7 @@ int gateway_write_obj(struct request *req)
 	if (sys->enable_write_cache && !req->local && !bypass_object_cache(req))
 		return object_cache_handle_request(req);
 
-	return gateway_forward_request(req);
+	return gateway_forward_request(req, NULL);
 }
 
 int gateway_create_and_write_obj(struct request *req)
@@ -309,10 +308,10 @@ int gateway_create_and_write_obj(struct request *req)
 	if (sys->enable_write_cache && !req->local && !bypass_object_cache(req))
 		return object_cache_handle_request(req);
 
-	return gateway_forward_request(req);
+	return gateway_forward_request(req, NULL);
 }
 
 int gateway_remove_obj(struct request *req)
 {
-	return gateway_forward_request(req);
+	return gateway_forward_request(req, NULL);
 }