Patchwork [v2] qemu-img: Initial progress printing support

login
register
mail settings
Submitter Jes Sorensen
Date March 30, 2011, 12:16 p.m.
Message ID <1301487385-1755-1-git-send-email-Jes.Sorensen@redhat.com>
Download mbox | patch
Permalink /patch/88914/
State New
Headers show

Comments

Jes Sorensen - March 30, 2011, 12:16 p.m.
From: Jes Sorensen <Jes.Sorensen@redhat.com>

This adds the basic infrastructure for supporting progress output
on the command line, as well as progress support for qemu-img commands
'rebase' and 'convert'.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Makefile.objs    |    2 +-
 qemu-common.h    |    4 ++
 qemu-img-cmds.hx |    4 +-
 qemu-img.c       |   38 ++++++++++++++++++++++-
 qemu-progress.c  |   89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 132 insertions(+), 5 deletions(-)
 create mode 100644 qemu-progress.c
Stefan Hajnoczi - March 30, 2011, 12:27 p.m.
On Wed, Mar 30, 2011 at 1:16 PM,  <Jes.Sorensen@redhat.com> wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> This adds the basic infrastructure for supporting progress output
> on the command line, as well as progress support for qemu-img commands
> 'rebase' and 'convert'.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  Makefile.objs    |    2 +-
>  qemu-common.h    |    4 ++
>  qemu-img-cmds.hx |    4 +-
>  qemu-img.c       |   38 ++++++++++++++++++++++-
>  qemu-progress.c  |   89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 132 insertions(+), 5 deletions(-)
>  create mode 100644 qemu-progress.c

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Kevin Wolf - March 31, 2011, 10:38 a.m.
Am 30.03.2011 14:16, schrieb Jes.Sorensen@redhat.com:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> This adds the basic infrastructure for supporting progress output
> on the command line, as well as progress support for qemu-img commands
> 'rebase' and 'convert'.
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>

Thanks, applied to the block branch.

I think we should consider turning the progress output on by default if
qemu-img is run from a terminal.

Kevin
Jes Sorensen - March 31, 2011, 11:15 a.m.
On 03/31/11 12:38, Kevin Wolf wrote:
> Am 30.03.2011 14:16, schrieb Jes.Sorensen@redhat.com:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> This adds the basic infrastructure for supporting progress output
>> on the command line, as well as progress support for qemu-img commands
>> 'rebase' and 'convert'.
>>
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Thanks, applied to the block branch.
> 
> I think we should consider turning the progress output on by default if
> qemu-img is run from a terminal.

Thanks!

I have been a little reluctant to do this because it will break the ABI
for tools running qemu-img from a GUI etc.

Cheers,
Jes
Kevin Wolf - March 31, 2011, 11:38 a.m.
Am 31.03.2011 13:15, schrieb Jes Sorensen:
> On 03/31/11 12:38, Kevin Wolf wrote:
>> Am 30.03.2011 14:16, schrieb Jes.Sorensen@redhat.com:
>>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>
>>> This adds the basic infrastructure for supporting progress output
>>> on the command line, as well as progress support for qemu-img commands
>>> 'rebase' and 'convert'.
>>>
>>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> Thanks, applied to the block branch.
>>
>> I think we should consider turning the progress output on by default if
>> qemu-img is run from a terminal.
> 
> Thanks!
> 
> I have been a little reluctant to do this because it will break the ABI
> for tools running qemu-img from a GUI etc.

That's the reason for the "from a terminal" part. If we check for
isatty(), we should handle these cases just fine.

Kevin
Stefan Hajnoczi - March 31, 2011, 11:49 a.m.
On Thu, Mar 31, 2011 at 12:38 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 31.03.2011 13:15, schrieb Jes Sorensen:
>> On 03/31/11 12:38, Kevin Wolf wrote:
>>> Am 30.03.2011 14:16, schrieb Jes.Sorensen@redhat.com:
>>>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>>
>>>> This adds the basic infrastructure for supporting progress output
>>>> on the command line, as well as progress support for qemu-img commands
>>>> 'rebase' and 'convert'.
>>>>
>>>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>
>>> Thanks, applied to the block branch.
>>>
>>> I think we should consider turning the progress output on by default if
>>> qemu-img is run from a terminal.
>>
>> Thanks!
>>
>> I have been a little reluctant to do this because it will break the ABI
>> for tools running qemu-img from a GUI etc.
>
> That's the reason for the "from a terminal" part. If we check for
> isatty(), we should handle these cases just fine.

