qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: mrezanin@redhat.com
Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option
Date: Wed, 19 Dec 2012 11:06:35 -0700	[thread overview]
Message-ID: <50D2022B.3050303@redhat.com> (raw)
In-Reply-To: <1355751593-31535-3-git-send-email-mrezanin@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3050 bytes --]

On 12/17/2012 06:39 AM, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@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.

> 
> 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 <mrezanin@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]

>  #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.

> +++ 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

  parent reply	other threads:[~2012-12-19 18:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-17 13:39 [Qemu-devel] [PATCH v7 0/4] Add subcommand compare for qemu-img mrezanin
2012-12-17 13:39 ` [Qemu-devel] [PATCH v7 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above mrezanin
2012-12-17 13:39   ` [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option mrezanin
2012-12-17 13:39     ` [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand mrezanin
2012-12-17 13:39       ` [Qemu-devel] [PATCH v7 4/4] Add qemu-img compare documentation mrezanin
2012-12-19 18:15         ` Eric Blake
2012-12-20 19:46           ` Miroslav Rezanina
2012-12-19 18:12       ` [Qemu-devel] [PATCH v7 3/4] qemu-img: Add compare subcommand Eric Blake
2012-12-20 19:44         ` Miroslav Rezanina
2012-12-19 18:06     ` Eric Blake [this message]
2012-12-20 19:31       ` [Qemu-devel] [PATCH v7 2/4] qemu-img: Add "Quiet mode" option Miroslav Rezanina
2012-12-19 17:54   ` [Qemu-devel] [PATCH v7 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above Eric Blake
2013-01-14 10:26 ` [Qemu-devel] [PATCH v8 0/4] Add subcommand compare for qemu-img Miroslav Rezanina
2013-01-14 10:26   ` [Qemu-devel] [PATCH v8 1/4] block: Add synchronous wrapper for bdrv_co_is_allocated_above Miroslav Rezanina
2013-01-14 21:33     ` Eric Blake
2013-01-14 10:26   ` [Qemu-devel] [PATCH v8 2/4] qemu-img: Add "Quiet mode" option Miroslav Rezanina
2013-01-14 23:01     ` Eric Blake
2013-01-14 10:26   ` [Qemu-devel] [PATCH v8 3/4] qemu-img: Add compare subcommand Miroslav Rezanina
2013-01-14 10:26   ` [Qemu-devel] [PATCH v8 4/4] Add qemu-img compare documentation Miroslav Rezanina
2013-01-14 23:24     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50D2022B.3050303@redhat.com \
    --to=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mrezanin@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).