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

[Wei Wang]

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 <address@hidden>
> > Signed-off-by: Wei Wang <address@hidden>
> > ---
> >   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