Yes, I think checking for a tty is enough precaution and allows users
to get the benefit of the progress bar.  TBH I'd probably forget to
add -p half the time :).

Stefan
Jes Sorensen - April 1, 2011, 1:41 p.m.
On 03/31/11 13:49, Stefan Hajnoczi wrote:
> On Thu, Mar 31, 2011 at 12:38 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 31.03.2011 13:15, schrieb Jes Sorensen:
>>> On 03/31/11 12:38, Kevin Wolf wrote:
>>> I have been a little reluctant to do this because it will break the ABI
>>> for tools running qemu-img from a GUI etc.
>>
>> That's the reason for the "from a terminal" part. If we check for
>> isatty(), we should handle these cases just fine.
> 
> Yes, I think checking for a tty is enough precaution and allows users
> to get the benefit of the progress bar.  TBH I'd probably forget to
> add -p half the time :).

Ok, this is fine with me - however how do you suggest we offer the
option to disable it on the command line, an additional flag?

Jes
Stefan Hajnoczi - April 1, 2011, 2:58 p.m.
On Fri, Apr 1, 2011 at 2:41 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> On 03/31/11 13:49, Stefan Hajnoczi wrote:
>> On Thu, Mar 31, 2011 at 12:38 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> Am 31.03.2011 13:15, schrieb Jes Sorensen:
>>>> On 03/31/11 12:38, Kevin Wolf wrote:
>>>> I have been a little reluctant to do this because it will break the ABI
>>>> for tools running qemu-img from a GUI etc.
>>>
>>> That's the reason for the "from a terminal" part. If we check for
>>> isatty(), we should handle these cases just fine.
>>
>> Yes, I think checking for a tty is enough precaution and allows users
>> to get the benefit of the progress bar.  TBH I'd probably forget to
>> add -p half the time :).
>
> Ok, this is fine with me - however how do you suggest we offer the
> option to disable it on the command line, an additional flag?

That would be safest, good idea.

Stefan
Paolo Bonzini - April 22, 2011, 12:37 p.m.
On 04/01/2011 04:58 PM, Stefan Hajnoczi wrote:
> On Fri, Apr 1, 2011 at 2:41 PM, Jes Sorensen<Jes.Sorensen@redhat.com>  wrote:
>> On 03/31/11 13:49, Stefan Hajnoczi wrote:
>>> On Thu, Mar 31, 2011 at 12:38 PM, Kevin Wolf<kwolf@redhat.com>  wrote:
>>>> Am 31.03.2011 13:15, schrieb Jes Sorensen:
>>>>> On 03/31/11 12:38, Kevin Wolf wrote:
>>>>> I have been a little reluctant to do this because it will break the ABI
>>>>> for tools running qemu-img from a GUI etc.
>>>>
>>>> That's the reason for the "from a terminal" part. If we check for
>>>> isatty(), we should handle these cases just fine.
>>>
>>> Yes, I think checking for a tty is enough precaution and allows users
>>> to get the benefit of the progress bar.  TBH I'd probably forget to
>>> add -p half the time :).
>>
>> Ok, this is fine with me - however how do you suggest we offer the
>> option to disable it on the command line, an additional flag?

If you also check stdout/stderr for isatty (probably the progress output 
should go to stderr), "> /dev/null" is enough.

