[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: Daniel Henrique Barboza
Subject: Re: [Qemu-block] [PATCH v2 1/3] block: introducing 'bdrv_co_delete_file' interface
Date: Mon, 25 Mar 2019 10:12:52 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0

On 3/25/19 9:10 AM, 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.

I'll move the code there in the next version.

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.

That makes sense for the problem that I'm aiming to fix.

However, since we're moving the code to file-posix.c, keeping the
bdrv_co_delete_file() in img_create will fix the issue for all other
formats that ends up using the file driver as well. ATM I have no
evidence that this error might happen with anything but LUKS,
but if/when it does, the code can handle it.

Now, making a counter-argument for what I just wrote, there are
more callers of bdrv_create besides img_create:

- bdrv_append_temp_snapshot
- bdrv_create_file
- bdrv_img_create
- enable_write_target (vvfat)
- img_convert
- img_dd

Moving the delete code from img_create to block_crypto_co_create_opts_luks(),
as you suggested,  will fix the case we know it can be problematic for these
callers as well. The drawback is that we will need to check inside LUKS if
the local file exists beforehand though.




reply via email to

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