From patchwork Thu Nov 4 10:38:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 1550844 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=Ld7pAPJJ; 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 4HlKvV5hX7z9sR4 for ; Thu, 4 Nov 2021 21:41:58 +1100 (AEDT) Received: from localhost ([::1]:37908 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1miaBw-0007Ha-J8 for incoming@patchwork.ozlabs.org; Thu, 04 Nov 2021 06:41:56 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56306) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9X-0003vd-BQ for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:27 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:26919) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9V-0006uA-4z for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636022363; 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=z6668KD6YY0BeZYP8+rehG/6fOcNrcpgLGR5j1kl6c4=; b=Ld7pAPJJtXmLKn/vL2Yqe8bXJPdpADGa1InwbYxfqd6uU9nnWF++qy4wxSYfLEAOYMm+4U BjCye3NSCKxLS2PTqFOsM7Xr4veaXoIWzBOqjsG3WV89Sa74O5pAAbBXy8b3U3yG2iW0lC IrcpC22ufdYIcF0B+am68/r0vk0iEHA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-359-qxdYRPBIOFiFGYkzRGk6bg-1; Thu, 04 Nov 2021 06:39:20 -0400 X-MC-Unique: qxdYRPBIOFiFGYkzRGk6bg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A748218125C0; Thu, 4 Nov 2021 10:39:19 +0000 (UTC) Received: from localhost (unknown [10.39.192.248]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 804B5A6C5D; Thu, 4 Nov 2021 10:39:04 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PATCH 1/7] stream: Traverse graph after modification Date: Thu, 4 Nov 2021 11:38:43 +0100 Message-Id: <20211104103849.46855-2-hreitz@redhat.com> In-Reply-To: <20211104103849.46855-1-hreitz@redhat.com> References: <20211104103849.46855-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@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.717, 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_H2=-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 , Hanna Reitz , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" bdrv_cor_filter_drop() modifies the block graph. That means that other parties can also modify the block graph before it returns. Therefore, we cannot assume that the result of a graph traversal we did before remains valid afterwards. We should thus fetch `base` and `unfiltered_base` afterwards instead of before. Signed-off-by: Hanna Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/stream.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/block/stream.c b/block/stream.c index 97bee482dc..e45113aed6 100644 --- a/block/stream.c +++ b/block/stream.c @@ -54,8 +54,8 @@ static int stream_prepare(Job *job) { StreamBlockJob *s = container_of(job, StreamBlockJob, common.job); BlockDriverState *unfiltered_bs = bdrv_skip_filters(s->target_bs); - BlockDriverState *base = bdrv_filter_or_cow_bs(s->above_base); - BlockDriverState *unfiltered_base = bdrv_skip_filters(base); + BlockDriverState *base; + BlockDriverState *unfiltered_base; Error *local_err = NULL; int ret = 0; @@ -63,6 +63,9 @@ static int stream_prepare(Job *job) bdrv_cor_filter_drop(s->cor_filter_bs); s->cor_filter_bs = NULL; + base = bdrv_filter_or_cow_bs(s->above_base); + unfiltered_base = bdrv_skip_filters(base); + if (bdrv_cow_child(unfiltered_bs)) { const char *base_id = NULL, *base_fmt = NULL; if (unfiltered_base) { From patchwork Thu Nov 4 10:38:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 1550842 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=YB6BxPI7; 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 4HlKsH2zCzz9sR4 for ; Thu, 4 Nov 2021 21:40:03 +1100 (AEDT) Received: from localhost ([::1]:33588 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1miaA4-0004Hx-R1 for incoming@patchwork.ozlabs.org; Thu, 04 Nov 2021 06:40:00 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56334) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9f-0004G4-SG for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:35 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:25865) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9e-0006v2-0P for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:35 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636022373; 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=wicCU2oy+FN3RifIdeaIhHv+GV5abyLPoa/GqoS4WOI=; b=YB6BxPI7QL73AXKaAHlRFMUqbDmGlqwT49k8p5OGViveAsi08LMjyLb9OHS20TdbpuCHrM ogehV9zTwvKGU+ZOl/4ewALpNUL7j+EW1b5DI7Z67clAQJdQIgKKaeBo6IbrzlbkCRzKiu pBIITHcmdFgzF1BDAaKfzhCUKINRq30= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-296-kWERbitVM5qJ-8IwiIILRA-1; Thu, 04 Nov 2021 06:39:30 -0400 X-MC-Unique: kWERbitVM5qJ-8IwiIILRA-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 2A2F1871805; Thu, 4 Nov 2021 10:39:29 +0000 (UTC) Received: from localhost (unknown [10.39.192.248]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8A1B819C79; Thu, 4 Nov 2021 10:39:21 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PATCH 2/7] block: Manipulate children list in .attach/.detach Date: Thu, 4 Nov 2021 11:38:44 +0100 Message-Id: <20211104103849.46855-3-hreitz@redhat.com> In-Reply-To: <20211104103849.46855-1-hreitz@redhat.com> References: <20211104103849.46855-1-hreitz@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=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=hreitz@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.717, 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_H2=-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 , Hanna Reitz , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The children list is specific to BDS parents. We should not modify it in the general children modification code, but let BDS parents deal with it in their .attach() and .detach() methods. This also has the advantage that a BdrvChild is removed from the children list before its .bs pointer can become NULL. BDS parents generally assume that their children's .bs pointer is never NULL, so this is actually a bug fix. Signed-off-by: Hanna Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index 580cb77a70..243ae206b5 100644 --- a/block.c +++ b/block.c @@ -1387,6 +1387,8 @@ static void bdrv_child_cb_attach(BdrvChild *child) { BlockDriverState *bs = child->opaque; + QLIST_INSERT_HEAD(&bs->children, child, next); + if (child->role & BDRV_CHILD_COW) { bdrv_backing_attach(child); } @@ -1403,6 +1405,8 @@ static void bdrv_child_cb_detach(BdrvChild *child) } bdrv_unapply_subtree_drain(child, bs); + + QLIST_REMOVE(child, next); } static int bdrv_child_cb_update_filename(BdrvChild *c, BlockDriverState *base, @@ -2747,7 +2751,7 @@ static void bdrv_child_free(void *opaque) static void bdrv_remove_empty_child(BdrvChild *child) { assert(!child->bs); - QLIST_SAFE_REMOVE(child, next); + assert(!child->next.le_prev); /* not in children list */ bdrv_child_free(child); } @@ -2913,7 +2917,6 @@ static int bdrv_attach_child_noperm(BlockDriverState *parent_bs, return ret; } - QLIST_INSERT_HEAD(&parent_bs->children, *child, next); /* * child is removed in bdrv_attach_child_common_abort(), so don't care to * abort this change separately. @@ -4851,7 +4854,6 @@ static void bdrv_remove_filter_or_cow_child_abort(void *opaque) BdrvRemoveFilterOrCowChild *s = opaque; BlockDriverState *parent_bs = s->child->opaque; - QLIST_INSERT_HEAD(&parent_bs->children, s->child, next); if (s->is_backing) { parent_bs->backing = s->child; } else { @@ -4906,7 +4908,6 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, }; tran_add(tran, &bdrv_remove_filter_or_cow_child_drv, s); - QLIST_SAFE_REMOVE(child, next); if (s->is_backing) { bs->backing = NULL; } else { From patchwork Thu Nov 4 10:38:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 1550843 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=CGX1wtu5; 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 4HlKt733CJz9sR4 for ; Thu, 4 Nov 2021 21:40:47 +1100 (AEDT) Received: from localhost ([::1]:34098 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1miaAn-0004dk-3l for incoming@patchwork.ozlabs.org; Thu, 04 Nov 2021 06:40:45 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56356) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9m-0004Zh-VL for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:43 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:46553) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9l-0006vc-25 for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636022380; 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=hGEGeFVzWLkPnTQJG9D/IwiYUT2c8nEAkpAE7miUqxw=; b=CGX1wtu5tZAr130vp9fpeiKdBnGBSWpEzoA3bAHnPs6Cm3Ly+bwzhtgTUbTbMUEP5MNVce lOfrc3MYft7Bd2SshITKXfcnSa7MY0xXTLeNT97cirvbMUrquRjbryTkI6gs+wiBlQIYuW rk4ElvV+vMGAOH6P8n4YRlVCapOFaWk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-437-8DDSh_tVOn6v14WtH_AUYg-1; Thu, 04 Nov 2021 06:39:39 -0400 X-MC-Unique: 8DDSh_tVOn6v14WtH_AUYg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6489110A8E01; Thu, 4 Nov 2021 10:39:38 +0000 (UTC) Received: from localhost (unknown [10.39.192.248]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 00BCF607A4; Thu, 4 Nov 2021 10:39:30 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PATCH 3/7] block: Unite remove_empty_child and child_free Date: Thu, 4 Nov 2021 11:38:45 +0100 Message-Id: <20211104103849.46855-4-hreitz@redhat.com> In-Reply-To: <20211104103849.46855-1-hreitz@redhat.com> References: <20211104103849.46855-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=hreitz@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.717, 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_H2=-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 , Hanna Reitz , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Now that bdrv_remove_empty_child() no longer removes the child from the parent's children list but only checks that it is not in such a list, it is only a wrapper around bdrv_child_free() that checks that the child is empty and unused. That should apply to all children that we free, so put those checks into bdrv_child_free() and drop bdrv_remove_empty_child(). Signed-off-by: Hanna Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/block.c b/block.c index 243ae206b5..b95f8dcf8f 100644 --- a/block.c +++ b/block.c @@ -2740,19 +2740,19 @@ static void bdrv_replace_child_noperm(BdrvChild *child, } } -static void bdrv_child_free(void *opaque) -{ - BdrvChild *c = opaque; - - g_free(c->name); - g_free(c); -} - -static void bdrv_remove_empty_child(BdrvChild *child) +/** + * Free the given @child. + * + * The child must be empty (i.e. `child->bs == NULL`) and it must be + * unused (i.e. not in a children list). + */ +static void bdrv_child_free(BdrvChild *child) { assert(!child->bs); assert(!child->next.le_prev); /* not in children list */ - bdrv_child_free(child); + + g_free(child->name); + g_free(child); } typedef struct BdrvAttachChildCommonState { @@ -2786,7 +2786,7 @@ static void bdrv_attach_child_common_abort(void *opaque) } bdrv_unref(bs); - bdrv_remove_empty_child(child); + bdrv_child_free(child); *s->child = NULL; } @@ -2859,7 +2859,7 @@ static int bdrv_attach_child_common(BlockDriverState *child_bs, if (ret < 0) { error_propagate(errp, local_err); - bdrv_remove_empty_child(new_child); + bdrv_child_free(new_child); return ret; } } @@ -2930,7 +2930,7 @@ static void bdrv_detach_child(BdrvChild *child) BlockDriverState *old_bs = child->bs; bdrv_replace_child_noperm(child, NULL); - bdrv_remove_empty_child(child); + bdrv_child_free(child); if (old_bs) { /* From patchwork Thu Nov 4 10:38:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 1550847 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=NK8srRRi; 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 4HlKzP3gPvz9sR4 for ; Thu, 4 Nov 2021 21:45:19 +1100 (AEDT) Received: from localhost ([::1]:43814 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1miaFA-0002ps-K7 for incoming@patchwork.ozlabs.org; Thu, 04 Nov 2021 06:45:16 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56414) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9t-0004w3-PW for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:49 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:40148) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9r-0006wI-JA for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:49 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636022386; 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=KiT5wJE3hpi6OY87AfVxU1M/IJt6n26tzfmI8okiQDQ=; b=NK8srRRi2LwZ/1FwafZzhzaDiKPYsD7nNjY/QSTcjuMyQ5tlrZH52UIQmk5uKKkYzkZfH/ 3HoX7z0Tr/ic20le0XW0FDH6ibTccbP3V00vc4DbHxXa+wz9CSjFSwl5dzjjxfhnN7vlFP l2XIh4nqeHrjmrLYoXt2UJnT1bIqTmE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-532-yLqMb_33Mg-OUR4aspFjvA-1; Thu, 04 Nov 2021 06:39:43 -0400 X-MC-Unique: yLqMb_33Mg-OUR4aspFjvA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BC4511005E65; Thu, 4 Nov 2021 10:39:42 +0000 (UTC) Received: from localhost (unknown [10.39.192.248]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3A4857092B; Thu, 4 Nov 2021 10:39:40 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PATCH 4/7] block: Drop detached child from ignore list Date: Thu, 4 Nov 2021 11:38:46 +0100 Message-Id: <20211104103849.46855-5-hreitz@redhat.com> In-Reply-To: <20211104103849.46855-1-hreitz@redhat.com> References: <20211104103849.46855-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=hreitz@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.717, 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_H2=-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 , Hanna Reitz , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" bdrv_attach_child_common_abort() restores the parent's AioContext. To do so, the child (which was supposed to be attached, but is now detached again by this abort handler) is added to the ignore list for the AioContext changing functions. However, since we modify a BDS's children list in the BdrvChildClass's .attach and .detach handlers, the child is already effectively detached from the parent by this point. We do not need to put it into the ignore list. Use this opportunity to clean up the empty line structure: Keep setting the ignore list, invoking the AioContext function, and freeing the ignore list in blocks separated by empty lines. Signed-off-by: Hanna Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- block.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index b95f8dcf8f..6d230ad3d1 100644 --- a/block.c +++ b/block.c @@ -2774,14 +2774,16 @@ static void bdrv_attach_child_common_abort(void *opaque) } if (bdrv_child_get_parent_aio_context(child) != s->old_parent_ctx) { - GSList *ignore = g_slist_prepend(NULL, child); + GSList *ignore; + /* No need to ignore `child`, because it has been detached already */ + ignore = NULL; child->klass->can_set_aio_ctx(child, s->old_parent_ctx, &ignore, &error_abort); g_slist_free(ignore); - ignore = g_slist_prepend(NULL, child); - child->klass->set_aio_ctx(child, s->old_parent_ctx, &ignore); + ignore = NULL; + child->klass->set_aio_ctx(child, s->old_parent_ctx, &ignore); g_slist_free(ignore); } From patchwork Thu Nov 4 10:38:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 1550845 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=eFxy7ktl; 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 4HlKxT6WKbz9sR4 for ; Thu, 4 Nov 2021 21:43:41 +1100 (AEDT) Received: from localhost ([::1]:41710 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1miaDb-0001Px-OH for incoming@patchwork.ozlabs.org; Thu, 04 Nov 2021 06:43:39 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56446) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9v-000538-Ub for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:51 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:45737) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mia9t-0006wd-QB for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:39:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636022389; 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=kocsaTnqsOQDRk27VonMvEt1oNRfZ10PzaAMGOTMEEI=; b=eFxy7ktlySgpazRRNHObBb1B7z6XEA739pXLdaUxGT6w8ekJmflFmde3O1zdE+w3uObjcp ZOmm5eBQLPCLJOhJRQSHqw+h7e8PUJmlC0W8dnpcKqF/m2qymDoEwz80DXZZaBZZmhyxus 93qyvh4Nh0kOV42IVbbibl4Xm70GWpc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-227-pKuV4kCzMH-qBQvTbPDhuQ-1; Thu, 04 Nov 2021 06:39:46 -0400 X-MC-Unique: pKuV4kCzMH-qBQvTbPDhuQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 07997871807; Thu, 4 Nov 2021 10:39:45 +0000 (UTC) Received: from localhost (unknown [10.39.192.248]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 772885D9D3; Thu, 4 Nov 2021 10:39:44 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PATCH 5/7] block: Pass BdrvChild ** to replace_child_noperm Date: Thu, 4 Nov 2021 11:38:47 +0100 Message-Id: <20211104103849.46855-6-hreitz@redhat.com> In-Reply-To: <20211104103849.46855-1-hreitz@redhat.com> References: <20211104103849.46855-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=hreitz@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.717, 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_H4=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 , Hanna Reitz , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" bdrv_replace_child_noperm() modifies BdrvChild.bs, and can potentially set it to NULL. That is dangerous, because BDS parents generally assume that their children's .bs pointer is never NULL. We therefore want to let bdrv_replace_child_noperm() set the corresponding BdrvChild pointer to NULL, too. This patch lays the foundation for it by passing a BdrvChild ** pointer to bdrv_replace_child_noperm() so that it can later use it to NULL the BdrvChild pointer immediately after setting BdrvChild.bs to NULL. Signed-off-by: Hanna Reitz --- block.c | 59 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/block.c b/block.c index 6d230ad3d1..ff45447686 100644 --- a/block.c +++ b/block.c @@ -87,7 +87,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, static bool bdrv_recurse_has_child(BlockDriverState *bs, BlockDriverState *child); -static void bdrv_replace_child_noperm(BdrvChild *child, +static void bdrv_replace_child_noperm(BdrvChild **child, BlockDriverState *new_bs); static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, BdrvChild *child, @@ -2254,6 +2254,7 @@ static int bdrv_drv_set_perm(BlockDriverState *bs, uint64_t perm, typedef struct BdrvReplaceChildState { BdrvChild *child; + BdrvChild **childp; BlockDriverState *old_bs; } BdrvReplaceChildState; @@ -2269,8 +2270,8 @@ static void bdrv_replace_child_abort(void *opaque) BdrvReplaceChildState *s = opaque; BlockDriverState *new_bs = s->child->bs; - /* old_bs reference is transparently moved from @s to @s->child */ - bdrv_replace_child_noperm(s->child, s->old_bs); + /* old_bs reference is transparently moved from @s to *s->childp */ + bdrv_replace_child_noperm(s->childp, s->old_bs); bdrv_unref(new_bs); } @@ -2287,21 +2288,23 @@ static TransactionActionDrv bdrv_replace_child_drv = { * * The function doesn't update permissions, caller is responsible for this. */ -static void bdrv_replace_child_tran(BdrvChild *child, BlockDriverState *new_bs, +static void bdrv_replace_child_tran(BdrvChild **childp, + BlockDriverState *new_bs, Transaction *tran) { BdrvReplaceChildState *s = g_new(BdrvReplaceChildState, 1); *s = (BdrvReplaceChildState) { - .child = child, - .old_bs = child->bs, + .child = *childp, + .childp = childp, + .old_bs = (*childp)->bs, }; tran_add(tran, &bdrv_replace_child_drv, s); if (new_bs) { bdrv_ref(new_bs); } - bdrv_replace_child_noperm(child, new_bs); - /* old_bs reference is transparently moved from @child to @s */ + bdrv_replace_child_noperm(childp, new_bs); + /* old_bs reference is transparently moved from *childp to @s */ } /* @@ -2672,9 +2675,10 @@ uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm) return permissions[qapi_perm]; } -static void bdrv_replace_child_noperm(BdrvChild *child, +static void bdrv_replace_child_noperm(BdrvChild **childp, BlockDriverState *new_bs) { + BdrvChild *child = *childp; BlockDriverState *old_bs = child->bs; int new_bs_quiesce_counter; int drain_saldo; @@ -2767,7 +2771,7 @@ static void bdrv_attach_child_common_abort(void *opaque) BdrvChild *child = *s->child; BlockDriverState *bs = child->bs; - bdrv_replace_child_noperm(child, NULL); + bdrv_replace_child_noperm(s->child, NULL); if (bdrv_get_aio_context(bs) != s->old_child_ctx) { bdrv_try_set_aio_context(bs, s->old_child_ctx, &error_abort); @@ -2867,7 +2871,7 @@ static int bdrv_attach_child_common(BlockDriverState *child_bs, } bdrv_ref(child_bs); - bdrv_replace_child_noperm(new_child, child_bs); + bdrv_replace_child_noperm(&new_child, child_bs); *child = new_child; @@ -2927,12 +2931,12 @@ static int bdrv_attach_child_noperm(BlockDriverState *parent_bs, return 0; } -static void bdrv_detach_child(BdrvChild *child) +static void bdrv_detach_child(BdrvChild **childp) { - BlockDriverState *old_bs = child->bs; + BlockDriverState *old_bs = (*childp)->bs; - bdrv_replace_child_noperm(child, NULL); - bdrv_child_free(child); + bdrv_replace_child_noperm(childp, NULL); + bdrv_child_free(*childp); if (old_bs) { /* @@ -3038,7 +3042,7 @@ void bdrv_root_unref_child(BdrvChild *child) BlockDriverState *child_bs; child_bs = child->bs; - bdrv_detach_child(child); + bdrv_detach_child(&child); bdrv_unref(child_bs); } @@ -4891,30 +4895,33 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, BdrvChild *child, Transaction *tran) { + BdrvChild **childp; BdrvRemoveFilterOrCowChild *s; - assert(child == bs->backing || child == bs->file); + if (child == bs->backing) { + childp = &bs->backing; + } else if (child == bs->file) { + childp = &bs->file; + } else { + g_assert_not_reached(); + } if (!child) { return; } if (child->bs) { - bdrv_replace_child_tran(child, NULL, tran); + bdrv_replace_child_tran(childp, NULL, tran); } s = g_new(BdrvRemoveFilterOrCowChild, 1); *s = (BdrvRemoveFilterOrCowChild) { .child = child, - .is_backing = (child == bs->backing), + .is_backing = (childp == &bs->backing), }; tran_add(tran, &bdrv_remove_filter_or_cow_child_drv, s); - if (s->is_backing) { - bs->backing = NULL; - } else { - bs->file = NULL; - } + *childp = NULL; } /* @@ -4950,7 +4957,7 @@ static int bdrv_replace_node_noperm(BlockDriverState *from, c->name, from->node_name); return -EPERM; } - bdrv_replace_child_tran(c, to, tran); + bdrv_replace_child_tran(&c, to, tran); } return 0; @@ -5099,7 +5106,7 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs, bdrv_drained_begin(old_bs); bdrv_drained_begin(new_bs); - bdrv_replace_child_tran(child, new_bs, tran); + bdrv_replace_child_tran(&child, new_bs, tran); found = g_hash_table_new(NULL, NULL); refresh_list = bdrv_topological_dfs(refresh_list, found, old_bs); From patchwork Thu Nov 4 10:38:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 1550848 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=W/3lBiqj; 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 4HlL0G3LqBz9sR4 for ; Thu, 4 Nov 2021 21:46:06 +1100 (AEDT) Received: from localhost ([::1]:46102 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1miaFv-0004Rx-N9 for incoming@patchwork.ozlabs.org; Thu, 04 Nov 2021 06:46:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56686) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1miaAR-0005eU-Bb for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:40:23 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:39097) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1miaAN-000723-5k for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:40:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636022418; 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=2xSKkkPNWwbycG9llUf8KgHMJXz4ncbW+VKFKHdBAeQ=; b=W/3lBiqjfQXiazD/7n5gweGHsqKBizA29lclNb5WiF4+LVKxWNrjaSah6SwqqPvsKc3XpD iWduOTCmF+6Auu2zG9aTjOXtbomnrYPNa8kP5jvgeGDJRB2f27z6HraiA9rC4+cevsUp7B EFy8HdfMaOEvlnG25VJd4ck8Gax0G5k= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-149-KTr82W0VN1uIVmfrysmOBg-1; Thu, 04 Nov 2021 06:40:14 -0400 X-MC-Unique: KTr82W0VN1uIVmfrysmOBg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BFFBE180831A; Thu, 4 Nov 2021 10:40:13 +0000 (UTC) Received: from localhost (unknown [10.39.192.248]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C1EC770F63; Thu, 4 Nov 2021 10:39:46 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PATCH 6/7] block: Let replace_child_noperm free children Date: Thu, 4 Nov 2021 11:38:48 +0100 Message-Id: <20211104103849.46855-7-hreitz@redhat.com> In-Reply-To: <20211104103849.46855-1-hreitz@redhat.com> References: <20211104103849.46855-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.129.124; envelope-from=hreitz@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.717, 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_H4=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 , Hanna Reitz , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" In most of the block layer, especially when traversing down from other BlockDriverStates, we assume that BdrvChild.bs can never be NULL. When it becomes NULL, it is expected that the corresponding BdrvChild pointer also becomes NULL and the BdrvChild object is freed. Therefore, once bdrv_replace_child_noperm() sets the BdrvChild.bs pointer to NULL, it should also immediately set the corresponding BdrvChild pointer (like bs->file or bs->backing) to NULL. In that context, it also makes sense for this function to free the child. Sometimes we cannot do so, though, because it is called in a transactional context where the caller might still want to reinstate the child in the abort branch (and free it only on commit), so this behavior has to remain optional. In bdrv_replace_child_tran()'s abort handler, we now rely on the fact that the BdrvChild passed to bdrv_replace_child_tran() must have had a non-NULL .bs pointer initially. Make a note of that and assert it. Signed-off-by: Hanna Reitz --- block.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index ff45447686..0a85ede8dc 100644 --- a/block.c +++ b/block.c @@ -87,8 +87,10 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, static bool bdrv_recurse_has_child(BlockDriverState *bs, BlockDriverState *child); +static void bdrv_child_free(BdrvChild *child); static void bdrv_replace_child_noperm(BdrvChild **child, - BlockDriverState *new_bs); + BlockDriverState *new_bs, + bool free_empty_child); static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, BdrvChild *child, Transaction *tran); @@ -2256,12 +2258,16 @@ typedef struct BdrvReplaceChildState { BdrvChild *child; BdrvChild **childp; BlockDriverState *old_bs; + bool free_empty_child; } BdrvReplaceChildState; static void bdrv_replace_child_commit(void *opaque) { BdrvReplaceChildState *s = opaque; + if (s->free_empty_child && !s->child->bs) { + bdrv_child_free(s->child); + } bdrv_unref(s->old_bs); } @@ -2270,8 +2276,23 @@ static void bdrv_replace_child_abort(void *opaque) BdrvReplaceChildState *s = opaque; BlockDriverState *new_bs = s->child->bs; - /* old_bs reference is transparently moved from @s to *s->childp */ - bdrv_replace_child_noperm(s->childp, s->old_bs); + /* + * old_bs reference is transparently moved from @s to s->child; + * pass &s->child here instead of s->childp, because *s->childp + * will have been cleared by bdrv_replace_child_tran()'s + * bdrv_replace_child_noperm() call if new_bs is NULL, and we must + * not pass a NULL *s->childp here. + */ + bdrv_replace_child_noperm(&s->child, s->old_bs, true); + /* + * The child was pre-existing, so s->old_bs must be non-NULL, and + * s->child thus must not have been freed + */ + assert(s->child != NULL); + if (!new_bs) { + /* As described above, *s->childp was cleared, so restore it */ + *s->childp = s->child; + } bdrv_unref(new_bs); } @@ -2287,23 +2308,40 @@ static TransactionActionDrv bdrv_replace_child_drv = { * Note: real unref of old_bs is done only on commit. * * The function doesn't update permissions, caller is responsible for this. + * + * (*childp)->bs must not be NULL. + * + * If @free_empty_child is true and @new_bs is NULL, the BdrvChild is + * freed (on commit). @free_empty_child should only be false if the + * caller will free the BDrvChild themselves (which may be important + * if this is in turn called in another transactional context). */ static void bdrv_replace_child_tran(BdrvChild **childp, BlockDriverState *new_bs, - Transaction *tran) + Transaction *tran, + bool free_empty_child) { BdrvReplaceChildState *s = g_new(BdrvReplaceChildState, 1); *s = (BdrvReplaceChildState) { .child = *childp, .childp = childp, .old_bs = (*childp)->bs, + .free_empty_child = free_empty_child, }; tran_add(tran, &bdrv_replace_child_drv, s); + /* The abort handler relies on this */ + assert(s->old_bs != NULL); + if (new_bs) { bdrv_ref(new_bs); } - bdrv_replace_child_noperm(childp, new_bs); + /* + * Pass free_empty_child=false, we will free the child (if + * necessary) in bdrv_replace_child_commit() (if our + * @free_empty_child parameter was true). + */ + bdrv_replace_child_noperm(childp, new_bs, false); /* old_bs reference is transparently moved from *childp to @s */ } @@ -2675,8 +2713,22 @@ uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm) return permissions[qapi_perm]; } +/** + * Replace (*childp)->bs by @new_bs. + * + * If @new_bs is NULL, *childp will be set to NULL, too: BDS parents + * generally cannot handle a BdrvChild with .bs == NULL, so clearing + * BdrvChild.bs should generally immediately be followed by the + * BdrvChild pointer being cleared as well. + * + * If @free_empty_child is true and @new_bs is NULL, the BdrvChild is + * freed. @free_empty_child should only be false if the caller will + * free the BdrvChild themselves (this may be important in a + * transactional context, where it may only be freed on commit). + */ static void bdrv_replace_child_noperm(BdrvChild **childp, - BlockDriverState *new_bs) + BlockDriverState *new_bs, + bool free_empty_child) { BdrvChild *child = *childp; BlockDriverState *old_bs = child->bs; @@ -2713,6 +2765,9 @@ static void bdrv_replace_child_noperm(BdrvChild **childp, } child->bs = new_bs; + if (!new_bs) { + *childp = NULL; + } if (new_bs) { QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); @@ -2742,6 +2797,10 @@ static void bdrv_replace_child_noperm(BdrvChild **childp, bdrv_parent_drained_end_single(child); drain_saldo++; } + + if (free_empty_child && !child->bs) { + bdrv_child_free(child); + } } /** @@ -2771,7 +2830,14 @@ static void bdrv_attach_child_common_abort(void *opaque) BdrvChild *child = *s->child; BlockDriverState *bs = child->bs; - bdrv_replace_child_noperm(s->child, NULL); + /* + * Pass free_empty_child=false, because we still need the child + * for the AioContext operations on the parent below; those + * BdrvChildClass methods all work on a BdrvChild object, so we + * need to keep it as an empty shell (after this function, it will + * not be attached to any parent, and it will not have a .bs). + */ + bdrv_replace_child_noperm(s->child, NULL, false); if (bdrv_get_aio_context(bs) != s->old_child_ctx) { bdrv_try_set_aio_context(bs, s->old_child_ctx, &error_abort); @@ -2793,7 +2859,6 @@ static void bdrv_attach_child_common_abort(void *opaque) bdrv_unref(bs); bdrv_child_free(child); - *s->child = NULL; } static TransactionActionDrv bdrv_attach_child_common_drv = { @@ -2871,7 +2936,9 @@ static int bdrv_attach_child_common(BlockDriverState *child_bs, } bdrv_ref(child_bs); - bdrv_replace_child_noperm(&new_child, child_bs); + bdrv_replace_child_noperm(&new_child, child_bs, true); + /* child_bs was non-NULL, so new_child must not have been freed */ + assert(new_child != NULL); *child = new_child; @@ -2935,8 +3002,7 @@ static void bdrv_detach_child(BdrvChild **childp) { BlockDriverState *old_bs = (*childp)->bs; - bdrv_replace_child_noperm(childp, NULL); - bdrv_child_free(*childp); + bdrv_replace_child_noperm(childp, NULL, true); if (old_bs) { /* @@ -4911,7 +4977,11 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, } if (child->bs) { - bdrv_replace_child_tran(childp, NULL, tran); + /* + * Pass free_empty_child=false, we will free the child in + * bdrv_remove_filter_or_cow_child_commit() + */ + bdrv_replace_child_tran(childp, NULL, tran, false); } s = g_new(BdrvRemoveFilterOrCowChild, 1); @@ -4920,8 +4990,6 @@ static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, .is_backing = (childp == &bs->backing), }; tran_add(tran, &bdrv_remove_filter_or_cow_child_drv, s); - - *childp = NULL; } /* @@ -4957,7 +5025,7 @@ static int bdrv_replace_node_noperm(BlockDriverState *from, c->name, from->node_name); return -EPERM; } - bdrv_replace_child_tran(&c, to, tran); + bdrv_replace_child_tran(&c, to, tran, true); } return 0; @@ -5106,7 +5174,9 @@ int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs, bdrv_drained_begin(old_bs); bdrv_drained_begin(new_bs); - bdrv_replace_child_tran(&child, new_bs, tran); + bdrv_replace_child_tran(&child, new_bs, tran, true); + /* @new_bs must have been non-NULL, so @child must not have been freed */ + assert(child != NULL); found = g_hash_table_new(NULL, NULL); refresh_list = bdrv_topological_dfs(refresh_list, found, old_bs); From patchwork Thu Nov 4 10:38:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanna Czenczek X-Patchwork-Id: 1550849 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=Oo1hvT6n; 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 4HlL0r6L5Sz9sR4 for ; Thu, 4 Nov 2021 21:46:36 +1100 (AEDT) Received: from localhost ([::1]:48084 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1miaGQ-0005l3-PZ for incoming@patchwork.ozlabs.org; Thu, 04 Nov 2021 06:46:34 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56646) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1miaAP-0005d8-O9 for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:40:22 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:28296) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1miaAN-000727-5w for qemu-devel@nongnu.org; Thu, 04 Nov 2021 06:40:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1636022418; 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=shVN2QICfYZ2g0c6Fg0kFCLqBxPQx2O4ZEwnCnj642E=; b=Oo1hvT6nJOvltTOXbvNclG15k1Y1qZ3q70kGm6Awpa0WDRzS7Hekg166uJOthIOjMpjdKy C+HqLOgGGj5g9YGTE0Tt/E1yr9DKNCEu5Nwzwiwr6SONKSKrYM0ce2A+EA95XMHuP8Gxqq 9lE5efkPwZR/B76f9EzpVxtL+H6Tsi0= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-186-AnM3GRueMXWxScWyiW4mBw-1; Thu, 04 Nov 2021 06:40:16 -0400 X-MC-Unique: AnM3GRueMXWxScWyiW4mBw-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0310D1030C29; Thu, 4 Nov 2021 10:40:16 +0000 (UTC) Received: from localhost (unknown [10.39.192.248]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9B3255C1D0; Thu, 4 Nov 2021 10:40:15 +0000 (UTC) From: Hanna Reitz To: qemu-block@nongnu.org Subject: [PATCH 7/7] iotests/030: Unthrottle parallel jobs in reverse Date: Thu, 4 Nov 2021 11:38:49 +0100 Message-Id: <20211104103849.46855-8-hreitz@redhat.com> In-Reply-To: <20211104103849.46855-1-hreitz@redhat.com> References: <20211104103849.46855-1-hreitz@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hreitz@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=hreitz@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.717, 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_H2=-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 , Hanna Reitz , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" See the comment for why this is necessary. Signed-off-by: Hanna Reitz --- tests/qemu-iotests/030 | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index 5fb65b4bef..567bf1da67 100755 --- a/tests/qemu-iotests/030 +++ b/tests/qemu-iotests/030 @@ -251,7 +251,16 @@ class TestParallelOps(iotests.QMPTestCase): speed=1024) self.assert_qmp(result, 'return', {}) - for job in pending_jobs: + # Do this in reverse: After unthrottling them, some jobs may finish + # before we have unthrottled all of them. This will drain their + # subgraph, and this will make jobs above them advance (despite those + # jobs on top being throttled). In the worst case, all jobs below the + # top one are finished before we can unthrottle it, and this makes it + # advance so far that it completes before we can unthrottle it - which + # results in an error. + # Starting from the top (i.e. in reverse) does not have this problem: + # When a job finishes, the ones below it are not advanced. + for job in reversed(pending_jobs): result = self.vm.qmp('block-job-set-speed', device=job, speed=0) self.assert_qmp(result, 'return', {})