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

[David Edmondson <dme@dme.org>]

On Monday, 2020-08-24 at 15:19:24 +02, Markus Armbruster wrote:

> 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".

Indeed.

>> 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)"?

Yes, thanks for catching it.

> Please add "(since 5.2)".

Will do.

>> +#
>>  # Since: 2.9
>>  ##
>>  { 'struct': 'BlockdevOptionsCurlBase',
>>    'data': { 'url': 'str',
>> +            '*blocksize': 'int',
>
> Should we use 'size' rather than 'int'?

Yes.

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

dme.
-- 
But are you safe Miss Gradenko?