From patchwork Tue Dec 3 13:57:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 296203 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 139542C008A for ; Wed, 4 Dec 2013 00:58:00 +1100 (EST) Received: from localhost ([::1]:42542 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VnqU9-0007CQ-JM for incoming@patchwork.ozlabs.org; Tue, 03 Dec 2013 08:57:57 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32936) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VnqTm-0007Bp-2A for qemu-devel@nongnu.org; Tue, 03 Dec 2013 08:57:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VnqTe-0007Hx-S0 for qemu-devel@nongnu.org; Tue, 03 Dec 2013 08:57:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55215) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VnqTe-0007Hn-Ka for qemu-devel@nongnu.org; Tue, 03 Dec 2013 08:57:26 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rB3DvFDp015672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 3 Dec 2013 08:57:15 -0500 Received: from localhost (ovpn-116-62.ams2.redhat.com [10.36.116.62]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rB3DvCIq002416 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Tue, 3 Dec 2013 08:57:14 -0500 From: Max Reitz To: qemu-devel@nongnu.org Date: Tue, 3 Dec 2013 14:57:52 +0100 Message-Id: <1386079072-5035-1-git-send-email-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , Max Reitz , Benoit Canet , Stefan Hajnoczi , Wenchao Xia Subject: [Qemu-devel] [PATCH v3] block: Close backing file early in bdrv_img_create X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 Reviewed-by: Kevin Wolf Reviewed-by: Wenchao Xia --- v3: - Reverted the revert in v2 and limited the BlockDriverState once again to the scope its used in (addressing Xia's comment); added a missing bdrv_unref() in contrast to v1 (addressing Kevin's comment to v1) v2: - Minimizing the changes prevents introducing a leak of the BlockDriverState in case of an error in bdrv_open() (thanks, Kevin). --- block.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index 382ea71..0468765 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; @@ -4601,6 +4601,7 @@ void bdrv_img_create(const char *filename, const char *fmt, error_get_pretty(local_err)); error_free(local_err); local_err = NULL; + bdrv_unref(bs); goto out; } bdrv_get_geometry(bs, &size); @@ -4608,6 +4609,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; @@ -4638,9 +4641,6 @@ out: free_option_parameters(create_options); free_option_parameters(param); - if (bs) { - bdrv_unref(bs); - } if (error_is_set(&local_err)) { error_propagate(errp, local_err); }