Re: [Qemu-devel] [PATCH v10 1/9] qcow2: Options' documentation fixes

[Leonid Bloch]

On 9/24/18 6:04 PM, Alberto Garcia wrote:
> On Fri 21 Sep 2018 07:23:02 PM CEST, Leonid Bloch wrote:
>> Signed-off-by: Leonid Bloch <address@hidden>
>> ---
>>   docs/qcow2-cache.txt | 20 +++++++++++++-------
>>   qemu-options.hx      |  9 ++++++---
>>   2 files changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
>> index 8a09a5cc5f..013991e21c 100644
>> --- a/docs/qcow2-cache.txt
>> +++ b/docs/qcow2-cache.txt
>> @@ -77,7 +77,7 @@ aforementioned L2 cache, and its size can also be 
>> configured.
>>   Choosing the right cache sizes
>>   ------------------------------
>>   In order to choose the cache sizes we need to know how they relate to
>> -the amount of allocated space.
>> +the amount of the allocated space.
> 
> I'm not a native English speaker, but the current version sounds correct
> to me. Why do you need to add an article there? :-?

I'm not a native speaker as well, but "the" seems to be needed there:
- Which allocated space?
- __The__ allocated space!

But I really do not insist on this one - it's clear enough. :)

> 
>> +For example, 1MB of L2 cache is needed to cover every 8 GB of the virtual
>> +image size (given that the default cluster size is used):
>>   
>> -   1048576 / 131072 = 8 GB of virtual disk covered by that cache
>> -    262144 /  32768 = 8 GB
>> +   8 GB / 8192 = 1 MB
>> +
>> +A default refcount cache is 4 times the cluster size, which defaults to
>> +256 KB (262144 bytes). This is sufficient for 8 GB of image size:
> 
> "A default ... which defaults to" sounds a bit strange I think.

Indeed. I'll reword. Thanks.

Leonid.

> 
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -742,15 +742,18 @@ image file)
>>   
>>   @item cache-size
>>   The maximum total size of the L2 table and refcount block caches in bytes
>> -(default: 1048576 bytes or 8 clusters, whichever is larger)
>> +(default: the sum of l2-cache-size and refcount-cache-size)
> 
> Oh, so this had been wrong all the time? Good that you saw it!
> 
> Berto
>