Re: [Qemu-devel] [RFC PATCH 2/4] block: add live block commit functionality

[Jeff Cody]

On 08/01/2012 03:07 AM, Paolo Bonzini wrote:
> Il 01/08/2012 08:32, Kevin Wolf ha scritto:
>>>>>>>> +enum {
>>>>>>>> +    /*
>>>>>>>> +     * Size of data buffer for populating the image file.  This 
>>>>>>>> should be large
>>>>>>>> +     * enough to process multiple clusters in a single call, so that 
>>>>>>>> populating
>>>>>>>> +     * contiguous regions of the image is efficient.
>>>>>>>> +     */
>>>>>>>> +    COMMIT_BUFFER_SIZE = 512 * 1024, /* in bytes */
>>>>>>>> +};
>>>>>>
>>>>>> Paolo's latest round of patches got to the point of making this
>>>>>> configurable for drive-mirror; is that something you should be copying 
>>>>>> here?
>>>>
>>>> Yes
>> Though its use is very limited for live commit. For the mirror it's
>> important because a larger number can mean that more data is
>> unnecessarily written, and the target can become larger than the source.
> 
> Note that the latest version of mirroring has _two_ knobs:
> 
> 1) granularity is what decides how much data could be written
> unnecessarily, because of the dirty bitmap.
> 
> 2) buffer size is what decides how much I/O is in flight at one time.
> 
> The default values are resp. the cluster size (or 64K for raw) and 10M.
>  The two together give mirroring some self-tuning capability.  For
> example in the first part of the mirroring you will likely proceed in
> 10M chunks with no concurrency; once you're synchronized, you'll
> probably send several chunks, perhaps all 64K if the guest does random
> writes.
> 
> Live commit as it is done now doesn't need any of this complication; it
> is just a background operation that does not need to compete with the
> guest.  So using a larger buffer is indeed always better, and 512K is a
> nice intermediate value between mirroring's 64K and 10M extremes.
> 

We may want these same adjust knobs for the 'second phase' of live
commit, however.  It will commit down the top (active) layer, which will
have similar properties to mirroring, since the guest can still be
writing to the active layer.  If we do need it for the second phase,
perhaps those controls will be useful now.


>> For live commit, I think using a larger buffer is always better.
>>
>> Hm, part of the difference is that I assume that commit uses
>> bdrv_is_allocated() to check whether some data must really be copied.
>> But then, there's no reason why mirroring couldn't do that as well. Paolo?
> 
> We copy a cluster at a time, and that's also the resolution of
> bdrv_is_allocated so we wouldn't gain anything.  Nice idea though, I had
> to mull about it to find the flaw. :)
> 
> Paolo
>