From nobody Wed Apr 16 08:02:49 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1528728626765815.5836855925927; Mon, 11 Jun 2018 07:50:26 -0700 (PDT) Received: from localhost ([::1]:49293 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fSO9I-0006Uw-0i for importer@patchew.org; Mon, 11 Jun 2018 10:50:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53403) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fSNmm-00050Y-QL for qemu-devel@nongnu.org; Mon, 11 Jun 2018 10:27:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fSNml-00020k-LD for qemu-devel@nongnu.org; Mon, 11 Jun 2018 10:27:08 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:33556 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fSNmi-0001zJ-Oc; Mon, 11 Jun 2018 10:27:04 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5BFA57D859; Mon, 11 Jun 2018 14:27:04 +0000 (UTC) Received: from localhost (unknown [10.40.205.155]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 080098442A; Mon, 11 Jun 2018 14:27:03 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Mon, 11 Jun 2018 16:26:08 +0200 Message-Id: <20180611142611.6609-27-mreitz@redhat.com> In-Reply-To: <20180611142611.6609-1-mreitz@redhat.com> References: <20180611142611.6609-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Mon, 11 Jun 2018 14:27:04 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Mon, 11 Jun 2018 14:27:04 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mreitz@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 26/29] throttle: Fix crash on reopen X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Peter Maydell , qemu-devel@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Alberto Garcia The throttle block filter can be reopened, and with this it is possible to change the throttle group that the filter belongs to. The way the code does that is the following: - On throttle_reopen_prepare(): create a new ThrottleGroupMember and attach it to the new throttle group. - On throttle_reopen_commit(): detach the old ThrottleGroupMember, delete it and replace it with the new one. The problem with this is that by replacing the ThrottleGroupMember the previous value of io_limits_disabled is lost, causing an assertion failure in throttle_co_drain_end(). This problem can be reproduced by reopening a throttle node: $QEMU -monitor stdio -object throttle-group,id=3Dtg0,x-iops-total=3D1000 \ -blockdev node-name=3Dhd0,driver=3Dqcow2,file.driver=3Dfile,file.filenam= e=3Dhd.qcow2 \ -blockdev node-name=3Droot,driver=3Dthrottle,throttle-group=3Dtg0,file= =3Dhd0,read-only=3Don (qemu) block_stream root block/throttle.c:214: throttle_co_drain_end: Assertion `tgm->io_limits_d= isabled' failed. Since we only want to change the throttle group on reopen there's no need to create a ThrottleGroupMember and discard the old one. It's easier if we simply detach it from its current group and attach it to the new one. Signed-off-by: Alberto Garcia Message-id: 20180608151536.7378-1-berto@igalia.com Signed-off-by: Max Reitz --- block/throttle.c | 54 +++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/block/throttle.c b/block/throttle.c index e298827f95..f617f23a12 100644 --- a/block/throttle.c +++ b/block/throttle.c @@ -36,9 +36,12 @@ static QemuOptsList throttle_opts =3D { }, }; =20 -static int throttle_configure_tgm(BlockDriverState *bs, - ThrottleGroupMember *tgm, - QDict *options, Error **errp) +/* + * If this function succeeds then the throttle group name is stored in + * @group and must be freed by the caller. + * If there's an error then @group remains unmodified. + */ +static int throttle_parse_options(QDict *options, char **group, Error **er= rp) { int ret; const char *group_name; @@ -63,8 +66,7 @@ static int throttle_configure_tgm(BlockDriverState *bs, goto fin; } =20 - /* Register membership to group with name group_name */ - throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs)); + *group =3D g_strdup(group_name); ret =3D 0; fin: qemu_opts_del(opts); @@ -75,6 +77,8 @@ static int throttle_open(BlockDriverState *bs, QDict *opt= ions, int flags, Error **errp) { ThrottleGroupMember *tgm =3D bs->opaque; + char *group; + int ret; =20 bs->file =3D bdrv_open_child(NULL, options, "file", bs, &child_file, false, errp); @@ -86,7 +90,14 @@ static int throttle_open(BlockDriverState *bs, QDict *op= tions, bs->supported_zero_flags =3D bs->file->bs->supported_zero_flags | BDRV_REQ_WRITE_UNCHANGED; =20 - return throttle_configure_tgm(bs, tgm, options, errp); + ret =3D throttle_parse_options(options, &group, errp); + if (ret =3D=3D 0) { + /* Register membership to group with name group_name */ + throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs)); + g_free(group); + } + + return ret; } =20 static void throttle_close(BlockDriverState *bs) @@ -162,35 +173,36 @@ static void throttle_attach_aio_context(BlockDriverSt= ate *bs, static int throttle_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, Error **errp) { - ThrottleGroupMember *tgm; + int ret; + char *group =3D NULL; =20 assert(reopen_state !=3D NULL); assert(reopen_state->bs !=3D NULL); =20 - reopen_state->opaque =3D g_new0(ThrottleGroupMember, 1); - tgm =3D reopen_state->opaque; - - return throttle_configure_tgm(reopen_state->bs, tgm, reopen_state->opt= ions, - errp); + ret =3D throttle_parse_options(reopen_state->options, &group, errp); + reopen_state->opaque =3D group; + return ret; } =20 static void throttle_reopen_commit(BDRVReopenState *reopen_state) { - ThrottleGroupMember *old_tgm =3D reopen_state->bs->opaque; - ThrottleGroupMember *new_tgm =3D reopen_state->opaque; + BlockDriverState *bs =3D reopen_state->bs; + ThrottleGroupMember *tgm =3D bs->opaque; + char *group =3D reopen_state->opaque; + + assert(group); =20 - throttle_group_unregister_tgm(old_tgm); - g_free(old_tgm); - reopen_state->bs->opaque =3D new_tgm; + if (strcmp(group, throttle_group_get_name(tgm))) { + throttle_group_unregister_tgm(tgm); + throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs)); + } + g_free(reopen_state->opaque); reopen_state->opaque =3D NULL; } =20 static void throttle_reopen_abort(BDRVReopenState *reopen_state) { - ThrottleGroupMember *tgm =3D reopen_state->opaque; - - throttle_group_unregister_tgm(tgm); - g_free(tgm); + g_free(reopen_state->opaque); reopen_state->opaque =3D NULL; } =20 --=20 2.17.1