Patchwork [3/8] block: bdrv_img_create(): move parameter list printing to qemu-img

login
register
mail settings
Submitter Luiz Capitulino
Date Oct. 17, 2012, 7:35 p.m.
Message ID <1350502556-4885-4-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/192137/
State New
Headers show

Comments

Luiz Capitulino - Oct. 17, 2012, 7:35 p.m.
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 <lcapitulino@redhat.com>
---
 block.c    |  4 ----
 qemu-img.c | 13 ++++++++++++-
 2 files changed, 12 insertions(+), 5 deletions(-)
Kevin Wolf - Oct. 18, 2012, 12:01 p.m.
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 <lcapitulino@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
Kevin Wolf - Oct. 18, 2012, 12:16 p.m.
Am 18.10.2012 14:01, schrieb 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 <lcapitulino@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.

The other thing is that I think you don't print the message any more
when creating the image fails. Did you check that qemu-iotests doesn't
need any updates?

Kevin
Luiz Capitulino - Oct. 18, 2012, 1:34 p.m.
On Thu, 18 Oct 2012 14:16:43 +0200
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 18.10.2012 14:01, schrieb 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 <lcapitulino@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.
> 
> The other thing is that I think you don't print the message any more
> when creating the image fails. Did you check that qemu-iotests doesn't
> need any updates?

I didn't. But I'll preserve today's behavior for v2 as I mentioned in my
last email.

Patch

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);
+    print_option_parameters(params);
+    free_option_parameters(params);
+    puts("");
+
 out:
     if (ret) {
         return 1;