From nobody Sat May 4 11:07:29 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=1559751339; cv=none; d=zoho.com; s=zohoarc; b=X+mV1GjyyVi0ew96sLrSh4H/6qr9RHUkgNBWJxDcnpOH5i4eFlTsA77CLwnBkKbpI2Dae3SpP0UJzfaz3exSI8UxaQ6/tnJJf9W4j1g5K4SOU2nqBT1UJ22VcytKtVpS6tEjfz6/RY5CKFZZ+9c6etONgPz4ZJUs/zNnQA8R16M= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1559751339; 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=fQBwaFj90fF+4pTbdizglfrw3zCZqHS2euwkFFgc/a4=; b=iAw7M+cVPE6KFcUJG0CeGqxQBmOiiJ1I+lRoSYyMhq2HzOpyM0x2hPEah0JbBVL3j6DWGEfqGfnM0iPQpr4DvTTVE2vW+c19XChceacoFiWLFAmhvYd6e8Cut+y6xUEScgwtYZcCTyQKZifw/pgji5Hy9oY9vMnvsX/7Bwip2pM= 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 1559751339016378.88395518388086; Wed, 5 Jun 2019 09:15:39 -0700 (PDT) Received: from localhost ([127.0.0.1]:45745 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYZX-0001gy-CK for importer@patchew.org; Wed, 05 Jun 2019 12:15:31 -0400 Received: from eggs.gnu.org ([209.51.188.92]:59924) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYVl-0007SO-K8 for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hYYVj-0002f6-NZ for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43880) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hYYVe-0002T7-TD; Wed, 05 Jun 2019 12:11:31 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2E7F3981D3; Wed, 5 Jun 2019 16:11:30 +0000 (UTC) Received: from localhost (unknown [10.40.205.15]) by smtp.corp.redhat.com (Postfix) with ESMTPS id ABD5B165F2; Wed, 5 Jun 2019 16:11:24 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 5 Jun 2019 18:11:15 +0200 Message-Id: <20190605161118.14544-2-mreitz@redhat.com> In-Reply-To: <20190605161118.14544-1-mreitz@redhat.com> References: <20190605161118.14544-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 05 Jun 2019 16:11: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 1/4] block: Introduce BdrvChild.parent_quiesce_counter 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 , qemu-devel@nongnu.org, Stefan Hajnoczi , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Commit 5cb2737e925042e6c7cd3fb0b01313950b03cddf laid out why bdrv_do_drained_end() must decrement the quiesce_counter after bdrv_drain_invoke(). It did not give a very good reason why it has to happen after bdrv_parent_drained_end(), instead only claiming symmetry to bdrv_do_drained_begin(). It turns out that delaying it for so long is wrong. Situation: We have an active commit job (i.e. a mirror job) from top to base for the following graph: filter | [file] | v top --[backing]--> base Now the VM is closed, which results in the job being cancelled and a bdrv_drain_all() happening pretty much simultaneously. Beginning the drain means the job is paused once whenever one of its nodes is quiesced. This is reversed when the drain ends. With how the code currently is, after base's drain ends (which means that it will have unpaused the job once), its quiesce_counter remains at 1 while it goes to undrain its parents (bdrv_parent_drained_end()). For some reason or another, undraining filter causes the job to be kicked and enter mirror_exit_common(), where it proceeds to invoke block_job_remove_all_bdrv(). Now base will be detached from the job. Because its quiesce_counter is still 1, it will unpause the job once more. So in total, undraining base will unpause the job twice. Eventually, this will lead to the job's pause_count going negative -- well, it would, were there not an assertion against this, which crashes qemu. The general problem is that if in bdrv_parent_drained_end() we undrain parent A, and then undrain parent B, which then leads to A detaching the child, bdrv_replace_child_noperm() will undrain A as if we had not done so yet; that is, one time too many. It follows that we cannot decrement the quiesce_counter after invoking bdrv_parent_drained_end(). Unfortunately, decrementing it before bdrv_parent_drained_end() would be wrong, too. Imagine the above situation in reverse: Undraining A leads to B detaching the child. If we had already decremented the quiesce_counter by that point, bdrv_replace_child_noperm() would undrain B one time too little; because it expects bdrv_parent_drained_end() to issue this undrain. But bdrv_parent_drained_end() won't do that, because B is no longer a parent. Therefore, we have to do something else. This patch opts for introducing a second quiesce_counter that counts how many times a child's parent has been quiesced (though c->role->drained_*). With that, bdrv_replace_child_noperm() just has to undrain the parent exactly that many times when removing a child, and it will always be right. Signed-off-by: Max Reitz --- include/block/block.h | 7 +++++++ include/block/block_int.h | 9 +++++++++ block.c | 15 +++++---------- block/io.c | 14 +++++++++++--- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index f9415ed740..3c084e8222 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -616,6 +616,13 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, B= drvChild *ignore, */ void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll); =20 +/** + * bdrv_parent_drained_end_single: + * + * End a quiesced section for the parent of @c. + */ +void bdrv_parent_drained_end_single(BdrvChild *c); + /** * bdrv_parent_drained_end: * diff --git a/include/block/block_int.h b/include/block/block_int.h index 06df2bda1b..84c0369fb7 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -729,6 +729,15 @@ struct BdrvChild { */ bool frozen; =20 + /* + * How many times the parent of this child has been drained + * (through role->drained_*). + * Usually, this is equal to bs->quiesce_counter (potentially + * reduced by bdrv_drain_all_count). It may differ while the + * child is entering or leaving a drained section. + */ + int parent_quiesce_counter; + QLIST_ENTRY(BdrvChild) next; QLIST_ENTRY(BdrvChild) next_parent; }; diff --git a/block.c b/block.c index e3e77feee0..b336a91415 100644 --- a/block.c +++ b/block.c @@ -2173,24 +2173,19 @@ static void bdrv_replace_child_noperm(BdrvChild *ch= ild, if (child->role->detach) { child->role->detach(child); } - if (old_bs->quiesce_counter && child->role->drained_end) { - int num =3D old_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++) { - child->role->drained_end(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 && child->role->drained_begin) { + if (new_bs->quiesce_counter) { int num =3D new_bs->quiesce_counter; if (child->role->parent_is_bds) { num -=3D bdrv_drain_all_count; diff --git a/block/io.c b/block/io.c index 9ba1bada36..112eed385c 100644 --- a/block/io.c +++ b/block/io.c @@ -55,6 +55,15 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, Bdr= vChild *ignore, } } =20 +void bdrv_parent_drained_end_single(BdrvChild *c) +{ + assert(c->parent_quiesce_counter > 0); + c->parent_quiesce_counter--; + if (c->role->drained_end) { + c->role->drained_end(c); + } +} + void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, bool ignore_bds_parents) { @@ -64,9 +73,7 @@ void bdrv_parent_drained_end(BlockDriverState *bs, BdrvCh= ild *ignore, if (c =3D=3D ignore || (ignore_bds_parents && c->role->parent_is_b= ds)) { continue; } - if (c->role->drained_end) { - c->role->drained_end(c); - } + bdrv_parent_drained_end_single(c); } } =20 @@ -96,6 +103,7 @@ static bool bdrv_parent_drained_poll(BlockDriverState *b= s, BdrvChild *ignore, =20 void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll) { + c->parent_quiesce_counter++; if (c->role->drained_begin) { c->role->drained_begin(c); } --=20 2.21.0 From nobody Sat May 4 11:07:29 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=1559751262; cv=none; d=zoho.com; s=zohoarc; b=VZPYuC91urMzPymnGoQOcPN73F5Q/j/uah33lQj0s++uTLW3Gud6QfSiVmFpqi0jPDzsUq2dfy6OWRSSoIvX5l+2rMteVN9a8a4MMmNV4LjeKyLFBE6yuZjUycXxQ0ZnpPNI/QHnJFW3z2MW3FKW4bem0TadoMv5paO7kjNW02Y= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1559751262; 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=xkEN71oKqAA9geGQG5up9u+v3KUbwA9ZHpjEimvastI=; b=X5MPHo6rwp/RDltNtmEqKeO45+FK43RucuJB/ee6MBbru1DQPsyrcXw1jKxxd6AVfYRAbjT54TghJ2+Nof6dcAQ2hUgRO1vPc0cJN63TRRqmLYVRgpjwbPcq+iAws95/VOtBLcXIDr9OPB9+7cSYk4PMfkL06Ni7J6flUaWczaQ= 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 1559751262898606.3282293023705; Wed, 5 Jun 2019 09:14:22 -0700 (PDT) Received: from localhost ([127.0.0.1]:45712 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYYK-0000iD-Bg for importer@patchew.org; Wed, 05 Jun 2019 12:14:16 -0400 Received: from eggs.gnu.org ([209.51.188.92]:59992) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYVt-0007Wz-Bc for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hYYVp-0002ta-It for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:62502) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hYYVj-0002ZJ-PQ; Wed, 05 Jun 2019 12:11:37 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6C55B2F8BD2; Wed, 5 Jun 2019 16:11:33 +0000 (UTC) Received: from localhost (unknown [10.40.205.15]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 297AF2DE98; Wed, 5 Jun 2019 16:11:31 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 5 Jun 2019 18:11:16 +0200 Message-Id: <20190605161118.14544-3-mreitz@redhat.com> In-Reply-To: <20190605161118.14544-1-mreitz@redhat.com> References: <20190605161118.14544-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 05 Jun 2019 16:11:33 +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 2/4] block: Make @parent_quiesced a bool 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 , qemu-devel@nongnu.org, Stefan Hajnoczi , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Reliably ending the drain on a BDS's parents is quite difficult. What we have to achieve is to undrain exactly those parents that have been added to the BDS while its quiesce_counter was elevated. If we move decrementing the quiesce_counter before the invocation of bdrv_parent_drained_end(), that leaves us with the parents that existed at the start of this function. That seems simple enough, because new parents are always added at the beginning of the list, so just iterating through it from start to finish should give us exactly the parents we are looking for. Unfortunately, there is a catch. Unquiescing one parent can lead to another deciding to detach the child. In the process, the BdrvChild object may be destroyed. That means using QLIST_FOREACH_SAFE() to iterate the list is actually wrong: The @next pointer may be stale by the time we get to the next iteration. Using QLIST_FOREACH() would be just as wrong, though, because the current BdrvChild object may be destroyed just as well. It follows that we cannot just iterate over the list from start to finish. Some ideas how to solve this problem: - Store a list of the parent pointers at the beginning of the function, then try to unquiesce them all (as long as they are in bs->parents). Does not work because pointers may be reused by new parents. - Add refcounts to BdrvChild objects. Kind of silly because only the parent really has a valid reference. Once that goes away, all fields must be cleared. Therefore, all we could guarantee is that the list pointers (next/next_parent) stay valid. Also, parents can simply change the child a BdrvChild refers to. If they do that, the BdrvChild object remains accessible, but (1) it refers to the wrong child, and (2) next_parent suddenly refers to a different list. So maybe this can be made to work, but it would always be kind of silly. - Check the difference between the parent_quiesce_counters and the actual bs->quiesce_counter. In theory, both should be equal after bdrv_parent_drained_end() (reduced by bdrv_drain_all_count for BDS parents). So we could reiterate the parent list (unquiescing one parent at a time) until all parents have the desired parent_quiesce_counter. In practice, this is difficult -- I have tried. bdrv_drain_all_count may be one too high because somewhere down the stack there is a bdrv_drain_all_end() currently running. Also, I suppose something can concurrently modify bs->quiesce_counter, and I am not sure how to handle such cases. In the best case, this would lead to rather complicated code that I could not trust but only pray that it works. In the worst case, my prayers are not heard. We can get around the whole issue by observing that it really does not matter whether a parent is quiesced one time or ten. We just have to quiesce it once the moment the child tries to change from being unquiesced to being quiesced. We have to unquiesce it when the child is unquiesced again. Therefore, we can just make the parent_quiesce_counter a boolean and call it parent_quiesced. When bdrv_parent_drained_end() sees bs->quiesce_counter going to 0, we unquiesce all parents. bdrv_parent_drained_begin() in turn quiesces all unquiesced parents. (This means we have to decrement bs->quiesce_counter before bdrv_parent_drained_end().) Signed-off-by: Max Reitz --- include/block/block.h | 10 +------ include/block/block_int.h | 9 +++---- block.c | 7 +++-- block/io.c | 55 ++++++++++++++++++++++++++++----------- tests/test-bdrv-drain.c | 31 +++++++++++++--------- 5 files changed, 65 insertions(+), 47 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 3c084e8222..687c03b275 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -620,18 +620,10 @@ void bdrv_parent_drained_begin_single(BdrvChild *c, b= ool poll); * bdrv_parent_drained_end_single: * * End a quiesced section for the parent of @c. + * c->parent_quiesced must be true. */ void bdrv_parent_drained_end_single(BdrvChild *c); =20 -/** - * bdrv_parent_drained_end: - * - * End a quiesced section of all users of @bs. This is part of - * bdrv_drained_end. - */ -void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, - bool ignore_bds_parents); - /** * bdrv_drain_poll: * diff --git a/include/block/block_int.h b/include/block/block_int.h index 84c0369fb7..58fca37ba3 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -730,13 +730,10 @@ struct BdrvChild { bool frozen; =20 /* - * How many times the parent of this child has been drained - * (through role->drained_*). - * Usually, this is equal to bs->quiesce_counter (potentially - * reduced by bdrv_drain_all_count). It may differ while the - * child is entering or leaving a drained section. + * Whether the parent of this child has been drained through + * role->drained_*. */ - int parent_quiesce_counter; + bool parent_quiesced; =20 QLIST_ENTRY(BdrvChild) next; QLIST_ENTRY(BdrvChild) next_parent; diff --git a/block.c b/block.c index b336a91415..6bc51e371f 100644 --- a/block.c +++ b/block.c @@ -2159,7 +2159,6 @@ static void bdrv_replace_child_noperm(BdrvChild *chil= d, BlockDriverState *new_bs) { BlockDriverState *old_bs =3D child->bs; - int i; =20 assert(!child->frozen); =20 @@ -2173,12 +2172,12 @@ static void bdrv_replace_child_noperm(BdrvChild *ch= ild, if (child->role->detach) { child->role->detach(child); } - while (child->parent_quiesce_counter) { + if (child->parent_quiesced) { bdrv_parent_drained_end_single(child); } QLIST_REMOVE(child, next_parent); } else { - assert(child->parent_quiesce_counter =3D=3D 0); + assert(!child->parent_quiesced); } =20 child->bs =3D new_bs; @@ -2191,7 +2190,7 @@ static void bdrv_replace_child_noperm(BdrvChild *chil= d, num -=3D bdrv_drain_all_count; } assert(num >=3D 0); - for (i =3D 0; i < num; i++) { + if (num) { bdrv_parent_drained_begin_single(child, true); } } diff --git a/block/io.c b/block/io.c index 112eed385c..2408abffd9 100644 --- a/block/io.c +++ b/block/io.c @@ -57,24 +57,47 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, Bd= rvChild *ignore, =20 void bdrv_parent_drained_end_single(BdrvChild *c) { - assert(c->parent_quiesce_counter > 0); - c->parent_quiesce_counter--; + assert(c->parent_quiesced); + c->parent_quiesced =3D false; if (c->role->drained_end) { c->role->drained_end(c); } } =20 -void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore, - bool ignore_bds_parents) +static void bdrv_parent_drained_end(BlockDriverState *bs) { - BdrvChild *c, *next; + BdrvChild *c; =20 - QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { - if (c =3D=3D ignore || (ignore_bds_parents && c->role->parent_is_b= ds)) { - continue; - } - bdrv_parent_drained_end_single(c); + if (bs->quiesce_counter) { + return; } + + /* + * The list of parents can change, so repeat until all are + * unquiesced (or until bs->quiesce_counter is no longer zero). + */ + do { + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (!c->parent_quiesced) { + continue; + } + + if (bs->quiesce_counter) { + /* + * We can just leave here. The first thing + * bdrv_parent_drained_end_single() does is to set + * c->parent_quiesced to false. If something decides + * to drain @bs while we are unquiescing some parent, + * it will thus redrain that parent (and everything + * else that we have already unquiesced). + */ + return; + } + + bdrv_parent_drained_end_single(c); + break; + } + } while (c); } =20 static bool bdrv_parent_drained_poll_single(BdrvChild *c) @@ -103,9 +126,11 @@ static bool bdrv_parent_drained_poll(BlockDriverState = *bs, BdrvChild *ignore, =20 void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll) { - c->parent_quiesce_counter++; - if (c->role->drained_begin) { - c->role->drained_begin(c); + if (!c->parent_quiesced) { + c->parent_quiesced =3D true; + if (c->role->drained_begin) { + c->role->drained_begin(c); + } } if (poll) { BDRV_POLL_WHILE(c->bs, bdrv_parent_drained_poll_single(c)); @@ -433,9 +458,9 @@ static void bdrv_do_drained_end(BlockDriverState *bs, b= ool recursive, =20 /* Re-enable things in child-to-parent order */ bdrv_drain_invoke(bs, false); - bdrv_parent_drained_end(bs, parent, ignore_bds_parents); - old_quiesce_counter =3D atomic_fetch_dec(&bs->quiesce_counter); + bdrv_parent_drained_end(bs); + if (old_quiesce_counter =3D=3D 1) { aio_enable_external(bdrv_get_aio_context(bs)); } diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index 12e2ecf517..75609bf6d0 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -446,10 +446,14 @@ static void test_multiparent(void) =20 do_drain_end(BDRV_SUBTREE_DRAIN, bs_b); =20 - g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 1); + /* + * @backing is still drained by @bs_a, so it has not unquiesced + * its parents yet (and @bs_a retains its qc of 2). + */ + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 2); g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 1); g_assert_cmpint(backing->quiesce_counter, =3D=3D, 1); - g_assert_cmpint(a_s->drain_count, =3D=3D, 1); + g_assert_cmpint(a_s->drain_count, =3D=3D, 2); g_assert_cmpint(b_s->drain_count, =3D=3D, 1); g_assert_cmpint(backing_s->drain_count, =3D=3D, 1); =20 @@ -505,27 +509,28 @@ static void test_graph_change_drain_subtree(void) do_drain_begin(BDRV_SUBTREE_DRAIN, bs_b); =20 bdrv_set_backing_hd(bs_b, backing, &error_abort); - g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 5); - g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 5); - g_assert_cmpint(backing->quiesce_counter, =3D=3D, 5); - g_assert_cmpint(a_s->drain_count, =3D=3D, 5); - g_assert_cmpint(b_s->drain_count, =3D=3D, 5); + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 4); /* 3 + !!2 */ + g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 3); /* !!3 + 2 */ + g_assert_cmpint(backing->quiesce_counter, =3D=3D, 5); /* 3 + 2 */ + g_assert_cmpint(a_s->drain_count, =3D=3D, 4); + g_assert_cmpint(b_s->drain_count, =3D=3D, 3); g_assert_cmpint(backing_s->drain_count, =3D=3D, 5); =20 bdrv_set_backing_hd(bs_b, NULL, &error_abort); - g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 3); + /* @backing remains quiesced, so it does not unquiesce @bs_a */ + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 4); g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 2); g_assert_cmpint(backing->quiesce_counter, =3D=3D, 3); - g_assert_cmpint(a_s->drain_count, =3D=3D, 3); + g_assert_cmpint(a_s->drain_count, =3D=3D, 4); g_assert_cmpint(b_s->drain_count, =3D=3D, 2); g_assert_cmpint(backing_s->drain_count, =3D=3D, 3); =20 bdrv_set_backing_hd(bs_b, backing, &error_abort); - g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 5); - g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 5); + g_assert_cmpint(bs_a->quiesce_counter, =3D=3D, 4); + g_assert_cmpint(bs_b->quiesce_counter, =3D=3D, 3); g_assert_cmpint(backing->quiesce_counter, =3D=3D, 5); - g_assert_cmpint(a_s->drain_count, =3D=3D, 5); - g_assert_cmpint(b_s->drain_count, =3D=3D, 5); + g_assert_cmpint(a_s->drain_count, =3D=3D, 4); + g_assert_cmpint(b_s->drain_count, =3D=3D, 3); g_assert_cmpint(backing_s->drain_count, =3D=3D, 5); =20 do_drain_end(BDRV_SUBTREE_DRAIN, bs_b); --=20 2.21.0 From nobody Sat May 4 11:07:29 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=1559751461; cv=none; d=zoho.com; s=zohoarc; b=isiJd6/GCPZsslet8BUMSvseMl3LcPsDfQm9mRjXy3kcRBJpghhqSFT1IXSt8ecAfRQlQqt0UEcs/U8WBk2VnhLvPzHsN3s2MDEyPlevtqwkl0nTw7bvGH7lP541vOMQujO5b42flg5Pu2zaxNL3Q0DTic9LrbFVSzcJm2ULWvQ= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1559751461; 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=PlS+OOdNcwEXR8a8OyJyixooJuIzWm2iEEmcBO0VRbw=; b=PVlwmF1JYecJE1LmAL1YCVjMYfcaFeH4z/knCBydscti+kyNLvcbF6V3XbeUyXRsemU2zPP8Su37F5mhKwJ7v/IeiE8AbGJu7A/EpVjtZFo5DhOTRiFKBeirkP5YOjkS3nwM4cHfk+uZsIXd3s4ONDg9+jPdm24cGq8eewEd1gc= 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 1559751461003334.954168043261; Wed, 5 Jun 2019 09:17:41 -0700 (PDT) Received: from localhost ([127.0.0.1]:45787 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYbb-0003NI-VZ for importer@patchew.org; Wed, 05 Jun 2019 12:17:40 -0400 Received: from eggs.gnu.org ([209.51.188.92]:60071) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYVw-0007Zy-K7 for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hYYVv-00036A-LJ for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39492) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hYYVp-0002fj-IU; Wed, 05 Jun 2019 12:11:43 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CCC3730860BD; Wed, 5 Jun 2019 16:11:35 +0000 (UTC) Received: from localhost (unknown [10.40.205.15]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 629672DE98; Wed, 5 Jun 2019 16:11:35 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 5 Jun 2019 18:11:17 +0200 Message-Id: <20190605161118.14544-4-mreitz@redhat.com> In-Reply-To: <20190605161118.14544-1-mreitz@redhat.com> References: <20190605161118.14544-1-mreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.44]); Wed, 05 Jun 2019 16:11:35 +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 3/4] iotests: Add @has_quit to vm.shutdown() 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 , qemu-devel@nongnu.org, Stefan Hajnoczi , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" If a test has issued a quit command already (which may be useful to do explicitly because the test wants to show its effects), QEMUMachine.shutdown() should not do so again. Otherwise, the VM may well return an ECONNRESET which will lead QEMUMachine.shutdown() to killing it, which then turns into a "qemu received signal 9" line. Signed-off-by: Max Reitz --- python/qemu/__init__.py | 5 +++-- tests/qemu-iotests/255 | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py index dbaf8a5311..25207a2970 100644 --- a/python/qemu/__init__.py +++ b/python/qemu/__init__.py @@ -332,13 +332,14 @@ class QEMUMachine(object): self._load_io_log() self._post_shutdown() =20 - def shutdown(self): + def shutdown(self, has_quit=3DFalse): """ Terminate the VM and clean up """ if self.is_running(): try: - self._qmp.cmd('quit') + if not has_quit: + self._qmp.cmd('quit') self._qmp.close() except: self._popen.kill() diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255 index 49433ec122..3632d507d0 100755 --- a/tests/qemu-iotests/255 +++ b/tests/qemu-iotests/255 @@ -132,4 +132,4 @@ with iotests.FilePath('src.qcow2') as src_path, \ vm.qmp_log('block-job-cancel', device=3D'job0') vm.qmp_log('quit') =20 - vm.shutdown() + vm.shutdown(has_quit=3DTrue) --=20 2.21.0 From nobody Sat May 4 11:07:29 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=1559751360; cv=none; d=zoho.com; s=zohoarc; b=D4AeSJvV9oKYRnj1s5b9ps6xU7of5b7yEWrmrJPecOXM2UfJfHPDwPOELpZE36DTPzfaZqnd/03LfY57zV2Yg5O1fSgprN9Kux0hBE6YsZSBzyNXlGHohV8un6e9ueO2PEHWJmzoqCPGxwtCxts8B1qyq4JEffobKVvOcuSKE50= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1559751360; 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=4hBOPyIuUMJtnLcAgCyaBmi1vX6fe8EYCt4Pu3u6vfE=; b=nETupDDDGd0XM+7dbqwRRWrWdQScBoiRxaf3an/Miwe9qtsL7GJvFkPvWUpOEya0g3IVztU4BVizBk6atg/jGKo016yKS48gpbqd58cUZXFaL/PVjSTYJk76PBaHcu/Ih8jgSJofoBjU81nxhDGQivLI+sBiInxKHBdpzgbV2jA= 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 1559751360570359.20726367179077; Wed, 5 Jun 2019 09:16:00 -0700 (PDT) Received: from localhost ([127.0.0.1]:45772 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYZz-0001vn-H5 for importer@patchew.org; Wed, 05 Jun 2019 12:15:59 -0400 Received: from eggs.gnu.org ([209.51.188.92]:60082) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hYYVx-0007aq-8O for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hYYVw-00037S-6M for qemu-devel@nongnu.org; Wed, 05 Jun 2019 12:11:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40164) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hYYVt-0002u3-DH; Wed, 05 Jun 2019 12:11:45 -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 9EBA1C0586C4; Wed, 5 Jun 2019 16:11:41 +0000 (UTC) Received: from localhost (unknown [10.40.205.15]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BABA32C8C3; Wed, 5 Jun 2019 16:11:37 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Wed, 5 Jun 2019 18:11:18 +0200 Message-Id: <20190605161118.14544-5-mreitz@redhat.com> In-Reply-To: <20190605161118.14544-1-mreitz@redhat.com> References: <20190605161118.14544-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]); Wed, 05 Jun 2019 16:11:41 +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 4/4] iotests: Test commit with a filter on the chain 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 , qemu-devel@nongnu.org, Stefan Hajnoczi , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Before the previous patches, the first case resulted in a failed assertion (which is noted as qemu receiving a SIGABRT in the test output), and the second usually triggered a segmentation fault. Signed-off-by: Max Reitz --- tests/qemu-iotests/040 | 40 +++++++++++++++++++++++++++++++++++++- tests/qemu-iotests/040.out | 4 ++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index b81133a474..aa0b1847e3 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -92,9 +92,10 @@ class TestSingleDrive(ImageCommitTestCase): =20 self.vm.add_device("scsi-hd,id=3Dscsi0,drive=3Ddrive0") self.vm.launch() + self.has_quit =3D False =20 def tearDown(self): - self.vm.shutdown() + self.vm.shutdown(has_quit=3Dself.has_quit) os.remove(test_img) os.remove(mid_img) os.remove(backing_img) @@ -109,6 +110,43 @@ class TestSingleDrive(ImageCommitTestCase): self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 52= 4288', backing_img).find("verification failed")) self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 5242= 88 524288', backing_img).find("verification failed")) =20 + def test_commit_with_filter_and_quit(self): + result =3D self.vm.qmp('object-add', qom_type=3D'throttle-group', = id=3D'tg') + self.assert_qmp(result, 'return', {}) + + # Add a filter outside of the backing chain + result =3D self.vm.qmp('blockdev-add', driver=3D'throttle', node_n= ame=3D'filter', throttle_group=3D'tg', file=3D'mid') + self.assert_qmp(result, 'return', {}) + + result =3D self.vm.qmp('block-commit', device=3D'drive0') + self.assert_qmp(result, 'return', {}) + + # Quit immediately, thus forcing a simultaneous cancel of the + # block job and a bdrv_drain_all() + result =3D self.vm.qmp('quit') + self.assert_qmp(result, 'return', {}) + + self.has_quit =3D True + + # Same as above, but this time we add the filter after starting the job + def test_commit_plus_filter_and_quit(self): + result =3D self.vm.qmp('object-add', qom_type=3D'throttle-group', = id=3D'tg') + self.assert_qmp(result, 'return', {}) + + result =3D self.vm.qmp('block-commit', device=3D'drive0') + self.assert_qmp(result, 'return', {}) + + # Add a filter outside of the backing chain + result =3D self.vm.qmp('blockdev-add', driver=3D'throttle', node_n= ame=3D'filter', throttle_group=3D'tg', file=3D'mid') + self.assert_qmp(result, 'return', {}) + + # Quit immediately, thus forcing a simultaneous cancel of the + # block job and a bdrv_drain_all() + result =3D self.vm.qmp('quit') + self.assert_qmp(result, 'return', {}) + + self.has_quit =3D True + def test_device_not_found(self): result =3D self.vm.qmp('block-commit', device=3D'nonexistent', top= =3D'%s' % mid_img) self.assert_qmp(result, 'error/class', 'DeviceNotFound') diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out index 802ffaa0c0..220a5fa82c 100644 --- a/tests/qemu-iotests/040.out +++ b/tests/qemu-iotests/040.out @@ -1,5 +1,5 @@ -........................................... +............................................... ---------------------------------------------------------------------- -Ran 43 tests +Ran 47 tests =20 OK --=20 2.21.0