From patchwork Mon Jun 6 14:42:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 630929 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rNd5r4Msrz9rxv for ; Tue, 7 Jun 2016 00:54:56 +1000 (AEST) Received: from localhost ([::1]:42953 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9vva-000698-Ki for incoming@patchwork.ozlabs.org; Mon, 06 Jun 2016 10:54:54 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36103) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9vjd-0003rV-Sv for qemu-devel@nongnu.org; Mon, 06 Jun 2016 10:42:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9vjc-0007Rs-NX for qemu-devel@nongnu.org; Mon, 06 Jun 2016 10:42:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56341) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9vjP-0007OU-QY; Mon, 06 Jun 2016 10:42:20 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5E5F0710A6; Mon, 6 Jun 2016 14:42:19 +0000 (UTC) Received: from localhost (ovpn-116-49.ams2.redhat.com [10.36.116.49]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u56EgHHR017442 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 6 Jun 2016 10:42:18 -0400 From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 6 Jun 2016 16:42:11 +0200 Message-Id: <20160606144212.24074-3-mreitz@redhat.com> In-Reply-To: <20160606144212.24074-1-mreitz@redhat.com> References: <20160606144212.24074-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 06 Jun 2016 14:42:19 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 2/3] block/mirror: Fix target backing BDS X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Currently, we are trying to move the backing BDS from the source to the target in bdrv_replace_in_backing_chain() which is called from mirror_exit(). However, mirror_complete() already tries to open the target's backing chain with a call to bdrv_open_backing_file(). First, we should only set the target's backing BDS once. Second, the mirroring block job has a better idea of what to set it to than the generic code in bdrv_replace_in_backing_chain() (in fact, the latter's conditions on when to move the backing BDS from source to target are not really correct). Therefore, remove that code from bdrv_replace_in_backing_chain() and leave it to mirror_complete(). However, mirror_complete() in turn pursues a questionable strategy by employing bdrv_open_backing_file(): On the one hand, because this may open the wrong backing file with drive-mirror in "existing" mode, or because it will not override a possibly wrong backing file in the blockdev-mirror case. On the other hand, we want to reuse the existing backing chain of the source instead of opening everything anew, because the latter results in having multiple BDSs for a single physical file and thus potentially concurrent access which we should try to avoid. Thus, instead of invoking bdrv_open_backing_file(), just set the correct backing BDS directly via bdrv_set_backing_hd(). Also, do so only when mirror_complete() is certain to succeed. In contrast to what bdrv_replace_in_backing_chain() did so far, we do not need to drop the source's backing file. Signed-off-by: Max Reitz --- block.c | 8 -------- block/mirror.c | 21 +++++++++++++-------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index 16463aa..792f5dd 100644 --- a/block.c +++ b/block.c @@ -2288,14 +2288,6 @@ void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new) change_parent_backing_link(old, new); - /* Change backing files if a previously independent node is added to the - * chain. For active commit, we replace top by its own (indirect) backing - * file and don't do anything here so we don't build a loop. */ - if (new->backing == NULL && !bdrv_chain_contains(backing_bs(old), new)) { - bdrv_set_backing_hd(new, backing_bs(old)); - bdrv_set_backing_hd(old, NULL); - } - bdrv_unref(old); } diff --git a/block/mirror.c b/block/mirror.c index 80fd3c7..217475b 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -742,15 +742,11 @@ static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp) static void mirror_complete(BlockJob *job, Error **errp) { MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); - Error *local_err = NULL; - int ret; + BlockDriverState *src, *target; + + src = blk_bs(job->blk); + target = blk_bs(s->target); - ret = bdrv_open_backing_file(blk_bs(s->target), NULL, "backing", - &local_err); - if (ret < 0) { - error_propagate(errp, local_err); - return; - } if (!s->synced) { error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id); return; @@ -777,6 +773,15 @@ static void mirror_complete(BlockJob *job, Error **errp) aio_context_release(replace_aio_context); } + /* Now we need to adjust the target's backing BDS. This is not necessary + * when having performed a commit operation. */ + if (!bdrv_chain_contains(backing_bs(src), target)) { + BlockDriverState *backing = s->is_none_mode ? src : s->base; + if (backing_bs(target) != backing) { + bdrv_set_backing_hd(target, backing); + } + } + s->should_complete = true; block_job_enter(&s->common); }