Re: [Qemu-devel] [PATCH/RFC] qemu-img: show image conversion speed

2013-11-19 Thread Stefan Hajnoczi 
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) * 10 / (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?

[Qemu-devel] [PATCH/RFC] qemu-img: show image conversion speed

2013-11-18 Thread Varad Gautam 
From: Varad Gautam varadgau...@live.com

Calculate and display write speed when converting image with the
-p parameter. qemu-progress:qemu_progress_print() now takes speed
parameter to print.

Signed-off-by: Varad Gautam varadgau...@gmail.com
---
 include/qemu-common.h |4 ++--
 qemu-img.c|   31 +--
 util/qemu-progress.c  |   11 +++
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 5054836..0e27c68 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -349,9 +349,9 @@ size_t qemu_iovec_memset(QEMUIOVector *qiov, size_t offset,
 
 bool buffer_is_zero(const void *buf, size_t len);
 
-void qemu_progress_init(int enabled, float min_skip);
+void qemu_progress_init(int enabled, float min_skip, float speed);
 void qemu_progress_end(void);
-void qemu_progress_print(float delta, int max);
+void qemu_progress_print(float delta, int max, float speed);
 const char *qemu_get_vm_name(void);
 
 #define QEMU_FILE_TYPE_BIOS   0
diff --git a/qemu-img.c b/qemu-img.c
index bf3fb4f..cf313ed 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -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);
 
 bs1 = bdrv_new_open(filename1, fmt1, BDRV_O_FLAGS, true, quiet);
 if (!bs1) {
@@ -970,7 +970,7 @@ static int img_compare(int argc, char **argv)
 total_sectors = MIN(total_sectors1, total_sectors2);
 progress_base = MAX(total_sectors1, total_sectors2);
 
-qemu_progress_print(0, 100);
+qemu_progress_print(0, 100, 0);
 
 if (strict  total_sectors1 != total_sectors2) {
 ret = 1;
@@ -1053,7 +1053,7 @@ static int img_compare(int argc, char **argv)
 }
 }
 sector_num += nb_sectors;
-qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+qemu_progress_print(((float) nb_sectors / progress_base)*100, 100, 0);
 }
 
 if (total_sectors1 != total_sectors2) {
@@ -1101,7 +1101,7 @@ static int img_compare(int argc, char **argv)
 }
 }
 sector_num += nb_sectors;
-qemu_progress_print(((float) nb_sectors / progress_base)*100, 100);
+qemu_progress_print(((float) nb_sectors / progress_base)*100, 100, 
0);
 }
 }
 
@@ -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);
 }
 /* signal EOF to align */
 bdrv_write_compressed(out_bs, 0, NULL, 0);
@@ -1475,6 +1475,7 @@ static int img_convert(int argc, char **argv)
 }
 
 for(;;) {
+time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 nb_sectors = total_sectors - sector_num;
 if (nb_sectors = 0) {
 break;
@@ -1547,7 +1548,9 @@ static int img_convert(int argc, char **argv)
 n -= n1;
 buf1 += n1 * 512;
 }
-qemu_progress_print(local_progress, 100);
+time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - time;
+write_speed = (sectors_to_bytes(n1) * 10 / (double) time ) 
/ 1048576 ;
+qemu_progress_print(local_progress, 100, write_speed);
 }
 }
 out:
@@ -2174,8 +2177,8 @@ static int img_rebase(int