From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vlyq5-000382-0k for qemu-devel@nongnu.org; Thu, 28 Nov 2013 05:28:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vlypy-0004kE-Gf for qemu-devel@nongnu.org; Thu, 28 Nov 2013 05:28:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38396) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vlypy-0004k1-8m for qemu-devel@nongnu.org; Thu, 28 Nov 2013 05:28:46 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rASASjxY006303 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 28 Nov 2013 05:28:45 -0500 Date: Thu, 28 Nov 2013 11:28:43 +0100 From: Kevin Wolf Message-ID: <20131128102843.GA4805@dhcp-200-207.str.redhat.com> References: <1385582728-17932-1-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1385582728-17932-1-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: Close backing file early in bdrv_img_create List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi 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 > --- > 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