Antoine Damhet <[email protected]> writes: > On Tue, Feb 17, 2026 at 09:53:11AM +0100, Markus Armbruster wrote: >> Antoine Damhet <[email protected]> writes: >> >> > S3 presigned URLs are signed for a specific HTTP method (typically GET >> > for our use cases). The curl block driver currently issues a HEAD >> > request to discover the backend features and the file size, which fails >> > with 403. >> > >> > Add a 'force-range' option that skips the HEAD request and instead >> > issues a minimal GET request (querying 1 byte from the server) to >> > extract the file size from the 'Content-Range' response header. To >> > achieve this the 'curl_header_cb' is redesigned to generically parse >> > HTTP headers. >> > >> > $ $QEMU -drive driver=http,\ >> > >> > 'url=https://s3.example.com/some.img?X-Amz-Security-Token=XXX', >> > force-range=true >> > >> > Enabling the 'force-range' option without the backend supporting it is >> > undefined behavior and untested >> >> "Undefined behavior" suggests it could do anything, even destroy data. >> I hope that's not the case. What is the case? >> >> What is "the backend"? The web server specified with @url? > > Undefined behavior was probably too strong of a wording. I have done way > more tests and have a clearer picture of what happens: > > The web server for @url will respond with `HTTP 200` and try to send the > whole file. Since we specified `CURLOPT_NOBODY` to the libcurl it stops > reading the socket after the headers and justs shuts it down. The > `force-range` mode is transparent for the user even if it can wastes a > few TCP packets. > > I'll rewrite the commit message to reflect the actual behavior of the > option in the v2.
Thanks! >> > but the libcurl should ignore the body >> > and stop reading after the HTTP headers then we would fail with the >> > expected `Server does not support 'range' (byte ranges).` error. >> > >> > Signed-off-by: Antoine Damhet <[email protected]> >> > --- >> >> [...] >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json >> > index b82af7425614..ff018c2d6bfb 100644 >> > --- a/qapi/block-core.json >> > +++ b/qapi/block-core.json >> > @@ -4582,12 +4582,17 @@ >> > # @cookie-secret: ID of a QCryptoSecret object providing the cookie >> > # data in a secure way. See @cookie for the format. (since 2.10) >> > # >> > +# @force-range: Don't issue a HEAD HTTP request to discover if the >> > +# backend supports range requests and rely only on GET requests. >> > +# This is especially useful for S3 presigned URLs. (since 11.0) Missing: Defaults to false. >> Unlike the commit message, this doesn't mention the need for "the >> backend" (whatever that may be) supporting it. > > Will rephrase "the backend" with "the http server". Should I document > the behavior of the http server missing the range requests here or is > the current description sufficient ? What do users need to know here? I think it's when and why to use @force-range. Drawbacks of using it if there are any. >> > +# >> > # Since: 2.9 >> > ## >> > { 'struct': 'BlockdevOptionsCurlHttp', >> > 'base': 'BlockdevOptionsCurlBase', >> > 'data': { '*cookie': 'str', >> > - '*cookie-secret': 'str'} } >> > + '*cookie-secret': 'str', >> > + '*force-range': 'bool'} } >> > >> > ## >> > # @BlockdevOptionsCurlHttps: >> > @@ -4605,13 +4610,18 @@ >> > # @cookie-secret: ID of a QCryptoSecret object providing the cookie >> > # data in a secure way. See @cookie for the format. (since 2.10) >> > # >> > +# @force-range: Don't issue a HEAD HTTP request to discover if the >> > +# backend supports range requests and rely only on GET requests. >> > +# This is especially useful for S3 presigned URLs. (since 11.0) >> > +# >> > # Since: 2.9 >> > ## >> >> @force-range is is duplicated between BlockdevOptionsCurlHttp and >> BlockdevOptionsCurlHttps. @cookie and @cookie-secret is already >> duplicated before the patch. Time to factor out a common base type? > > This would be only on the QAPI ? looking something like: > > ``` > { 'struct': 'BlockdevOptionsCurlHttps', > - 'base': 'BlockdevOptionsCurlBase', > - 'data': { '*cookie': 'str', > - '*sslverify': 'bool', > - '*cookie-secret': 'str', > - '*force-range': 'bool'} } > + 'base': 'BlockdevOptionsCurlHttp', > + 'data': { '*sslverify': 'bool' } } > ``` Looks good to me. > ? Would you rather see this in a separate commit or is the same patch OK > ? I'd prefer a separate commit.
