| Message ID | 20180301135856.22698-2-rjones@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] block: curl: Allow arbitrary HTTP request headers to be set. | expand |
On Thu, Mar 1, 2018 at 4:00 PM Richard W.M. Jones <rjones@redhat.com> wrote: > Allow arbitrary HTTP request headers to be set, like this: > > qemu-img create -f qcow2 \ > -b 'json:{ "file.driver":"http", > "file.url":"http://192.168.0.249/scratch/test.img", > "file.header": ["Authorization: letmein"] }' \ > Since oVirt 4.2, you don't need the Authorization header. In any version, the Authorization header was used only in by the proxy (running on engine host). You really want to access the host directly and not the proxy, unless you cannot access the host. Authorization in ovirt works like this now: 1. User start a transfer operation using engine API/UI 2. Engine creates a ticket authorizing the operation on a some host and in ovirt-imageio-proxy 3. Engine returns random transfter url and proxy url to the user. 4. User can access the the image via the transfer or proxy url 5. User finish the operation via engine API/UI 6. Engine remove the tickets, invalidating the transfer/proxy urls So the only thing qemu-img needs is a url. Other issue that you need to consider is that you cannot create images via the http. Images are created only via engine API/UI. What ovirt-imageio give you is a way to read and write data to existing image. The url you get is mostly like an open file descriptor you can use with read() and write(). Note that you cannot access yet a chain of image via a url, since ovirt generate a random url per image in a chain. For example, if you have this chain: base-img <- top-image (top) oVirt will provide these urls to access a single image of the chain: https://host.address:54322/images/random-id1 (top-image) https://host.address:54322/images/random-id <https://host.address:54322/images/random-id1>2 (base-image) So qemu will not be able to consume the chain, as the base image is not available at https://host.address:54322/images/ <https://host.address:54322/images/random-id1>base-imge We plan to support this use case in 4.3, by using a ticket-per-chain instead of ticker-per-image. In this case oVirt will generate this url: https://host.address:54322/images/random-id1/top-image And the base image will be accessible at: https://host.address:54322/images/random-id1/base-image Nir > test.qcow2 > > which adds the ‘Authorization: letmein’ header to the outgoing request > for the backing file. Multiple headers can be set, and curl built-in > headers can be removed (using "Header:"). > > Note this uses the same format as curl's CURLOPT_HTTPHEADER, thus > pulling in curl API guarantees into qemu, but curl has had very strong > API backward compatibility since the start of the project. > But otherwise, supporting custom headers may be useful for other use cases. > > Signed-off-by: Richard W.M. Jones <rjones@redhat.com> > --- > block/curl.c | 21 ++++++++++++++++++++- > qapi/block-core.json | 10 ++++++++-- > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/block/curl.c b/block/curl.c > index aa42535783..972673ba5c 100644 > --- a/block/curl.c > +++ b/block/curl.c > @@ -83,6 +83,7 @@ static CURLMcode __curl_multi_socket_action(CURLM > *multi_handle, > #define CURL_BLOCK_OPT_TIMEOUT "timeout" > #define CURL_BLOCK_OPT_COOKIE "cookie" > #define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret" > +#define CURL_BLOCK_OPT_HEADER_PATTERN "header." > #define CURL_BLOCK_OPT_USERNAME "username" > #define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret" > #define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username" > @@ -134,6 +135,7 @@ typedef struct BDRVCURLState { > bool sslverify; > uint64_t timeout; > char *cookie; > + struct curl_slist *headers; > bool accept_range; > AioContext *aio_context; > QemuMutex mutex; > @@ -486,6 +488,9 @@ static int curl_init_state(BDRVCURLState *s, CURLState > *state) > if (s->cookie) { > curl_easy_setopt(state->curl, CURLOPT_COOKIE, s->cookie); > } > + if (s->headers) { > + curl_easy_setopt(state->curl, CURLOPT_HTTPHEADER, s->headers); > + } > curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, (long)s->timeout); > curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, > (void *)curl_read_cb); > @@ -680,7 +685,7 @@ static int curl_open(BlockDriverState *bs, QDict > *options, int flags, > double d; > const char *secretid; > const char *protocol_delimiter; > - int ret; > + int i, nr, ret; > > > if (flags & BDRV_O_RDWR) { > @@ -740,6 +745,18 @@ static int curl_open(BlockDriverState *bs, QDict > *options, int flags, > s->cookie = g_strdup(cookie); > } > > + s->headers = NULL; > + nr = qdict_array_entries(options, CURL_BLOCK_OPT_HEADER_PATTERN); > + for (i = 0; i < nr; ++i) { > + char key[32]; > + const char *header; > + > + snprintf(key, sizeof key, CURL_BLOCK_OPT_HEADER_PATTERN "%d", i); > + header = qdict_get_str(options, key); > + s->headers = curl_slist_append(s->headers, header); > + qdict_del(options, key); > + } > + > file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL); > if (file == NULL) { > error_setg(errp, "curl block driver requires an 'url' option"); > @@ -847,6 +864,7 @@ out: > state->curl = NULL; > out_noclean: > qemu_mutex_destroy(&s->mutex); > + curl_slist_free_all(s->headers); > g_free(s->cookie); > g_free(s->url); > g_free(s->username); > @@ -945,6 +963,7 @@ static void curl_close(BlockDriverState *bs) > curl_detach_aio_context(bs); > qemu_mutex_destroy(&s->mutex); > > + curl_slist_free_all(s->headers); > g_free(s->cookie); > g_free(s->url); > g_free(s->username); > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 5c5921bfb7..ca1ebdbef1 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -3047,12 +3047,15 @@ > # @cookie-secret: ID of a QCryptoSecret object providing the cookie data > in a > # secure way. See @cookie for the format. (since 2.10) > # > +# @header: List of HTTP request headers, see CURLOPT_HTTPHEADER(3). > +# > # Since: 2.9 > ## > { 'struct': 'BlockdevOptionsCurlHttp', > 'base': 'BlockdevOptionsCurlBase', > 'data': { '*cookie': 'str', > - '*cookie-secret': 'str'} } > + '*cookie-secret': 'str', > + '*header': [ 'str' ] } } > > ## > # @BlockdevOptionsCurlHttps: > @@ -3070,13 +3073,16 @@ > # @cookie-secret: ID of a QCryptoSecret object providing the cookie data > in a > # secure way. See @cookie for the format. (since 2.10) > # > +# @header: List of HTTP request headers, see CURLOPT_HTTPHEADER(3). > +# > # Since: 2.9 > ## > { 'struct': 'BlockdevOptionsCurlHttps', > 'base': 'BlockdevOptionsCurlBase', > 'data': { '*cookie': 'str', > '*sslverify': 'bool', > - '*cookie-secret': 'str'} } > + '*cookie-secret': 'str', > + '*header': [ 'str' ] } } > > ## > # @BlockdevOptionsCurlFtp: > -- > 2.13.2 > > >
On Thu, Mar 01, 2018 at 03:24:48PM +0000, Nir Soffer wrote: > Other issue that you need to consider is that you cannot > create images via the http. Images are created only via > engine API/UI. We've got a separate bit of Python creating the image so I think we're ok here. > What ovirt-imageio give you is a way to > read and write data to existing image. The url you get is > mostly like an open file descriptor you can use with read() > and write(). OK. In fact in my experiment with oVirt 4.2 it *did* work without the Authorization header, but we believed that was a bug in oVirt, we didn't know it was designed to do that, thanks for clarifying it. In any case I'm pretty sure we do need ca.pem (second patch). > Note that you cannot access yet a chain of image via a url, since > ovirt generate a random url per image in a chain. For example, > if you have this chain: > > base-img <- top-image (top) I think we're OK for what virt-v2v is doing. Thanks, Rich.
On Thu, Mar 01, 2018 at 01:58:55PM +0000, Richard W.M. Jones wrote: > Allow arbitrary HTTP request headers to be set, like this: > > qemu-img create -f qcow2 \ > -b 'json:{ "file.driver":"http", > "file.url":"http://192.168.0.249/scratch/test.img", > "file.header": ["Authorization: letmein"] }' \ > test.qcow2 > > which adds the ‘Authorization: letmein’ header to the outgoing request > for the backing file. Multiple headers can be set, and curl built-in > headers can be removed (using "Header:"). > > Note this uses the same format as curl's CURLOPT_HTTPHEADER, thus > pulling in curl API guarantees into qemu, but curl has had very strong > API backward compatibility since the start of the project. It doesn't appear like there's anything really curl specific about this - isn't 'Key: value' just the standard HTTP format (minus the \r\n line ending of course. IOW, I don't see any problem with using this format. > > Signed-off-by: Richard W.M. Jones <rjones@redhat.com> > --- > block/curl.c | 21 ++++++++++++++++++++- > qapi/block-core.json | 10 ++++++++-- > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 5c5921bfb7..ca1ebdbef1 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -3047,12 +3047,15 @@ > # @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a > # secure way. See @cookie for the format. (since 2.10) > # > +# @header: List of HTTP request headers, see CURLOPT_HTTPHEADER(3). Put a '(since 2.12)' anntation on end of the docs for this new field > +# > # Since: 2.9 > ## > { 'struct': 'BlockdevOptionsCurlHttp', > 'base': 'BlockdevOptionsCurlBase', > 'data': { '*cookie': 'str', > - '*cookie-secret': 'str'} } > + '*cookie-secret': 'str', > + '*header': [ 'str' ] } } > > ## > # @BlockdevOptionsCurlHttps: > @@ -3070,13 +3073,16 @@ > # @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a > # secure way. See @cookie for the format. (since 2.10) > # > +# @header: List of HTTP request headers, see CURLOPT_HTTPHEADER(3). Same here about version annotation. > +# > # Since: 2.9 > ## > { 'struct': 'BlockdevOptionsCurlHttps', > 'base': 'BlockdevOptionsCurlBase', > 'data': { '*cookie': 'str', > '*sslverify': 'bool', > - '*cookie-secret': 'str'} } > + '*cookie-secret': 'str', > + '*header': [ 'str' ] } } Regards, Daniel
On Thu, Mar 01, 2018 at 04:11:18PM +0000, Daniel P. Berrangé wrote: > On Thu, Mar 01, 2018 at 01:58:55PM +0000, Richard W.M. Jones wrote: > > Allow arbitrary HTTP request headers to be set, like this: > > > > qemu-img create -f qcow2 \ > > -b 'json:{ "file.driver":"http", > > "file.url":"http://192.168.0.249/scratch/test.img", > > "file.header": ["Authorization: letmein"] }' \ > > test.qcow2 > > > > which adds the ‘Authorization: letmein’ header to the outgoing request > > for the backing file. Multiple headers can be set, and curl built-in > > headers can be removed (using "Header:"). > > > > Note this uses the same format as curl's CURLOPT_HTTPHEADER, thus > > pulling in curl API guarantees into qemu, but curl has had very strong > > API backward compatibility since the start of the project. > > It doesn't appear like there's anything really curl specific about > this - isn't 'Key: value' just the standard HTTP format (minus the > \r\n line ending of course. IOW, I don't see any problem with using > this format. I'm going to withdraw this patch is it's not needed. However this does introduce a dependency on curl because of this from CURLOPT_HTTPHEADER(3): "If you add a header that is otherwise generated and used by libcurl internally, your added one will be used instead. If you add a header with no content as in 'Accept:' (no data on the right side of the colon), the internally used header will get disabled. With this option you can add new head‐ ers, replace internal headers and remove internal headers. To add a header with no content (nothing to the right side of the colon), use the form 'MyHeader;' (note the ending semicolon)." > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index 5c5921bfb7..ca1ebdbef1 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -3047,12 +3047,15 @@ > > # @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a > > # secure way. See @cookie for the format. (since 2.10) > > # > > +# @header: List of HTTP request headers, see CURLOPT_HTTPHEADER(3). > > Put a '(since 2.12)' anntation on end of the docs for this new field Will change the second patch accordingly, thanks. Rich.
diff --git a/block/curl.c b/block/curl.c index aa42535783..972673ba5c 100644 --- a/block/curl.c +++ b/block/curl.c @@ -83,6 +83,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle, #define CURL_BLOCK_OPT_TIMEOUT "timeout" #define CURL_BLOCK_OPT_COOKIE "cookie" #define CURL_BLOCK_OPT_COOKIE_SECRET "cookie-secret" +#define CURL_BLOCK_OPT_HEADER_PATTERN "header." #define CURL_BLOCK_OPT_USERNAME "username" #define CURL_BLOCK_OPT_PASSWORD_SECRET "password-secret" #define CURL_BLOCK_OPT_PROXY_USERNAME "proxy-username" @@ -134,6 +135,7 @@ typedef struct BDRVCURLState { bool sslverify; uint64_t timeout; char *cookie; + struct curl_slist *headers; bool accept_range; AioContext *aio_context; QemuMutex mutex; @@ -486,6 +488,9 @@ static int curl_init_state(BDRVCURLState *s, CURLState *state) if (s->cookie) { curl_easy_setopt(state->curl, CURLOPT_COOKIE, s->cookie); } + if (s->headers) { + curl_easy_setopt(state->curl, CURLOPT_HTTPHEADER, s->headers); + } curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, (long)s->timeout); curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, (void *)curl_read_cb); @@ -680,7 +685,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, double d; const char *secretid; const char *protocol_delimiter; - int ret; + int i, nr, ret; if (flags & BDRV_O_RDWR) { @@ -740,6 +745,18 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, s->cookie = g_strdup(cookie); } + s->headers = NULL; + nr = qdict_array_entries(options, CURL_BLOCK_OPT_HEADER_PATTERN); + for (i = 0; i < nr; ++i) { + char key[32]; + const char *header; + + snprintf(key, sizeof key, CURL_BLOCK_OPT_HEADER_PATTERN "%d", i); + header = qdict_get_str(options, key); + s->headers = curl_slist_append(s->headers, header); + qdict_del(options, key); + } + file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL); if (file == NULL) { error_setg(errp, "curl block driver requires an 'url' option"); @@ -847,6 +864,7 @@ out: state->curl = NULL; out_noclean: qemu_mutex_destroy(&s->mutex); + curl_slist_free_all(s->headers); g_free(s->cookie); g_free(s->url); g_free(s->username); @@ -945,6 +963,7 @@ static void curl_close(BlockDriverState *bs) curl_detach_aio_context(bs); qemu_mutex_destroy(&s->mutex); + curl_slist_free_all(s->headers); g_free(s->cookie); g_free(s->url); g_free(s->username); diff --git a/qapi/block-core.json b/qapi/block-core.json index 5c5921bfb7..ca1ebdbef1 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3047,12 +3047,15 @@ # @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a # secure way. See @cookie for the format. (since 2.10) # +# @header: List of HTTP request headers, see CURLOPT_HTTPHEADER(3). +# # Since: 2.9 ## { 'struct': 'BlockdevOptionsCurlHttp', 'base': 'BlockdevOptionsCurlBase', 'data': { '*cookie': 'str', - '*cookie-secret': 'str'} } + '*cookie-secret': 'str', + '*header': [ 'str' ] } } ## # @BlockdevOptionsCurlHttps: @@ -3070,13 +3073,16 @@ # @cookie-secret: ID of a QCryptoSecret object providing the cookie data in a # secure way. See @cookie for the format. (since 2.10) # +# @header: List of HTTP request headers, see CURLOPT_HTTPHEADER(3). +# # Since: 2.9 ## { 'struct': 'BlockdevOptionsCurlHttps', 'base': 'BlockdevOptionsCurlBase', 'data': { '*cookie': 'str', '*sslverify': 'bool', - '*cookie-secret': 'str'} } + '*cookie-secret': 'str', + '*header': [ 'str' ] } } ## # @BlockdevOptionsCurlFtp:
Allow arbitrary HTTP request headers to be set, like this: qemu-img create -f qcow2 \ -b 'json:{ "file.driver":"http", "file.url":"http://192.168.0.249/scratch/test.img", "file.header": ["Authorization: letmein"] }' \ test.qcow2 which adds the ‘Authorization: letmein’ header to the outgoing request for the backing file. Multiple headers can be set, and curl built-in headers can be removed (using "Header:"). Note this uses the same format as curl's CURLOPT_HTTPHEADER, thus pulling in curl API guarantees into qemu, but curl has had very strong API backward compatibility since the start of the project. Signed-off-by: Richard W.M. Jones <rjones@redhat.com> --- block/curl.c | 21 ++++++++++++++++++++- qapi/block-core.json | 10 ++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-)