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