Re: [Qemu-devel] [PATCH v11 7/9] Add XBZRLE to ram_save_block and ram_save_live

[Juan Quintela]

Orit Wasserman <address@hidden> wrote:
> On 06/01/2012 02:42 PM, Juan Quintela wrote:
>> We are still not using this value, and we are sending it anyway (with a
>> value of zero).  What happens when we start using if for a checksum, and
>> we migration to a new version that "expects" it to be valid?  I would
>> preffer not to sent it, or sent the correct value.
> I think I will remove it, checksum should be used for all migration
> not just XBZRLE.
> I guess we can add it to the protocol in the future.

Agreed.
>> On the other hand ... Why are we doing the stage == 1 test?  stage 1
>> normally only sent part of the pages, so we could use the generic code
>> there?  It would just return -1 as bytes_sent, and do the same code that
>> we have now?
>
> we need to add the pages to the cache in stage 1 (for the next stage),
> and there is no need for checking if the page is cached.
> and send the pages from the cache for consistency

My question is: If we remove the check and just call the other function,
everything works, no?  So , why add the special case?

If it dont' work, we need to change it, because nothing warantees that
we fill the cache during stage .

>>>              void *host;
>>>  
>>>              host = host_from_stream_offset(f, addr, flags);
>>> +            if (!host) {
>>> +                return -EINVAL;
>>> +            }
>> 
>> Why is this check only needed now?
> I wish I knew, looks like it is missing in upstream.
> Do you think I should fix it separately ?

Yeap.

Thanks, Juan.