On Tue, 2020-12-08 at 19:54 +0300, Vladimir Sementsov-Ogievskiy wrote: > 08.12.2020 19:27, Maxim Levitsky wrote: > > 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 <mlevi...@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. > > > > Oh yes, you are right. File is created by bdrv_create_file() in > qcow2_co_create_opts() not in qcow2_co_create(). Still, I think, you should > remove the file on any failure after bdrv_create_file() call, but you remove > it only on the last failure point..
You are right! The bulk of the code that can fail is in qcow2_co_create_opts but there are indeed few error conditions prior to that. Thanks for pointing that out. I'll fix that. Best regards, Maxim Levitsky > >