[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