Message ID | 20201209203326.879381-5-mlevitsk@redhat.com |
---|---|
State | New |
Headers | show |
Series | qcow2: don't leave partially initialized file on image creation | expand |
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); >
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 --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);