qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] block: Be more verbose in create fallback
@ 2023-07-20 14:00 Hanna Czenczek
  2023-07-20 17:01 ` Eric Blake
  2023-08-18 16:00 ` Kevin Wolf
  0 siblings, 2 replies; 3+ messages in thread
From: Hanna Czenczek @ 2023-07-20 14:00 UTC (permalink / raw)
  To: qemu-block; +Cc: qemu-devel, Hanna Czenczek, Kevin Wolf

For image creation code, we have central fallback code for protocols
that do not support creating new images (like NBD or iscsi).  So for
them, you can only specify existing paths/exports that are overwritten
to make clean new images.  In such a case, if the given path cannot be
opened (assuming a pre-existing image there), we print an error message
that tries to describe what is going on: That with this protocol, you
cannot create new images, but only overwrite existing ones; and the
given path could not be opened as a pre-existing image.

However, the current message is confusing, because it does not say that
the protocol in question does not support creating new images, but
instead that "image creation" is unsupported.  This can be interpreted
to mean that `qemu-img create` will not work in principle, which is not
true.  Be more verbose for clarity.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2217204
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 block.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index a307c151a8..f530dd9c02 100644
--- a/block.c
+++ b/block.c
@@ -661,8 +661,10 @@ int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv,
     blk = blk_co_new_open(filename, NULL, options,
                           BDRV_O_RDWR | BDRV_O_RESIZE, errp);
     if (!blk) {
-        error_prepend(errp, "Protocol driver '%s' does not support image "
-                      "creation, and opening the image failed: ",
+        error_prepend(errp, "Protocol driver '%s' does not support creating "
+                      "new images, so an existing image must be selected as "
+                      "the target; however, opening the given target as an "
+                      "existing image failed: ",
                       drv->format_name);
         return -EINVAL;
     }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] block: Be more verbose in create fallback
  2023-07-20 14:00 [PATCH] block: Be more verbose in create fallback Hanna Czenczek
@ 2023-07-20 17:01 ` Eric Blake
  2023-08-18 16:00 ` Kevin Wolf
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Blake @ 2023-07-20 17:01 UTC (permalink / raw)
  To: Hanna Czenczek; +Cc: qemu-block, qemu-devel, Kevin Wolf

On Thu, Jul 20, 2023 at 04:00:24PM +0200, Hanna Czenczek wrote:
> For image creation code, we have central fallback code for protocols
> that do not support creating new images (like NBD or iscsi).  So for
> them, you can only specify existing paths/exports that are overwritten
> to make clean new images.  In such a case, if the given path cannot be
> opened (assuming a pre-existing image there), we print an error message
> that tries to describe what is going on: That with this protocol, you
> cannot create new images, but only overwrite existing ones; and the
> given path could not be opened as a pre-existing image.
> 
> However, the current message is confusing, because it does not say that
> the protocol in question does not support creating new images, but
> instead that "image creation" is unsupported.  This can be interpreted
> to mean that `qemu-img create` will not work in principle, which is not
> true.  Be more verbose for clarity.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2217204
> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
> ---
>  block.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Definitely more vebose, but I don't see that as a bad thing.

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block.c b/block.c
> index a307c151a8..f530dd9c02 100644
> --- a/block.c
> +++ b/block.c
> @@ -661,8 +661,10 @@ int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv,
>      blk = blk_co_new_open(filename, NULL, options,
>                            BDRV_O_RDWR | BDRV_O_RESIZE, errp);
>      if (!blk) {
> -        error_prepend(errp, "Protocol driver '%s' does not support image "
> -                      "creation, and opening the image failed: ",
> +        error_prepend(errp, "Protocol driver '%s' does not support creating "
> +                      "new images, so an existing image must be selected as "
> +                      "the target; however, opening the given target as an "
> +                      "existing image failed: ",
>                        drv->format_name);
>          return -EINVAL;
>      }
> -- 
> 2.41.0
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] block: Be more verbose in create fallback
  2023-07-20 14:00 [PATCH] block: Be more verbose in create fallback Hanna Czenczek
  2023-07-20 17:01 ` Eric Blake
@ 2023-08-18 16:00 ` Kevin Wolf
  1 sibling, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2023-08-18 16:00 UTC (permalink / raw)
  To: Hanna Czenczek; +Cc: qemu-block, qemu-devel

Am 20.07.2023 um 16:00 hat Hanna Czenczek geschrieben:
> For image creation code, we have central fallback code for protocols
> that do not support creating new images (like NBD or iscsi).  So for
> them, you can only specify existing paths/exports that are overwritten
> to make clean new images.  In such a case, if the given path cannot be
> opened (assuming a pre-existing image there), we print an error message
> that tries to describe what is going on: That with this protocol, you
> cannot create new images, but only overwrite existing ones; and the
> given path could not be opened as a pre-existing image.
> 
> However, the current message is confusing, because it does not say that
> the protocol in question does not support creating new images, but
> instead that "image creation" is unsupported.  This can be interpreted
> to mean that `qemu-img create` will not work in principle, which is not
> true.  Be more verbose for clarity.
> 
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2217204
> Signed-off-by: Hanna Czenczek <hreitz@redhat.com>

Thanks, applied to the block branch.

Kevin



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-08-18 16:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-20 14:00 [PATCH] block: Be more verbose in create fallback Hanna Czenczek
2023-07-20 17:01 ` Eric Blake
2023-08-18 16:00 ` Kevin Wolf

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