[sheepdog] [PATCH v4 1/2] sheep/http: add range support for swift insterface
Robin Dong
robin.k.dong at gmail.com
Tue Jan 21 02:03:37 CET 2014
2014/1/20 Liu Yuan <namei.unix at gmail.com>
> On Mon, Jan 20, 2014 at 09:42:29PM +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>
> > ---
> > 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 | 43 +++++++++++++++++++++++++++++++++----------
> > 3 files changed, 66 insertions(+), 10 deletions(-)
> >
> > diff --git a/sheep/http/http.c b/sheep/http/http.c
> > index 165c818..08f894b 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;
>
> I think you should return "Requested Range Not Satisfiable"
>
> > + req->data_length = max - req->offset;
> > + sd_debug("HTTP_RANGE: %"PRIx64" %"PRIx64, req->offset,
> max);
> > + }
> >
> > req->status = UNKNOWN;
> >
> > return OK;
> > +
> > +invalid_range:
> > + sd_err("invalid range %s", p);
> > + return BAD_REQUEST;
> > }
> >
> > 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..4e9b921 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,27 @@ 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)
> > + len = onode->size - off;
> > + if (off >= onode->size)
>
> I think this check is redundant. Check (off + len - 1) > onode->size is
> enough
>
> > + len = 0;
> > + }
> > + req->data_length = len;
> > + http_response_header(req, OK);
> > +
>
> I tried the Range: bytes=0-1, but it returns the whole file to me. Also
> please
> add this to test script.
>
> Also if offset + len - 1 > onode-size, I think you should return
> "Requested Range Not Satisfiable"
>
In system-calls of unix, if the argument "length" reach out of the file in
read(), it will return all the data after "offset".
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
in this situation.
>
> Thanks
> Yuan
>
--
--
Best Regard
Robin Dong
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wpkg.org/pipermail/sheepdog/attachments/20140121/80eb1531/attachment-0004.html>
More information about the sheepdog
mailing list