Re: [PATCH v3 2/2] block: qcow2: remove the created file on initialization error

[Maxim Levitsky <mlevitsk@redhat.com>]

On Tue, 2020-12-08 at 18:47 +0300, Vladimir Sementsov-Ogievskiy wrote:
> 08.12.2020 17:21, 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>
> > ---
> >   block/qcow2.c | 13 +++++++++++++
> >   1 file changed, 13 insertions(+)
> > 
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 3a90ef2786..3bc2096b72 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -3848,6 +3848,19 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv,
> >       /* Create the qcow2 image (format layer) */
> >       ret = qcow2_co_create(create_options, errp);
> >       if (ret < 0) {
> > +
> > +        Error *local_delete_err = NULL;
> > +        int r_del = bdrv_co_delete_file(bs, &local_delete_err);
> > +        /*
> > +         * ENOTSUP will happen if the block driver doesn't support
> > +         * the 'bdrv_co_delete_file' interface. This is a predictable
> > +         * scenario and shouldn't be reported back to the user.
> > +         */
> > +        if ((r_del < 0) && (r_del != -ENOTSUP)) {
> > +            error_report_err(local_delete_err);
> > +        } else {
> > +            error_free(local_delete_err);
> > +        }
> >           goto finish;
> >       }
> >   
> > 
> 
> Hi!
> 
> As I understand, qcow2_co_create is a new interface and qcow2_co_create_opts() is old, and now works as a wrapper on qcow2_co_create.
> 
> I think it's better to do the cleanup in qcow2_co_create, to bring the feature both to new and old interface in the same way.

I think that the new interface doesn't need this fix, since 
using the new interface is only possible from qmp which 
forces the user to explicitly create and open the file 
prior to formatting it with qcow2 format.

Thus it is logical to make the user remove it as well if creation fails.

Best regards,
	Maxim Levitsky

> 
>