Message ID | 20190903135708.21624-2-danielhb413@gmail.com |
---|---|
State | New |
Headers | show |
Series | delete created files when block_crypto_co_create_opts_luks fails | expand |
Am 03.09.2019 um 15:57 hat Daniel Henrique Barboza geschrieben: > Adding to Block Drivers the capability of being able to clean up > its created files can be useful in certain situations. For the > LUKS driver, for instance, a failure in one of its authentication > steps can leave files in the host that weren't there before. > > This patch adds the 'bdrv_co_delete_file' interface to block > drivers and add it to the 'file' driver in file-posix.c.The Missing space between "file-posix.c.The" > implementation is given by 'raw_co_delete_file'. > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > --- > block/file-posix.c | 28 ++++++++++++++++++++++++++++ > include/block/block_int.h | 6 ++++++ > 2 files changed, 34 insertions(+) > > diff --git a/block/file-posix.c b/block/file-posix.c > index fbeb0068db..52756de522 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -2390,6 +2390,33 @@ static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts, > return raw_co_create(&options, errp); > } > > +/** > + * Co-routine function that erases a regular file. > + */ We don't have function comments for other BdrvDriver callbacks. It may be appropriate to have a comment when there is something special going on here that differs from a normal implementation. But this specific comment is even redundant with the function name, so I think we don't need it. > +static int coroutine_fn raw_co_delete_file(BlockDriverState *bs, > + Error **errp) > +{ > + struct stat st; > + int ret; > + > + if (!(stat(bs->filename, &st) == 0) || !S_ISREG(st.st_mode)) { > + ret = -ENOENT; > + error_setg_errno(errp, -ret, "%s is not a regular file", > + bs->filename); > + goto done; There is no cleanup code, so a direct return -ENOENT would be simpler. > + } > + > + ret = unlink(bs->filename); > + if (ret < 0) { > + ret = -errno; > + error_setg_errno(errp, -ret, "Error when deleting file %s", > + bs->filename); > + } > + > +done: > + return ret; > +} > + > /* > * Find allocation range in @bs around offset @start. > * May change underlying file descriptor's file offset. > @@ -2942,6 +2969,7 @@ BlockDriver bdrv_file = { > .bdrv_co_block_status = raw_co_block_status, > .bdrv_co_invalidate_cache = raw_co_invalidate_cache, > .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes, > + .bdrv_co_delete_file = raw_co_delete_file, > > .bdrv_co_preadv = raw_co_preadv, > .bdrv_co_pwritev = raw_co_pwritev, > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 0422acdf1c..a959ec2d1e 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -314,6 +314,12 @@ struct BlockDriver { > */ > int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs); > > + /* > + * Delete a local created file. > + */ No reason to restrict this to "local", even if for now file-posix is the only driver that implements it. This can change later. The comment fits in a single line. > + int coroutine_fn (*bdrv_co_delete_file)(BlockDriverState *bs, > + Error **errp); > + Kevin
diff --git a/block/file-posix.c b/block/file-posix.c index fbeb0068db..52756de522 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -2390,6 +2390,33 @@ static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts, return raw_co_create(&options, errp); } +/** + * Co-routine function that erases a regular file. + */ +static int coroutine_fn raw_co_delete_file(BlockDriverState *bs, + Error **errp) +{ + struct stat st; + int ret; + + if (!(stat(bs->filename, &st) == 0) || !S_ISREG(st.st_mode)) { + ret = -ENOENT; + error_setg_errno(errp, -ret, "%s is not a regular file", + bs->filename); + goto done; + } + + ret = unlink(bs->filename); + if (ret < 0) { + ret = -errno; + error_setg_errno(errp, -ret, "Error when deleting file %s", + bs->filename); + } + +done: + return ret; +} + /* * Find allocation range in @bs around offset @start. * May change underlying file descriptor's file offset. @@ -2942,6 +2969,7 @@ BlockDriver bdrv_file = { .bdrv_co_block_status = raw_co_block_status, .bdrv_co_invalidate_cache = raw_co_invalidate_cache, .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes, + .bdrv_co_delete_file = raw_co_delete_file, .bdrv_co_preadv = raw_co_preadv, .bdrv_co_pwritev = raw_co_pwritev, diff --git a/include/block/block_int.h b/include/block/block_int.h index 0422acdf1c..a959ec2d1e 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -314,6 +314,12 @@ struct BlockDriver { */ int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs); + /* + * Delete a local created file. + */ + int coroutine_fn (*bdrv_co_delete_file)(BlockDriverState *bs, + Error **errp); + /* * Flushes all data that was already written to the OS all the way down to * the disk (for example file-posix.c calls fsync()).
Adding to Block Drivers the capability of being able to clean up its created files can be useful in certain situations. For the LUKS driver, for instance, a failure in one of its authentication steps can leave files in the host that weren't there before. This patch adds the 'bdrv_co_delete_file' interface to block drivers and add it to the 'file' driver in file-posix.c.The implementation is given by 'raw_co_delete_file'. Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- block/file-posix.c | 28 ++++++++++++++++++++++++++++ include/block/block_int.h | 6 ++++++ 2 files changed, 34 insertions(+)