Re: [Qemu-devel] [PATCH 3/8] block: bdrv_img_create(): move parameter list printing to qemu-img

[Kevin Wolf]

Am 17.10.2012 21:35, schrieb Luiz Capitulino:
> Today, bdrv_img_create() prints the parameter list used to create the
> new image to stdout, like this:
> 
>    Formatting '/tmp/a', fmt=qcow2 size=10737418240 encryption=off 
> cluster_size=65536 lazy_refcounts=off
> 
> As the transaction QMP command calls bdrv_img_create(), this message
> is also printed when using QMP.
> 
> This commit moves the printing of the parameter list to qemu-img instead.
> This way we avoid printing it in QMP and from whatever bdrv_img_create()
> usage we might have in the future.
> 
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>

I believe the idea was that this message is printed before actually
creating the image, which in the case of preallocation could take a while.

> ---
>  block.c    |  4 ----
>  qemu-img.c | 13 ++++++++++++-
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 254a5c2..bdb53af 100644
> --- a/block.c
> +++ b/block.c
> @@ -4411,10 +4411,6 @@ int bdrv_img_create(const char *filename, const char 
> *fmt,
>          }
>      }
>  
> -    printf("Formatting '%s', fmt=%s ", filename, fmt);
> -    print_option_parameters(param);
> -    puts("");
> -
>      ret = bdrv_create(drv, filename, param);
>  
>      if (ret < 0) {
> diff --git a/qemu-img.c b/qemu-img.c
> index b841012..e482443 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -301,6 +301,7 @@ static int img_create(int argc, char **argv)
>      const char *filename;
>      const char *base_filename = NULL;
>      char *options = NULL;
> +    QEMUOptionParameter *params = NULL;
>  
>      for(;;) {
>          c = getopt(argc, argv, "F:b:f:he6o:");
> @@ -362,7 +363,17 @@ static int img_create(int argc, char **argv)
>      }
>  
>      ret = bdrv_img_create(filename, fmt, base_filename, base_fmt,
> -                          options, img_size, BDRV_O_FLAGS, NULL);
> +                          options, img_size, BDRV_O_FLAGS, &params);
> +    if (ret < 0) {
> +        goto out;
> +    }
> +
> +    assert(params);
> +    printf("Formatting '%s', fmt=%s ", filename, fmt);

If we do want to move the message to the end of the operation,
s/Formatting/Formatted/ would make more sense. Could possibly break some
scripts, though.

> +    print_option_parameters(params);
> +    free_option_parameters(params);
> +    puts("");
> +
>  out:
>      if (ret) {
>          return 1;

Kevin