From patchwork Thu Mar 28 16:25:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10875287 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5C80513B5 for ; Thu, 28 Mar 2019 16:27:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4856A28908 for ; Thu, 28 Mar 2019 16:27:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3C2E928A71; Thu, 28 Mar 2019 16:27:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 85C0228908 for ; Thu, 28 Mar 2019 16:27:19 +0000 (UTC) Received: from localhost ([127.0.0.1]:39049 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9Xs6-0002Xj-G9 for patchwork-qemu-devel@patchwork.kernel.org; Thu, 28 Mar 2019 12:27:18 -0400 Received: from eggs.gnu.org ([209.51.188.92]:50192) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9Xqk-0001bZ-Dn for qemu-devel@nongnu.org; Thu, 28 Mar 2019 12:25:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h9Xqi-0004NI-M8 for qemu-devel@nongnu.org; Thu, 28 Mar 2019 12:25:54 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:33979) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1h9Xqh-0004Mu-Pf; Thu, 28 Mar 2019 12:25:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=7Ps109C7qn47P+ruAxmyGhOalU8z8WRLsZZVTg2E0TA=; b=GNMZsWVlU/30/lemCvUi3ZG6dnfhKd8Ux77KiPNv9auCLOaQ8zEx4676YyohdUmNXTjJ6UNoC3T214UfLL/1UZTG/ItaiRcG6ozQLmD0+Mu1XTtEfDgskhLF25PGl1j8rxPA5HiBeJl3EGHvek5UlwaXP1XehJUfdPhl6hp5CT3KzBYacfj+rE1GKGWMB8xZCxkFsocpP0jC985gi1WlPHbTGpXewMeCEHqRU2q7ni2GXqH7Kb/1saLrK9uIQS3Zd0X1Vhy1OJScw2p/qNJK0MLLjfqSFtSF8Sivig00ROgNL+mpqpqgBUeBPDSUCvqM9EJoDe1m/JqadRuMnIjnRA==; Received: from 87-100-202-60.bb.dnainternet.fi ([87.100.202.60] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1h9Xqf-0007VJ-PW; Thu, 28 Mar 2019 17:25:49 +0100 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1h9XqR-0003Xy-AU; Thu, 28 Mar 2019 18:25:35 +0200 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 28 Mar 2019 18:25:09 +0200 Message-Id: X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH for-4.0 v2 1/3] block: continue until base is found in bdrv_freeze_backing_chain() et al 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 , Vladimir Sementsov-Ogievskiy , Alberto Garcia , qemu-block@nongnu.org, Max Reitz , Andrey Shinkevich Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP All three functions that handle the BdrvChild.frozen attribute walk the backing chain from 'bs' to 'base' and stop either when 'base' is found or at the end of the chain if 'base' is NULL. However if 'base' is not found then the functions return without errors as if it was NULL. This is wrong: if the caller passed an incorrect parameter that means that there is a bug in the code. Signed-off-by: Alberto Garcia Reviewed-by: Vladimir Sementsov-Ogievskiy --- block.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index 0a93ee9ac8..3050854528 100644 --- a/block.c +++ b/block.c @@ -4218,14 +4218,15 @@ BlockDriverState *bdrv_find_base(BlockDriverState *bs) /* * Return true if at least one of the backing links between @bs and * @base is frozen. @errp is set if that's the case. + * @base must be reachable from @bs, or NULL. */ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, Error **errp) { BlockDriverState *i; - for (i = bs; i != base && i->backing; i = backing_bs(i)) { - if (i->backing->frozen) { + for (i = bs; i != base; i = backing_bs(i)) { + if (i->backing && i->backing->frozen) { error_setg(errp, "Cannot change '%s' link from '%s' to '%s'", i->backing->name, i->node_name, backing_bs(i)->node_name); @@ -4240,6 +4241,7 @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, * Freeze all backing links between @bs and @base. * If any of the links is already frozen the operation is aborted and * none of the links are modified. + * @base must be reachable from @bs, or NULL. * Returns 0 on success. On failure returns < 0 and sets @errp. */ int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, @@ -4251,8 +4253,10 @@ int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, return -EPERM; } - for (i = bs; i != base && i->backing; i = backing_bs(i)) { - i->backing->frozen = true; + for (i = bs; i != base; i = backing_bs(i)) { + if (i->backing) { + i->backing->frozen = true; + } } return 0; @@ -4261,14 +4265,17 @@ int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, /* * Unfreeze all backing links between @bs and @base. The caller must * ensure that all links are frozen before using this function. + * @base must be reachable from @bs, or NULL. */ void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base) { BlockDriverState *i; - for (i = bs; i != base && i->backing; i = backing_bs(i)) { - assert(i->backing->frozen); - i->backing->frozen = false; + for (i = bs; i != base; i = backing_bs(i)) { + if (i->backing) { + assert(i->backing->frozen); + i->backing->frozen = false; + } } } From patchwork Thu Mar 28 16:25:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10875285 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C0A381874 for ; Thu, 28 Mar 2019 16:27:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id ACEFE28A6B for ; Thu, 28 Mar 2019 16:27:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A082028B4D; Thu, 28 Mar 2019 16:27:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9A6CF28A71 for ; Thu, 28 Mar 2019 16:27:20 +0000 (UTC) Received: from localhost ([127.0.0.1]:39051 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9Xs7-0002Xv-RH for patchwork-qemu-devel@patchwork.kernel.org; Thu, 28 Mar 2019 12:27:19 -0400 Received: from eggs.gnu.org ([209.51.188.92]:50191) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9Xqk-0001bY-Dr for qemu-devel@nongnu.org; Thu, 28 Mar 2019 12:25:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h9Xqi-0004NR-Mr for qemu-devel@nongnu.org; Thu, 28 Mar 2019 12:25:54 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:33981) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1h9Xqh-0004Mt-Oe; Thu, 28 Mar 2019 12:25:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=nkWk4JE2tlYtImsap3v9W3cWOko+4zNJcUdfmlS1j68=; b=RjJ4w67NDtm4/zKyyVAgJmirsVzk5xgdbQuEksYtRmFLLVQHm6ylaLdQ3uwYaBTqwKJiYkbDazJ6/eHHw795qz6KpbyrTIG5v4Ev2GMN1MeicQ8SsQyOr9IMa6HS7Sz2t0/qtg/REkUZkcLwJb3I3azwre37/C0XyZ+KG4Ma1BeZJHvwP/uq4xoujaJSdIcvlXCy8Ty86Dj2e4OrrrcJaa8Og0lCrxFp7Y+IdlSypI91wKT4cKaCpUcGHiMykbi9wVp7MoiD4sueTq2d14l9RT8qAtA//asVzyuMWxgdRxJbnN75bhCTn/6BZOFNCMEGvP9nlSUwC4Y3unw9W1VwWg==; Received: from 87-100-202-60.bb.dnainternet.fi ([87.100.202.60] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1h9Xqf-0007VK-QY; Thu, 28 Mar 2019 17:25:49 +0100 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1h9XqR-0003Y0-Bk; Thu, 28 Mar 2019 18:25:35 +0200 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 28 Mar 2019 18:25:10 +0200 Message-Id: <9ddb290573f04bdcf98985e569001efb5d19a9de.1553789986.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH for-4.0 v2 2/3] block: freeze the backing chain earlier in stream_start() 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 , Vladimir Sementsov-Ogievskiy , Alberto Garcia , qemu-block@nongnu.org, Max Reitz , Andrey Shinkevich Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Commit 6585493369819a48d34a86d57ec6b97cb5cd9bc0 added code to freeze the backing chain from 'top' to 'base' for the duration of the block-stream job. The problem is that the freezing happens too late in stream_start(): during the bdrv_reopen_set_read_only() call earlier in that function another job can jump in and remove the base image. If that happens we have an invalid chain and QEMU crashes. This patch puts the bdrv_freeze_backing_chain() call at the beginning of the function. Signed-off-by: Alberto Garcia Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/stream.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/block/stream.c b/block/stream.c index 6253c86fae..bfaebb861a 100644 --- a/block/stream.c +++ b/block/stream.c @@ -238,11 +238,16 @@ void stream_start(const char *job_id, BlockDriverState *bs, BlockDriverState *iter; bool bs_read_only; + if (bdrv_freeze_backing_chain(bs, base, errp) < 0) { + return; + } + /* Make sure that the image is opened in read-write mode */ bs_read_only = bdrv_is_read_only(bs); if (bs_read_only) { if (bdrv_reopen_set_read_only(bs, false, errp) != 0) { - return; + bs_read_only = false; + goto fail; } } @@ -269,11 +274,6 @@ void stream_start(const char *job_id, BlockDriverState *bs, &error_abort); } - if (bdrv_freeze_backing_chain(bs, base, errp) < 0) { - job_early_fail(&s->common.job); - goto fail; - } - s->base = base; s->backing_file_str = g_strdup(backing_file_str); s->bs_read_only = bs_read_only; @@ -288,4 +288,5 @@ fail: if (bs_read_only) { bdrv_reopen_set_read_only(bs, true, NULL); } + bdrv_unfreeze_backing_chain(bs, base); } From patchwork Thu Mar 28 16:25:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alberto Garcia X-Patchwork-Id: 10875283 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 86B2A186D for ; Thu, 28 Mar 2019 16:27:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6E32128A6B for ; Thu, 28 Mar 2019 16:27:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5EE7028BAF; Thu, 28 Mar 2019 16:27:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 85CEE28A6B for ; Thu, 28 Mar 2019 16:27:19 +0000 (UTC) Received: from localhost ([127.0.0.1]:39053 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9Xs7-0002YH-9Y for patchwork-qemu-devel@patchwork.kernel.org; Thu, 28 Mar 2019 12:27:19 -0400 Received: from eggs.gnu.org ([209.51.188.92]:50189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9Xqk-0001bX-DM for qemu-devel@nongnu.org; Thu, 28 Mar 2019 12:25:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h9Xqi-0004NQ-Mc for qemu-devel@nongnu.org; Thu, 28 Mar 2019 12:25:54 -0400 Received: from fanzine.igalia.com ([91.117.99.155]:33988) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1h9Xqi-0004My-4C; Thu, 28 Mar 2019 12:25:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=m/aFFkGCA/OJoAk3GZeKk9aS56ZXz4NI1n41kqcJBKQ=; b=U1wbmwk4FZ0ccMPxKiuAh/lC3Ngyxt+gxv6d16qqMCQ5hhQPe8YVc9vTCThC+bM1Y8AT3xzARWki45Gg076yhCugXp4Cv7Bdi7eOIOvovxWcDQiJtFV2ZB8M/U1uKqbUl1rMm4uZR8+ZSwpx0zLNacjLGNo1oEBarP5YWw8+yaiMu0obtLC5OmAoDycsi0XNjuWPgEcG6Qprp5H2Jvw3iEtTZtYa0j4Upotg7JSQdWPY5D9NL+D30/fLqoPvFM18oLRC6KiPZ5xXm006UYmjkEELuhhp3MlXAeVRKje1lFEVyxHal8MGAGHHvoN655K3OfZ1ZwYGXEi4Ob1fdK8/vA==; Received: from 87-100-202-60.bb.dnainternet.fi ([87.100.202.60] helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1h9Xqg-0007VO-PB; Thu, 28 Mar 2019 17:25:50 +0100 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1h9XqR-0003Y2-D7; Thu, 28 Mar 2019 18:25:35 +0200 From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 28 Mar 2019 18:25:11 +0200 Message-Id: <001949bcb5ba28d72ab14731dbcf8e6b239ebe78.1553789986.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH for-4.0 v2 3/3] block: test block-stream with a base node that is used by block-commit 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 , Vladimir Sementsov-Ogievskiy , Alberto Garcia , qemu-block@nongnu.org, Max Reitz , Andrey Shinkevich Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP The base node of a block-stream operation indicates the first image from the backing chain starting from which no data is copied to the top node. The block-stream job allows others to use that base image, so a second block-stream job could be writing to it at the same time. An important restriction is that the base image must not disappear while the stream job is ongoing. stream_start() freezes the backing chain from top to base with that purpose but it does it too late in the code so there is a race condition there. This bug was fixed in the previous commit, and this patch contains an iotest for this scenario. Signed-off-by: Alberto Garcia Reviewed-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/030 | 17 +++++++++++++++++ tests/qemu-iotests/030.out | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index 276e06b5ba..c6311d1825 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -314,6 +314,23 @@ class TestParallelOps(iotests.QMPTestCase): self.wait_until_completed(drive='commit-drive0') + # In this case the base node of the stream job is the same as the + # top node of commit job. Since block-commit removes the top node + # when it finishes, this is not allowed. + def test_overlapping_4(self): + self.assert_no_active_block_jobs() + + # Commit from node2 into node0 + result = self.vm.qmp('block-commit', device='drive0', top=self.imgs[2], base=self.imgs[0]) + self.assert_qmp(result, 'return', {}) + + # Stream from node2 into node4 + result = self.vm.qmp('block-stream', device='node4', base_node='node2', job_id='node4') + self.assert_qmp(result, 'error/class', 'GenericError') + + self.wait_until_completed() + self.assert_no_active_block_jobs() + # Test a block-stream and a block-commit job in parallel # Here the stream job is supposed to finish quickly in order to reproduce # the scenario that triggers the bug fixed in 3d5d319e1221 and 1a63a907507 diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out index 42314e9c00..4fd1c2dcd2 100644 --- a/tests/qemu-iotests/030.out +++ b/tests/qemu-iotests/030.out @@ -1,5 +1,5 @@ -........................ +......................... ---------------------------------------------------------------------- -Ran 24 tests +Ran 25 tests OK