On 06/06/2013 03:26 PM, Peter Lieven wrote:
>>> @@ -514,8 +512,8 @@
>>> ##
>>> { 'type': 'MigrationStats',
>>> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
>>> - 'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
>>> - 'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
>>> + 'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
>>> + 'dirty-pages-rate' : 'int' } }
>> This hunk is questionable. Removing something that we have previously
>> sent
>> over the wire may break clients that are expecting this field to exist.
>> Rather than reverting the entire patch, you should consider keeping this
>> field present in QMP, even if you now always populate it with 0.
>>
> You are right. I might still account zero pages in the bulk phase to
> give this
> field a meaning. These pages are very likely not written on the
> destination except
> for the cornercases. What do you think?I don't care if you document a slightly different meaning for the statistic; as long as it is documented. Where I do care is removing something that previous versions provided; I think your later versions of this series adequately took care of this by reverting only a subset of the original patch. I'm okay with the current qemu.git behavior that always displays returns "skipped":0, or if you post a later patch that again makes it print a non-zero number that you find useful. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
