The Friday 15 Aug 2014 à 15:06:12 (+0200), Max Reitz wrote :
> On 15.08.2014 13:18, Benoît Canet wrote:
> >The Saturday 02 Aug 2014 à 01:49:16 (+0200), Max Reitz wrote :
> >>Now that bdrv_amend_options() supports a status callback, use it to
> >>display a progress report.
> >>
> >>Signed-off-by: Max Reitz <mre...@redhat.com>
> >>---
> >> qemu-img-cmds.hx | 4 ++--
> >> qemu-img.c | 25 ++++++++++++++++++++++---
> >> qemu-img.texi | 2 +-
> >> 3 files changed, 25 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> >>index 55aec6b..d83f3d0 100644
> >>--- a/qemu-img-cmds.hx
> >>+++ b/qemu-img-cmds.hx
> >>@@ -70,8 +70,8 @@ STEXI
> >> ETEXI
> >> DEF("amend", img_amend,
> >>- "amend [-q] [-f fmt] [-t cache] -o options filename")
> >>+ "amend [-p] [-q] [-f fmt] [-t cache] -o options filename")
> >> STEXI
> >>-@item amend [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options}
> >>@var{filename}
> >>+@item amend [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options}
> >>@var{filename}
> >> @end table
> >> ETEXI
> >>diff --git a/qemu-img.c b/qemu-img.c
> >>index 90d6b79..4b6406b 100644
> >>--- a/qemu-img.c
> >>+++ b/qemu-img.c
> >>@@ -2732,6 +2732,12 @@ out:
> >> return 0;
> >> }
> >>+static void amend_status_cb(BlockDriverState *bs,
> >>+ int64_t offset, int64_t total_work_size)
> >>+{
> >>+ qemu_progress_print(100.f * offset / total_work_size, 0);
> >>+}
> >>+
> >> static int img_amend(int argc, char **argv)
> >> {
> >> int c, ret = 0;
> >>@@ -2740,12 +2746,12 @@ static int img_amend(int argc, char **argv)
> >> QemuOpts *opts = NULL;
> >> const char *fmt = NULL, *filename, *cache;
> >> int flags;
> >>- bool quiet = false;
> >>+ bool quiet = false, progress = false;
> >> BlockDriverState *bs = NULL;
> >> cache = BDRV_DEFAULT_CACHE;
> >> for (;;) {
> >>- c = getopt(argc, argv, "ho:f:t:q");
> >>+ c = getopt(argc, argv, "ho:f:t:pq");
> >> if (c == -1) {
> >> break;
> >> }
> >>@@ -2775,6 +2781,9 @@ static int img_amend(int argc, char **argv)
> >> case 't':
> >> cache = optarg;
> >> break;
> >>+ case 'p':
> >>+ progress = true;
> >>+ break;
> >> case 'q':
> >> quiet = true;
> >> break;
> >>@@ -2785,6 +2794,11 @@ static int img_amend(int argc, char **argv)
> >> error_exit("Must specify options (-o)");
> >> }
> >>+ if (quiet) {
> >>+ progress = false;
> >>+ }
> >>+ qemu_progress_init(progress, 1.0);
> >>+
> >> filename = (optind == argc - 1) ? argv[argc - 1] : NULL;
> >> if (fmt && has_help_option(options)) {
> >> /* If a format is explicitly specified (and possibly no filename
> >> is
> >>@@ -2827,13 +2841,18 @@ static int img_amend(int argc, char **argv)
> >> goto out;
> >> }
> >>- ret = bdrv_amend_options(bs, opts, NULL);
> >>+ /* In case the driver does not call amend_status_cb() */
> >>+ qemu_progress_print(0.f, 0);
> >>+ ret = bdrv_amend_options(bs, opts, &amend_status_cb);
> >>+ qemu_progress_print(100.f, 0);
> >> if (ret < 0) {
> >> error_report("Error while amending options: %s", strerror(-ret));
> >> goto out;
> >> }
> >> out:
> >>+ qemu_progress_end();
> >>+
> >> if (bs) {
> >> bdrv_unref(bs);
> >> }
> >>diff --git a/qemu-img.texi b/qemu-img.texi
> >>index cb68948..2c66603 100644
> >>--- a/qemu-img.texi
> >>+++ b/qemu-img.texi
> >>@@ -397,7 +397,7 @@ After using this command to grow a disk image, you must
> >>use file system and
> >> partitioning tools inside the VM to actually begin using the new space on
> >> the
> >> device.
> >>-@item amend [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
> >>+@item amend [-p] [-f @var{fmt}] [-t @var{cache}] -o @var{options}
> >>@var{filename}
> >> Amends the image format specific @var{options} for the image file
> >> @var{filename}. Not all file formats support this operation.
> >>--
> >>2.0.3
> >>
> >This patchs is fine and sweet.
> >However could we add an assert(amend_status_cb) in bdrv_amend_options
> >somewhere in the series ?
> >So if a coder pass NULL as callback he will have a clue near the root cause.
>
> Judging from your response to patch 4, this seems no longer necessary...?
I was thinking about avoiding adding burden to the qemu know how
(like "oh I added a field to BlockDriverState so I must modify bdrv_swap")
but I trust your jugement on this one.
>
> >Reviewed-by: Benoît Canet <benoit.ca...@nodalink.com>
>
> Just now seeing your new email address, hopefully I don't end up
> accidentally typing out the old one when adding the tag to commits. :-)
>
> Max
>
