[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Close backing file early in bdrv_img_cre
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] block: Close backing file early in bdrv_img_create |
Date: |
Thu, 28 Nov 2013 11:28:43 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 27.11.2013 um 21:05 hat Max Reitz geschrieben:
> Leaving the backing file open although it is not needed anymore can
> cause problems if it is opened through a block driver which allows
> exclusive access only and if the create function of the block driver
> used for the top image (the one being created) tries to close and reopen
> the image file (which will include opening the backing file a second
> time).
>
> In particular, this will happen with a backing file opened through
> qemu-nbd and using qcow2 as the top image file format (which reopens the
> image to flush it to disk).
>
> In addition, the BlockDriverState in bdrv_img_create() is used for the
> backing file only; it should therefore be made local to the respective
> block.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index 382ea71..e5a8a4c 100644
> --- a/block.c
> +++ b/block.c
> @@ -4504,7 +4504,6 @@ void bdrv_img_create(const char *filename, const char
> *fmt,
> {
> QEMUOptionParameter *param = NULL, *create_options = NULL;
> QEMUOptionParameter *backing_fmt, *backing_file, *size;
> - BlockDriverState *bs = NULL;
> BlockDriver *drv, *proto_drv;
> BlockDriver *backing_drv = NULL;
> Error *local_err = NULL;
> @@ -4583,6 +4582,7 @@ void bdrv_img_create(const char *filename, const char
> *fmt,
> size = get_option_parameter(param, BLOCK_OPT_SIZE);
> if (size && size->value.n == -1) {
> if (backing_file && backing_file->value.s) {
> + BlockDriverState *bs;
> uint64_t size;
> char buf[32];
> int back_flags;
More context:
/* backing files always opened read-only */
back_flags =
flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
bs = bdrv_new("");
ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
backing_drv, &local_err);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not open '%s': %s",
backing_file->value.s,
error_get_pretty(local_err));
error_free(local_err);
local_err = NULL;
goto out;
}
bdrv_get_geometry(bs, &size);
size *= 512;
> @@ -4608,6 +4608,8 @@ void bdrv_img_create(const char *filename, const char
> *fmt,
>
> snprintf(buf, sizeof(buf), "%" PRId64, size);
> set_option_parameter(param, BLOCK_OPT_SIZE, buf);
> +
> + bdrv_unref(bs);
> } else {
> error_setg(errp, "Image creation needs a size parameter");
> goto out;
bs is now leaked if bdrv_open() fails.
Kevin