| Message ID | 20200716113359.15656-1-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: cleanup created file when qcow2_co_create | expand |
On Thu, 2020-07-16 at 14:33 +0300, Maxim Levitsky wrote: > This is basically the same thing as commit > 'crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails' > does but for qcow2 files to ensure that we don't leave qcow2 files > when creation fails. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > block/qcow2.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index fadf3422f8..8b848924b5 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3794,6 +3794,17 @@ 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); > + } > goto finish; > } > Any update on this? Do you think we can add this to 5.1 or is it too late? Best regards, Maxim Levitsky
On Thu, 2020-07-16 at 14:33 +0300, Maxim Levitsky wrote: > This is basically the same thing as commit > 'crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails' > does but for qcow2 files to ensure that we don't leave qcow2 files > when creation fails. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Very kind ping on this patch. Best regards, Maxim levitsky > --- > block/qcow2.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index fadf3422f8..8b848924b5 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3794,6 +3794,17 @@ 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); > + } > goto finish; > } >
On Thu 16 Jul 2020 01:33:59 PM CEST, Maxim Levitsky wrote: > 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); > + } I think you're leaking the error on ENOTSUP. Berto
On Tue, 2020-09-01 at 15:30 +0200, Alberto Garcia wrote: > On Thu 16 Jul 2020 01:33:59 PM CEST, Maxim Levitsky wrote: > > 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); > > + } > > I think you're leaking the error on ENOTSUP. Indeed, just like in the original code from where I copy&pasta'ed this from. I'll send a patch to fix this and update the patch as well. Thanks, Best regards, Maxim Levitsky > > Berto >
diff --git a/block/qcow2.c b/block/qcow2.c index fadf3422f8..8b848924b5 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3794,6 +3794,17 @@ 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); + } goto finish; }
This is basically the same thing as commit 'crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails' does but for qcow2 files to ensure that we don't leave qcow2 files when creation fails. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- block/qcow2.c | 11 +++++++++++ 1 file changed, 11 insertions(+)