[sheepdog] [PATCH v2 1/3] shared lib: return SD_RES_XXX instead of mixing std errno and sd error code

Hitoshi Mitake mitake.hitoshi at lab.ntt.co.jp
Mon Apr 27 04:11:30 CEST 2015


At Fri, 24 Apr 2015 10:01:31 +0800,
Liu Yuan wrote:
> 
> From: Liu Yuan <liuyuan at cmss.chinamobile.com>
> 
> Have all the sd_xxx functions return SD_RES_XXX, we will have a unified error
> handling for user program and more importantly, std error can't cover all the
> error cases that is specific to the sheepdog cluster, such as SD_RES_NO_VDI
> and SD_RES_NO_OBJ.
> 
> Signed-off-by: Liu Yuan <liuyuan at cmss.chinamobile.com>
> ---
>  lib/shared/sheep.c | 63 +++++++++++++++++++++++++-----------------------------
>  lib/shared/vdi.c   | 35 ++++++++++++++++--------------
>  2 files changed, 48 insertions(+), 50 deletions(-)

Applied, thanks.
Hitoshi

> 
> diff --git a/lib/shared/sheep.c b/lib/shared/sheep.c
> index 4c1b1f1..8568436 100644
> --- a/lib/shared/sheep.c
> +++ b/lib/shared/sheep.c
> @@ -62,6 +62,9 @@ static int sheep_submit_sdreq(struct sd_cluster *c, struct sd_req *hdr,
>  		ret = xwrite(c->sockfd, data, wlen);
>  out:
>  	sd_mutex_unlock(&c->submit_mutex);
> +	if (unlikely(ret < 0))
> +		return -SD_RES_EIO;
> +
>  	return ret;
>  }
>  
> @@ -86,7 +89,7 @@ int sd_run_sdreq(struct sd_cluster *c, struct sd_req *hdr, void *data)
>  
>  	ret = xread(c->sockfd, rsp, sizeof(*rsp));
>  	if (ret < 0)
> -		return ret;
> +		return SD_RES_SYSTEM_ERROR;
>  
>  	if (rlen > rsp->data_length)
>  		rlen = rsp->data_length;
> @@ -94,20 +97,10 @@ int sd_run_sdreq(struct sd_cluster *c, struct sd_req *hdr, void *data)
>  	if (rlen) {
>  		ret = xread(c->sockfd, data, rlen);
>  		if (ret < 0)
> -			return ret;
> -	}
> -
> -	switch (rsp->result) {
> -	case SD_RES_SUCCESS:
> -		break;
> -	case SD_RES_NO_OBJ:
> -	case SD_RES_NO_VDI:
> -		return -ENOENT;
> -	default:
> -		return -EIO;
> +			return SD_RES_SYSTEM_ERROR;
>  	}
>  
> -	return 0;
> +	return rsp->result;
>  }
>  
>  static void aio_end_request(struct sd_request *req, int ret)
> @@ -173,7 +166,6 @@ static void end_sheep_request(struct sheep_request *req)
>  	free(req);
>  }
>  
> -/* FIXME: handle submit failure */
>  static int submit_sheep_request(struct sheep_request *req)
>  {
>  	struct sd_req hdr = {};
> @@ -282,9 +274,6 @@ static int sheep_aiocb_submit(struct sheep_aiocb *aiocb)
>  		}
>  
>  		req = alloc_sheep_request(aiocb, oid, cow_oid, len, start);
> -		if (IS_ERR(req))
> -			return PTR_ERR(req);
> -
>  		if (vid && !cow_oid)
>  			goto submit;
>  
> @@ -337,9 +326,6 @@ static int submit_request(struct sd_request *req)
>  {
>  	struct sheep_aiocb *aiocb = sheep_aiocb_setup(req);
>  
> -	if (IS_ERR(aiocb))
> -		return PTR_ERR(aiocb);
> -
>  	return sheep_aiocb_submit(aiocb);
>  }
>  
> @@ -429,7 +415,7 @@ static int sheep_handle_reply(struct sd_cluster *c)
>  	if (ret < 0) {
>  		req = fetch_first_inflight_request(c);
>  		if (req != NULL) {
> -			req->aiocb->ret = EIO;
> +			req->aiocb->ret = SD_RES_EIO;
>  			goto end_request;
>  		}
>  		goto err;
> @@ -441,7 +427,7 @@ static int sheep_handle_reply(struct sd_cluster *c)
>  	if (rsp.data_length > 0) {
>  		ret = xread(c->sockfd, req->buf, req->length);
>  		if (ret < 0) {
> -			req->aiocb->ret = EIO;
> +			req->aiocb->ret = SD_RES_EIO;
>  			goto end_request;
>  		}
>  	}
> @@ -513,12 +499,12 @@ static int init_cluster_handlers(struct sd_cluster *c)
>  
>  	c->request_fd = eventfd(0, 0);
>  	if (c->request_fd < 0)
> -		return -errno;
> +		return -SD_RES_SYSTEM_ERROR;
>  
>  	c->reply_fd = eventfd(0, 0);
>  	if (c->reply_fd < 0) {
>  		close(c->request_fd);
> -		return -errno;
> +		return -SD_RES_SYSTEM_ERROR;
>  	}
>  
>  	ret = pthread_create(&thread, NULL, request_handler, c);
> @@ -538,7 +524,7 @@ static int init_cluster_handlers(struct sd_cluster *c)
>  	}
>  	c->reply_thread = thread;
>  
> -	return 0;
> +	return SD_RES_SUCCESS;
>  }
>  
>  struct sd_cluster *sd_connect(char *host)
> @@ -552,33 +538,39 @@ struct sd_cluster *sd_connect(char *host)
>  
>  	ip = strtok(h, ":");
>  	if (!ip) {
> -		errno = EINVAL;
> +		errno = SD_RES_INVALID_PARMS;
>  		goto err;
>  	}
>  
>  	pt = strtok(NULL, ":");
>  	if (!pt) {
> -		errno = EINVAL;
> +		errno = SD_RES_INVALID_PARMS;
>  		goto err;
>  	}
>  
>  	if (sscanf(pt, "%u", &port) != 1) {
> -		errno = EINVAL;
> +		errno = SD_RES_INVALID_PARMS;
>  		goto err;
>  	}
>  
>  	fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> -	if (fd < 0)
> +	if (fd < 0) {
> +		errno = SD_RES_SYSTEM_ERROR;
>  		goto err;
> +	}
>  
>  	ret = setsockopt(fd, SOL_SOCKET, SO_LINGER, &linger_opt,
>  			 sizeof(linger_opt));
> -	if (ret < 0)
> +	if (ret < 0) {
> +		errno = SD_RES_SYSTEM_ERROR;
>  		goto err_close;
> +	}
>  
>  	ret = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &value, sizeof(value));
> -	if (ret < 0)
> +	if (ret < 0) {
> +		errno = SD_RES_SYSTEM_ERROR;
>  		goto err_close;
> +	}
>  
>  	addr.sin_family = AF_INET;
>  	addr.sin_port = htons(port);
> @@ -587,13 +579,15 @@ struct sd_cluster *sd_connect(char *host)
>  	case 1:
>  		break;
>  	default:
> -		errno = EINVAL;
> +		errno = SD_RES_INVALID_PARMS;
>  		goto err_close;
>  	}
>  
>  	ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr));
> -	if (ret < 0)
> +	if (ret < 0) {
> +		errno = SD_RES_SYSTEM_ERROR;
>  		goto err_close;
> +	}
>  
>  	c = xzalloc(sizeof(*c));
>  	c->sockfd = fd;
> @@ -638,5 +632,6 @@ int sd_disconnect(struct sd_cluster *c)
>  	close(c->reply_fd);
>  	close(c->sockfd);
>  	free(c);
> -	return 0;
> +
> +	return SD_RES_SUCCESS;
>  }
> diff --git a/lib/shared/vdi.c b/lib/shared/vdi.c
> index 39b8acb..ce10e35 100644
> --- a/lib/shared/vdi.c
> +++ b/lib/shared/vdi.c
> @@ -23,12 +23,12 @@ static int lock_vdi(struct sd_vdi *vdi)
>  	hdr.data_length = SD_MAX_VDI_LEN;
>  	hdr.flags = SD_FLAG_CMD_WRITE;
>  	ret = sd_run_sdreq(vdi->cluster, &hdr, vdi->name);
> -	if (ret < 0)
> +	if (ret != SD_RES_SUCCESS)
>  		return ret;
>  
>  	vdi->vid = rsp->vdi.vdi_id;
>  
> -	return 0;
> +	return SD_RES_SUCCESS;
>  }
>  
>  static int unlock_vdi(struct sd_vdi *vdi)
> @@ -40,9 +40,10 @@ static int unlock_vdi(struct sd_vdi *vdi)
>  	hdr.vdi.type = LOCK_TYPE_NORMAL;
>  	hdr.vdi.base_vdi_id = vdi->vid;
>  	ret = sd_run_sdreq(vdi->cluster, &hdr, NULL);
> -	if (ret < 0)
> +	if (ret != SD_RES_SUCCESS)
>  		return ret;
> -	return 0;
> +
> +	return SD_RES_SUCCESS;
>  }
>  
>  static struct sd_vdi *alloc_vdi(struct sd_cluster *c, char *name)
> @@ -71,8 +72,8 @@ struct sd_vdi *sd_vdi_open(struct sd_cluster *c, char *name)
>  	int ret;
>  
>  	ret = lock_vdi(new);
> -	if (ret < 0) {
> -		errno = -ret;
> +	if (ret != SD_RES_SUCCESS) {
> +		errno = ret;
>  		goto out_free;
>  	}
>  
> @@ -81,13 +82,13 @@ struct sd_vdi *sd_vdi_open(struct sd_cluster *c, char *name)
>  	hdr.obj.oid = vid_to_vdi_oid(new->vid);
>  	hdr.obj.offset = 0;
>  	ret = sd_run_sdreq(c, &hdr, new->inode);
> -	if (ret < 0) {
> -		errno = -ret;
> +	if (ret != SD_RES_SUCCESS) {
> +		errno = ret;
>  		goto out_unlock;
>  	}
>  
>  	if (vdi_is_snapshot(new->inode)) {
> -		errno = EINVAL;
> +		errno = SD_RES_INVALID_PARMS;
>  		goto out_unlock;
>  	}
>  
> @@ -123,8 +124,10 @@ static struct sd_request *alloc_request(struct sd_vdi *vdi, void *buf,
>  	int fd;
>  
>  	fd = eventfd(0, 0);
> -	if (fd < 0)
> -		return ERR_PTR(-errno);
> +	if (fd < 0) {
> +		errno = SD_RES_SYSTEM_ERROR;
> +		return NULL;
> +	}
>  	req = xzalloc(sizeof(*req));
>  	req->efd = fd;
>  	req->data = buf;
> @@ -142,8 +145,8 @@ int sd_vdi_read(struct sd_vdi *vdi, void *buf, size_t count, off_t offset)
>  	struct sd_request *req = alloc_request(vdi, buf, count, offset, false);
>  	int ret;
>  
> -	if (IS_ERR(req))
> -		return PTR_ERR(req);
> +	if (!req)
> +		return errno;
>  
>  	queue_request(req);
>  
> @@ -159,8 +162,8 @@ int sd_vdi_write(struct sd_vdi *vdi, void *buf, size_t count, off_t offset)
>  	struct sd_request *req = alloc_request(vdi, buf, count, offset, true);
>  	int ret;
>  
> -	if (IS_ERR(req))
> -		return PTR_ERR(req);
> +	if (!req)
> +		return errno;
>  
>  	queue_request(req);
>  
> @@ -176,7 +179,7 @@ int sd_vdi_close(struct sd_vdi *vdi)
>  	int ret;
>  
>  	ret = unlock_vdi(vdi);
> -	if (ret < 0) {
> +	if (ret != SD_RES_SUCCESS) {
>  		fprintf(stderr, "failed to unlock %s\n", vdi->name);
>  		return ret;
>  	}
> -- 
> 1.9.1
> 
> -- 
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> https://lists.wpkg.org/mailman/listinfo/sheepdog



More information about the sheepdog mailing list