From patchwork Tue Jan 18 16:27:27 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581425 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=WJJhfNHG; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZYl3gDvz9sRR for ; Wed, 19 Jan 2022 03:51:55 +1100 (AEDT) Received: from localhost ([::1]:52174 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9ri4-0004Di-C0 for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:51:52 -0500 Received: from eggs.gnu.org ([209.51.188.92]:38652) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rTq-0003sv-T7 for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:37:10 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:46839) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rTp-0005vQ-Ca for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:37:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523827; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZQgl8oN+/Auu1+Z3PypozVm42DQuUmaIhnOqXTYhhQI=; b=WJJhfNHGomm1ufxkBQlTDjhvznzBi03pTkZvNRXnp97aiiX/f87OyPOxi9LHWRfGJvWgFS FCPq/oSugnpcBNZ0yQigwYroUOXMZrwkgw9Pg7R5kUWHfa9BvmiUSF4giHG/CkmJ6xuzPo HYLm5uZFf9fDuH4Tt8nbLQcEBQArNHo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-511-2iXkCnDjNzaZxRoa82yVkg-1; Tue, 18 Jan 2022 11:36:53 -0500 X-MC-Unique: 2iXkCnDjNzaZxRoa82yVkg-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7408886A8D3; Tue, 18 Jan 2022 16:28:19 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7FB54348F8; Tue, 18 Jan 2022 16:28:09 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 01/12] introduce BDRV_POLL_WHILE_UNLOCKED Date: Tue, 18 Jan 2022 11:27:27 -0500 Message-Id: <20220118162738.1366281-2-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/block-global-state.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 419fe8427f..7ad9496f56 100644 --- a/include/block/block-global-state.h +++ b/include/block/block-global-state.h @@ -158,6 +158,11 @@ void bdrv_drain_all(void); AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \ cond); }) +#define BDRV_POLL_WHILE_UNLOCKED(bs, cond) ({ \ + BlockDriverState *bs_ = (bs); \ + AIO_WAIT_WHILE_UNLOCKED(bdrv_get_aio_context(bs_), \ + cond); }) + int bdrv_has_zero_init_1(BlockDriverState *bs); int bdrv_has_zero_init(BlockDriverState *bs); int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, From patchwork Tue Jan 18 16:27:28 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581437 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=jDmEmvRV; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZt60kTgz9s1l for ; Wed, 19 Jan 2022 04:06:05 +1100 (AEDT) Received: from localhost ([::1]:50000 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rvn-0005jS-Fn for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 12:06:03 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39288) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVM-00078J-Po for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:44 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:46617) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVL-00065G-2h for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:44 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523922; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C0blrVanM5+tLYMTkO5FDUx1zxDw+Kbo9e9GCpvpPhc=; b=jDmEmvRVvtYYL/ywdjlaQdHx45wuqAjaRYzqKL+MFNlG889lCPtRkWhc6+lpj03xy7wmaA 7r39k4Tr3HPhXiirwG6uC0/g05zO80Nv7dACzWAQVGZuG+9G4nDq0ZFGHh/JPg/m9QIYeB dlUSZ31S9cv4rK3Fv3dpmHBeJlDbmBI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-251-wWx_anlCOj6KCRxUGfXXsA-1; Tue, 18 Jan 2022 11:38:39 -0500 X-MC-Unique: wWx_anlCOj6KCRxUGfXXsA-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3217E10B6CDA; Tue, 18 Jan 2022 16:28:20 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 68DDC34D52; Tue, 18 Jan 2022 16:28:19 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 02/12] block/io.c: make bdrv_do_drained_begin_quiesce static and introduce bdrv_drained_begin_no_poll Date: Tue, 18 Jan 2022 11:27:28 -0500 Message-Id: <20220118162738.1366281-3-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Using bdrv_do_drained_begin_quiesce() in bdrv_child_cb_drained_end() is not a good idea: the callback might be called when running a drain in a coroutine, and bdrv_drained_begin_poll() does not handle that case, resulting in assertion failure. Instead, bdrv_do_drained_begin with no recursion and poll will accomplish the same thing (invoking bdrv_do_drained_begin_quiesce) but will firstly check if we are already in a coroutine, and exit from that via bdrv_co_yield_to_drain(). Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 +- block/io.c | 7 ++++++- include/block/block-io.h | 8 +++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 668986c879..08fde585f4 100644 --- a/block.c +++ b/block.c @@ -1206,7 +1206,7 @@ static char *bdrv_child_get_parent_desc(BdrvChild *c) static void bdrv_child_cb_drained_begin(BdrvChild *child) { BlockDriverState *bs = child->opaque; - bdrv_do_drained_begin_quiesce(bs, NULL, false); + bdrv_drained_begin_no_poll(bs); } static bool bdrv_child_cb_drained_poll(BdrvChild *child) diff --git a/block/io.c b/block/io.c index 3be08cad29..5123b7b713 100644 --- a/block/io.c +++ b/block/io.c @@ -425,7 +425,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, } } -void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, +static void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, BdrvChild *parent, bool ignore_bds_parents) { assert(!qemu_in_coroutine()); @@ -487,6 +487,11 @@ void bdrv_subtree_drained_begin(BlockDriverState *bs) bdrv_do_drained_begin(bs, true, NULL, false, true); } +void bdrv_drained_begin_no_poll(BlockDriverState *bs) +{ + bdrv_do_drained_begin(bs, false, NULL, false, false); +} + /** * This function does not poll, nor must any of its recursively called * functions. The *drained_end_counter pointee will be incremented diff --git a/include/block/block-io.h b/include/block/block-io.h index a69bc5bd36..34a991649c 100644 --- a/include/block/block-io.h +++ b/include/block/block-io.h @@ -238,13 +238,15 @@ bool bdrv_drain_poll(BlockDriverState *bs, bool recursive, void bdrv_drained_begin(BlockDriverState *bs); /** - * bdrv_do_drained_begin_quiesce: + * bdrv_drained_begin_no_poll: * * Quiesces a BDS like bdrv_drained_begin(), but does not wait for already * running requests to complete. + * Same as bdrv_drained_begin(), but do not poll for the subgraph to + * actually become unquiesced. Therefore, no graph changes will occur + * with this function. */ -void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, - BdrvChild *parent, bool ignore_bds_parents); +void bdrv_drained_begin_no_poll(BlockDriverState *bs); /** * Like bdrv_drained_begin, but recursively begins a quiesced section for From patchwork Tue Jan 18 16:27:29 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581428 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=JerFrF9F; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZfW6WMjz9sRR for ; Wed, 19 Jan 2022 03:56:03 +1100 (AEDT) Received: from localhost ([::1]:33702 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rm5-0002wE-Ii for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:56:01 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39104) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUr-0006Ig-J4 for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:13 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:57082) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUo-00061L-OG for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:13 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523883; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=uGKVVxmZwVjsknZwXqGVwij7wFLBfaxRdt9+qmwKoJU=; b=JerFrF9FCrIoV27JZFCI5F1U53oh+1tIqy1aL+BnQT8/60VeJepVgQ2TNVhY4TAn69c6Je S0dfTRvnlAeGBqpBqOZacP4n+Xy0Z5y5EYHIfnS+uY3ce6o7T2er/SHV4wrCimxvN2JLlV xU6NfM9TGOQ5xtlpOdN1UGiTVUE5oYU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-467-GrrHE2WKNq6yR45vEVzsKA-1; Tue, 18 Jan 2022 11:37:55 -0500 X-MC-Unique: GrrHE2WKNq6yR45vEVzsKA-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 13FCA12CD92; Tue, 18 Jan 2022 16:28:21 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4B63E348F8; Tue, 18 Jan 2022 16:28:20 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 03/12] block.c: bdrv_replace_child_noperm: first remove the child, and then call ->detach() Date: Tue, 18 Jan 2022 11:27:29 -0500 Message-Id: <20220118162738.1366281-4-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Doing the opposite can make ->detach() (more precisely bdrv_unapply_subtree_drain() in bdrv_child_cb_detach) undo the subtree_drain just performed to protect the removal of the child from the graph, thus making the fully-enabled assert_bdrv_graph_writable fail. Note that assert_bdrv_graph_writable is not yet fully enabled. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index 08fde585f4..29de2b62b5 100644 --- a/block.c +++ b/block.c @@ -2861,14 +2861,16 @@ static void bdrv_replace_child_noperm(BdrvChild **childp, } if (old_bs) { - /* Detach first so that the recursive drain sections coming from @child + assert_bdrv_graph_writable(old_bs); + QLIST_REMOVE(child, next_parent); + /* + * Detach first so that the recursive drain sections coming from @child * are already gone and we only end the drain sections that came from - * elsewhere. */ + * elsewhere. + */ if (child->klass->detach) { child->klass->detach(child); } - assert_bdrv_graph_writable(old_bs); - QLIST_REMOVE(child, next_parent); } child->bs = new_bs; From patchwork Tue Jan 18 16:27:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581434 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ERGBlA7k; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZnP0MLZz9s1l for ; Wed, 19 Jan 2022 04:02:01 +1100 (AEDT) Received: from localhost ([::1]:45044 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rrq-0002DY-QX for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 12:01:58 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39266) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVL-00075S-S0 for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:43 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:32058) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVK-00065B-2f for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:43 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523921; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1mN4YprAtPYLW25ybvOA6+PmPtLEmh4URozvfVnbu10=; b=ERGBlA7kVU3JU4hzMc1Jkjdw82qFHiEoFQ6CVqsA7gBfszG/aFj9yoOkE6uVSs8dUpD9dt mrN9feqD4FgsjpqpHPV21emwav0Fwsgc6YzvDkKnkNxrRWiJoyJGlplRQvVJnNQ4NrozdA X7z5DLaTlDD1tvd6qxtUUfPHykO6gFs= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-213-oRJkJsH1PKuYVcqVOBXReA-1; Tue, 18 Jan 2022 11:38:39 -0500 X-MC-Unique: oRJkJsH1PKuYVcqVOBXReA-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id EA3AC10C44BB; Tue, 18 Jan 2022 16:28:21 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2CEF034D3F; Tue, 18 Jan 2022 16:28:21 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 04/12] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child Date: Tue, 18 Jan 2022 11:27:30 -0500 Message-Id: <20220118162738.1366281-5-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Doing the opposite can make adding the child node to a non-drained node, as apply_subtree_drain is only done in ->attach() and thus make assert_bdrv_graph_writable fail. This can happen for example during a transaction rollback (test 245, test_io_with_graph_changes): 1. a node is removed from the graph, thus it is undrained 2. then something happens, and we need to roll back the transactions through tran_abort() 3. at this point, the current code would first attach the undrained node to the graph via QLIST_INSERT_HEAD, and then call ->attach() that will take care of restoring the drain with apply_subtree_drain(), leaving the node undrained between the two operations. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index 29de2b62b5..fb5bc3077a 100644 --- a/block.c +++ b/block.c @@ -2879,8 +2879,6 @@ static void bdrv_replace_child_noperm(BdrvChild **childp, } if (new_bs) { - assert_bdrv_graph_writable(new_bs); - QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); /* * Detaching the old node may have led to the new node's @@ -2897,6 +2895,10 @@ static void bdrv_replace_child_noperm(BdrvChild **childp, if (child->klass->attach) { child->klass->attach(child); } + + assert_bdrv_graph_writable(new_bs); + QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); + } /* From patchwork Tue Jan 18 16:27:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581427 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=QKFA1tF7; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZbY31R6z9sRR for ; Wed, 19 Jan 2022 03:53:29 +1100 (AEDT) Received: from localhost ([::1]:55796 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rjb-0006rQ-3y for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:53:27 -0500 Received: from eggs.gnu.org ([209.51.188.92]:38432) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rTJ-0002Qf-N2 for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:36:37 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:42908) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rTH-0005qS-UM for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:36:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523795; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oCNhTywy/F6op3ibTVRQmfx5IM0UrdJWaXcz6og+PZk=; b=QKFA1tF7eJe/bW5xjDHUxNuOpFoegEQUGrV2psdGNJW+yCmpnwDDm9qf60d0okD8gWyajJ HKEMEU1UHLizHxcYQk64nLKHy92cpu80V0OuFzv71iYZmA8JM1ipoCuX6JmWR591vKvxIR jdg8WBbpFgGwuxZ8hMQtSKPt6RmBDGI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-645-k5OKaxp_OSiE8q8bpCxxMg-1; Tue, 18 Jan 2022 11:36:30 -0500 X-MC-Unique: k5OKaxp_OSiE8q8bpCxxMg-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id CC7EB8143EF; Tue, 18 Jan 2022 16:28:22 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0ECBF348F8; Tue, 18 Jan 2022 16:28:21 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 05/12] test-bdrv-drain.c: adapt test to the coming subtree drains Date: Tue, 18 Jan 2022 11:27:31 -0500 Message-Id: <20220118162738.1366281-6-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" There will be 2 problems in this test when we will add subtree drains in bdrv_replace_child_noperm: - First of all, inconsistency between block_job_create under aiocontext lock that internally calls blk_insert_bs and therefore bdrv_replace_child_noperm, and blk_insert_bs that is called two lines above in the same test without aiocontext. There seems to be no reason on why we need the lock there, so move the aiocontext lock further down. - test_detach_indirect: here it is simply a matter of wrong callbacks used. In the original test, there was only a subtree drain, so overriding .drained_begin was not a problem. Now that we want to have additional subtree drains, we risk to call the test callback to early, or multiple times. We do not want that, so override the callback only when we actually want to use it. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 5a35dc391d..f6af206748 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -912,12 +912,12 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, blk_insert_bs(blk_target, target, &error_abort); blk_set_allow_aio_context_change(blk_target, true); - aio_context_acquire(ctx); tjob = block_job_create("job0", &test_job_driver, NULL, src, 0, BLK_PERM_ALL, 0, 0, NULL, NULL, &error_abort); tjob->bs = src; job = &tjob->common; + aio_context_acquire(ctx); block_job_add_bdrv(job, "target", target, 0, BLK_PERM_ALL, &error_abort); switch (result) { @@ -1322,15 +1322,18 @@ static void detach_by_parent_aio_cb(void *opaque, int ret) } } +static BdrvChildClass detach_by_driver_cb_class; + static void detach_by_driver_cb_drained_begin(BdrvChild *child) { + /* restore .drained_begin cb, we don't need it anymore. */ + detach_by_driver_cb_class.drained_begin = child_of_bds.drained_begin; + aio_bh_schedule_oneshot(qemu_get_current_aio_context(), detach_indirect_bh, &detach_by_parent_data); child_of_bds.drained_begin(child); } -static BdrvChildClass detach_by_driver_cb_class; - /* * Initial graph: * @@ -1362,8 +1365,6 @@ static void test_detach_indirect(bool by_parent_cb) if (!by_parent_cb) { detach_by_driver_cb_class = child_of_bds; - detach_by_driver_cb_class.drained_begin = - detach_by_driver_cb_drained_begin; } /* Create all involved nodes */ @@ -1421,6 +1422,12 @@ static void test_detach_indirect(bool by_parent_cb) acb = blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, NULL); g_assert(acb != NULL); + if (!by_parent_cb) { + /* set .drained_begin cb to run only in the following drain. */ + detach_by_driver_cb_class.drained_begin = + detach_by_driver_cb_drained_begin; + } + /* Drain and check the expected result */ bdrv_subtree_drained_begin(parent_b); From patchwork Tue Jan 18 16:27:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581430 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=iKO6AYCh; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZhY27nDz9s1l for ; Wed, 19 Jan 2022 03:57:48 +1100 (AEDT) Received: from localhost ([::1]:38302 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rnk-00069e-NI for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:57:44 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39166) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUt-0006P8-Cs for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:15 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:35115) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUo-00061X-Oz for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:14 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523886; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9N1PNDVUG/C5XbnDQHJ/4SUfJSxUMs/3JNk+hdgVuwU=; b=iKO6AYCh4+0wP4JaSWHPp2dP3mKx2M/RfuXJ7c4sRY7STwOgvmn3LwbI4hC6MdOIibj1Ia b4/6G+3IAB3YMnwsaos3gR9mesf29owaljtIRixifysujeZcAF/l9Bjsudbd6SyP/3IGUw XkyiopQDx9F9BIk5yoeydULxOfB73tk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-363-iZ7KR0nxNVieLzaD8-eIzg-1; Tue, 18 Jan 2022 11:38:00 -0500 X-MC-Unique: iZ7KR0nxNVieLzaD8-eIzg-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id AF5CE1142342; Tue, 18 Jan 2022 16:28:23 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id E5A7C348F8; Tue, 18 Jan 2022 16:28:22 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 06/12] test-bdrv-drain.c: remove test_detach_by_parent_cb() Date: Tue, 18 Jan 2022 11:27:32 -0500 Message-Id: <20220118162738.1366281-7-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" This test uses a callback of an I/O function (blk_aio_preadv) to modify the graph, using bdrv_attach_child. This is simply not allowed anymore. I/O cannot change the graph. Before "block/io.c: make bdrv_do_drained_begin_quiesce static and introduce bdrv_drained_begin_no_poll", the test would simply be at risk of failure, because if bdrv_replace_child_noperm() (called to modify the graph) would call a drain, then one callback of .drained_begin() is bdrv_do_drained_begin_quiesce, that specifically asserts that we are not in a coroutine. Now that we fixed the behavior, the drain will invoke a bh in the main loop, so we don't have such problem. However, this test is still illegal and fails because we forbid graph changes from I/O paths. Once we add the required subtree_drains to protect bdrv_replace_child_noperm(), the key problem in this test is in: acb = blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, NULL); /* Drain and check the expected result */ bdrv_subtree_drained_begin(parent_b); because the detach_by_parent_aio_cb calls detach_indirect_bh(), that modifies the graph and is invoked during bdrv_subtree_drained_begin(). The call stack is the following: 1. blk_aio_preadv() creates a coroutine, increments in_flight counter and enters the coroutine running blk_aio_read_entry() 2. blk_aio_read_entry() performs the read and then schedules a bh to complete (blk_aio_complete) 3. at this point, subtree_drained_begin() kicks in and waits for all in_flight requests, polling 4. polling allows the bh to be scheduled, so blk_aio_complete runs 5. blk_aio_complete *first* invokes the callback (detach_by_parent_aio_cb) and then decrements the in_flight counter 6. Here we have the problem: detach_by_parent_aio_cb modifies the graph, so both bdrv_unref_child() and bdrv_attach_child() will have subtree_drains inside. And this causes a deadlock, because the nested drain will wait for in_flight counter to go to zero, which is only happening once the drain itself finishes. Different story is test_detach_by_driver_cb(): in this case, detach_by_parent_aio_cb() does not call detach_indirect_bh(), but it is instead called as a bh running in the main loop by detach_by_driver_cb_drained_begin(), the callback for .drained_begin(). This test was added in 231281ab42 and part of the series "Drain fixes and cleanups, part 3" https://lists.nongnu.org/archive/html/qemu-block/2018-05/msg01132.html but as explained above I believe that it is not valid anymore, and can be discarded. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 46 +++++++++--------------------------- 1 file changed, 11 insertions(+), 35 deletions(-) diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index f6af206748..af3501c46d 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -1296,7 +1296,6 @@ struct detach_by_parent_data { BdrvChild *child_b; BlockDriverState *c; BdrvChild *child_c; - bool by_parent_cb; }; static struct detach_by_parent_data detach_by_parent_data; @@ -1314,12 +1313,7 @@ static void detach_indirect_bh(void *opaque) static void detach_by_parent_aio_cb(void *opaque, int ret) { - struct detach_by_parent_data *data = &detach_by_parent_data; - g_assert_cmpint(ret, ==, 0); - if (data->by_parent_cb) { - detach_indirect_bh(data); - } } static BdrvChildClass detach_by_driver_cb_class; @@ -1341,31 +1335,24 @@ static void detach_by_driver_cb_drained_begin(BdrvChild *child) * \ / \ * A B C * - * by_parent_cb == true: Test that parent callbacks don't poll - * - * PA has a pending write request whose callback changes the child nodes of - * PB: It removes B and adds C instead. The subtree of PB is drained, which - * will indirectly drain the write request, too. - * - * by_parent_cb == false: Test that bdrv_drain_invoke() doesn't poll + * Test that bdrv_drain_invoke() doesn't poll * * PA's BdrvChildClass has a .drained_begin callback that schedules a BH * that does the same graph change. If bdrv_drain_invoke() calls it, the * state is messed up, but if it is only polled in the single * BDRV_POLL_WHILE() at the end of the drain, this should work fine. */ -static void test_detach_indirect(bool by_parent_cb) +static void test_detach_indirect(void) { BlockBackend *blk; BlockDriverState *parent_a, *parent_b, *a, *b, *c; BdrvChild *child_a, *child_b; BlockAIOCB *acb; + BDRVTestState *s; QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0); - if (!by_parent_cb) { - detach_by_driver_cb_class = child_of_bds; - } + detach_by_driver_cb_class = child_of_bds; /* Create all involved nodes */ parent_a = bdrv_new_open_driver(&bdrv_test, "parent-a", BDRV_O_RDWR, @@ -1384,10 +1371,8 @@ static void test_detach_indirect(bool by_parent_cb) /* If we want to get bdrv_drain_invoke() to call aio_poll(), the driver * callback must not return immediately. */ - if (!by_parent_cb) { - BDRVTestState *s = parent_a->opaque; - s->sleep_in_drain_begin = true; - } + s = parent_a->opaque; + s->sleep_in_drain_begin = true; /* Set child relationships */ bdrv_ref(b); @@ -1399,7 +1384,7 @@ static void test_detach_indirect(bool by_parent_cb) bdrv_ref(a); bdrv_attach_child(parent_a, a, "PA-A", - by_parent_cb ? &child_of_bds : &detach_by_driver_cb_class, + &detach_by_driver_cb_class, BDRV_CHILD_DATA, &error_abort); g_assert_cmpint(parent_a->refcnt, ==, 1); @@ -1417,16 +1402,13 @@ static void test_detach_indirect(bool by_parent_cb) .parent_b = parent_b, .child_b = child_b, .c = c, - .by_parent_cb = by_parent_cb, }; acb = blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, NULL); g_assert(acb != NULL); - if (!by_parent_cb) { - /* set .drained_begin cb to run only in the following drain. */ - detach_by_driver_cb_class.drained_begin = - detach_by_driver_cb_drained_begin; - } + /* set .drained_begin cb to run only in the following drain. */ + detach_by_driver_cb_class.drained_begin = + detach_by_driver_cb_drained_begin; /* Drain and check the expected result */ bdrv_subtree_drained_begin(parent_b); @@ -1462,14 +1444,9 @@ static void test_detach_indirect(bool by_parent_cb) bdrv_unref(c); } -static void test_detach_by_parent_cb(void) -{ - test_detach_indirect(true); -} - static void test_detach_by_driver_cb(void) { - test_detach_indirect(false); + test_detach_indirect(); } static void test_append_to_drained(void) @@ -2224,7 +2201,6 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/detach/drain_all", test_detach_by_drain_all); g_test_add_func("/bdrv-drain/detach/drain", test_detach_by_drain); g_test_add_func("/bdrv-drain/detach/drain_subtree", test_detach_by_drain_subtree); - g_test_add_func("/bdrv-drain/detach/parent_cb", test_detach_by_parent_cb); g_test_add_func("/bdrv-drain/detach/driver_cb", test_detach_by_driver_cb); g_test_add_func("/bdrv-drain/attach/drain", test_append_to_drained); From patchwork Tue Jan 18 16:27:33 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581441 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=GQyqUW49; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4Jdb1903knz9s9c for ; Wed, 19 Jan 2022 04:12:11 +1100 (AEDT) Received: from localhost ([::1]:58334 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9s1f-0003HN-CF for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 12:12:07 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39318) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVQ-0007Fk-Mr for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:49 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:44399) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVO-00066J-Td for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:48 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523926; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CXYNcuDLnPPpWTS1yf2y5Z0dCqv/nWo5hZI1K16GCuo=; b=GQyqUW499EoTD8298u0GEKDHOL6+ke0udgx/RXaX4nlEgdb/suG9uMMEY599hQJZeg1/lf 0O75wp0i38KarEVaaLsXK7h0gtb9AjqMc3hPP4YgZKaoqRtnPyz59Och763HjoRXHa7FM0 JNG3h++0hBWyjNGwDbIbSIANa1AXBpU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-49-wjIU9_2_N4a-KLgHZlcF4g-1; Tue, 18 Jan 2022 11:38:45 -0500 X-MC-Unique: wjIU9_2_N4a-KLgHZlcF4g-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 92EEA10CDCEF; Tue, 18 Jan 2022 16:28:24 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id C8F4C34D38; Tue, 18 Jan 2022 16:28:23 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 07/12] block/io.c: introduce bdrv_subtree_drained_{begin/end}_unlocked Date: Tue, 18 Jan 2022 11:27:33 -0500 Message-Id: <20220118162738.1366281-8-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Same as the locked version, but use BDRV_POLL_UNLOCKED Signed-off-by: Emanuele Giuseppe Esposito --- block/io.c | 50 +++++++++++++++++++++++++++++----------- include/block/block-io.h | 2 ++ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/block/io.c b/block/io.c index 5123b7b713..9d5167f64a 100644 --- a/block/io.c +++ b/block/io.c @@ -244,6 +244,7 @@ typedef struct { bool begin; bool recursive; bool poll; + bool unlock; BdrvChild *parent; bool ignore_bds_parents; int *drained_end_counter; @@ -334,7 +335,7 @@ static bool bdrv_drain_poll_top_level(BlockDriverState *bs, bool recursive, static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, BdrvChild *parent, bool ignore_bds_parents, - bool poll); + bool poll, bool unlock); static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, BdrvChild *parent, bool ignore_bds_parents, int *drained_end_counter); @@ -352,7 +353,8 @@ static void bdrv_co_drain_bh_cb(void *opaque) if (data->begin) { assert(!data->drained_end_counter); bdrv_do_drained_begin(bs, data->recursive, data->parent, - data->ignore_bds_parents, data->poll); + data->ignore_bds_parents, data->poll, + data->unlock); } else { assert(!data->poll); bdrv_do_drained_end(bs, data->recursive, data->parent, @@ -374,6 +376,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, BdrvChild *parent, bool ignore_bds_parents, bool poll, + bool unlock, int *drained_end_counter) { BdrvCoDrainData data; @@ -394,6 +397,7 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs, .parent = parent, .ignore_bds_parents = ignore_bds_parents, .poll = poll, + .unlock = unlock, .drained_end_counter = drained_end_counter, }; @@ -441,13 +445,13 @@ static void bdrv_do_drained_begin_quiesce(BlockDriverState *bs, static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, BdrvChild *parent, bool ignore_bds_parents, - bool poll) + bool poll, bool unlock) { BdrvChild *child, *next; if (qemu_in_coroutine()) { bdrv_co_yield_to_drain(bs, true, recursive, parent, ignore_bds_parents, - poll, NULL); + poll, unlock, NULL); return; } @@ -458,7 +462,7 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, bs->recursive_quiesce_counter++; QLIST_FOREACH_SAFE(child, &bs->children, next, next) { bdrv_do_drained_begin(child->bs, true, child, ignore_bds_parents, - false); + false, false); } } @@ -473,23 +477,35 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive, */ if (poll) { assert(!ignore_bds_parents); - BDRV_POLL_WHILE(bs, bdrv_drain_poll_top_level(bs, recursive, parent)); + if (unlock) { + BDRV_POLL_WHILE_UNLOCKED(bs, + bdrv_drain_poll_top_level(bs, recursive, + parent)); + } else { + BDRV_POLL_WHILE(bs, + bdrv_drain_poll_top_level(bs, recursive, parent)); + } } } void bdrv_drained_begin(BlockDriverState *bs) { - bdrv_do_drained_begin(bs, false, NULL, false, true); + bdrv_do_drained_begin(bs, false, NULL, false, true, false); } void bdrv_subtree_drained_begin(BlockDriverState *bs) { - bdrv_do_drained_begin(bs, true, NULL, false, true); + bdrv_do_drained_begin(bs, true, NULL, false, true, false); +} + +void bdrv_subtree_drained_begin_unlocked(BlockDriverState *bs) +{ + bdrv_do_drained_begin(bs, true, NULL, false, true, true); } void bdrv_drained_begin_no_poll(BlockDriverState *bs) { - bdrv_do_drained_begin(bs, false, NULL, false, false); + bdrv_do_drained_begin(bs, false, NULL, false, false, false); } /** @@ -517,7 +533,7 @@ static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive, if (qemu_in_coroutine()) { bdrv_co_yield_to_drain(bs, false, recursive, parent, ignore_bds_parents, - false, drained_end_counter); + false, false, drained_end_counter); return; } assert(bs->quiesce_counter > 0); @@ -561,12 +577,19 @@ void bdrv_subtree_drained_end(BlockDriverState *bs) BDRV_POLL_WHILE(bs, qatomic_read(&drained_end_counter) > 0); } +void bdrv_subtree_drained_end_unlocked(BlockDriverState *bs) +{ + int drained_end_counter = 0; + bdrv_do_drained_end(bs, true, NULL, false, &drained_end_counter); + BDRV_POLL_WHILE_UNLOCKED(bs, qatomic_read(&drained_end_counter) > 0); +} + void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent) { int i; for (i = 0; i < new_parent->recursive_quiesce_counter; i++) { - bdrv_do_drained_begin(child->bs, true, child, false, true); + bdrv_do_drained_begin(child->bs, true, child, false, true, false); } } @@ -651,7 +674,8 @@ void bdrv_drain_all_begin(void) assert(qemu_in_main_thread()); if (qemu_in_coroutine()) { - bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true, NULL); + bdrv_co_yield_to_drain(NULL, true, false, NULL, true, true, false, + NULL); return; } @@ -676,7 +700,7 @@ void bdrv_drain_all_begin(void) AioContext *aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); - bdrv_do_drained_begin(bs, false, NULL, true, false); + bdrv_do_drained_begin(bs, false, NULL, true, false, false); aio_context_release(aio_context); } diff --git a/include/block/block-io.h b/include/block/block-io.h index 34a991649c..a329ae24c1 100644 --- a/include/block/block-io.h +++ b/include/block/block-io.h @@ -253,6 +253,7 @@ void bdrv_drained_begin_no_poll(BlockDriverState *bs); * exclusive access to all child nodes as well. */ void bdrv_subtree_drained_begin(BlockDriverState *bs); +void bdrv_subtree_drained_begin_unlocked(BlockDriverState *bs); /** * bdrv_drained_end: @@ -285,6 +286,7 @@ void bdrv_drained_end_no_poll(BlockDriverState *bs, int *drained_end_counter); * End a quiescent section started by bdrv_subtree_drained_begin(). */ void bdrv_subtree_drained_end(BlockDriverState *bs); +void bdrv_subtree_drained_end_unlocked(BlockDriverState *bs); bool bdrv_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, uint32_t granularity, Error **errp); From patchwork Tue Jan 18 16:27:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581433 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=IQuH4Ysa; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZkP3crfz9s1l for ; Wed, 19 Jan 2022 03:59:25 +1100 (AEDT) Received: from localhost ([::1]:42280 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rpL-0000Ma-AX for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:59:23 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39168) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUt-0006PO-Jy for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:15 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:54607) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUo-00061O-Q1 for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523884; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JUit603A3uNQkUhU7v54C81byKLM5MT2P7CEzhQOJII=; b=IQuH4YsaiFFWXpHw3EZr7bD9azmr/DC0vmt99YUSWL47FBA9bsogfk2w3/h6DaFK4jPycm 4eimRDeQzddj5rLgDmLDiveyPXJz3HTtJ01PoxO9BBgPsmtIOR8OtGY9PMs6Trm8eK+If8 HokmrxdJGZn8GsKzHxk3VHxK/sI6PbI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-176-eRtUkNbtMjSOIhko0xO9RA-1; Tue, 18 Jan 2022 11:37:52 -0500 X-MC-Unique: eRtUkNbtMjSOIhko0xO9RA-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7451B134E91; Tue, 18 Jan 2022 16:28:25 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id AC572348F8; Tue, 18 Jan 2022 16:28:24 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 08/12] reopen: add a transaction to drain_end nodes picked in bdrv_reopen_parse_file_or_backing Date: Tue, 18 Jan 2022 11:27:34 -0500 Message-Id: <20220118162738.1366281-9-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Depending on the options given to reopen_state, bdrv_reopen_parse_file_or_backing could pick another bs that could be from another graph, and thus not protected by subtree_drained_begin called by the callers of this function. We can't simply drain-undrain here, because of transactions. To simplify the logic, transactions always assume that they are run under drain, so the various subtree_drain introduced so far always take care of covering tran_commit(). And since we cannot directly do it, as the transaction is created/committed higher above, we can just add a new transaction to the list that just executes subtree_drained_end to match the drained_begin done in this function. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index fb5bc3077a..fcc44a49a0 100644 --- a/block.c +++ b/block.c @@ -4522,6 +4522,10 @@ int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, return bdrv_reopen(bs, opts, true, errp); } +TransactionActionDrv bdrv_drv_subtree_end = { + .clean = (void (*)(void *)) bdrv_subtree_drained_end_unlocked, +}; + /* * Take a BDRVReopenState and check if the value of 'backing' in the * reopen_state->options QDict is valid or not. @@ -4550,6 +4554,7 @@ static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state, const char *child_name = is_backing ? "backing" : "file"; QObject *value; const char *str; + int ret = 0; assert(qemu_in_main_thread()); @@ -4573,6 +4578,8 @@ static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state, "cycle", str, child_name, bs->node_name); return -EINVAL; } + /* This will be paired with a drained_end in tran_commit */ + bdrv_subtree_drained_begin_unlocked(new_child_bs); break; default: /* @@ -4583,18 +4590,19 @@ static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state, } if (old_child_bs == new_child_bs) { - return 0; + goto end; } if (old_child_bs) { if (bdrv_skip_implicit_filters(old_child_bs) == new_child_bs) { - return 0; + goto end; } if (old_child_bs->implicit) { error_setg(errp, "Cannot replace implicit %s child of %s", child_name, bs->node_name); - return -EPERM; + ret = -EPERM; + goto end; } } @@ -4605,7 +4613,8 @@ static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state, */ error_setg(errp, "'%s' is a %s filter node that does not support a " "%s child", bs->node_name, bs->drv->format_name, child_name); - return -EINVAL; + ret = -EINVAL; + goto end; } if (is_backing) { @@ -4614,8 +4623,14 @@ static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state, reopen_state->old_file_bs = old_child_bs; } - return bdrv_set_file_or_backing_noperm(bs, new_child_bs, is_backing, + ret = bdrv_set_file_or_backing_noperm(bs, new_child_bs, is_backing, tran, errp); + +end: + if (new_child_bs) { + tran_add(tran, &bdrv_drv_subtree_end, new_child_bs); + } + return ret; } /* From patchwork Tue Jan 18 16:27:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581429 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=WegK+I8F; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZfW6hjLz9sSs for ; Wed, 19 Jan 2022 03:56:03 +1100 (AEDT) Received: from localhost ([::1]:33616 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rm5-0002sw-8b for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:56:01 -0500 Received: from eggs.gnu.org ([209.51.188.92]:38978) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUa-00064C-Ps for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:37:56 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:40146) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUY-0005zx-6J for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:37:55 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523873; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OIS+nEi2zL0K3rc9EeZsqcQV+p5qmb4EJ5SeDrUI3VU=; b=WegK+I8FlcSwm+DQwEql4QAsRiGaIbAXi8Lsv1ng99ikOaLn9ar99KAWA94KlvUi/2RJ94 ymowuj5p07KUcVMsyknLhhD/WZv5lL96UZW4O2IToahAb+L+rnfWMG8DNvBF6lhu96Rfci ILlRitQ/jN8rWgmpzS6ESFZ+Ux/zNDw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-198-UDJw0UwdMaq-ODb60sCRmQ-1; Tue, 18 Jan 2022 11:37:52 -0500 X-MC-Unique: UDJw0UwdMaq-ODb60sCRmQ-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id AF57C1856EE1; Tue, 18 Jan 2022 16:28:27 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8D79934D44; Tue, 18 Jan 2022 16:28:25 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 09/12] jobs: ensure sleep in job_sleep_ns is fully performed Date: Tue, 18 Jan 2022 11:27:35 -0500 Message-Id: <20220118162738.1366281-10-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" If a drain happens while a job is sleeping, the timeout gets cancelled and the job continues once the drain ends. This is especially bad for the sleep performed in commit and stream jobs, since that is dictated by ratelimit to maintain a certain speed. Basically the execution path is the followig: 1. job calls job_sleep_ns, and yield with a timer in @ns ns. 2. meanwhile, a drain is executed, and child_job_drained_{begin/end} could be executed as ->drained_begin() and ->drained_end() callbacks. Therefore child_job_drained_begin() enters the job, that continues execution in job_sleep_ns() and calls job_pause_point_locked(). 3. job_pause_point_locked() detects that we are in the middle of a drain, and firstly deletes any existing timer and then yields again, waiting for ->drained_end(). 4. Once draining is finished, child_job_drained_end() runs and resumes the job. At this point, the timer has been lost and we just resume without checking if enough time has passed. This fix implies that from now onwards, job_sleep_ns will force the job to sleep @ns, even if it is wake up (purposefully or not) in the middle of the sleep. Therefore qemu-iotests test might run a little bit slower, depending on the speed of the job. Setting a job speed to values like "1" is not allowed anymore (unless you want to wait forever). Because of this fix, test_stream_parallel() in tests/qemu-iotests/030 takes too long, since speed of stream job is just 1024 and before it was skipping all the wait thanks to the drains. Increase the speed to 256 * 1024. Exactly the same happens for test 151. Instead we need to sleep less in test_cancel_ready() test-blockjob.c, so that the job will be able to exit the sleep and transition to ready before the main loop asserts. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 28 +++++++++++++++++----------- tests/qemu-iotests/030 | 2 +- tests/qemu-iotests/151 | 4 ++-- tests/unit/test-blockjob.c | 2 +- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/job.c b/job.c index 83921dd79b..6ef2adead4 100644 --- a/job.c +++ b/job.c @@ -584,17 +584,15 @@ static void coroutine_fn job_do_yield_locked(Job *job, uint64_t ns) assert(job->busy); } -void coroutine_fn job_pause_point(Job *job) +/* Called with job_mutex held, but releases it temporarly. */ +static void coroutine_fn job_pause_point_locked(Job *job) { assert(job && job_started(job)); - job_lock(); if (!job_should_pause_locked(job)) { - job_unlock(); return; } if (job_is_cancelled_locked(job)) { - job_unlock(); return; } @@ -614,13 +612,20 @@ void coroutine_fn job_pause_point(Job *job) job->paused = false; job_state_transition_locked(job, status); } - job_unlock(); if (job->driver->resume) { + job_unlock(); job->driver->resume(job); + job_lock(); } } +void coroutine_fn job_pause_point(Job *job) +{ + JOB_LOCK_GUARD(); + job_pause_point_locked(job); +} + void job_yield(Job *job) { WITH_JOB_LOCK_GUARD() { @@ -641,21 +646,22 @@ void job_yield(Job *job) void coroutine_fn job_sleep_ns(Job *job, int64_t ns) { - WITH_JOB_LOCK_GUARD() { - assert(job->busy); + int64_t end_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + ns; + JOB_LOCK_GUARD(); + assert(job->busy); + do { /* Check cancellation *before* setting busy = false, too! */ if (job_is_cancelled_locked(job)) { return; } if (!job_should_pause_locked(job)) { - job_do_yield_locked(job, - qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + ns); + job_do_yield_locked(job, end_ns); } - } - job_pause_point(job); + job_pause_point_locked(job); + } while (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) < end_ns); } /* Assumes the job_mutex is held */ diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index 567bf1da67..969b246d0f 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -248,7 +248,7 @@ class TestParallelOps(iotests.QMPTestCase): pending_jobs.append(job_id) result = self.vm.qmp('block-stream', device=node_name, job_id=job_id, bottom=f'node{i-1}', - speed=1024) + speed=256*1024) self.assert_qmp(result, 'return', {}) # Do this in reverse: After unthrottling them, some jobs may finish diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151 index 93d14193d0..5998beb5c4 100755 --- a/tests/qemu-iotests/151 +++ b/tests/qemu-iotests/151 @@ -129,7 +129,7 @@ class TestActiveMirror(iotests.QMPTestCase): sync='full', copy_mode='write-blocking', buf_size=(1048576 // 4), - speed=1) + speed=1024*1024) self.assert_qmp(result, 'return', {}) # Start an unaligned request to a dirty area @@ -154,7 +154,7 @@ class TestActiveMirror(iotests.QMPTestCase): target='target-node', sync='full', copy_mode='write-blocking', - speed=1) + speed=1024*1024) self.vm.hmp_qemu_io('source', 'break write_aio A') self.vm.hmp_qemu_io('source', 'aio_write 0 1M') # 1 diff --git a/tests/unit/test-blockjob.c b/tests/unit/test-blockjob.c index c926db7b5d..0b3010b94d 100644 --- a/tests/unit/test-blockjob.c +++ b/tests/unit/test-blockjob.c @@ -184,7 +184,7 @@ static int coroutine_fn cancel_job_run(Job *job, Error **errp) job_transition_to_ready(&s->common.job); } - job_sleep_ns(&s->common.job, 100000); + job_sleep_ns(&s->common.job, 100); } return 0; From patchwork Tue Jan 18 16:27:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581431 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=a4HSOukW; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZk06tNgz9s1l for ; Wed, 19 Jan 2022 03:59:04 +1100 (AEDT) Received: from localhost ([::1]:41146 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9rp0-00082K-PR for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:59:02 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39068) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUq-0006FB-PF for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:12 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:41321) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUo-00060q-NZ for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:12 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523881; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=h9oH9KFDshDkkrZj6LuueG0DMST4LEuRstXa0K9NWyU=; b=a4HSOukWM5eSBp5iOigN/wNfeNdBmaKs9ybtC6WOeP0Ni5iHo63IOjY8OJOhyK/ykKLwec GwgC1Tk+z8kfZpjvA9XYMGsA0sRFVPBVRVu2iAi1AFJB3he62l6pws7VhpBoY1fcPZwT+e zQ9bg10D4I6nrhXjYHVAvx9bcLbIDFY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-607-rc7TMCNfMlyHGNhO-Go2Vg-1; Tue, 18 Jan 2022 11:37:56 -0500 X-MC-Unique: rc7TMCNfMlyHGNhO-Go2Vg-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 92A4F1856EFB; Tue, 18 Jan 2022 16:28:28 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id C8C7334D3A; Tue, 18 Jan 2022 16:28:27 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 10/12] block.c: add subtree_drains where needed Date: Tue, 18 Jan 2022 11:27:36 -0500 Message-Id: <20220118162738.1366281-11-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Protect bdrv_replace_child_noperm, as it modifies the graph by adding/removing elements to .children and .parents list of a bs. Use the newly introduced bdrv_subtree_drained_{begin/end}_unlocked drains to achieve that and be free from the aiocontext lock. One important criteria to keep in mind is that if the caller of bdrv_replace_child_noperm creates a transaction, we need to make sure that the whole transaction is under the same drain block. This is imperative, as having multiple drains also in the .abort() class of functions causes discrepancies in the drained counters (as nodes are put back into the original positions), making it really hard to retourn all to zero and leaving the code very buggy. See https://patchew.org/QEMU/20211213104014.69858-1-eesposit@redhat.com/ for more explanations. Unfortunately we still need to have bdrv_subtree_drained_begin/end in bdrv_detach_child() releasing and then holding the AioContext lock, since it later invokes bdrv_try_set_aio_context() that is not safe yet. Once all is cleaned up, we can also remove the acquire/release locks in job_unref, artificially added because of this. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/block.c b/block.c index fcc44a49a0..6196c95aae 100644 --- a/block.c +++ b/block.c @@ -3114,8 +3114,22 @@ static void bdrv_detach_child(BdrvChild **childp) BlockDriverState *old_bs = (*childp)->bs; assert(qemu_in_main_thread()); + if (old_bs) { + /* + * TODO: this is called by job_unref with lock held, because + * afterwards it calls bdrv_try_set_aio_context. + * Once all of this is fixed, take care of removing + * the aiocontext lock and make this function _unlocked. + */ + bdrv_subtree_drained_begin(old_bs); + } + bdrv_replace_child_noperm(childp, NULL, true); + if (old_bs) { + bdrv_subtree_drained_end(old_bs); + } + if (old_bs) { /* * Update permissions for old node. We're just taking a parent away, so @@ -3154,6 +3168,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, Transaction *tran = tran_new(); assert(qemu_in_main_thread()); + bdrv_subtree_drained_begin_unlocked(child_bs); ret = bdrv_attach_child_common(child_bs, child_name, child_class, child_role, perm, shared_perm, opaque, @@ -3168,6 +3183,7 @@ out: tran_finalize(tran, ret); /* child is unset on failure by bdrv_attach_child_common_abort() */ assert((ret < 0) == !child); + bdrv_subtree_drained_end_unlocked(child_bs); bdrv_unref(child_bs); return child; @@ -3197,6 +3213,9 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, assert(qemu_in_main_thread()); + bdrv_subtree_drained_begin_unlocked(parent_bs); + bdrv_subtree_drained_begin_unlocked(child_bs); + ret = bdrv_attach_child_noperm(parent_bs, child_bs, child_name, child_class, child_role, &child, tran, errp); if (ret < 0) { @@ -3211,6 +3230,9 @@ BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, out: tran_finalize(tran, ret); /* child is unset on failure by bdrv_attach_child_common_abort() */ + bdrv_subtree_drained_end_unlocked(child_bs); + bdrv_subtree_drained_end_unlocked(parent_bs); + assert((ret < 0) == !child); bdrv_unref(child_bs); @@ -3456,6 +3478,11 @@ int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, assert(qemu_in_main_thread()); + bdrv_subtree_drained_begin_unlocked(bs); + if (backing_hd) { + bdrv_subtree_drained_begin_unlocked(backing_hd); + } + ret = bdrv_set_backing_noperm(bs, backing_hd, tran, errp); if (ret < 0) { goto out; @@ -3464,6 +3491,10 @@ int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, ret = bdrv_refresh_perms(bs, errp); out: tran_finalize(tran, ret); + if (backing_hd) { + bdrv_subtree_drained_end_unlocked(backing_hd); + } + bdrv_subtree_drained_end_unlocked(bs); return ret; } @@ -5266,7 +5297,8 @@ static int bdrv_replace_node_common(BlockDriverState *from, assert(qemu_get_current_aio_context() == qemu_get_aio_context()); assert(bdrv_get_aio_context(from) == bdrv_get_aio_context(to)); - bdrv_drained_begin(from); + bdrv_subtree_drained_begin_unlocked(from); + bdrv_subtree_drained_begin_unlocked(to); /* * Do the replacement without permission update. @@ -5298,7 +5330,8 @@ static int bdrv_replace_node_common(BlockDriverState *from, out: tran_finalize(tran, ret); - bdrv_drained_end(from); + bdrv_subtree_drained_end_unlocked(to); + bdrv_subtree_drained_end_unlocked(from); bdrv_unref(from); return ret; @@ -5345,6 +5378,9 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, assert(!bs_new->backing); + bdrv_subtree_drained_begin_unlocked(bs_new); + bdrv_subtree_drained_begin_unlocked(bs_top); + ret = bdrv_attach_child_noperm(bs_new, bs_top, "backing", &child_of_bds, bdrv_backing_role(bs_new), &bs_new->backing, tran, errp); @@ -5360,6 +5396,8 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, ret = bdrv_refresh_perms(bs_new, errp); out: tran_finalize(tran, ret); + bdrv_subtree_drained_end_unlocked(bs_top); + bdrv_subtree_drained_end_unlocked(bs_new); bdrv_refresh_limits(bs_top, NULL, NULL); @@ -5379,8 +5417,8 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs, assert(qemu_in_main_thread()); bdrv_ref(old_bs); - bdrv_drained_begin(old_bs); - bdrv_drained_begin(new_bs); + bdrv_subtree_drained_begin_unlocked(old_bs); + bdrv_subtree_drained_begin_unlocked(new_bs); bdrv_replace_child_tran(&child, new_bs, tran, true); /* @new_bs must have been non-NULL, so @child must not have been freed */ @@ -5394,8 +5432,8 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs, tran_finalize(tran, ret); - bdrv_drained_end(old_bs); - bdrv_drained_end(new_bs); + bdrv_subtree_drained_end_unlocked(new_bs); + bdrv_subtree_drained_end_unlocked(old_bs); bdrv_unref(old_bs); return ret; From patchwork Tue Jan 18 16:27:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581422 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=i+1Z0xAa; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZTg4nk2z9sSs for ; Wed, 19 Jan 2022 03:48:23 +1100 (AEDT) Received: from localhost ([::1]:44972 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9ref-0007nR-1L for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 11:48:21 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39374) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVW-0007Pp-Fg for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:54 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:46999) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rVU-00066x-Kx for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:53 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523932; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hKHLrZHyUcnoBxO5vObGu0F6VvFauzwbCCLT3dnhHGs=; b=i+1Z0xAa49l3o6YDIOGweYj/IreCjjmiCrOAMJsJ0mpPKIRxR2WOCefYvCiSXpp7IzEsZP HVPFy7sqVFSiJHT8yw855eI70s3yFfNbZihf0AOUXXuunXJr2BlKwRmAt4JuOxexuPpA+6 rGOOriftMQamYmCFXcc7YXOORDJfoig= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-605-sVX_1yfUOiuEalazIhX0tg-1; Tue, 18 Jan 2022 11:38:48 -0500 X-MC-Unique: sVX_1yfUOiuEalazIhX0tg-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 747DC108668D; Tue, 18 Jan 2022 16:28:29 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id AB170348F8; Tue, 18 Jan 2022 16:28:28 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 11/12] block/io.c: fully enable assert_bdrv_graph_writable Date: Tue, 18 Jan 2022 11:27:37 -0500 Message-Id: <20220118162738.1366281-12-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" As explained in the TODO, complete the function by checking that the node is also drained. In this way, we can ensure that modify the bs is thread safe, as the drain makes sure that no I/O concurrently reads the field, and all writes are under BQL. Signed-off-by: Emanuele Giuseppe Esposito --- block/io.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/block/io.c b/block/io.c index 9d5167f64a..d7b0707662 100644 --- a/block/io.c +++ b/block/io.c @@ -765,12 +765,7 @@ void bdrv_drain_all(void) void assert_bdrv_graph_writable(BlockDriverState *bs) { - /* - * TODO: this function is incomplete. Because the users of this - * assert lack the necessary drains, check only for BQL. - * Once the necessary drains are added, - * assert also for qatomic_read(&bs->quiesce_counter) > 0 - */ + assert(qatomic_read(&bs->quiesce_counter) > 0); assert(qemu_in_main_thread()); } From patchwork Tue Jan 18 16:27:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1581436 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=i0jd7eR3; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4JdZrl2d6cz9s1l for ; Wed, 19 Jan 2022 04:04:53 +1100 (AEDT) Received: from localhost ([::1]:48802 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9ruc-0004uW-7x for incoming@patchwork.ozlabs.org; Tue, 18 Jan 2022 12:04:50 -0500 Received: from eggs.gnu.org ([209.51.188.92]:39170) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUt-0006Pd-MU for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:15 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:40965) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9rUo-000614-Qd for qemu-devel@nongnu.org; Tue, 18 Jan 2022 11:38:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1642523882; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=57igYvlsN0D5JR36F1J5QcRbp3wZctA3qP6mPCCwrn8=; b=i0jd7eR3Dak2p/73LH3Mh5QFYh34G7+uvLpKYJrmjlfsIsAPsdRa7q3zYoGb99DMDzs76F sdnh64D08cBbusRf4kLfBYIKNhMXqnPngYb3GLbjfhA8fvLqBvj6QTdU9Axq98KkYJ3WW2 r1ZOSl6PumXwm7wpr0Ov6Q9kGgpj9Jk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-60-g3buZYuNP-i67QEuNKxS5A-1; Tue, 18 Jan 2022 11:37:59 -0500 X-MC-Unique: g3buZYuNP-i67QEuNKxS5A-1 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5426214453B; Tue, 18 Jan 2022 16:28:30 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8D237348F8; Tue, 18 Jan 2022 16:28:29 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH 12/12] block.c: additional assert qemu in main tread Date: Tue, 18 Jan 2022 11:27:38 -0500 Message-Id: <20220118162738.1366281-13-eesposit@redhat.com> In-Reply-To: <20220118162738.1366281-1-eesposit@redhat.com> References: <20220118162738.1366281-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , Vladimir Sementsov-Ogievskiy , Emanuele Giuseppe Esposito , qemu-devel@nongnu.org, Hanna Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Add some missing assertion in static functions of block.c Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 ++ block/block-backend.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/block.c b/block.c index 6196c95aae..7961f5a984 100644 --- a/block.c +++ b/block.c @@ -5227,6 +5227,7 @@ static int bdrv_replace_node_noperm(BlockDriverState *from, BdrvChild *c, *next; assert(to != NULL); + assert(qemu_in_main_thread()); QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { assert(c->bs == from); @@ -6767,6 +6768,7 @@ void bdrv_invalidate_cache_all(Error **errp) static bool bdrv_has_bds_parent(BlockDriverState *bs, bool only_active) { BdrvChild *parent; + assert(qemu_in_main_thread()); QLIST_FOREACH(parent, &bs->parents, next_parent) { if (parent->klass->parent_is_bds) { diff --git a/block/block-backend.c b/block/block-backend.c index 9229ff7ca7..048ba83f37 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -754,6 +754,9 @@ BlockDriverState *blk_bs(BlockBackend *blk) static BlockBackend *bdrv_first_blk(BlockDriverState *bs) { BdrvChild *child; + + assert(qemu_in_main_thread()); + QLIST_FOREACH(child, &bs->parents, next_parent) { if (child->klass == &child_root) { return child->opaque;