diff mbox series

[v5,4/4] block: qcow2: remove the created file on initialization error

Message ID 20201209203326.879381-5-mlevitsk@redhat.com (mailing list archive)
State New, archived
Headers show
Series qcow2: don't leave partially initialized file on image creation | expand

Commit Message

Maxim Levitsky Dec. 9, 2020, 8:33 p.m. UTC
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>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Dec. 10, 2020, 11 a.m. UTC | #1
09.12.2020 23:33, Maxim Levitsky wrote:
> 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>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
>   block/qcow2.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 3a90ef2786..68c9182f92 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3847,12 +3847,14 @@ 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;

Ooops :) After this patch the function always returns zero :)

> -finish:
> +
>       qobject_unref(qdict);
>       bdrv_unref(bs);
>       bdrv_unref(data_bs);
>
Maxim Levitsky Dec. 10, 2020, 11:02 a.m. UTC | #2
On Thu, 2020-12-10 at 14:00 +0300, Vladimir Sementsov-Ogievskiy wrote:
> 09.12.2020 23:33, Maxim Levitsky wrote:
> > 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>
> > Reviewed-by: Alberto Garcia <berto@igalia.com>
> > ---
> >   block/qcow2.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 3a90ef2786..68c9182f92 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -3847,12 +3847,14 @@ 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;
> 
> Ooops :) After this patch the function always returns zero :)

Indeed :-(

These small changes done in the last minute are the most dangerous.

Best regards,
	Maxim Levitsky
> 
> > -finish:
> > +
> >       qobject_unref(qdict);
> >       bdrv_unref(bs);
> >       bdrv_unref(data_bs);
> > 
> 
>
diff mbox series

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index 3a90ef2786..68c9182f92 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3847,12 +3847,14 @@  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);