Paolo

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 42301fd..c980b25 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@  oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o async.o
-block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
+block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
diff --git a/qemu-common.h b/qemu-common.h
index 8ecb488..82e27c1 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -334,6 +334,10 @@  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_progress_init(int enabled, float min_skip);
+void qemu_progress_end(void);
+void qemu_progress_print(float percent, int max);
+
 /* Convert a byte between binary and BCD.  */
 static inline uint8_t to_bcd(uint8_t val)
 {
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 6c7176f..3072d38 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -28,7 +28,7 @@  STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [-c] [-f fmt] [-O output_fmt] [-o options] [-s snapshot_name] filename [filename2 [...]] output_filename")
+    "convert [-c] [-p] [-f fmt] [-O output_fmt] [-o options] [-s snapshot_name] filename [filename2 [...]] output_filename")
 STEXI
 @item convert [-c] [-f @var{fmt}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
@@ -46,7 +46,7 @@  STEXI
 ETEXI
 
 DEF("rebase", img_rebase,
-    "rebase [-f fmt] [-u] -b backing_file [-F backing_fmt] filename")
+    "rebase [-f fmt] [-p] [-u] -b backing_file [-F backing_fmt] filename")
 STEXI
 @item rebase [-f @var{fmt}] [-u] -b @var{backing_file} [-F @var{backing_fmt}] @var{filename}
 ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 7e3cc4c..074388c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -77,6 +77,7 @@  static void help(void)
            "       match exactly. The image doesn't need a working backing file before\n"
            "       rebasing in this case (useful for renaming the backing file)\n"
            "  '-h' with or without a command shows this help and lists the supported formats\n"
+           "  '-p' show progress of command (only certain commands)\n"
            "\n"
            "Parameters to snapshot subcommand:\n"
            "  'snapshot' is the name of the snapshot to create, apply or delete\n"
@@ -567,6 +568,7 @@  static int compare_sectors(const uint8_t *buf1, const uint8_t *buf2, int n,
 static int img_convert(int argc, char **argv)
 {
     int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
+    int progress = 0;
     const char *fmt, *out_fmt, *out_baseimg, *out_filename;
     BlockDriver *drv, *proto_drv;
     BlockDriverState **bs = NULL, *out_bs = NULL;
@@ -579,13 +581,14 @@  static int img_convert(int argc, char **argv)
     QEMUOptionParameter *out_baseimg_param;
     char *options = NULL;
     const char *snapshot_name = NULL;
+    float local_progress;
 
     fmt = NULL;
     out_fmt = "raw";
     out_baseimg = NULL;
     compress = 0;
     for(;;) {
-        c = getopt(argc, argv, "f:O:B:s:hce6o:");
+        c = getopt(argc, argv, "f:O:B:s:hce6o:p");
         if (c == -1) {
             break;
         }
@@ -620,6 +623,9 @@  static int img_convert(int argc, char **argv)
         case 's':
             snapshot_name = optarg;
             break;
+        case 'p':
+            progress = 1;
+            break;
         }
     }
 
@@ -642,6 +648,9 @@  static int img_convert(int argc, char **argv)
         goto out;
     }
         
+    qemu_progress_init(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;
+        local_progress = (float)100 /
+            (nb_sectors / MIN(nb_sectors, (cluster_sectors)));
+
         for(;;) {
             int64_t bs_num;
             int remainder;
@@ -832,6 +846,7 @@  static int img_convert(int argc, char **argv)
                 }
             }
             sector_num += n;
+            qemu_progress_print(local_progress, 100);
         }
         /* signal EOF to align */
         bdrv_write_compressed(out_bs, 0, NULL, 0);
@@ -839,6 +854,10 @@  static int img_convert(int argc, char **argv)
         int has_zero_init = bdrv_has_zero_init(out_bs);
 
         sector_num = 0; // total number of sectors converted so far
+        nb_sectors = total_sectors - sector_num;
+        local_progress = (float)100 /
+            (nb_sectors / MIN(nb_sectors, (IO_BUF_SIZE / 512)));
+
         for(;;) {
             nb_sectors = total_sectors - sector_num;
             if (nb_sectors <= 0) {
@@ -912,9 +931,11 @@  static int img_convert(int argc, char **argv)
                 n -= n1;
                 buf1 += n1 * 512;
             }
+            qemu_progress_print(local_progress, 100);
         }
     }
 out:
