Re: [RFC PATCH 5/9] block/curl: Allow the blocksize to be specified by the user

[Markus Armbruster <armbru@redhat.com>]

David Edmondson <david.edmondson@oracle.com> writes:

> Rather than a fixed 256kB blocksize, allow the user to specify the
> size used. It must be a non-zero power of two, defaulting to 256kB.

Nitpick: any power of two is non-zero.  Scratch "non-zero".

> Signed-off-by: David Edmondson <david.edmondson@oracle.com>
> ---
[...]
> diff --git a/docs/system/device-url-syntax.rst.inc b/docs/system/device-url-syntax.rst.inc
> index bc38b9df38..ee504ee41a 100644
> --- a/docs/system/device-url-syntax.rst.inc
> +++ b/docs/system/device-url-syntax.rst.inc
> @@ -194,6 +194,13 @@ These are specified using a special URL syntax.
>        Add an offset, in bytes, to all range requests sent to the
>        remote server.
>  
> +   ``blocksize``
> +      The size of all IO requests sent to the remote server. This
> +      value may optionally have the suffix 'T', 'G', 'M', 'K', 'k' or
> +      'b'. If it does not have a suffix, it will be assumed to be in
> +      bytes. The value must be a non-zero power of two.  It defaults
> +      to 256kB.
> +
>     Note that when passing options to qemu explicitly, ``driver`` is the
>     value of <protocol>.
>  
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index d6f5e91cc3..cd16197e1e 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3764,10 +3764,14 @@
>  # @proxy-password-secret: ID of a QCryptoSecret object providing a password
>  #                         for proxy authentication (defaults to no password)
>  #
> +# @blocksize: Size of all IO requests sent to the remote server; must
> +#             be a non-zero power of two (defaults to 1 256kB)

Scratch "non-zero".

"(defaults to 1 256kB)" confuses me.  Do you mean "(defaults to 256kB)"?

Please add "(since 5.2)".

> +#
>  # Since: 2.9
>  ##
>  { 'struct': 'BlockdevOptionsCurlBase',
>    'data': { 'url': 'str',
> +            '*blocksize': 'int',

Should we use 'size' rather than 'int'?

>              '*timeout': 'int',
>              '*username': 'str',
>              '*password-secret': 'str',