From nobody Sat May 18 06:04:18 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1563802483; cv=none; d=zoho.com; s=zohoarc; b=XaPDbfKsuAE2DLRSBiEcg/hwMl3NQo2NtsMGxI+4lP/GgTqif3wMj9wbezI2/NbXY3COo+vrgFFnd2iocaeXWknBe1YTjjEpqOgiZohyXcwXyrwtZbie5CgxXGHoBD4maCvuODQJRY+/HA9OHkiTS25v3DlTH7sDAOqDZYTGRfU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1563802483; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=fTiJBYyr/+4YvwamZbj7bLCpEZQ6tWg7hC5KVcpyzCM=; b=ldKqmopJ+tPzJeK1v9QVjATOEzC3BrfGrDHQSIKvZ4BXILpayDmn2rnVjcqXuskMq/jie1sKa5kvcfH4wot6vuFZiGv17YPR/AarMS7/gKhDhsbZoOJ5Xpi5tDssuV5qfyzv3wXn51xYeSWTWTzW9AjAuuhcmvttf8asdWJmc68= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1563802483256394.04710618987394; Mon, 22 Jul 2019 06:34:43 -0700 (PDT) Received: from localhost ([::1]:33636 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYSf-00086b-Ms for importer@patchew.org; Mon, 22 Jul 2019 09:34:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52752) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYSF-00075d-Kl for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpYSD-00053p-Hb for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52922) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpYS7-0004tz-Ll; Mon, 22 Jul 2019 09:34:09 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6D0C7308222F; Mon, 22 Jul 2019 13:34:06 +0000 (UTC) Received: from localhost (unknown [10.40.205.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BF6425C548; Mon, 22 Jul 2019 13:34:03 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 22 Jul 2019 15:33:43 +0200 Message-Id: <20190722133347.15122-2-mreitz@redhat.com> In-Reply-To: <20190722133347.15122-1-mreitz@redhat.com> References: <20190722133347.15122-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Mon, 22 Jul 2019 13:34:06 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 v3 1/5] block: Keep subtree drained in drop_intermediate X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" bdrv_drop_intermediate() calls BdrvChildRole.update_filename(). That may poll, thus changing the graph, which potentially breaks the QLIST_FOREACH_SAFE() loop. Just keep the whole subtree drained. This is probably the right thing to do anyway (dropping nodes while the subtree is not drained seems wrong). Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf --- block.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block.c b/block.c index 9c94f7f28a..818885d467 100644 --- a/block.c +++ b/block.c @@ -4499,6 +4499,7 @@ int bdrv_drop_intermediate(BlockDriverState *top, Blo= ckDriverState *base, int ret =3D -EIO; =20 bdrv_ref(top); + bdrv_subtree_drained_begin(top); =20 if (!top->drv || !base->drv) { goto exit; @@ -4570,6 +4571,7 @@ int bdrv_drop_intermediate(BlockDriverState *top, Blo= ckDriverState *base, =20 ret =3D 0; exit: + bdrv_subtree_drained_end(top); bdrv_unref(top); return ret; } --=20 2.21.0 From nobody Sat May 18 06:04:18 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1563802492; cv=none; d=zoho.com; s=zohoarc; b=cJN80QXkPpAGNwhjxA1WJOOaG2b5yPj3pBfC0c71UUd+JLOLN2VMLQ/okEtVEdWFnlpbSy6OAPeGskLI2YhB/WnNgKh+R1VrQMXPDqga/x5V1ytrzsQSjiL2cL8ALng/oVVXK+t6JtpcbNNxT39xPcyjX/RKMsVNZiPZih2GbT4= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1563802492; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=do+jPpEIJWFAaXSxfdN7yYOw/9ts3aq3KpAJ5AQMNWw=; b=fIHNizn30Dcf/OsQ1XyPZ2dU+lhhD45ci4++eork3cry6ut+HsBuHlGi/fVfM38qIAbBzt4XlIAQuHYt+x20MOdrt0QQwLFiZyzkwmGg57mjIlxEDOMHSswaRvgDuLBIpSQAm9gU/5bhUJb+DveINBuuewCrAyhtT8xKYydBCWg= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1563802492904346.3107314774861; Mon, 22 Jul 2019 06:34:52 -0700 (PDT) Received: from localhost ([::1]:33642 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYSp-0000Im-Ja for importer@patchew.org; Mon, 22 Jul 2019 09:34:51 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52835) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYSO-0007WY-6k for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpYSM-0005Fg-U1 for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53086) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpYSK-0005BT-9u; Mon, 22 Jul 2019 09:34:20 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9B7AC308212D; Mon, 22 Jul 2019 13:34:19 +0000 (UTC) Received: from localhost (unknown [10.40.205.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C697E60A97; Mon, 22 Jul 2019 13:34:15 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 22 Jul 2019 15:33:44 +0200 Message-Id: <20190722133347.15122-3-mreitz@redhat.com> In-Reply-To: <20190722133347.15122-1-mreitz@redhat.com> References: <20190722133347.15122-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Mon, 22 Jul 2019 13:34:19 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 v3 2/5] block: Reduce (un)drains when replacing a child X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Currently, bdrv_replace_child_noperm() undrains the parent until it is completely undrained, then re-drains it after attaching the new child node. This is a problem with bdrv_drop_intermediate(): We want to keep the whole subtree drained, including parents, while the operation is under way. bdrv_replace_child_noperm() breaks this by allowing every parent to become unquiesced briefly, and then redraining it. In fact, there is no reason why the parent should become unquiesced and be allowed to submit requests to the new child node if that new node is supposed to be kept drained. So if anything, we have to drain the parent before detaching the old child node. Conversely, we have to undrain it only after attaching the new child node. Thus, change the whole drain algorithm here: Calculate the number of times we have to drain/undrain the parent before replacing the child node then drain it (if necessary), replace the child node, and then undrain it. Signed-off-by: Max Reitz --- block.c | 49 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index 818885d467..153e2a8805 100644 --- a/block.c +++ b/block.c @@ -2238,13 +2238,27 @@ static void bdrv_replace_child_noperm(BdrvChild *ch= ild, BlockDriverState *new_bs) { BlockDriverState *old_bs =3D child->bs; - int i; + int new_bs_quiesce_counter; + int drain_saldo; =20 assert(!child->frozen); =20 if (old_bs && new_bs) { assert(bdrv_get_aio_context(old_bs) =3D=3D bdrv_get_aio_context(ne= w_bs)); } + + new_bs_quiesce_counter =3D (new_bs ? new_bs->quiesce_counter : 0); + drain_saldo =3D new_bs_quiesce_counter - child->parent_quiesce_counter; + + /* + * If the new child node is drained but the old one was not, flush + * all outstanding requests to the old child node. + */ + while (drain_saldo > 0 && child->role->drained_begin) { + bdrv_parent_drained_begin_single(child, true); + drain_saldo--; + } + if (old_bs) { /* Detach first so that the recursive drain sections coming from @= child * are already gone and we only end the drain sections that came f= rom @@ -2252,28 +2266,22 @@ static void bdrv_replace_child_noperm(BdrvChild *ch= ild, if (child->role->detach) { child->role->detach(child); } - while (child->parent_quiesce_counter) { - bdrv_parent_drained_end_single(child); - } QLIST_REMOVE(child, next_parent); - } else { - assert(child->parent_quiesce_counter =3D=3D 0); } =20 child->bs =3D new_bs; =20 if (new_bs) { QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); - if (new_bs->quiesce_counter) { - int num =3D new_bs->quiesce_counter; - if (child->role->parent_is_bds) { - num -=3D bdrv_drain_all_count; - } - assert(num >=3D 0); - for (i =3D 0; i < num; i++) { - bdrv_parent_drained_begin_single(child, true); - } - } + + /* + * Detaching the old node may have led to the new node's + * quiesce_counter having been decreased. Not a problem, we + * just need to recognize this here and then invoke + * drained_end appropriately more often. + */ + assert(new_bs->quiesce_counter <=3D new_bs_quiesce_counter); + drain_saldo +=3D new_bs->quiesce_counter - new_bs_quiesce_counter; =20 /* Attach only after starting new drained sections, so that recurs= ive * drain sections coming from @child don't get an extra .drained_b= egin @@ -2282,6 +2290,15 @@ static void bdrv_replace_child_noperm(BdrvChild *chi= ld, child->role->attach(child); } } + + /* + * If the old child node was drained but the new one is not, allow + * requests to come in only after the new node has been attached. + */ + while (drain_saldo < 0 && child->role->drained_end) { + bdrv_parent_drained_end_single(child); + drain_saldo++; + } } =20 /* --=20 2.21.0 From nobody Sat May 18 06:04:18 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1563802524; cv=none; d=zoho.com; s=zohoarc; b=QjDt8MGajmp5bLeF38S4Jpc0aNGf7GsIhn4hFfkqEEfyT7miwpgxA3PxHv2kXCMJ70uIIuyppR8So3r++2Y+i4ISPqJd3GW1IQ7OqEMz8QlDgB/BDV9lJX5It6vSJDCSReSC3KuTFr/8klxgmr9c+AmpG5+usC/R9JsgrOB/H9k= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1563802524; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=UW5mhA+xlPrgIvOx9CR989lUYi0xEbGNS2etwUOHW30=; b=bmc7XC5oPfSKA4jc3XqWxQHz5LxN3ygDwxp032UmnZ3PiTbdypvKLQU01vLZZUtvFxRP5Ch4XXw4UnWngZwxQQ6aSmz9Hzd3JFWrkymVCcZW8piICcSJRG9+JnwhZa+WT/+yObIPdzwAy9yM7O78yXxeyHyzUnMe+LXwlMdlyLs= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1563802524929593.2941694897535; Mon, 22 Jul 2019 06:35:24 -0700 (PDT) Received: from localhost ([::1]:33659 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYTL-000264-Mk for importer@patchew.org; Mon, 22 Jul 2019 09:35:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52894) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYSU-0007uL-GC for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpYST-0005MA-4I for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57308) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpYSQ-0005Io-6K; Mon, 22 Jul 2019 09:34:26 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8184E330265; Mon, 22 Jul 2019 13:34:25 +0000 (UTC) Received: from localhost (unknown [10.40.205.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DA2DF600C4; Mon, 22 Jul 2019 13:34:23 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 22 Jul 2019 15:33:45 +0200 Message-Id: <20190722133347.15122-4-mreitz@redhat.com> In-Reply-To: <20190722133347.15122-1-mreitz@redhat.com> References: <20190722133347.15122-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 22 Jul 2019 13:34:25 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 v3 3/5] tests: Test polling in bdrv_drop_intermediate() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Signed-off-by: Max Reitz --- tests/test-bdrv-drain.c | 167 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 03fa1142a1..1600d41e9a 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -100,6 +100,13 @@ static void bdrv_test_child_perm(BlockDriverState *bs,= BdrvChild *c, nperm, nshared); } =20 +static int bdrv_test_change_backing_file(BlockDriverState *bs, + const char *backing_file, + const char *backing_fmt) +{ + return 0; +} + static BlockDriver bdrv_test =3D { .format_name =3D "test", .instance_size =3D sizeof(BDRVTestState), @@ -111,6 +118,8 @@ static BlockDriver bdrv_test =3D { .bdrv_co_drain_end =3D bdrv_test_co_drain_end, =20 .bdrv_child_perm =3D bdrv_test_child_perm, + + .bdrv_change_backing_file =3D bdrv_test_change_backing_file, }; =20 static void aio_ret_cb(void *opaque, int ret) @@ -1671,6 +1680,161 @@ static void test_blockjob_commit_by_drained_end(voi= d) bdrv_unref(bs_child); } =20 + +typedef struct TestSimpleBlockJob { + BlockJob common; + bool should_complete; + bool *did_complete; +} TestSimpleBlockJob; + +static int coroutine_fn test_simple_job_run(Job *job, Error **errp) +{ + TestSimpleBlockJob *s =3D container_of(job, TestSimpleBlockJob, common= .job); + + while (!s->should_complete) { + job_sleep_ns(job, 0); + } + + return 0; +} + +static void test_simple_job_clean(Job *job) +{ + TestSimpleBlockJob *s =3D container_of(job, TestSimpleBlockJob, common= .job); + *s->did_complete =3D true; +} + +static const BlockJobDriver test_simple_job_driver =3D { + .job_driver =3D { + .instance_size =3D sizeof(TestSimpleBlockJob), + .free =3D block_job_free, + .user_resume =3D block_job_user_resume, + .drain =3D block_job_drain, + .run =3D test_simple_job_run, + .clean =3D test_simple_job_clean, + }, +}; + +static int drop_intermediate_poll_update_filename(BdrvChild *child, + BlockDriverState *new_ba= se, + const char *filename, + Error **errp) +{ + /* + * We are free to poll here, which may change the block graph, if + * it is not drained. + */ + + /* If the job is not drained: Complete it, schedule job_exit() */ + aio_poll(qemu_get_current_aio_context(), false); + /* If the job is not drained: Run job_exit(), finish the job */ + aio_poll(qemu_get_current_aio_context(), false); + + return 0; +} + +/** + * Test a poll in the midst of bdrv_drop_intermediate(). + * + * bdrv_drop_intermediate() calls BdrvChildRole.update_filename(), + * which can yield or poll. This may lead to graph changes, unless + * the whole subtree in question is drained. + * + * We test this on the following graph: + * + * Job + * + * | + * job-node + * | + * v + * + * job-node + * + * | + * backing + * | + * v + * + * node-2 --chain--> node-1 --chain--> node-0 + * + * We drop node-1 with bdrv_drop_intermediate(top=3Dnode-1, base=3Dnode-0). + * + * This first updates node-2's backing filename by invoking + * drop_intermediate_poll_update_filename(), which polls twice. This + * causes the job to finish, which in turns causes the job-node to be + * deleted. + * + * bdrv_drop_intermediate() uses a QLIST_FOREACH_SAFE() loop, so it + * already has a pointer to the BdrvChild edge between job-node and + * node-1. When it tries to handle that edge, we probably get a + * segmentation fault because the object no longer exists. + * + * + * The solution is for bdrv_drop_intermediate() to drain top's + * subtree. This prevents graph changes from happening just because + * BdrvChildRole.update_filename() yields or polls. Thus, the block + * job is paused during that drained section and must finish before or + * after. + * + * (In addition, bdrv_replace_child() must keep the job paused.) + */ +static void test_drop_intermediate_poll(void) +{ + static BdrvChildRole chain_child_role; + BlockDriverState *chain[3]; + TestSimpleBlockJob *job; + BlockDriverState *job_node; + bool job_has_completed =3D false; + int i; + int ret; + + chain_child_role =3D child_backing; + chain_child_role.update_filename =3D drop_intermediate_poll_update_fil= ename; + + for (i =3D 0; i < 3; i++) { + char name[32]; + snprintf(name, 32, "node-%i", i); + + chain[i] =3D bdrv_new_open_driver(&bdrv_test, name, 0, &error_abor= t); + } + + job_node =3D bdrv_new_open_driver(&bdrv_test, "job-node", BDRV_O_RDWR, + &error_abort); + bdrv_set_backing_hd(job_node, chain[1], &error_abort); + + /* + * Establish the chain last, so the chain links are the first + * elements in the BDS.parents lists + */ + for (i =3D 0; i < 3; i++) { + if (i) { + /* Takes the reference to chain[i - 1] */ + chain[i]->backing =3D bdrv_attach_child(chain[i], chain[i - 1], + "chain", &chain_child_ro= le, + &error_abort); + } + } + + job =3D block_job_create("job", &test_simple_job_driver, NULL, job_nod= e, + 0, BLK_PERM_ALL, 0, 0, NULL, NULL, &error_abort= ); + + /* The job has a reference now */ + bdrv_unref(job_node); + + job->did_complete =3D &job_has_completed; + + job_start(&job->common.job); + job->should_complete =3D true; + + g_assert(!job_has_completed); + ret =3D bdrv_drop_intermediate(chain[1], chain[0], NULL); + g_assert(ret =3D=3D 0); + g_assert(job_has_completed); + + bdrv_unref(chain[2]); +} + int main(int argc, char **argv) { int ret; @@ -1757,6 +1921,9 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/blockjob/commit_by_drained_end", test_blockjob_commit_by_drained_end); =20 + g_test_add_func("/bdrv-drain/bdrv_drop_intermediate/poll", + test_drop_intermediate_poll); + ret =3D g_test_run(); qemu_event_destroy(&done_event); return ret; --=20 2.21.0 From nobody Sat May 18 06:04:18 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1563802518; cv=none; d=zoho.com; s=zohoarc; b=aqbKjDJX/saXWZAxC6MV2dZT6BUG73Wlgp3vjDC4LtMOAhG6fa4nDdBq5LJ1BGvEeWWQbmHhmzguywxUFngSEZHP5+6PMKxOe5Ony4Tc6qro5RIi/FzzHDoTeVjacszKlHGKjZeKSdgdm4pGp+53BWMBlaKY9N4WWNshY9lQreY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1563802518; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=Xrz3fp7mgfEpTVsMqnDJSwx2+Z3LXRX9aecqhO8m1kg=; b=fsN/0jqOvfwomGwqYTd9n8QB4nkPcV5Xh36BGRqsTF1dGJZIcmKDOG4QmfeXnXQD3kqIBj269vJrJydZA9yZ/J0MNBIT1xe+aXMTSoKtBQ7SmYKlN5r9A7owWVEPBbyEW0jPFtQeJvsjp9d2S8X2bTffmHS/Rl4VFbcjZyT8YzE= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1563802518420349.4876625541956; Mon, 22 Jul 2019 06:35:18 -0700 (PDT) Received: from localhost ([::1]:33656 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYTF-0001kb-2t for importer@patchew.org; Mon, 22 Jul 2019 09:35:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52953) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYSb-0008GT-8R for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpYSZ-0005Ru-EB for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:24701) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpYSV-0005NY-6X; Mon, 22 Jul 2019 09:34:31 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 80CFC3082E23; Mon, 22 Jul 2019 13:34:30 +0000 (UTC) Received: from localhost (unknown [10.40.205.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7BC8A60BF1; Mon, 22 Jul 2019 13:34:29 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 22 Jul 2019 15:33:46 +0200 Message-Id: <20190722133347.15122-5-mreitz@redhat.com> In-Reply-To: <20190722133347.15122-1-mreitz@redhat.com> References: <20190722133347.15122-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Mon, 22 Jul 2019 13:34:30 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 v3 4/5] tests: Test mid-drain bdrv_replace_child_noperm() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Add a test for what happens when you call bdrv_replace_child_noperm() for various drain situations ({old,new} child {drained,not drained}). Most importantly, if both the old and the new child are drained, the parent must not be undrained at any point. Signed-off-by: Max Reitz --- tests/test-bdrv-drain.c | 308 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 308 insertions(+) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 1600d41e9a..9dffd86c47 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -1835,6 +1835,311 @@ static void test_drop_intermediate_poll(void) bdrv_unref(chain[2]); } =20 + +typedef struct BDRVReplaceTestState { + bool was_drained; + bool was_undrained; + bool has_read; + + int drain_count; + + bool yield_before_read; + Coroutine *io_co; + Coroutine *drain_co; +} BDRVReplaceTestState; + +static void bdrv_replace_test_close(BlockDriverState *bs) +{ +} + +/** + * If @bs has a backing file: + * Yield if .yield_before_read is true (and wait for drain_begin to + * wake us up). + * Forward the read to bs->backing. Set .has_read to true. + * If drain_begin has woken us, wake it in turn. + * + * Otherwise: + * Set .has_read to true and return success. + */ +static int coroutine_fn bdrv_replace_test_co_preadv(BlockDriverState *bs, + uint64_t offset, + uint64_t bytes, + QEMUIOVector *qiov, + int flags) +{ + BDRVReplaceTestState *s =3D bs->opaque; + + if (bs->backing) { + int ret; + + g_assert(!s->drain_count); + + s->io_co =3D qemu_coroutine_self(); + if (s->yield_before_read) { + s->yield_before_read =3D false; + qemu_coroutine_yield(); + } + s->io_co =3D NULL; + + ret =3D bdrv_preadv(bs->backing, offset, qiov); + s->has_read =3D true; + + /* Wake up drain_co if it runs */ + if (s->drain_co) { + aio_co_wake(s->drain_co); + } + + return ret; + } + + s->has_read =3D true; + return 0; +} + +/** + * If .drain_count is 0, wake up .io_co if there is one; and set + * .was_drained. + * Increment .drain_count. + */ +static void coroutine_fn bdrv_replace_test_co_drain_begin(BlockDriverState= *bs) +{ + BDRVReplaceTestState *s =3D bs->opaque; + + if (!s->drain_count) { + /* Keep waking io_co up until it is done */ + s->drain_co =3D qemu_coroutine_self(); + while (s->io_co) { + aio_co_wake(s->io_co); + s->io_co =3D NULL; + qemu_coroutine_yield(); + } + s->drain_co =3D NULL; + + s->was_drained =3D true; + } + s->drain_count++; +} + +/** + * Reduce .drain_count, set .was_undrained once it reaches 0. + * If .drain_count reaches 0 and the node has a backing file, issue a + * read request. + */ +static void coroutine_fn bdrv_replace_test_co_drain_end(BlockDriverState *= bs) +{ + BDRVReplaceTestState *s =3D bs->opaque; + + g_assert(s->drain_count > 0); + if (!--s->drain_count) { + int ret; + + s->was_undrained =3D true; + + if (bs->backing) { + char data; + QEMUIOVector qiov =3D QEMU_IOVEC_INIT_BUF(qiov, &data, 1); + + /* Queue a read request post-drain */ + ret =3D bdrv_replace_test_co_preadv(bs, 0, 1, &qiov, 0); + g_assert(ret >=3D 0); + } + } +} + +static BlockDriver bdrv_replace_test =3D { + .format_name =3D "replace_test", + .instance_size =3D sizeof(BDRVReplaceTestState), + + .bdrv_close =3D bdrv_replace_test_close, + .bdrv_co_preadv =3D bdrv_replace_test_co_preadv, + + .bdrv_co_drain_begin =3D bdrv_replace_test_co_drain_begin, + .bdrv_co_drain_end =3D bdrv_replace_test_co_drain_end, + + .bdrv_child_perm =3D bdrv_format_default_perms, +}; + +static void coroutine_fn test_replace_child_mid_drain_read_co(void *opaque) +{ + int ret; + char data; + + ret =3D blk_co_pread(opaque, 0, 1, &data, 0); + g_assert(ret >=3D 0); +} + +/** + * We test two things: + * (1) bdrv_replace_child_noperm() must not undrain the parent if both + * children are drained. + * (2) bdrv_replace_child_noperm() must never flush I/O requests to a + * drained child. If the old child is drained, it must flush I/O + * requests after the new one has been attached. If the new child + * is drained, it must flush I/O requests before the old one is + * detached. + * + * To do so, we create one parent node and two child nodes; then + * attach one of the children (old_child_bs) to the parent, then + * drain both old_child_bs and new_child_bs according to + * old_drain_count and new_drain_count, respectively, and finally + * we invoke bdrv_replace_node() to replace old_child_bs by + * new_child_bs. + * + * The test block driver we use here (bdrv_replace_test) has a read + * function that: + * - For the parent node, can optionally yield, and then forwards the + * read to bdrv_preadv(), + * - For the child node, just returns immediately. + * + * If the read yields, the drain_begin function will wake it up. + * + * The drain_end function issues a read on the parent once it is fully + * undrained (which simulates requests starting to come in again). + */ +static void do_test_replace_child_mid_drain(int old_drain_count, + int new_drain_count) +{ + BlockBackend *parent_blk; + BlockDriverState *parent_bs; + BlockDriverState *old_child_bs, *new_child_bs; + BDRVReplaceTestState *parent_s; + BDRVReplaceTestState *old_child_s, *new_child_s; + Coroutine *io_co; + int i; + + parent_bs =3D bdrv_new_open_driver(&bdrv_replace_test, "parent", 0, + &error_abort); + parent_s =3D parent_bs->opaque; + + parent_blk =3D blk_new(qemu_get_aio_context(), + BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL); + blk_insert_bs(parent_blk, parent_bs, &error_abort); + + old_child_bs =3D bdrv_new_open_driver(&bdrv_replace_test, "old-child",= 0, + &error_abort); + new_child_bs =3D bdrv_new_open_driver(&bdrv_replace_test, "new-child",= 0, + &error_abort); + old_child_s =3D old_child_bs->opaque; + new_child_s =3D new_child_bs->opaque; + + /* So that we can read something */ + parent_bs->total_sectors =3D 1; + old_child_bs->total_sectors =3D 1; + new_child_bs->total_sectors =3D 1; + + bdrv_ref(old_child_bs); + parent_bs->backing =3D bdrv_attach_child(parent_bs, old_child_bs, "chi= ld", + &child_backing, &error_abort); + + for (i =3D 0; i < old_drain_count; i++) { + bdrv_drained_begin(old_child_bs); + } + for (i =3D 0; i < new_drain_count; i++) { + bdrv_drained_begin(new_child_bs); + } + + if (!old_drain_count) { + /* + * Start a read operation that will yield, so it will not + * complete before the node is drained. + */ + parent_s->yield_before_read =3D true; + io_co =3D qemu_coroutine_create(test_replace_child_mid_drain_read_= co, + parent_blk); + qemu_coroutine_enter(io_co); + } + + /* If we have started a read operation, it should have yielded */ + g_assert(!parent_s->has_read); + + /* Reset drained status so we can see what bdrv_replace_node() does */ + parent_s->was_drained =3D false; + parent_s->was_undrained =3D false; + + g_assert(parent_bs->quiesce_counter =3D=3D old_drain_count); + bdrv_replace_node(old_child_bs, new_child_bs, &error_abort); + g_assert(parent_bs->quiesce_counter =3D=3D new_drain_count); + + if (!old_drain_count && !new_drain_count) { + /* + * From undrained to undrained drains and undrains the parent, + * because bdrv_replace_node() contains a drained section for + * @old_child_bs. + */ + g_assert(parent_s->was_drained && parent_s->was_undrained); + } else if (!old_drain_count && new_drain_count) { + /* + * From undrained to drained should drain the parent and keep + * it that way. + */ + g_assert(parent_s->was_drained && !parent_s->was_undrained); + } else if (old_drain_count && !new_drain_count) { + /* + * From drained to undrained should undrain the parent and + * keep it that way. + */ + g_assert(!parent_s->was_drained && parent_s->was_undrained); + } else /* if (old_drain_count && new_drain_count) */ { + /* + * From drained to drained must not undrain the parent at any + * point + */ + g_assert(!parent_s->was_drained && !parent_s->was_undrained); + } + + if (!old_drain_count || !new_drain_count) { + /* + * If !old_drain_count, we have started a read request before + * bdrv_replace_node(). If !new_drain_count, the parent must + * have been undrained at some point, and + * bdrv_replace_test_co_drain_end() starts a read request + * then. + */ + g_assert(parent_s->has_read); + } else { + /* + * If the parent was never undrained, there is no way to start + * a read request. + */ + g_assert(!parent_s->has_read); + } + + /* A drained child must have not received any request */ + g_assert(!(old_drain_count && old_child_s->has_read)); + g_assert(!(new_drain_count && new_child_s->has_read)); + + for (i =3D 0; i < new_drain_count; i++) { + bdrv_drained_end(new_child_bs); + } + for (i =3D 0; i < old_drain_count; i++) { + bdrv_drained_end(old_child_bs); + } + + /* + * By now, bdrv_replace_test_co_drain_end() must have been called + * at some point while the new child was attached to the parent. + */ + g_assert(parent_s->has_read); + g_assert(new_child_s->has_read); + + blk_unref(parent_blk); + bdrv_unref(parent_bs); + bdrv_unref(old_child_bs); + bdrv_unref(new_child_bs); +} + +static void test_replace_child_mid_drain(void) +{ + int old_drain_count, new_drain_count; + + for (old_drain_count =3D 0; old_drain_count < 2; old_drain_count++) { + for (new_drain_count =3D 0; new_drain_count < 2; new_drain_count++= ) { + do_test_replace_child_mid_drain(old_drain_count, new_drain_cou= nt); + } + } +} + int main(int argc, char **argv) { int ret; @@ -1924,6 +2229,9 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/bdrv_drop_intermediate/poll", test_drop_intermediate_poll); =20 + g_test_add_func("/bdrv-drain/replace_child/mid-drain", + test_replace_child_mid_drain); + ret =3D g_test_run(); qemu_event_destroy(&done_event); return ret; --=20 2.21.0 From nobody Sat May 18 06:04:18 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1563802541; cv=none; d=zoho.com; s=zohoarc; b=GYvOtm7ip3yID3C+2LykoVKoaoToilkEfySRUF9JeTOk1qCARKXHycS0ENQae42dNuNpU0B4LzxKisYbTi7wLcHXxMZQMEkbBWi5PxELW3kJjLOogAM8S6WsrzN5odSWNRvp2AuZg1q3bTEktWDlgfPoWjrr79u62TsAsFFWAVk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1563802541; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=2WkCVpwIu65GlPOs6xtxv/JMWAFgBlt5SimtkBIojAE=; b=YTe4Siq/22QlD533SoLUsRCU8yptVKhfW+KUsI6AAhIysB5LvrwYlz/d5dcRYDUbWiSBrjY+7AwQOMC2aH53QEP1+yG9qtVQuWoicW8DtTqXan+Ql9dWnTicNAgs+mID/tVO1aSDT/2PHOZm6drH6W1Wz5FZrKjht/UkDrQfyHc= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1563802541968622.1325042234448; Mon, 22 Jul 2019 06:35:41 -0700 (PDT) Received: from localhost ([::1]:33666 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYTc-0003IU-Sq for importer@patchew.org; Mon, 22 Jul 2019 09:35:40 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53027) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpYSo-0000cj-JJ for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpYSi-0005cq-Nc for qemu-devel@nongnu.org; Mon, 22 Jul 2019 09:34:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45038) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpYSd-0005Ta-4h; Mon, 22 Jul 2019 09:34:39 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 37CBFC065128; Mon, 22 Jul 2019 13:34:37 +0000 (UTC) Received: from localhost (unknown [10.40.205.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BE6DF608A5; Mon, 22 Jul 2019 13:34:35 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 22 Jul 2019 15:33:47 +0200 Message-Id: <20190722133347.15122-6-mreitz@redhat.com> In-Reply-To: <20190722133347.15122-1-mreitz@redhat.com> References: <20190722133347.15122-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 22 Jul 2019 13:34:37 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 v3 5/5] iotests: Add test for concurrent stream/commit X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" We already have 030 for that in general, but this tests very specific cases of both jobs finishing concurrently. Signed-off-by: Max Reitz --- tests/qemu-iotests/258 | 163 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/258.out | 33 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 197 insertions(+) create mode 100755 tests/qemu-iotests/258 create mode 100644 tests/qemu-iotests/258.out diff --git a/tests/qemu-iotests/258 b/tests/qemu-iotests/258 new file mode 100755 index 0000000000..b84cf02254 --- /dev/null +++ b/tests/qemu-iotests/258 @@ -0,0 +1,163 @@ +#!/usr/bin/env python +# +# Very specific tests for adjacent commit/stream block jobs +# +# Copyright (C) 2019 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# Creator/Owner: Max Reitz + +import iotests +from iotests import log, qemu_img, qemu_io_silent, \ + filter_qmp_testfiles, filter_qmp_imgfmt + +# Need backing file and change-backing-file support +iotests.verify_image_format(supported_fmts=3D['qcow2', 'qed']) +iotests.verify_platform(['linux']) + + +# Returns a node for blockdev-add +def node(node_name, path, backing=3DNone, fmt=3DNone, throttle=3DNone): + if fmt is None: + fmt =3D iotests.imgfmt + + res =3D { + 'node-name': node_name, + 'driver': fmt, + 'file': { + 'driver': 'file', + 'filename': path + } + } + + if backing is not None: + res['backing'] =3D backing + + if throttle: + res['file'] =3D { + 'driver': 'throttle', + 'throttle-group': throttle, + 'file': res['file'] + } + + return res + +# Finds a node in the debug block graph +def find_graph_node(graph, node_id): + return next(node for node in graph['nodes'] if node['id'] =3D=3D node_= id) + + +def test_concurrent_finish(write_to_stream_node): + log('') + log('=3D=3D=3D Commit and stream finish concurrently (letting %s write= ) =3D=3D=3D' % \ + ('stream' if write_to_stream_node else 'commit')) + log('') + + # All chosen in such a way that when the commit job wants to + # finish, it polls and thus makes stream finish concurrently -- + # and the other way around, depending on whether the commit job + # is finalized before stream completes or not. + + with iotests.FilePath('node4.img') as node4_path, \ + iotests.FilePath('node3.img') as node3_path, \ + iotests.FilePath('node2.img') as node2_path, \ + iotests.FilePath('node1.img') as node1_path, \ + iotests.FilePath('node0.img') as node0_path, \ + iotests.VM() as vm: + + # It is important to use raw for the base layer (so that + # permissions are just handed through to the protocol layer) + assert qemu_img('create', '-f', 'raw', node0_path, '64M') =3D=3D 0 + + stream_throttle=3DNone + commit_throttle=3DNone + + for path in [node1_path, node2_path, node3_path, node4_path]: + assert qemu_img('create', '-f', iotests.imgfmt, path, '64M') = =3D=3D 0 + + if write_to_stream_node: + # This is what (most of the time) makes commit finish + # earlier and then pull in stream + assert qemu_io_silent(node2_path, + '-c', 'write %iK 64K' % (65536 - 192), + '-c', 'write %iK 64K' % (65536 - 64)) = =3D=3D 0 + + stream_throttle=3D'tg' + else: + # And this makes stream finish earlier + assert qemu_io_silent(node1_path, + '-c', 'write %iK 64K' % (65536 - 64)) = =3D=3D 0 + + commit_throttle=3D'tg' + + vm.launch() + + vm.qmp_log('object-add', + qom_type=3D'throttle-group', + id=3D'tg', + props=3D{ + 'x-iops-write': 1, + 'x-iops-write-max': 1 + }) + + vm.qmp_log('blockdev-add', + filters=3D[filter_qmp_testfiles, filter_qmp_imgfmt], + **node('node4', node4_path, throttle=3Dstream_throttle, + backing=3Dnode('node3', node3_path, + backing=3Dnode('node2', node2_path, + backing=3Dnode('node1', node1_path, + backing=3Dnode('node0', node0_path, throttle=3Dcommit= _throttle, + fmt=3D'raw')))))) + + vm.qmp_log('block-commit', + job_id=3D'commit', + device=3D'node4', + filter_node_name=3D'commit-filter', + top_node=3D'node1', + base_node=3D'node0', + auto_finalize=3DFalse) + + vm.qmp_log('block-stream', + job_id=3D'stream', + device=3D'node3', + base_node=3D'commit-filter') + + if write_to_stream_node: + vm.run_job('commit', auto_finalize=3DFalse, auto_dismiss=3DTru= e) + vm.run_job('stream', auto_finalize=3DTrue, auto_dismiss=3DTrue) + else: + # No, the jobs do not really finish concurrently here, + # the stream job does complete strictly before commit. + # But still, this is close enough for what we want to + # test. + vm.run_job('stream', auto_finalize=3DTrue, auto_dismiss=3DTrue) + vm.run_job('commit', auto_finalize=3DFalse, auto_dismiss=3DTru= e) + + # Assert that the backing node of node3 is node 0 now + graph =3D vm.qmp('x-debug-query-block-graph')['return'] + for edge in graph['edges']: + if edge['name'] =3D=3D 'backing' and \ + find_graph_node(graph, edge['parent'])['name'] =3D=3D 'node= 3': + assert find_graph_node(graph, edge['child'])['name'] =3D= =3D 'node0' + break + + +def main(): + log('Running tests:') + test_concurrent_finish(True) + test_concurrent_finish(False) + +if __name__ =3D=3D '__main__': + main() diff --git a/tests/qemu-iotests/258.out b/tests/qemu-iotests/258.out new file mode 100644 index 0000000000..ce6e9ba3e5 --- /dev/null +++ b/tests/qemu-iotests/258.out @@ -0,0 +1,33 @@ +Running tests: + +=3D=3D=3D Commit and stream finish concurrently (letting stream write) =3D= =3D=3D + +{"execute": "object-add", "arguments": {"id": "tg", "props": {"x-iops-writ= e": 1, "x-iops-write-max": 1}, "qom-type": "throttle-group"}} +{"return": {}} +{"execute": "blockdev-add", "arguments": {"backing": {"backing": {"backing= ": {"backing": {"driver": "raw", "file": {"driver": "file", "filename": "TE= ST_DIR/PID-node0.img"}, "node-name": "node0"}, "driver": "IMGFMT", "file": = {"driver": "file", "filename": "TEST_DIR/PID-node1.img"}, "node-name": "nod= e1"}, "driver": "IMGFMT", "file": {"driver": "file", "filename": "TEST_DIR/= PID-node2.img"}, "node-name": "node2"}, "driver": "IMGFMT", "file": {"drive= r": "file", "filename": "TEST_DIR/PID-node3.img"}, "node-name": "node3"}, "= driver": "IMGFMT", "file": {"driver": "throttle", "file": {"driver": "file"= , "filename": "TEST_DIR/PID-node4.img"}, "throttle-group": "tg"}, "node-nam= e": "node4"}} +{"return": {}} +{"execute": "block-commit", "arguments": {"auto-finalize": false, "base-no= de": "node0", "device": "node4", "filter-node-name": "commit-filter", "job-= id": "commit", "top-node": "node1"}} +{"return": {}} +{"execute": "block-stream", "arguments": {"base-node": "commit-filter", "d= evice": "node3", "job-id": "stream"}} +{"return": {}} +{"execute": "job-finalize", "arguments": {"id": "commit"}} +{"return": {}} +{"data": {"id": "commit", "type": "commit"}, "event": "BLOCK_JOB_PENDING",= "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"data": {"device": "commit", "len": 67108864, "offset": 67108864, "speed"= : 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"micr= oseconds": "USECS", "seconds": "SECS"}} +{"data": {"device": "stream", "len": 67108864, "offset": 67108864, "speed"= : 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"micr= oseconds": "USECS", "seconds": "SECS"}} + +=3D=3D=3D Commit and stream finish concurrently (letting commit write) =3D= =3D=3D + +{"execute": "object-add", "arguments": {"id": "tg", "props": {"x-iops-writ= e": 1, "x-iops-write-max": 1}, "qom-type": "throttle-group"}} +{"return": {}} +{"execute": "blockdev-add", "arguments": {"backing": {"backing": {"backing= ": {"backing": {"driver": "raw", "file": {"driver": "throttle", "file": {"d= river": "file", "filename": "TEST_DIR/PID-node0.img"}, "throttle-group": "t= g"}, "node-name": "node0"}, "driver": "IMGFMT", "file": {"driver": "file", = "filename": "TEST_DIR/PID-node1.img"}, "node-name": "node1"}, "driver": "IM= GFMT", "file": {"driver": "file", "filename": "TEST_DIR/PID-node2.img"}, "n= ode-name": "node2"}, "driver": "IMGFMT", "file": {"driver": "file", "filena= me": "TEST_DIR/PID-node3.img"}, "node-name": "node3"}, "driver": "IMGFMT", = "file": {"driver": "file", "filename": "TEST_DIR/PID-node4.img"}, "node-nam= e": "node4"}} +{"return": {}} +{"execute": "block-commit", "arguments": {"auto-finalize": false, "base-no= de": "node0", "device": "node4", "filter-node-name": "commit-filter", "job-= id": "commit", "top-node": "node1"}} +{"return": {}} +{"execute": "block-stream", "arguments": {"base-node": "commit-filter", "d= evice": "node3", "job-id": "stream"}} +{"return": {}} +{"data": {"device": "stream", "len": 67108864, "offset": 67108864, "speed"= : 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"micr= oseconds": "USECS", "seconds": "SECS"}} +{"execute": "job-finalize", "arguments": {"id": "commit"}} +{"return": {}} +{"data": {"id": "commit", "type": "commit"}, "event": "BLOCK_JOB_PENDING",= "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} +{"data": {"device": "commit", "len": 67108864, "offset": 67108864, "speed"= : 0, "type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"micr= oseconds": "USECS", "seconds": "SECS"}} diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index b34c8e3c0c..a854ecdc80 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -269,3 +269,4 @@ 254 rw auto backing quick 255 rw auto quick 256 rw auto quick +258 rw quick --=20 2.21.0