If the qcow initialization fails, we should remove the file if it was
already created, to avoid leaving stale files around.

We already do this for luks raw images.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/qcow2.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 3a90ef2786..b5169b7cad 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3847,12 +3847,13 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
 
     /* Create the qcow2 image (format layer) */
     ret = qcow2_co_create(create_options, errp);
+
+finish:
     if (ret < 0) {
-        goto finish;
+        bdrv_co_delete_file_noerr(bs);
+        bdrv_co_delete_file_noerr(data_bs);
     }
 
-    ret = 0;
-finish:
     qobject_unref(qdict);
     bdrv_unref(bs);
     bdrv_unref(data_bs);
-- 
2.26.2

On Wed 09 Dec 2020 05:44:41 PM CET, Maxim Levitsky wrote:
> @@ -3847,12 +3847,13 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
>  
>      /* Create the qcow2 image (format layer) */
>      ret = qcow2_co_create(create_options, errp);
> +
> +finish:
>      if (ret < 0) {
> -        goto finish;
> +        bdrv_co_delete_file_noerr(bs);
> +        bdrv_co_delete_file_noerr(data_bs);
>      }
>  
> -    ret = 0;

Many/most functions in qcow2.c force ret to be 0 on success, we could
also keep that here (although in practice I don't think that ret can be
greater than 0 in this case, or that the caller would care).

Either way,

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

On Wed, 2020-12-09 at 18:41 +0100, Alberto Garcia wrote:
> On Wed 09 Dec 2020 05:44:41 PM CET, Maxim Levitsky wrote:
> > @@ -3847,12 +3847,13 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
> >  
> >      /* Create the qcow2 image (format layer) */
> >      ret = qcow2_co_create(create_options, errp);
> > +
> > +finish:
> >      if (ret < 0) {
> > -        goto finish;
> > +        bdrv_co_delete_file_noerr(bs);
> > +        bdrv_co_delete_file_noerr(data_bs);
> >      }
> >  
> > -    ret = 0;
> 
> Many/most functions in qcow2.c force ret to be 0 on success, we could
> also keep that here (although in practice I don't think that ret can be
> greater than 0 in this case, or that the caller would care).

I also noticed this when I was sending the patches, and I wasn't sure
if I want to keep that 'ret = 0' or not.
I will add it back.

Best regards,
	Maxim Levitsky

> 
> Either way,
> 
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> 
> Berto
>