Message ID | 20190807142114.17569-3-danielhb413@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | delete created files when block_crypto_co_create_opts_luks fails | expand |
On 8/7/19 10:21 AM, Daniel Henrique Barboza wrote: > Using the new 'bdrv_co_delete_file' interface, bdrv_delete_file > can be used in a way similar of the existing bdrv_create_file to > to clean up a created file. > > The logic is also similar to what is already done in bdrv_create_file: > a qemu_coroutine is created if needed, a specialized function > bdrv_delete_co_entry is used to call the bdrv_co_delete_file > co-routine of the driver, if the driver implements it. > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > block.c | 77 +++++++++++++++++++++++++++++++++++++++++++ > include/block/block.h | 1 + > 2 files changed, 78 insertions(+) > > diff --git a/block.c b/block.c > index cbd8da5f3b..1e20250627 100644 > --- a/block.c > +++ b/block.c > @@ -547,6 +547,83 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) > return ret; > } > > +typedef struct DeleteCo { > + BlockDriver *drv; > + BlockDriverState *bs; > + int ret; > + Error *err; > +} DeleteCo; > + > +static void coroutine_fn bdrv_delete_co_entry(void *opaque) > +{ > + Error *local_err = NULL; > + DeleteCo *dco = opaque; > + > + assert(dco->bs); > + > + dco->ret = dco->drv->bdrv_co_delete_file(dco->bs, &local_err); > + error_propagate(&dco->err, local_err); > +} > + > +int bdrv_delete_file(const char *filename, Error **errp) > +{ > + BlockDriver *drv = bdrv_find_protocol(filename, true, NULL); > + BlockDriverState *bs = bdrv_open(filename, NULL, NULL, > + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL); > + DeleteCo dco = { > + .drv = drv, > + .bs = bs, > + .ret = NOT_DONE, > + .err = NULL, > + }; > + Coroutine *co; > + int ret; > + > + if (!drv) { > + error_setg(errp, "File '%s' has unknown format", filename); > + ret = -ENOENT; > + goto out; > + } > + I was going to say that ENOENT is a weird error here, but I see it used for !drv a few other places in block.c too, alongside EINVAL and ENOMEDIUM. ENOMEDIUM loks like the most popular. > + if (!drv->bdrv_co_delete_file) { > + error_setg(errp, "Driver '%s' does not support image delete", > + drv->format_name); > + ret = -ENOTSUP; > + goto out; > + } > + > + if (!bs) { > + error_setg(errp, "Could not open image '%s' for erasing", > + filename); > + ret = 1; Please keep all errors negative (or at least consistent within a function). I'm also wondering if we want a version of delete that doesn't try to open a file directly -- i.e. a version that exists like this: bdrv_co_delete_file(BlockDriverState *bs, Error **errp); That simply dispatches based on bs->drv to the correct routine. Then, you are free to have bdrv_delete_file handle the open (and let the opening figure out what driver it needs), and just hand off the bds to bdrv_co_delete_file. I'm not the authority for block.c, though, so maaaybe I'm giving you bad advice here. Kevin's away on PTO for a bit and gave you advice most recently, so I might try to gently ask him for more feedback next week. > + goto out; > + } > + > + if (qemu_in_coroutine()) { > + /* Fast-path if already in coroutine context */ > + bdrv_delete_co_entry(&dco); > + } else { > + co = qemu_coroutine_create(bdrv_delete_co_entry, &dco); > + qemu_coroutine_enter(co); > + while (dco.ret == NOT_DONE) { > + aio_poll(qemu_get_aio_context(), true); > + } > + } > + > + ret = dco.ret; > + if (ret < 0) { > + if (dco.err) { > + error_propagate(errp, dco.err); > + } else { > + error_setg_errno(errp, -ret, "Could not delete image"); > + } > + } > + > +out: > + bdrv_unref(bs); > + return ret; > +} > + > /** > * Try to get @bs's logical and physical block size. > * On success, store them in @bsz struct and return 0. > diff --git a/include/block/block.h b/include/block/block.h > index 50a07c1c33..5e83532364 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -369,6 +369,7 @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, > int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, > Error **errp); > void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base); > +int bdrv_delete_file(const char *filename, Error **errp); > > > typedef struct BdrvCheckResult { >
On 8/28/19 11:07 PM, John Snow wrote: > > On 8/7/19 10:21 AM, Daniel Henrique Barboza wrote: >> Using the new 'bdrv_co_delete_file' interface, bdrv_delete_file >> can be used in a way similar of the existing bdrv_create_file to >> to clean up a created file. >> >> The logic is also similar to what is already done in bdrv_create_file: >> a qemu_coroutine is created if needed, a specialized function >> bdrv_delete_co_entry is used to call the bdrv_co_delete_file >> co-routine of the driver, if the driver implements it. >> >> Suggested-by: Daniel P. Berrangé <berrange@redhat.com> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >> --- >> block.c | 77 +++++++++++++++++++++++++++++++++++++++++++ >> include/block/block.h | 1 + >> 2 files changed, 78 insertions(+) >> >> diff --git a/block.c b/block.c >> index cbd8da5f3b..1e20250627 100644 >> --- a/block.c >> +++ b/block.c >> @@ -547,6 +547,83 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) >> return ret; >> } >> >> +typedef struct DeleteCo { >> + BlockDriver *drv; >> + BlockDriverState *bs; >> + int ret; >> + Error *err; >> +} DeleteCo; >> + >> +static void coroutine_fn bdrv_delete_co_entry(void *opaque) >> +{ >> + Error *local_err = NULL; >> + DeleteCo *dco = opaque; >> + >> + assert(dco->bs); >> + >> + dco->ret = dco->drv->bdrv_co_delete_file(dco->bs, &local_err); >> + error_propagate(&dco->err, local_err); >> +} >> + >> +int bdrv_delete_file(const char *filename, Error **errp) >> +{ >> + BlockDriver *drv = bdrv_find_protocol(filename, true, NULL); >> + BlockDriverState *bs = bdrv_open(filename, NULL, NULL, >> + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL); >> + DeleteCo dco = { >> + .drv = drv, >> + .bs = bs, >> + .ret = NOT_DONE, >> + .err = NULL, >> + }; >> + Coroutine *co; >> + int ret; >> + >> + if (!drv) { >> + error_setg(errp, "File '%s' has unknown format", filename); >> + ret = -ENOENT; >> + goto out; >> + } >> + > I was going to say that ENOENT is a weird error here, but I see it used > for !drv a few other places in block.c too, alongside EINVAL and > ENOMEDIUM. ENOMEDIUM loks like the most popular. Didn't spend too much time thinking about it. I copied the same behavior from bdrv_create_file: --------- int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) { (...) drv = bdrv_find_protocol(filename, true, errp); if (drv == NULL) { return -ENOENT; } ----- I can change to ENOMEDIUM if it's indeed more informative than ENOENT. > >> + if (!drv->bdrv_co_delete_file) { >> + error_setg(errp, "Driver '%s' does not support image delete", >> + drv->format_name); >> + ret = -ENOTSUP; >> + goto out; >> + } >> + >> + if (!bs) { >> + error_setg(errp, "Could not open image '%s' for erasing", >> + filename); >> + ret = 1; > Please keep all errors negative (or at least consistent within a function). Got it. I'll fix it in the re-spin. > > > I'm also wondering if we want a version of delete that doesn't try to > open a file directly -- i.e. a version that exists like this: > > bdrv_co_delete_file(BlockDriverState *bs, Error **errp); > > That simply dispatches based on bs->drv to the correct routine. > > Then, you are free to have bdrv_delete_file handle the open (and let the > opening figure out what driver it needs), and just hand off the bds to > bdrv_co_delete_file. > > I'm not the authority for block.c, though, so maaaybe I'm giving you bad > advice here. Kevin's away on PTO for a bit and gave you advice most > recently, so I might try to gently ask him for more feedback next week. I appreciate. I'm not acquainted with the block code at all - I'm playing by ear since the first version. Any tip is appreciated :) Thanks, DHB > >> + goto out; >> + } >> + >> + if (qemu_in_coroutine()) { >> + /* Fast-path if already in coroutine context */ >> + bdrv_delete_co_entry(&dco); >> + } else { >> + co = qemu_coroutine_create(bdrv_delete_co_entry, &dco); >> + qemu_coroutine_enter(co); >> + while (dco.ret == NOT_DONE) { >> + aio_poll(qemu_get_aio_context(), true); >> + } >> + } >> + >> + ret = dco.ret; >> + if (ret < 0) { >> + if (dco.err) { >> + error_propagate(errp, dco.err); >> + } else { >> + error_setg_errno(errp, -ret, "Could not delete image"); >> + } >> + } >> + >> +out: >> + bdrv_unref(bs); >> + return ret; >> +} >> + >> /** >> * Try to get @bs's logical and physical block size. >> * On success, store them in @bsz struct and return 0. >> diff --git a/include/block/block.h b/include/block/block.h >> index 50a07c1c33..5e83532364 100644 >> --- a/include/block/block.h >> +++ b/include/block/block.h >> @@ -369,6 +369,7 @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, >> int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, >> Error **errp); >> void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base); >> +int bdrv_delete_file(const char *filename, Error **errp); >> >> >> typedef struct BdrvCheckResult { >>
Am 29.08.2019 um 04:07 hat John Snow geschrieben: > > > On 8/7/19 10:21 AM, Daniel Henrique Barboza wrote: > > Using the new 'bdrv_co_delete_file' interface, bdrv_delete_file > > can be used in a way similar of the existing bdrv_create_file to > > to clean up a created file. > > > > The logic is also similar to what is already done in bdrv_create_file: > > a qemu_coroutine is created if needed, a specialized function > > bdrv_delete_co_entry is used to call the bdrv_co_delete_file > > co-routine of the driver, if the driver implements it. > > > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com> > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > --- > > block.c | 77 +++++++++++++++++++++++++++++++++++++++++++ > > include/block/block.h | 1 + > > 2 files changed, 78 insertions(+) > > > > diff --git a/block.c b/block.c > > index cbd8da5f3b..1e20250627 100644 > > --- a/block.c > > +++ b/block.c > > @@ -547,6 +547,83 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) > > return ret; > > } > > > > +typedef struct DeleteCo { > > + BlockDriver *drv; > > + BlockDriverState *bs; > > + int ret; > > + Error *err; > > +} DeleteCo; > > + > > +static void coroutine_fn bdrv_delete_co_entry(void *opaque) > > +{ > > + Error *local_err = NULL; > > + DeleteCo *dco = opaque; > > + > > + assert(dco->bs); > > + > > + dco->ret = dco->drv->bdrv_co_delete_file(dco->bs, &local_err); > > + error_propagate(&dco->err, local_err); > > +} > > + > > +int bdrv_delete_file(const char *filename, Error **errp) > > +{ > > + BlockDriver *drv = bdrv_find_protocol(filename, true, NULL); > > + BlockDriverState *bs = bdrv_open(filename, NULL, NULL, > > + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL); > > + DeleteCo dco = { > > + .drv = drv, > > + .bs = bs, > > + .ret = NOT_DONE, > > + .err = NULL, > > + }; > > + Coroutine *co; > > + int ret; > > + > > + if (!drv) { > > + error_setg(errp, "File '%s' has unknown format", filename); > > + ret = -ENOENT; > > + goto out; > > + } > > + > > I was going to say that ENOENT is a weird error here, but I see it used > for !drv a few other places in block.c too, alongside EINVAL and > ENOMEDIUM. ENOMEDIUM loks like the most popular. > > > + if (!drv->bdrv_co_delete_file) { > > + error_setg(errp, "Driver '%s' does not support image delete", > > + drv->format_name); > > + ret = -ENOTSUP; > > + goto out; > > + } > > + > > + if (!bs) { > > + error_setg(errp, "Could not open image '%s' for erasing", > > + filename); > > + ret = 1; > > Please keep all errors negative (or at least consistent within a function). > > > I'm also wondering if we want a version of delete that doesn't try to > open a file directly -- i.e. a version that exists like this: > > bdrv_co_delete_file(BlockDriverState *bs, Error **errp); > > That simply dispatches based on bs->drv to the correct routine. > > Then, you are free to have bdrv_delete_file handle the open (and let the > opening figure out what driver it needs), and just hand off the bds to > bdrv_co_delete_file. > > I'm not the authority for block.c, though, so maaaybe I'm giving you bad > advice here. Kevin's away on PTO for a bit and gave you advice most > recently, so I might try to gently ask him for more feedback next week. Yes, this was definitely the idea I had in mind when I suggested that bdrv_co_delete_file() should take a BDS. And I think the callers that want to call this function (for failures during image creation) all already have a BDS pointer, so nobody will actually need the additional open. const char *filename only works for the local filesystem (and even then I think not for all filenames) and some URLs, so this is not what we want to have in a public interface to identify an image file. Kevin
On 9/3/19 6:22 AM, Kevin Wolf wrote: > Am 29.08.2019 um 04:07 hat John Snow geschrieben: >> >> On 8/7/19 10:21 AM, Daniel Henrique Barboza wrote: >>> Using the new 'bdrv_co_delete_file' interface, bdrv_delete_file >>> can be used in a way similar of the existing bdrv_create_file to >>> to clean up a created file. >>> >>> The logic is also similar to what is already done in bdrv_create_file: >>> a qemu_coroutine is created if needed, a specialized function >>> bdrv_delete_co_entry is used to call the bdrv_co_delete_file >>> co-routine of the driver, if the driver implements it. >>> >>> Suggested-by: Daniel P. Berrangé <berrange@redhat.com> >>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> >>> --- >>> block.c | 77 +++++++++++++++++++++++++++++++++++++++++++ >>> include/block/block.h | 1 + >>> 2 files changed, 78 insertions(+) >>> >>> diff --git a/block.c b/block.c >>> index cbd8da5f3b..1e20250627 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -547,6 +547,83 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) >>> return ret; >>> } >>> >>> +typedef struct DeleteCo { >>> + BlockDriver *drv; >>> + BlockDriverState *bs; >>> + int ret; >>> + Error *err; >>> +} DeleteCo; >>> + >>> +static void coroutine_fn bdrv_delete_co_entry(void *opaque) >>> +{ >>> + Error *local_err = NULL; >>> + DeleteCo *dco = opaque; >>> + >>> + assert(dco->bs); >>> + >>> + dco->ret = dco->drv->bdrv_co_delete_file(dco->bs, &local_err); >>> + error_propagate(&dco->err, local_err); >>> +} >>> + >>> +int bdrv_delete_file(const char *filename, Error **errp) >>> +{ >>> + BlockDriver *drv = bdrv_find_protocol(filename, true, NULL); >>> + BlockDriverState *bs = bdrv_open(filename, NULL, NULL, >>> + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL); >>> + DeleteCo dco = { >>> + .drv = drv, >>> + .bs = bs, >>> + .ret = NOT_DONE, >>> + .err = NULL, >>> + }; >>> + Coroutine *co; >>> + int ret; >>> + >>> + if (!drv) { >>> + error_setg(errp, "File '%s' has unknown format", filename); >>> + ret = -ENOENT; >>> + goto out; >>> + } >>> + >> I was going to say that ENOENT is a weird error here, but I see it used >> for !drv a few other places in block.c too, alongside EINVAL and >> ENOMEDIUM. ENOMEDIUM loks like the most popular. >> >>> + if (!drv->bdrv_co_delete_file) { >>> + error_setg(errp, "Driver '%s' does not support image delete", >>> + drv->format_name); >>> + ret = -ENOTSUP; >>> + goto out; >>> + } >>> + >>> + if (!bs) { >>> + error_setg(errp, "Could not open image '%s' for erasing", >>> + filename); >>> + ret = 1; >> Please keep all errors negative (or at least consistent within a function). >> >> >> I'm also wondering if we want a version of delete that doesn't try to >> open a file directly -- i.e. a version that exists like this: >> >> bdrv_co_delete_file(BlockDriverState *bs, Error **errp); >> >> That simply dispatches based on bs->drv to the correct routine. >> >> Then, you are free to have bdrv_delete_file handle the open (and let the >> opening figure out what driver it needs), and just hand off the bds to >> bdrv_co_delete_file. >> >> I'm not the authority for block.c, though, so maaaybe I'm giving you bad >> advice here. Kevin's away on PTO for a bit and gave you advice most >> recently, so I might try to gently ask him for more feedback next week. > Yes, this was definitely the idea I had in mind when I suggested that > bdrv_co_delete_file() should take a BDS. > > And I think the callers that want to call this function (for failures > during image creation) all already have a BDS pointer, so nobody will > actually need the additional open. > > const char *filename only works for the local filesystem (and even then > I think not for all filenames) and some URLs, so this is not what we > want to have in a public interface to identify an image file. Hmpf, I understood your idead wrong in the v4 review and ended up changing the co_routine (bdrv_co_delete_file) to use the BlockDriverState instead of the public interface bdrv_delete_file that will be called inside crypto.c. I'll respin it again with this change. Thanks for clarifying! > > Kevin
Am 03.09.2019 um 11:55 hat Daniel Henrique Barboza geschrieben: > > > On 9/3/19 6:22 AM, Kevin Wolf wrote: > > Am 29.08.2019 um 04:07 hat John Snow geschrieben: > > > > > > On 8/7/19 10:21 AM, Daniel Henrique Barboza wrote: > > > > Using the new 'bdrv_co_delete_file' interface, bdrv_delete_file > > > > can be used in a way similar of the existing bdrv_create_file to > > > > to clean up a created file. > > > > > > > > The logic is also similar to what is already done in bdrv_create_file: > > > > a qemu_coroutine is created if needed, a specialized function > > > > bdrv_delete_co_entry is used to call the bdrv_co_delete_file > > > > co-routine of the driver, if the driver implements it. > > > > > > > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com> > > > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > > > --- > > > > block.c | 77 +++++++++++++++++++++++++++++++++++++++++++ > > > > include/block/block.h | 1 + > > > > 2 files changed, 78 insertions(+) > > > > > > > > diff --git a/block.c b/block.c > > > > index cbd8da5f3b..1e20250627 100644 > > > > --- a/block.c > > > > +++ b/block.c > > > > @@ -547,6 +547,83 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) > > > > return ret; > > > > } > > > > +typedef struct DeleteCo { > > > > + BlockDriver *drv; > > > > + BlockDriverState *bs; > > > > + int ret; > > > > + Error *err; > > > > +} DeleteCo; > > > > + > > > > +static void coroutine_fn bdrv_delete_co_entry(void *opaque) > > > > +{ > > > > + Error *local_err = NULL; > > > > + DeleteCo *dco = opaque; > > > > + > > > > + assert(dco->bs); > > > > + > > > > + dco->ret = dco->drv->bdrv_co_delete_file(dco->bs, &local_err); > > > > + error_propagate(&dco->err, local_err); > > > > +} > > > > + > > > > +int bdrv_delete_file(const char *filename, Error **errp) > > > > +{ > > > > + BlockDriver *drv = bdrv_find_protocol(filename, true, NULL); > > > > + BlockDriverState *bs = bdrv_open(filename, NULL, NULL, > > > > + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL); > > > > + DeleteCo dco = { > > > > + .drv = drv, > > > > + .bs = bs, > > > > + .ret = NOT_DONE, > > > > + .err = NULL, > > > > + }; > > > > + Coroutine *co; > > > > + int ret; > > > > + > > > > + if (!drv) { > > > > + error_setg(errp, "File '%s' has unknown format", filename); > > > > + ret = -ENOENT; > > > > + goto out; > > > > + } > > > > + > > > I was going to say that ENOENT is a weird error here, but I see it used > > > for !drv a few other places in block.c too, alongside EINVAL and > > > ENOMEDIUM. ENOMEDIUM loks like the most popular. > > > > > > > + if (!drv->bdrv_co_delete_file) { > > > > + error_setg(errp, "Driver '%s' does not support image delete", > > > > + drv->format_name); > > > > + ret = -ENOTSUP; > > > > + goto out; > > > > + } > > > > + > > > > + if (!bs) { > > > > + error_setg(errp, "Could not open image '%s' for erasing", > > > > + filename); > > > > + ret = 1; > > > Please keep all errors negative (or at least consistent within a function). > > > > > > > > > I'm also wondering if we want a version of delete that doesn't try to > > > open a file directly -- i.e. a version that exists like this: > > > > > > bdrv_co_delete_file(BlockDriverState *bs, Error **errp); > > > > > > That simply dispatches based on bs->drv to the correct routine. > > > > > > Then, you are free to have bdrv_delete_file handle the open (and let the > > > opening figure out what driver it needs), and just hand off the bds to > > > bdrv_co_delete_file. > > > > > > I'm not the authority for block.c, though, so maaaybe I'm giving you bad > > > advice here. Kevin's away on PTO for a bit and gave you advice most > > > recently, so I might try to gently ask him for more feedback next week. > > Yes, this was definitely the idea I had in mind when I suggested that > > bdrv_co_delete_file() should take a BDS. > > > > And I think the callers that want to call this function (for failures > > during image creation) all already have a BDS pointer, so nobody will > > actually need the additional open. > > > > const char *filename only works for the local filesystem (and even then > > I think not for all filenames) and some URLs, so this is not what we > > want to have in a public interface to identify an image file. > > Hmpf, I understood your idead wrong in the v4 review and ended up > changing the co_routine (bdrv_co_delete_file) to use the BlockDriverState > instead of the public interface bdrv_delete_file that will be called inside > crypto.c. > > I'll respin it again with this change. Thanks for clarifying! Yes, I see that I only really commented on the BlockDriver callback, so it wasn't very clear what I actually meant. Sorry for that. Kevin
diff --git a/block.c b/block.c index cbd8da5f3b..1e20250627 100644 --- a/block.c +++ b/block.c @@ -547,6 +547,83 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) return ret; } +typedef struct DeleteCo { + BlockDriver *drv; + BlockDriverState *bs; + int ret; + Error *err; +} DeleteCo; + +static void coroutine_fn bdrv_delete_co_entry(void *opaque) +{ + Error *local_err = NULL; + DeleteCo *dco = opaque; + + assert(dco->bs); + + dco->ret = dco->drv->bdrv_co_delete_file(dco->bs, &local_err); + error_propagate(&dco->err, local_err); +} + +int bdrv_delete_file(const char *filename, Error **errp) +{ + BlockDriver *drv = bdrv_find_protocol(filename, true, NULL); + BlockDriverState *bs = bdrv_open(filename, NULL, NULL, + BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL); + DeleteCo dco = { + .drv = drv, + .bs = bs, + .ret = NOT_DONE, + .err = NULL, + }; + Coroutine *co; + int ret; + + if (!drv) { + error_setg(errp, "File '%s' has unknown format", filename); + ret = -ENOENT; + goto out; + } + + if (!drv->bdrv_co_delete_file) { + error_setg(errp, "Driver '%s' does not support image delete", + drv->format_name); + ret = -ENOTSUP; + goto out; + } + + if (!bs) { + error_setg(errp, "Could not open image '%s' for erasing", + filename); + ret = 1; + goto out; + } + + if (qemu_in_coroutine()) { + /* Fast-path if already in coroutine context */ + bdrv_delete_co_entry(&dco); + } else { + co = qemu_coroutine_create(bdrv_delete_co_entry, &dco); + qemu_coroutine_enter(co); + while (dco.ret == NOT_DONE) { + aio_poll(qemu_get_aio_context(), true); + } + } + + ret = dco.ret; + if (ret < 0) { + if (dco.err) { + error_propagate(errp, dco.err); + } else { + error_setg_errno(errp, -ret, "Could not delete image"); + } + } + +out: + bdrv_unref(bs); + return ret; +} + /** * Try to get @bs's logical and physical block size. * On success, store them in @bsz struct and return 0. diff --git a/include/block/block.h b/include/block/block.h index 50a07c1c33..5e83532364 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -369,6 +369,7 @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, Error **errp); void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base); +int bdrv_delete_file(const char *filename, Error **errp); typedef struct BdrvCheckResult {
Using the new 'bdrv_co_delete_file' interface, bdrv_delete_file can be used in a way similar of the existing bdrv_create_file to to clean up a created file. The logic is also similar to what is already done in bdrv_create_file: a qemu_coroutine is created if needed, a specialized function bdrv_delete_co_entry is used to call the bdrv_co_delete_file co-routine of the driver, if the driver implements it. Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- block.c | 77 +++++++++++++++++++++++++++++++++++++++++++ include/block/block.h | 1 + 2 files changed, 78 insertions(+)