Hi Eric, thanks for review, reply inline. ----- Original Message ----- > From: "Eric Blake" <ebl...@redhat.com> > To: mreza...@redhat.com > Cc: qemu-devel@nongnu.org, kw...@redhat.com, pbonz...@redhat.com, > stefa...@redhat.com > Sent: Wednesday, December 19, 2012 7:06:35 PM > Subject: Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option > > On 12/17/2012 06:39 AM, mreza...@redhat.com wrote: > > From: Miroslav Rezanina <mreza...@redhat.com> > > Your git send-email settings threaded each message as a reply to the > other, rather than the more typical setting of threading messages > only > as a reply to the cover letter. You may want to do 'git config > format.thread shallow' rather than your current deep approach.
I use temporary machine for sending patches as my usual one was not usable for me in this case, so setting can be incorrect. I will take better care of this next time. > > > > > There can be need to turn output to stdout off. This patch adds a > > -q option that > > s/be need/be a need/ > > > enable "Quiet mode". In Quiet mode, only errors are printed out. > > > > Signed-off-by: Miroslav Rezanina <mreza...@redhat.com> > > --- > > block.c | 11 +++--- > > block.h | 2 +- > > blockdev.c | 6 ++-- > > qemu-img-cmds.hx | 28 +++++++-------- > > qemu-img.c | 108 > > +++++++++++++++++++++++++++++++++++++++++-------------- > > qemu-img.texi | 3 ++ > > 6 files changed, 108 insertions(+), 50 deletions(-) > > > > @@ -1275,7 +1275,7 @@ void qmp_drive_mirror(const char *device, > > const char *target, > > ret = bdrv_img_create(target, format, > > source->filename, > > source->drv->format_name, > > - NULL, -1, flags); > > + NULL, -1, flags,false); > > Space after comma. > > > @@ -10,27 +10,27 @@ STEXI > > ETEXI > > > > DEF("check", img_check, > > - "check [-f fmt] [-r [leaks | all]] filename") > > + "check [-q] [-f fmt] [-r [leaks | all]] filename") > > Is it really better to list [-q] to all of the sub-commands, or would > it > be easier to simply declare that -q is a universal option that can > appear before sub-commands, as in: > qemu-img [-q] command [command options] > -q is not useable for all commands so it is listed this ways as it is same for other options. > > #ifdef _WIN32 > > #include <windows.h> > > @@ -86,6 +87,7 @@ static void help(void) > > " 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" > > + " '-q' Quiet mode - do not print any output (except > > errors)\n" > > s/Quiet/quiet/ for consistency with the nearby lines not starting > with a > capital. I use Quiet mode as name with capital Q, so I have to choose what consistency to break - I choose the start of the description. I have no problem with using quiet instead of Quiet. > > > +++ b/qemu-img.texi > > @@ -54,6 +54,9 @@ indicates that target image must be compressed > > (qcow format only) > > with or without a command shows help and lists the supported > > formats > > @item -p > > display progress bar (convert and rebase commands only) > > +@item -q > > +Quiet mode - do not print any output (except errors). There's no > > progres bar > > s/progres/progress/ > > > +in case both @var{-q} and @var{-p} options are used. > > Should it be a hard error if both -q and -p are given? Otherwise, > when > dealing with conflicting options, it's more typical to have the > semantic > of last one wins: 'qemu-img -q -p' prints progress, and only > 'qemu-img > -p -q' is quiet. > Depends on point of view - There's no conflict for -p and -q as -p adds progress bar to output and -q suppress output. You can imagine (in theory) -q as redirecting stdout to /dev/null. > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > >