On Wed, Jan 03, 2018 at 11:08:49AM +0100, Juan Quintela wrote: > Peter Xu <pet...@redhat.com> wrote: > > We have quite a few lines in migration_thread() that calculates some > > statistics for the migration interations. Isolate it into a single > > function to improve readability. > > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > > > > +static void migration_update_statistics(MigrationState *s, > > > migration_update_counters()?
Sure, or... > > statistics for me mean that they are only used for informative > purposes. Here we *act* on that values. > > > > > > - qemu_file_reset_rate_limit(s->to_dst_file); > > - initial_time = current_time; > > - initial_bytes = qemu_ftell(s->to_dst_file); > > - } > > + /* Conditionally update statistics */ > > No need for the comment. If we think it is needed just rename the > function to: > conditionally_update_statistics()? > > I still preffer the: > migration_update_counters. ... migration_update_counters_conditionally()? > > > > diff --git a/migration/migration.h b/migration/migration.h > > index 3ab5506233..248f7d9a5c 100644 > > --- a/migration/migration.h > > +++ b/migration/migration.h > > @@ -90,6 +90,19 @@ struct MigrationState > > QEMUBH *cleanup_bh; > > QEMUFile *to_dst_file; > > > > + /* > > + * Migration thread statistic variables, mostly used in > > + * migration_thread() iterations only. > > + */ > > + uint64_t initial_bytes; > > /* bytes already send at the beggining of current interation */ > uint64_t iteration_initial_bytes; > > > + int64_t initial_time; > /* time at the start of current iteration */ > int64_t iteration_start_time; > > What do you think? Will change both. Thanks, -- Peter Xu
