[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v2 1/3] block: introducing 'bdrv_co_delete_file'

From: Kevin Wolf
Subject: Re: [Qemu-block] [PATCH v2 1/3] block: introducing 'bdrv_co_delete_file' interface
Date: Mon, 25 Mar 2019 16:06:09 +0100
User-agent: Mutt/1.11.3 (2019-02-01)

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é <address@hidden>
> > > Signed-off-by: Daniel Henrique Barboza <address@hidden>
> > 
> > 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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]