From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39143) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XIHbA-00084X-Pu for qemu-devel@nongnu.org; Fri, 15 Aug 2014 09:31:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XIHb5-0004Sz-MX for qemu-devel@nongnu.org; Fri, 15 Aug 2014 09:31:16 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:42431 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XIHb5-0004St-CS for qemu-devel@nongnu.org; Fri, 15 Aug 2014 09:31:11 -0400 Date: Fri, 15 Aug 2014 15:30:18 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140815133018.GA595@irqsave.net> References: <1406936961-20356-1-git-send-email-mreitz@redhat.com> <1406936961-20356-3-git-send-email-mreitz@redhat.com> <20140815111858.GB9305@irqsave.net> <53EE05C4.7010704@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <53EE05C4.7010704@redhat.com> 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: Max Reitz Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi The Friday 15 Aug 2014 =E0 15:06:12 (+0200), Max Reitz wrote : > 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 > >> 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} @v= ar{filename} > >>+@item amend [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{option= s} @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 =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; > >> 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)"); > >> } > >>+ 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 fil= ename is > >>@@ -2827,13 +2841,18 @@ static int img_amend(int argc, char **argv) > >> goto out; > >> } > >>- 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(-= 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, yo= u must use file system and > >> partitioning tools inside the VM to actually begin using the new sp= ace on the > >> device. > >>-@item amend [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{fi= lename} > >>+@item amend [-p] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @v= ar{filename} > >> 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 = somewhere in the series ? > >So if a coder pass NULL as callback he will have a clue near the root = cause. >=20 > 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. >=20 > >Reviewed-by: Beno=EEt Canet >=20 > 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. :-) >=20 > Max >=20