From nobody Sun Feb 8 22:50:09 2026 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; dkim=fail; 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 Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1532379063265580.9908005913762; Mon, 23 Jul 2018 13:51:03 -0700 (PDT) Received: from localhost ([::1]:36526 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhhnI-0003V1-5U for importer@patchew.org; Mon, 23 Jul 2018 16:51:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41676) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fhhJI-0002Tt-OH for qemu-devel@nongnu.org; Mon, 23 Jul 2018 16:20:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fhhJG-00046v-JH for qemu-devel@nongnu.org; Mon, 23 Jul 2018 16:20:00 -0400 Received: from mail-oi0-x235.google.com ([2607:f8b0:4003:c06::235]:42539) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fhhJG-00045R-Cu; Mon, 23 Jul 2018 16:19:58 -0400 Received: by mail-oi0-x235.google.com with SMTP id n84-v6so3465019oib.9; Mon, 23 Jul 2018 13:19:58 -0700 (PDT) Received: from localhost (76-251-165-188.lightspeed.austtx.sbcglobal.net. [76.251.165.188]) by smtp.gmail.com with ESMTPSA id i132-v6sm11471143oif.20.2018.07.23.13.19.55 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 23 Jul 2018 13:19:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=jXsN2Q+sxs+JhQkNlK+/daRT2a5Egg638Mhim1g4XHs=; b=qsF+HW8XdVYDpOUyw8Zm04MaxIMui/D5Ctrq/Q3i8VXQgAx3kaWJ9twdJ/sI3Y5nWx E4s5WLaeh/MdLSa4kcmEemIO88h1oEfpd15H4zE9xmNPuez1PNwLrcqzKhK0SIup0E8y h6rFZV+WOHULiTlQtmI4CuqyGlrYf8U8gWsE7lASeF/1bVb7OQ25upZkG6oLn7hkcdmW ltgIS0/Nf3ZQrDyeVgf+3AcTGqs3MOwSnPuHnNH5dl1StXQD+xJiekqez+RSoJVlyo7i GJA8H9MedXKwtCBtgj0AOLonpMV5dynXJrJTShfnsMsAB1m50J1gzQmJT4istMgjlb1V 6LBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=jXsN2Q+sxs+JhQkNlK+/daRT2a5Egg638Mhim1g4XHs=; b=Y0tp1UtAq063sfuLdeiaG30Ygy6vlf7mGIuhA5+xq/OeJ+rpRx61I56+joq+iZTzsN bzIFd+lZMk8VGfZLhpT2VRWhwMoBO5z6MKRIhkB81xx+PTGNXR9GdXe1hP8kFWGV3q5T E7F7VaBjmkM0d0CuW4l7WhQ++kav0AshzrsFPuJyT3O+88Eybo+8pDsYBekMCilRB+6T DbI8t7QEa04BWcHM4BSOldQWxLa6NubezvBZkPCA0Mg2vOthu/Jx67FuCO+7yzBW5QKz IJ70gonhG0szYDrH14ogMpk9JZcDnP3OP3p0z0gbVuUS3atOBiXqRUQplyySwulPBRPN 5T+A== X-Gm-Message-State: AOUpUlFUSFkGDAdcVIBbE+wG6C84jJbMzqF8VTMmdIGCi+ajRzQS4xWY yRl0ktKq8X8Z6/QzhCGPFHlm1CDRPYw= X-Google-Smtp-Source: AAOMgpcm71oNfiEXdORaRCC+Pmv03cU2QKgo5z8CNXmy0chsrJuOqGWLHCfkuAyQwWz8/jnd6B43zw== X-Received: by 2002:aca:3c02:: with SMTP id j2-v6mr348883oia.50.1532377197182; Mon, 23 Jul 2018 13:19:57 -0700 (PDT) From: Michael Roth To: qemu-devel@nongnu.org Date: Mon, 23 Jul 2018 15:16:50 -0500 Message-Id: <20180723201748.25573-42-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com> References: <20180723201748.25573-1-mdroth@linux.vnet.ibm.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4003:c06::235 Subject: [Qemu-devel] [PATCH 41/99] 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: Alberto Garcia , qemu-stable@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 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 (cherry picked from commit bc33c047d1ec0b35c9cd8be62bcefae2da28654f) Signed-off-by: Michael Roth --- block/throttle.c | 54 +++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/block/throttle.c b/block/throttle.c index 95ed06acd8..026f408aa6 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); @@ -84,7 +88,14 @@ static int throttle_open(BlockDriverState *bs, QDict *op= tions, bs->supported_write_flags =3D bs->file->bs->supported_write_flags; bs->supported_zero_flags =3D bs->file->bs->supported_zero_flags; =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) @@ -160,35 +171,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