[sheepdog] [PATCH v5 1/2] sheep/http: add range support for swift insterface

[Liu Yuan]

On Tue, Jan 21, 2014 at 04:14:39PM +0800, Robin Dong wrote:
> From: Robin Dong <sanbai at taobao.com>
> 
> In swift spec, "Range: bytes=13-15" in http header means "read the three bytes
> of data after a 13-byte offset". We add this support by parsing http header and
> read specific bytes in range.
> 
> We don't support multi-ranges at present.
> 
> Signed-off-by: Robin Dong <sanbai at taobao.com>
> ---
> v4-->v5:
>     1. return "Requested Range Not Satisfiable" instead of "Bad Request"
>     2. fix the problem that "Range: bytes=0-1" will return whole file
>     3. add test-case for range about [0,x] and [x,EOF]
> v3-->v4:
>     1. do not check "off" and "onode->size" twice
>     2. check return value of http_request_write() with "len"
> v2-->v3:
>     1. add scissor lines for patch header
>     2. change "%lu" to PRIx64 for format of output
> v1-->v2:
>     1. use req->offset and req->data_length instead of req->range[2]
> 
>  sheep/http/http.c | 32 ++++++++++++++++++++++++++++++++
>  sheep/http/http.h |  1 +
>  sheep/http/kv.c   | 45 +++++++++++++++++++++++++++++++++++----------
>  3 files changed, 68 insertions(+), 10 deletions(-)
> 
> diff --git a/sheep/http/http.c b/sheep/http/http.c
> index 165c818..7b25651 100644
> --- a/sheep/http/http.c
> +++ b/sheep/http/http.c
> @@ -169,10 +169,42 @@ static int request_init_operation(struct http_request *req)
>  	req->uri = FCGX_GetParam("DOCUMENT_URI", env);
>  	if (!req->uri)
>  		return BAD_REQUEST;
> +	p = FCGX_GetParam("HTTP_RANGE", env);
> +	if (p && p[0] != '\0') {
> +		const char prefix[] = "bytes=";
> +		char *left, *right, num[64];
> +		uint64_t max;
> +		left = strstr(p, prefix);
> +		if (!p)
> +			goto invalid_range;
> +		right = strchr(left, '-');
> +		strncpy(num, left + sizeof(prefix) - 1, right - left);
> +		req->offset = strtoll(num, &endp, 10);
> +		if (num == endp)
> +			goto invalid_range;
> +		strcpy(num, right + 1);
> +		/*
> +		 * In swift spec, the second number of RANGE should be included
> +		 * which means [num1, num2], but our common means for read and
> +		 * write data by 'offset' and 'len' is [num1, num2), so we
> +		 * should add 1 to num2.
> +		 */
> +		max = strtoll(num, &endp, 10) + 1;
> +		if (num == endp)
> +			goto invalid_range;
> +		if (max <= req->offset)
> +			goto invalid_range;
> +		req->data_length = max - req->offset;
> +		sd_debug("HTTP_RANGE: %"PRIu64" %"PRIu64, req->offset, max);
> +	}
>  
>  	req->status = UNKNOWN;
>  
>  	return OK;
> +
> +invalid_range:
> +	sd_err("invalid range %s", p);
> +	return REQUEST_RANGE_NOT_SATISFIABLE;
>  }
>  
>  static int http_init_request(struct http_request *req)
> diff --git a/sheep/http/http.h b/sheep/http/http.h
> index 20246dc..f437f82 100644
> --- a/sheep/http/http.h
> +++ b/sheep/http/http.h
> @@ -48,6 +48,7 @@ struct http_request {
>  	enum http_opcode opcode;
>  	enum http_status status;
>  	uint64_t data_length;
> +	uint64_t offset;
>  };
>  
>  struct http_driver {
> diff --git a/sheep/http/kv.c b/sheep/http/kv.c
> index 182b263..6ee2230 100644
> --- a/sheep/http/kv.c
> +++ b/sheep/http/kv.c
> @@ -858,24 +858,34 @@ static int onode_free_data(struct kv_onode *onode)
>  	return ret;
>  }
>  
> -static int onode_read_extents(struct kv_onode *onode, struct http_request *req)
> +static int onode_read_extents(struct kv_onode *onode, struct http_request *req,
> +			      uint64_t off, uint64_t len)
>  {
>  	struct onode_extent *ext;
> -	uint64_t size, total, total_size, offset, done = 0, i;
> +	uint64_t size, total, total_size, offset, done = 0, i, ext_len;
>  	int ret;
>  	char *data_buf = NULL;
>  	uint64_t read_buffer_size = MIN(MAX_RW_BUFFER, onode->size);
>  
>  	data_buf = xmalloc(read_buffer_size);
> -	total_size = onode->size;
> +	total_size = len;
>  	for (i = 0; i < onode->nr_extent; i++) {
>  		ext = onode->o_extent + i;
> -		total = min(ext->count * SD_DATA_OBJ_SIZE, total_size);
> -		offset = ext->start * SD_DATA_OBJ_SIZE;
> +		ext_len = ext->count * SD_DATA_OBJ_SIZE;
> +		if (off >= ext_len) {
> +			off -= ext_len;
> +			continue;
> +		}
> +		total = min(ext_len - off, total_size);
> +		offset = ext->start * SD_DATA_OBJ_SIZE + off;
> +		off = 0;
> +		done = 0;
>  		while (done < total) {
>  			size = MIN(total - done, read_buffer_size);
>  			ret = vdi_read_write(onode->data_vid, data_buf,
>  					     size, offset, true);
> +			sd_debug("vdi_read_write size: %"PRIx64", offset: %"
> +				 PRIx64, size, offset);
>  			if (ret != SD_RES_SUCCESS) {
>  				sd_err("Failed to read for vid %"PRIx32,
>  				       onode->data_vid);
> @@ -953,12 +963,29 @@ out:
>  static int onode_read_data(struct kv_onode *onode, struct http_request *req)
>  {
>  	int ret;
> +	uint64_t off = 0, len = onode->size;
> +
> +	if (req->offset || req->data_length) {
> +		off = req->offset;
> +		len = req->data_length;
> +		if ((off + len - 1) > onode->size) {
> +			if (onode->size > off)
> +				len = onode->size - off;
> +			else
> +				len = 0;
> +		}
> +	}
> +	req->data_length = len;
> +	http_response_header(req, OK);
> +
> +	if (!len)
> +		return SD_RES_SUCCESS;
>  
>  	if (!onode->inlined)
> -		return onode_read_extents(onode, req);
> +		return onode_read_extents(onode, req, off, len);

req has offset and data_length field, why you need to pass them by extra para?

Thanks
Yuan