Re: [Qemu-devel] [PATCH 3/4] ram: Print block stats also in the complete case

[Peter Xu <peterx@redhat.com>]

On Fri, Jun 02, 2017 at 06:55:30PM +0100, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
> > To make things easier, I just moved it to populate_ram_info().
> 
> Blk is not RAM; better to create a separate function.

Agree...

Maybe rename populate_ram_info() into more general name?
migrate_info_populate()?

(Also, I think auto convergence is not RAM-related as well, so same
 comment may apply to patch 2)

Thanks,

> 
> Dave
> 
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > ---
> >  migration/migration.c | 26 ++++++++------------------
> >  1 file changed, 8 insertions(+), 18 deletions(-)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 7f79da0..8289544 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -509,6 +509,14 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
> >          info->cpu_throttle_percentage = cpu_throttle_get_percentage();
> >      }
> >  
> > +    if (blk_mig_active()) {
> > +        info->has_disk = true;
> > +        info->disk = g_malloc0(sizeof(*info->disk));
> > +        info->disk->transferred = blk_mig_bytes_transferred();
> > +        info->disk->remaining = blk_mig_bytes_remaining();
> > +        info->disk->total = blk_mig_bytes_total();
> > +    }
> > +
> >      if (s->state != MIGRATION_STATUS_COMPLETED) {
> >          info->ram->remaining = ram_bytes_remaining();
> >          info->ram->dirty_pages_rate = ram_dirty_pages_rate();
> > @@ -540,15 +548,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >          info->setup_time = s->setup_time;
> >  
> >          populate_ram_info(info, s);
> > -
> > -        if (blk_mig_active()) {
> > -            info->has_disk = true;
> > -            info->disk = g_malloc0(sizeof(*info->disk));
> > -            info->disk->transferred = blk_mig_bytes_transferred();
> > -            info->disk->remaining = blk_mig_bytes_remaining();
> > -            info->disk->total = blk_mig_bytes_total();
> > -        }
> > -
> >          break;
> >      case MIGRATION_STATUS_POSTCOPY_ACTIVE:
> >          /* Mostly the same as active; TODO add some postcopy stats */
> > @@ -562,15 +561,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >          info->setup_time = s->setup_time;
> >  
> >          populate_ram_info(info, s);
> > -
> > -        if (blk_mig_active()) {
> > -            info->has_disk = true;
> > -            info->disk = g_malloc0(sizeof(*info->disk));
> > -            info->disk->transferred = blk_mig_bytes_transferred();
> > -            info->disk->remaining = blk_mig_bytes_remaining();
> > -            info->disk->total = blk_mig_bytes_total();
> > -        }
> > -
> >          break;
> >      case MIGRATION_STATUS_COLO:
> >          info->has_status = true;
> > -- 
> > 2.9.4
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- 
Peter Xu