On Mon, Nov 18, 2013 at 07:25:09PM +0530, Varad Gautam wrote: > Calculate and display write speed when converting image with the > -p parameter. qemu-progress:qemu_progress_print() now takes speed > parameter to print.
How well does this approach work in your testing? Calculating a new speed for every I/O request could lead to very volatile output. If the value jumps around too much it becomes unusable; "moving averages" solve this problem. Adding speed with hardcoded 'kB/s' units in qemu-progress.c is unfortunate. qemu-progress.c does not know about units. Perhaps the speed units should be passed in when providing speed values. > @@ -945,7 +945,7 @@ static int img_compare(int argc, char **argv) > filename2 = argv[optind++]; > > /* Initialize before goto out */ > - qemu_progress_init(progress, 2.0); > + qemu_progress_init(progress, 2.0, 0); Perhaps compare should report read speed. > @@ -1127,7 +1127,7 @@ static int img_convert(int argc, char **argv) > const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename; > BlockDriver *drv, *proto_drv; > BlockDriverState **bs = NULL, *out_bs = NULL; > - int64_t total_sectors, nb_sectors, sector_num, bs_offset; > + int64_t total_sectors, nb_sectors, sector_num, bs_offset, time; > uint64_t bs_sectors; > uint8_t * buf = NULL; > const uint8_t *buf1; > @@ -1136,7 +1136,7 @@ static int img_convert(int argc, char **argv) > QEMUOptionParameter *out_baseimg_param; > char *options = NULL; > const char *snapshot_name = NULL; > - float local_progress = 0; > + float local_progress = 0, write_speed = 0; > int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */ > bool quiet = false; > Error *local_err = NULL; > @@ -1223,7 +1223,7 @@ static int img_convert(int argc, char **argv) > out_filename = argv[argc - 1]; > > /* Initialize before goto out */ > - qemu_progress_init(progress, 2.0); > + qemu_progress_init(progress, 2.0, write_speed); > > if (options && is_help_option(options)) { > ret = print_block_option_help(out_filename, out_fmt); > @@ -1237,7 +1237,7 @@ static int img_convert(int argc, char **argv) > goto out; > } > > - qemu_progress_print(0, 100); > + qemu_progress_print(0, 100, write_speed); > > bs = g_malloc0(bs_n * sizeof(BlockDriverState *)); > > @@ -1460,7 +1460,7 @@ static int img_convert(int argc, char **argv) > } > } > sector_num += n; > - qemu_progress_print(local_progress, 100); > + qemu_progress_print(local_progress, 100, write_speed); > } Write speed is not calculated for compressed writes. > - qemu_progress_print(local_progress, 100); > + time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - time; > + write_speed = (sectors_to_bytes(n1) * 1000000000 / (double) time > ) / 1048576 ; Please use constants for these magic numbers (e.g. NANOSECONDS_PER_SECOND and MEGABYTE). By the way, shouldn't the speed be in KB/s? > @@ -2174,8 +2177,8 @@ static int img_rebase(int argc, char **argv) > } > filename = argv[optind++]; > > - qemu_progress_init(progress, 2.0); > - qemu_progress_print(0, 100); > + qemu_progress_init(progress, 2.0, 0); > + qemu_progress_print(0, 100, 0); Perhaps we should provide the write speed?