+    qemu_progress_end();
     free_option_parameters(create_options);
     free_option_parameters(param);
     qemu_free(buf);
@@ -1184,6 +1205,7 @@  static int img_rebase(int argc, char **argv)
     const char *fmt, *out_basefmt, *out_baseimg;
     int c, flags, ret;
     int unsafe = 0;
+    int progress = 0;
 
     /* Parse commandline parameters */
     fmt = NULL;
@@ -1191,7 +1213,7 @@  static int img_rebase(int argc, char **argv)
     out_basefmt = NULL;
 
     for(;;) {
-        c = getopt(argc, argv, "uhf:F:b:");
+        c = getopt(argc, argv, "uhf:F:b:p");
         if (c == -1) {
             break;
         }
@@ -1212,6 +1234,9 @@  static int img_rebase(int argc, char **argv)
         case 'u':
             unsafe = 1;
             break;
+        case 'p':
+            progress = 1;
+            break;
         }
     }
 
@@ -1220,6 +1245,9 @@  static int img_rebase(int argc, char **argv)
     }
     filename = argv[optind++];
 
+    qemu_progress_init(progress, 2.0);
+    qemu_progress_print(0, 100);
+
     /*
      * Open the images.
      *
@@ -1295,12 +1323,15 @@  static int img_rebase(int argc, char **argv)
         int n;
         uint8_t * buf_old;
         uint8_t * buf_new;
+        float local_progress;
 
         buf_old = qemu_malloc(IO_BUF_SIZE);
         buf_new = qemu_malloc(IO_BUF_SIZE);
 
         bdrv_get_geometry(bs, &num_sectors);
 
+        local_progress = (float)100 /
+            (num_sectors / MIN(num_sectors, (IO_BUF_SIZE / 512)));
         for (sector = 0; sector < num_sectors; sector += n) {
 
             /* How many sectors can we handle with the next read? */
@@ -1348,6 +1379,7 @@  static int img_rebase(int argc, char **argv)
 
                 written += pnum;
             }
+            qemu_progress_print(local_progress, 100);
         }
 
         qemu_free(buf_old);
@@ -1368,6 +1400,7 @@  static int img_rebase(int argc, char **argv)
             out_baseimg, strerror(-ret));
     }
 
+    qemu_progress_print(100, 0);
     /*
      * TODO At this point it is possible to check if any clusters that are
      * allocated in the COW file are the same in the backing file. If so, they
@@ -1375,6 +1408,7 @@  static int img_rebase(int argc, char **argv)
      * backing file, in case of a crash this would lead to corruption.
      */
 out:
+    qemu_progress_end();
     /* Cleanup */
     if (!unsafe) {
         bdrv_delete(bs_old_backing);
diff --git a/qemu-progress.c b/qemu-progress.c
new file mode 100644
index 0000000..656e065
--- /dev/null
+++ b/qemu-progress.c
@@ -0,0 +1,89 @@ 
+/*
+ * QEMU progress printing utility functions
+ *
+ * Copyright (C) 2011 Jes Sorensen <Jes.Sorensen@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu-common.h"
+#include "osdep.h"
+#include "sysemu.h"
+#include <stdio.h>
+
+struct progress_state {
+    int enabled;
+    float current;
+    float last_print;
+    float min_skip;
+};
+
+static struct progress_state state;
+
+/*
+ * Simple progress print function.
+ * @percent relative percent of current operation
+ * @max percent of total operation
+ */
+static void progress_simple_print(void)
+{
+    if (state.enabled) {
+        printf("    (%3.2f/100%%)\r", state.current);
+        fflush(stdout);
+    }
+}
+
+static void progress_simple_end(void)
+{
+    if (state.enabled) {
+        printf("\n");
+    }
+}
+
+void qemu_progress_init(int enabled, float min_skip)
+{
+    state.enabled = enabled;
+    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;
+    }
+    if (current > 100) {
+        current = 100;
+    }
+    state.current = current;
+
+    if (current > (state.last_print + state.min_skip) ||
+        (current == 100) || (current == 0)) {
+        state.last_print = state.current;
+        progress_simple_print();
+    }
+}