Am 25.03.2019 um 13:18 hat Daniel P. Berrangé geschrieben: > On Mon, Mar 25, 2019 at 01:10:46PM +0100, Kevin Wolf wrote: > > Am 22.03.2019 um 18:52 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 LUKS driver. The implementation is provided > > > in a new 'bdrv_co_delete_file_generic' function inside block.c. This > > > function is made public in case other block drivers wants to > > > support this cleanup interface as well. > > > > > > Suggested-by: Daniel P. Berrangé <berra...@redhat.com> > > > Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com> > > > > This is still wrong. Consider a LUKS image that is accessed via http:// > > rather than in a local file. > > > > Instead of the "generic" implementation in block.c (which isn't actually > > generic, but very specific to local files), this needs to be the > > implementation for the file driver in block/file-posix.c. > > > > The call of bdrv_co_delete_file() must then be in the error path of the > > same function that called bdrv_create_file(), such as > > block_crypto_co_create_opts_luks() in your specific example. > > I thought it was previously suggested that we did *not* want to > put the calls in that kind of place, as it would lead us to delete > files that the user had pre-created ?
We definitely don't want to put them in .bdrv_co_create (i.e. the blockdev-create path) because there we only got an opened block node passed and we can't just destroy that node if we can't create a format on it. Here I'm suggesting .bdrv_co_create_opts, though, which has an explicit bdrv_create_file() call in all format drivers (it creates both protocol and format layer at once; this is what qemu-img create/convert use). At the point where bdrv_create_file() returns, the original file is already overwritten or at least truncated, so removing the file in an error path even if it existed before could be reasonable enough. Kevin