[sheepdog] [PATCH v4 1/2] sheep/http: add range support for swift insterface
Liu Yuan
namei.unix at gmail.com
Tue Jan 21 03:59:33 CET 2014
On Tue, Jan 21, 2014 at 09:03:37AM +0800, Robin Dong wrote:
> 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.
>
Okay, we can serve a short read.
Thanks
Yuan
More information about the sheepdog
mailing list