On Tue, Mar 29, 2011 at 9:51 AM, <jes.soren...@redhat.com> wrote: > diff --git a/qemu-common.h b/qemu-common.h > index 7a96dd1..a3a4dde 100644 > --- a/qemu-common.h > +++ b/qemu-common.h > @@ -330,6 +330,11 @@ void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t > count); > void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count, > size_t skip); > > +void qemu_init_progress(int enabled, float min_skip);
Please call it qemu_progress_init() to be consistent with the other qemu_progress_*() functions. > @@ -642,6 +648,9 @@ static int img_convert(int argc, char **argv) > goto out; > } > > + qemu_init_progress(progress, 2.0); > + qemu_progress_print(0, 100); > + > bs = qemu_mallocz(bs_n * sizeof(BlockDriverState *)); > > total_sectors = 0; > @@ -773,6 +782,11 @@ static int img_convert(int argc, char **argv) > } > cluster_sectors = cluster_size >> 9; > sector_num = 0; > + > + nb_sectors = total_sectors - sector_num; sector_num is always 0 here, why subtract it? > + local_progress = (float)100 / > + (nb_sectors / MIN(nb_sectors, (cluster_sectors))); This seems to calculate the percentage increment for each iteration. This increment value is wrong for the final iteration unless nb_sectors is a multiple of cluster_sectors, so we'll overshoot 100%. It would be nice if the caller didn't have to calculate progress themselves. How about: void qemu_progress_begin(bool enabled, uint64_t max); void qemu_progress_end(void); void qemu_progress_add(uint64_t increment); You set the maximum absolute value and then tell it how much progress has been made each iteration: qemu_progress_begin(true, total_sectors); for (...) { nbytes = ...; qemu_progress_add(nbytes); } qemu_progress_end(); > +void qemu_init_progress(int enabled, float min_skip) > +{ > + state.enabled = enabled; > + if (!enabled) { > + return; > + } This early return is not needed. > + state.min_skip = min_skip; > +} > + > +void qemu_progress_end(void) > +{ > + progress_simple_end(); > +} > + > +void qemu_progress_print(float percent, int max) > +{ > + float current; > + > + if (max == 0) { > + current = percent; > + } else { > + current = state.current + percent / 100 * max; > + } > + state.current = current; > + > + if (current > (state.last_print + state.min_skip) || > + (current == 100) || (current == 0)) { Comparing against (float)100 may not match due to floating point representation. > + state.last_print = state.current; > + progress_simple_print(); > + } > +} > + > +int qemu_progress_get_current(void) > +{ > + return state.current; > +} This function is unused. Stefan