Re: [PATCH v1 1/2] migration/xbzrle: replace transferred xbzrle bytes with encoded bytes

[Wei Wang <wei.w.wang@intel.com>]

On 04/20/2020 05:29 PM, Daniel P. Berrangé wrote:
> On Mon, Apr 20, 2020 at 11:06:42AM +0800, Wei Wang wrote:
>> Like compressed_size which indicates how many bytes are compressed, we
>> need encoded_size to understand how many bytes are encoded with xbzrle
>> during migration.
>>
>> Replace the old xbzrle_counter.bytes, instead of adding a new counter,
>> because we don't find a usage of xbzrle_counter.bytes currently, which
>> includes 3 more bytes of the migration transfer protocol header (in
>> addition to the encoding header). The encoded_size will further be used
>> to calculate the encoding rate.
>>
>> Signed-off-by: Yi Sun <yi.y.sun@intel.com>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> ---
>>   migration/migration.c |  2 +-
>>   migration/ram.c       | 18 +++++++++---------
>>   monitor/hmp-cmds.c    |  4 ++--
>>   qapi/migration.json   |  6 +++---
>>   4 files changed, 15 insertions(+), 15 deletions(-)
>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index eca2981d0a..bf195ff6ac 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -62,7 +62,7 @@
>>   #
>>   # @cache-size: XBZRLE cache size
>>   #
>> -# @bytes: amount of bytes already transferred to the target VM
>> +# @encoded_size: amount of bytes encoded
> Woah, this is part of QEMU's public API, so it isn't permissible to just
> arbitrarily remove a field with no warning, and replace it with a new
> field reporting different data. Adding a new field is allowed, but any
> existing field should be deprecated first, if there is a genuine need
> to remove it. If it isn't costly though, just leave the existing field
> unchanged.
>
> I would also note that the other fields in this struct use a hyphen, not
> an underscore.
OK. Thanks for reviewing and pointing it out.
We can add it as a new filed using hyphen in this case.
Will wait for other comments to post out a new version.

Best,
Wei