Re: [Qemu-devel] [PATCH] xbzrle: don't check the value in the vm ram repeatedly

[陈梁]

> On 03/29/2014 01:52 AM, address@hidden wrote:
>> From: ChenLiang <address@hidden>
>> 
>> xbzrle_encode_buffer checks the value in the ram repeatedly.
>> It is risk if runs xbzrle_encode_buffer on changing data.
>> And it is not necessary.
>> 
>> Reported-by: Dr. David Alan Gilbert <address@hidden>
>> Signed-off-by: ChenLiang <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>> ---
>> xbzrle.c | 4 ++++
>> 1 file changed, 4 insertions(+)
> 
> Insufficient.  Observe what we did at line 52 when looking for the zero-run:
> 
>>>        /* word at a time for speed */
>>>        if (!res) {
>>>            while (i < slen &&
>>>                   (*(long *)(old_buf + i)) == (*(long *)(new_buf + i))) {
> 
> This dereferences 8 bytes at new_buf[i] (on a 64-bit machine)...
> 
>>>                i += sizeof(long);
>>>                zrun_len += sizeof(long);
>>>            }
>>> 
>>>            /* go over the rest */
>>>            while (i < slen && old_buf[i] == new_buf[i]) {
> 
> ...and this also dereferences those same bytes.  But your argument is
> that new_buf[i] is volatile.
> 
> You MUST read new_buf[i] into a temporary variable, and if the first
> read found a difference, then the "go over the rest" analysis must be on
> that temporary, rather than going back to reading directly from new_buf.
> 
It is ok, we just need to guarantee that the pages in cache are same to the 
page in dest side. 
Don‘t care about whether they are same to src side. Because the modified pages 
during this
time will be sent at next time.
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>