<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2014/1/20 Liu Yuan <span dir="ltr"><<a href="mailto:namei.unix@gmail.com" target="_blank">namei.unix@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Mon, Jan 20, 2014 at 09:42:29PM +0800, Robin Dong wrote:<br>
> From: Robin Dong <<a href="mailto:sanbai@taobao.com">sanbai@taobao.com</a>><br>
><br>
> In swift spec, "Range: bytes=13-15" in http header means "read the three bytes<br>
> of data after a 13-byte offset". We add this support by parsing http header and<br>
> read specific bytes in range.<br>
><br>
> We don't support multi-ranges at present.<br>
><br>
> Signed-off-by: Robin Dong <<a href="mailto:sanbai@taobao.com">sanbai@taobao.com</a>><br>
> ---<br>
> v3-->v4:<br>
> 1. do not check "off" and "onode->size" twice<br>
> 2. check return value of http_request_write() with "len"<br>
> v2-->v3:<br>
> 1. add scissor lines for patch header<br>
> 2. change "%lu" to PRIx64 for format of output<br>
> v1-->v2:<br>
> 1. use req->offset and req->data_length instead of req->range[2]<br>
><br>
> sheep/http/http.c | 32 ++++++++++++++++++++++++++++++++<br>
> sheep/http/http.h | 1 +<br>
> sheep/http/kv.c | 43 +++++++++++++++++++++++++++++++++----------<br>
> 3 files changed, 66 insertions(+), 10 deletions(-)<br>
><br>
> diff --git a/sheep/http/http.c b/sheep/http/http.c<br>
> index 165c818..08f894b 100644<br>
> --- a/sheep/http/http.c<br>
> +++ b/sheep/http/http.c<br>
> @@ -169,10 +169,42 @@ static int request_init_operation(struct http_request *req)<br>
> req->uri = FCGX_GetParam("DOCUMENT_URI", env);<br>
> if (!req->uri)<br>
> return BAD_REQUEST;<br>
> + p = FCGX_GetParam("HTTP_RANGE", env);<br>
> + if (p && p[0] != '\0') {<br>
> + const char prefix[] = "bytes=";<br>
> + char *left, *right, num[64];<br>
> + uint64_t max;<br>
> + left = strstr(p, prefix);<br>
> + if (!p)<br>
> + goto invalid_range;<br>
> + right = strchr(left, '-');<br>
> + strncpy(num, left + sizeof(prefix) - 1, right - left);<br>
> + req->offset = strtoll(num, &endp, 10);<br>
> + if (num == endp)<br>
> + goto invalid_range;<br>
> + strcpy(num, right + 1);<br>
> + /*<br>
> + * In swift spec, the second number of RANGE should be included<br>
> + * which means [num1, num2], but our common means for read and<br>
> + * write data by 'offset' and 'len' is [num1, num2), so we<br>
> + * should add 1 to num2.<br>
> + */<br>
> + max = strtoll(num, &endp, 10) + 1;<br>
> + if (num == endp)<br>
> + goto invalid_range;<br>
> + if (max <= req->offset)<br>
> + goto invalid_range;<br>
<br>
</div></div>I think you should return "Requested Range Not Satisfiable"<br>
<div><div class="h5"><br>
> + req->data_length = max - req->offset;<br>
> + sd_debug("HTTP_RANGE: %"PRIx64" %"PRIx64, req->offset, max);<br>
> + }<br>
><br>
> req->status = UNKNOWN;<br>
><br>
> return OK;<br>
> +<br>
> +invalid_range:<br>
> + sd_err("invalid range %s", p);<br>
> + return BAD_REQUEST;<br>
> }<br>
><br>
> static int http_init_request(struct http_request *req)<br>
> diff --git a/sheep/http/http.h b/sheep/http/http.h<br>
> index 20246dc..f437f82 100644<br>
> --- a/sheep/http/http.h<br>
> +++ b/sheep/http/http.h<br>
> @@ -48,6 +48,7 @@ struct http_request {<br>
> enum http_opcode opcode;<br>
> enum http_status status;<br>
> uint64_t data_length;<br>
> + uint64_t offset;<br>
> };<br>
><br>
> struct http_driver {<br>
> diff --git a/sheep/http/kv.c b/sheep/http/kv.c<br>
> index 182b263..4e9b921 100644<br>
> --- a/sheep/http/kv.c<br>
> +++ b/sheep/http/kv.c<br>
> @@ -858,24 +858,34 @@ static int onode_free_data(struct kv_onode *onode)<br>
> return ret;<br>
> }<br>
><br>
> -static int onode_read_extents(struct kv_onode *onode, struct http_request *req)<br>
> +static int onode_read_extents(struct kv_onode *onode, struct http_request *req,<br>
> + uint64_t off, uint64_t len)<br>
> {<br>
> struct onode_extent *ext;<br>
> - uint64_t size, total, total_size, offset, done = 0, i;<br>
> + uint64_t size, total, total_size, offset, done = 0, i, ext_len;<br>
> int ret;<br>
> char *data_buf = NULL;<br>
> uint64_t read_buffer_size = MIN(MAX_RW_BUFFER, onode->size);<br>
><br>
> data_buf = xmalloc(read_buffer_size);<br>
> - total_size = onode->size;<br>
> + total_size = len;<br>
> for (i = 0; i < onode->nr_extent; i++) {<br>
> ext = onode->o_extent + i;<br>
> - total = min(ext->count * SD_DATA_OBJ_SIZE, total_size);<br>
> - offset = ext->start * SD_DATA_OBJ_SIZE;<br>
> + ext_len = ext->count * SD_DATA_OBJ_SIZE;<br>
> + if (off >= ext_len) {<br>
> + off -= ext_len;<br>
> + continue;<br>
> + }<br>
> + total = min(ext_len - off, total_size);<br>
> + offset = ext->start * SD_DATA_OBJ_SIZE + off;<br>
> + off = 0;<br>
> + done = 0;<br>
> while (done < total) {<br>
> size = MIN(total - done, read_buffer_size);<br>
> ret = vdi_read_write(onode->data_vid, data_buf,<br>
> size, offset, true);<br>
> + sd_debug("vdi_read_write size: %"PRIx64", offset: %"<br>
> + PRIx64, size, offset);<br>
> if (ret != SD_RES_SUCCESS) {<br>
> sd_err("Failed to read for vid %"PRIx32,<br>
> onode->data_vid);<br>
> @@ -953,12 +963,27 @@ out:<br>
> static int onode_read_data(struct kv_onode *onode, struct http_request *req)<br>
> {<br>
> int ret;<br>
> + uint64_t off = 0, len = onode->size;<br>
> +<br>
> + if (req->offset && req->data_length) {<br>
> + off = req->offset;<br>
> + len = req->data_length;<br>
> + if ((off + len - 1) > onode->size)<br>
> + len = onode->size - off;<br>
> + if (off >= onode->size)<br>
<br>
</div></div>I think this check is redundant. Check (off + len - 1) > onode->size is enough<br>
<div class="im"><br>
> + len = 0;<br>
> + }<br>
> + req->data_length = len;<br>
> + http_response_header(req, OK);<br>
> +<br>
<br>
</div>I tried the Range: bytes=0-1, but it returns the whole file to me. Also please<br>
add this to test script.<br>
<br>
Also if offset + len - 1 > onode-size, I think you should return<br>
"Requested Range Not Satisfiable"<br></blockquote><div>In system-calls of unix, if the argument "length" reach out of the file in read(), it will return all the data after "offset".</div><div>
I think it's better to be consistent with common APIs since the spec of SWIFT doesn't tell us what exactly we should do</div><div>in this situation.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Thanks<br>
<span class="HOEnZb"><font color="#888888">Yuan<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br>--<br>Best Regard<br>Robin Dong
</div></div>