From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60884) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XIHD6-00083C-Os for qemu-devel@nongnu.org; Fri, 15 Aug 2014 09:06:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XIHD1-0004kV-CM for qemu-devel@nongnu.org; Fri, 15 Aug 2014 09:06:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62438) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XIHD1-0004kF-38 for qemu-devel@nongnu.org; Fri, 15 Aug 2014 09:06:19 -0400 Message-ID: <53EE05C4.7010704@redhat.com> Date: Fri, 15 Aug 2014 15:06:12 +0200 From: Max Reitz MIME-Version: 1.0 References: <1406936961-20356-1-git-send-email-mreitz@redhat.com> <1406936961-20356-3-git-send-email-mreitz@redhat.com> <20140815111858.GB9305@irqsave.net> In-Reply-To: <20140815111858.GB9305@irqsave.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 2/7] qemu-img: Add progress output for amend List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?windows-1252?Q?Beno=EEt_Canet?= Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 15.08.2014 13:18, Beno=EEt Canet wrote: > The Saturday 02 Aug 2014 =E0 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 >> --- >> 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 >> =20 >> 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} @va= r{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; >> } >> =20 >> +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 =3D 0; >> @@ -2740,12 +2746,12 @@ static int img_amend(int argc, char **argv) >> QemuOpts *opts =3D NULL; >> const char *fmt =3D NULL, *filename, *cache; >> int flags; >> - bool quiet =3D false; >> + bool quiet =3D false, progress =3D false; >> BlockDriverState *bs =3D NULL; >> =20 >> cache =3D BDRV_DEFAULT_CACHE; >> for (;;) { >> - c =3D getopt(argc, argv, "ho:f:t:q"); >> + c =3D getopt(argc, argv, "ho:f:t:pq"); >> if (c =3D=3D -1) { >> break; >> } >> @@ -2775,6 +2781,9 @@ static int img_amend(int argc, char **argv) >> case 't': >> cache =3D optarg; >> break; >> + case 'p': >> + progress =3D true; >> + break; >> case 'q': >> quiet =3D true; >> break; >> @@ -2785,6 +2794,11 @@ static int img_amend(int argc, char **argv) >> error_exit("Must specify options (-o)"); >> } >> =20 >> + if (quiet) { >> + progress =3D false; >> + } >> + qemu_progress_init(progress, 1.0); >> + >> filename =3D (optind =3D=3D argc - 1) ? argv[argc - 1] : NULL; >> if (fmt && has_help_option(options)) { >> /* If a format is explicitly specified (and possibly no file= name is >> @@ -2827,13 +2841,18 @@ static int img_amend(int argc, char **argv) >> goto out; >> } >> =20 >> - ret =3D bdrv_amend_options(bs, opts, NULL); >> + /* In case the driver does not call amend_status_cb() */ >> + qemu_progress_print(0.f, 0); >> + ret =3D 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(-r= et)); >> goto out; >> } >> =20 >> 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 spa= ce on the >> device. >> =20 >> -@item amend [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{fil= ename} >> +@item amend [-p] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @va= r{filename} >> =20 >> Amends the image format specific @var{options} for the image file >> @var{filename}. Not all file formats support this operation. >> --=20 >> 2.0.3 >> > This patchs is fine and sweet. > However could we add an assert(amend_status_cb) in bdrv_amend_options s= omewhere in the series ? > So if a coder pass NULL as callback he will have a clue near the root c= ause. Judging from your response to patch 4, this seems no longer necessary...? > Reviewed-by: Beno=EEt Canet Just now seeing your new email address, hopefully I don't end up=20 accidentally typing out the old one when adding the tag to commits. :-) Max