From nobody Fri May 3 18:03:59 2024 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 1509980116312833.6387775964578; Mon, 6 Nov 2017 06:55:16 -0800 (PST) Received: from localhost ([::1]:48563 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eBinv-0002q2-8E for importer@patchew.org; Mon, 06 Nov 2017 09:55:11 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42758) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eBin6-0002Yz-03 for qemu-devel@nongnu.org; Mon, 06 Nov 2017 09:54:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eBin1-0000vk-3S for qemu-devel@nongnu.org; Mon, 06 Nov 2017 09:54:20 -0500 Received: from fanzine.igalia.com ([91.117.99.155]:40308) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eBin0-0000vN-QB; Mon, 06 Nov 2017 09:54:15 -0500 Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1eBimx-0002F6-B5; Mon, 06 Nov 2017 15:54:11 +0100 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1eBime-00039Y-7o; Mon, 06 Nov 2017 16:53:52 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Message-Id:Date:Subject:Cc:To:From; bh=dPI7uNIQzzGMo2PkIb7YvCNTvGXJQW1PW0gM+lsh1SA=; b=YdkH2ZUnRHu8SYn11Z5v3dxX4pOBHmcV55qaVGrL0ttbGqmKifpmi8Ea85FS1NJTdrjVDPSE3cCrQ/ki2XQNmEHxoNWkLe3kOgHHQzfd4E1C/eT9U+q3pXvbyc5pzMWo106gJuiMfgQlqUjFQOAlCCvCxxQny/AcX9A0j6zbWlqjHXcxh0zW2lL6H384KGPtrPGuA8yssSP1BrQj09wd+XVocjw4Vcd2sB5B7lB+k2uNTKLfVn66dhQMVtpTw5saQIHceQoDIjDL06H7tNgIuY/+k7JP4i9+d3m0oLnvWg4X8Kr92fR20QadL9aspu8P3cMGfeQXpXT83YmFoMGa0g==; From: Alberto Garcia To: qemu-devel@nongnu.org Date: Mon, 6 Nov 2017 16:53:45 +0200 Message-Id: <20171106145345.12038-1-berto@igalia.com> X-Mailer: git-send-email 2.11.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [PATCH] block: Close a BlockDriverState completely even when bs->drv is NULL 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 , Alberto Garcia , qemu-block@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" bdrv_close() skips much of its logic when bs->drv is NULL. This is fine when we're closing a BlockDriverState that has just been created (because e.g the initialization process failed), but it's not enough in other cases. For example, when a valid qcow2 image is found to be corrupted then QEMU marks it as such in the file header and then sets bs->drv to NULL in order to make the BlockDriverState unusable. When that BDS is later closed then many of its data structures are not freed (leaking their memory) and none of its children are detached. This results in bdrv_close_all() failing to close all BDSs and making this assertion fail when QEMU is being shut down: bdrv_close_all: Assertion `QTAILQ_EMPTY(&all_bdrv_states)' failed. This patch makes bdrv_close() do the full uninitialization process in all cases. This fixes the problem with corrupted images and still works fine with freshly created BDSs. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake --- block.c | 57 +++++++++++++++++++++++-------------------= ---- tests/qemu-iotests/060 | 13 +++++++++++ tests/qemu-iotests/060.out | 12 ++++++++++ 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/block.c b/block.c index 684cb018da..389a6edad2 100644 --- a/block.c +++ b/block.c @@ -3179,6 +3179,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state) static void bdrv_close(BlockDriverState *bs) { BdrvAioNotifier *ban, *ban_next; + BdrvChild *child, *next; =20 assert(!bs->job); assert(!bs->refcnt); @@ -3188,43 +3189,41 @@ static void bdrv_close(BlockDriverState *bs) bdrv_drain(bs); /* in case flush left pending I/O */ =20 if (bs->drv) { - BdrvChild *child, *next; - bs->drv->bdrv_close(bs); bs->drv =3D NULL; + } =20 - bdrv_set_backing_hd(bs, NULL, &error_abort); + bdrv_set_backing_hd(bs, NULL, &error_abort); =20 - if (bs->file !=3D NULL) { - bdrv_unref_child(bs, bs->file); - bs->file =3D NULL; - } + if (bs->file !=3D NULL) { + bdrv_unref_child(bs, bs->file); + bs->file =3D NULL; + } =20 - QLIST_FOREACH_SAFE(child, &bs->children, next, next) { - /* TODO Remove bdrv_unref() from drivers' close function and u= se - * bdrv_unref_child() here */ - if (child->bs->inherits_from =3D=3D bs) { - child->bs->inherits_from =3D NULL; - } - bdrv_detach_child(child); + QLIST_FOREACH_SAFE(child, &bs->children, next, next) { + /* TODO Remove bdrv_unref() from drivers' close function and use + * bdrv_unref_child() here */ + if (child->bs->inherits_from =3D=3D bs) { + child->bs->inherits_from =3D NULL; } - - g_free(bs->opaque); - bs->opaque =3D NULL; - atomic_set(&bs->copy_on_read, 0); - bs->backing_file[0] =3D '\0'; - bs->backing_format[0] =3D '\0'; - bs->total_sectors =3D 0; - bs->encrypted =3D false; - bs->sg =3D false; - QDECREF(bs->options); - QDECREF(bs->explicit_options); - bs->options =3D NULL; - bs->explicit_options =3D NULL; - QDECREF(bs->full_open_options); - bs->full_open_options =3D NULL; + bdrv_detach_child(child); } =20 + g_free(bs->opaque); + bs->opaque =3D NULL; + atomic_set(&bs->copy_on_read, 0); + bs->backing_file[0] =3D '\0'; + bs->backing_format[0] =3D '\0'; + bs->total_sectors =3D 0; + bs->encrypted =3D false; + bs->sg =3D false; + QDECREF(bs->options); + QDECREF(bs->explicit_options); + bs->options =3D NULL; + bs->explicit_options =3D NULL; + QDECREF(bs->full_open_options); + bs->full_open_options =3D NULL; + bdrv_release_named_dirty_bitmaps(bs); assert(QLIST_EMPTY(&bs->dirty_bitmaps)); =20 diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index 66a8fa4aea..1cebe4c775 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -291,6 +291,19 @@ _make_test_img 64M poke_file "$TEST_IMG" "48" "\x00\x00\x00\x00\x00\x00\x00\x0= 0" $QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io =20 +echo +echo "=3D=3D=3D Testing the QEMU shutdown with a corrupted image =3D=3D=3D" +echo +_make_test_img 64M +poke_file "$TEST_IMG" "$rt_offset" "\x00\x00\x00\x00\x00\x00\x00\x0= 0" +echo "{'execute': 'qmp_capabilities'} + {'execute': 'human-monitor-command', + 'arguments': {'command-line': 'qemu-io drive \"write 0 512\"'}} + {'execute': 'quit'}" \ + | $QEMU -qmp stdio -nographic -nodefaults \ + -drive if=3Dnone,node-name=3Ddrive,file=3D"$TEST_IMG",driver= =3Dqcow2 \ + | _filter_qmp | _filter_qemu_io + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index cfd78f87a9..a5093fab8e 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -220,4 +220,16 @@ can't open device TEST_DIR/t.IMGFMT: Image does not co= ntain a reference count ta Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 qcow2: Marking image as corrupt: Preventing invalid allocation of L2 table= at offset 0; further corruption events will be suppressed write failed: Input/output error + +=3D=3D=3D Testing the QEMU shutdown with a corrupted image =3D=3D=3D + +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 +qcow2: Marking image as corrupt: Preventing invalid write on metadata (ove= rlaps with refcount table); further corruption events will be suppressed +QMP_VERSION +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "BLOCK_IMAGE_CORRUPTED", "data": {"device": "none0", "msg": "Preventing i= nvalid write on metadata (overlaps with refcount table)", "offset": 65536, = "node-name": "drive", "fatal": true, "size": 65536}} +write failed: Input/output error +{"return": ""} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "SHUTDOWN", "data": {"guest": false}} *** done --=20 2.11.0