Re: [Qemu-devel] [PATCH 4/5] ram: Use MigrationStats for statistics

[Peter Xu <peterx@redhat.com>]

On Tue, Jun 06, 2017 at 07:33:45PM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > On Mon, Jun 05, 2017 at 01:34:45PM +0100, Dr. David Alan Gilbert wrote:
> >> * Juan Quintela (quintela@redhat.com) wrote:
> >> > RAM Statistics need to survive migration to make info migrate work, so we
> >> > need to store them outside of RAMState.  As we already have an struct
> >> > with those fields, just used them. (MigrationStats and XBZRLECacheStats).
> >> > 
> >> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> 
> >> Hmm OK; this feels very much like it's the opposite of 180f61f from
> >> March; these variables keep moving around over the last couple of months
> >> - are they going to stay still now?
> >
> > O:-)
> >
> > Meanwhile, I don't know whether it'll be necessary to remove all the
> > functions like ram_bytes_transferred(), e.g., it would be just:
> >
> > uint64_t ram_bytes_transferred(void)
> > {
> > -    return ram_state.bytes_transferred;
> > +    return ram_counters.transferred;
> > }
> >
> > But I'm okay with either.
> 
> That value was only used for filling the statistics.  And we are filling
> a struct from another struct of the exact same type.  Going through an
> exported function looks stranger.
> 
> And as said in $commit, the idea was that creating a new counter was
> easy, right now you have to:
> 
> - add it to MigrationParam (still have to do this)
> - add it to MigrationParams (still have to do this)
> - create the field in MigrationStats or RAMState
> - create a function that exports it
> - add that function in ram.h to export it
> - add it on qmp_query (still have to do this)
> 
> So, we are moving from 6 steps to 3 steps.  I think we are much better
> now, no? O:-)

Hmm, okay!

(as long as we won't move these functions back one day :-)

-- 
Peter Xu