From nobody Mon May 6 15:12:16 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=virtuozzo.com ARC-Seal: i=1; a=rsa-sha256; t=1558626780; cv=none; d=zoho.com; s=zohoarc; b=mAlfq1UN+ih+PnBsV75wnavBN353RDXxdlvKvDCO06oBTody3ZTfmwMYHikrCS7oG57FDx7zCChKDEVAnARGSV0pIdc6gyD9v6Ht/Dc+3EQaMscWCapVxHjxzaK5Le8/xbaU0Wth0XJM10SqdOdEo2n4wsAscvudDFqVGf/l26o= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1558626780; h=Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=8l7wExIp+e2wubAlsaL5po4uRlkZpxAXmIfqTZuQQQw=; b=SrO7me9bpA1YvjzGYOvuoHZp4sEhtOD+zPGbslp8Lxv8adB+/XbSjpJdR/J4yuNkYn9mfHx9xHlLjQ9pV2ZIGh4OgBjpyoqdFXnrSzd0/GnVE8I7gPaPsDvsh/YhCw9MHhRU7Gc2VvXAoBhVN8eBKnnBWqMdmJMBBxK2yf8nIes= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (209.51.188.17 [209.51.188.17]) by mx.zohomail.com with SMTPS id 1558626780385384.88205262794986; Thu, 23 May 2019 08:53:00 -0700 (PDT) Received: from localhost ([127.0.0.1]:39362 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hTq1T-0006NU-3c for importer@patchew.org; Thu, 23 May 2019 11:52:51 -0400 Received: from eggs.gnu.org ([209.51.188.92]:53835) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hTpwU-0003AB-PY for qemu-devel@nongnu.org; Thu, 23 May 2019 11:47:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hTpwT-0007vz-9s for qemu-devel@nongnu.org; Thu, 23 May 2019 11:47:42 -0400 Received: from relay.sw.ru ([185.231.240.75]:34906) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hTpwS-0007u4-QL; Thu, 23 May 2019 11:47:41 -0400 Received: from [10.94.3.0] (helo=kvm.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1hTpwL-0002xl-Mv; Thu, 23 May 2019 18:47:33 +0300 From: Vladimir Sementsov-Ogievskiy To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Thu, 23 May 2019 18:47:31 +0300 Message-Id: <20190523154733.54944-2-vsementsov@virtuozzo.com> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20190523154733.54944-1-vsementsov@virtuozzo.com> References: <20190523154733.54944-1-vsementsov@virtuozzo.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 185.231.240.75 Subject: [Qemu-devel] [PATCH 1/3] iotests: add test 255 to check bitmap life after snapshot + commit 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: fam@euphon.net, kwolf@redhat.com, vsementsov@virtuozzo.com, mreitz@redhat.com, den@openvz.org, jsnow@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Two testcases with persistent bitmaps are not added here, as there are bugs to be fixed soon. Signed-off-by: Vladimir Sementsov-Ogievskiy --- python/qemu/__init__.py | 4 +- tests/qemu-iotests/255 | 81 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/255.out | 17 ++++++++ tests/qemu-iotests/group | 1 + 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/255 create mode 100644 tests/qemu-iotests/255.out diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py index 81d9657ec0..4c4317a55e 100644 --- a/python/qemu/__init__.py +++ b/python/qemu/__init__.py @@ -402,7 +402,7 @@ class QEMUMachine(object): self._qmp.clear_events() return events =20 - def event_wait(self, name, timeout=3D60.0, match=3DNone): + def event_wait(self, name, timeout=3D60.0, match=3DNone, fail_on=3DNon= e): """ Wait for specified timeout on named event in QMP; optionally filter results by match. @@ -430,6 +430,7 @@ class QEMUMachine(object): =20 # Search cached events for event in self._events: + assert event['event'] !=3D fail_on if (event['event'] =3D=3D name) and event_match(event, match): self._events.remove(event) return event @@ -437,6 +438,7 @@ class QEMUMachine(object): # Poll for new events while True: event =3D self._qmp.pull_event(wait=3Dtimeout) + assert event['event'] !=3D fail_on if (event['event'] =3D=3D name) and event_match(event, match): return event self._events.append(event) diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255 new file mode 100755 index 0000000000..36712689d3 --- /dev/null +++ b/tests/qemu-iotests/255 @@ -0,0 +1,81 @@ +#!/usr/bin/env python +# +# Tests for temporary external snapshot when we have bitmaps. +# +# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import iotests +from iotests import qemu_img_create, file_path, log + +iotests.verify_image_format(supported_fmts=3D['qcow2']) + +base, top =3D file_path('base', 'top') +size =3D 64 * 1024 * 3 + + +def print_bitmap(msg, vm): + result =3D vm.qmp('query-block')['return'][0] + if 'dirty-bitmaps' in result: + bitmap =3D result['dirty-bitmaps'][0] + log('{}: name=3D{} dirty-clusters=3D{}'.format(msg, bitmap['name'], + bitmap['count'] // 64 // 1024)) + else: + log(msg + ': not found') + + +def test(persistent, restart): + assert persistent or not restart + log("\nTestcase {}persistent {} restart\n".format( + '' if persistent else 'non-', 'with' if restart else 'without'= )) + + qemu_img_create('-f', iotests.imgfmt, base, str(size)) + + vm =3D iotests.VM().add_drive(base) + vm.launch() + + vm.qmp_log('block-dirty-bitmap-add', node=3D'drive0', name=3D'bitmap0', + persistent=3Dpersistent) + vm.hmp_qemu_io('drive0', 'write 0 64K') + print_bitmap('initial bitmap', vm) + + vm.qmp_log('blockdev-snapshot-sync', device=3D'drive0', snapshot_file= =3Dtop, + format=3Diotests.imgfmt, filters=3D[iotests.filter_qmp_test= files]) + vm.hmp_qemu_io('drive0', 'write 64K 512') + print_bitmap('check, that no bitmaps in snapshot', vm) + + if restart: + log("... Restart ...") + vm.shutdown() + vm =3D iotests.VM().add_drive(top) + vm.launch() + + vm.qmp_log('block-commit', device=3D'drive0', top=3Dtop, + filters=3D[iotests.filter_qmp_testfiles]) + log(vm.event_wait('BLOCK_JOB_READY', fail_on=3D'BLOCK_JOB_COMPLETED'), + filters=3D[iotests.filter_qmp_event]) + vm.qmp_log('block-job-complete', device=3D'drive0') + log(vm.event_wait('BLOCK_JOB_COMPLETED'), + filters=3D[iotests.filter_qmp_event]) + print_bitmap('check merged bitmap', vm) + + vm.hmp_qemu_io('drive0', 'write 128K 64K') + print_bitmap('check updated bitmap', vm) + + vm.shutdown() + + +test(persistent=3DFalse, restart=3DFalse) diff --git a/tests/qemu-iotests/255.out b/tests/qemu-iotests/255.out new file mode 100644 index 0000000000..2bffb486d2 --- /dev/null +++ b/tests/qemu-iotests/255.out @@ -0,0 +1,17 @@ + +Testcase non-persistent without restart + +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "no= de": "drive0", "persistent": false}} +{"return": {}} +initial bitmap: name=3Dbitmap0 dirty-clusters=3D1 +{"execute": "blockdev-snapshot-sync", "arguments": {"device": "drive0", "f= ormat": "qcow2", "snapshot-file": "TEST_DIR/PID-top"}} +{"return": {}} +check, that no bitmaps in snapshot: not found +{"execute": "block-commit", "arguments": {"device": "drive0", "top": "TEST= _DIR/PID-top"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "= type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds":= "USECS", "seconds": "SECS"}} +{"execute": "block-job-complete", "arguments": {"device": "drive0"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "= type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microsecon= ds": "USECS", "seconds": "SECS"}} +check merged bitmap: name=3Dbitmap0 dirty-clusters=3D2 +check updated bitmap: name=3Dbitmap0 dirty-clusters=3D3 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 52b7c16e15..2758f48143 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -251,3 +251,4 @@ 249 rw auto quick 252 rw auto backing quick 253 rw auto quick +255 rw auto quick --=20 2.18.0 From nobody Mon May 6 15:12:16 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=virtuozzo.com ARC-Seal: i=1; a=rsa-sha256; t=1558626906; cv=none; d=zoho.com; s=zohoarc; b=dqno0GjEiTdhbNdbm1OdH54oXFtLysoTTQamtbOhXfj40olW/UqFoWRYU0jLUVthUkaq1fJEj2IVNJZ4cTLQfjIjkDF/cPl0qlDt6b3A4orHu/Ubt687HT1OCvDi2RPWuLS3rpT8USjxfGsNQN9GHHuhgZdk6PD+L4G3UPVx5Vs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1558626906; h=Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=xgRttQa2vwoe+iSSl9FOfgzzNRerXQGFhNnLR3q6mNU=; b=RxQMCidi0eK0N6gVm0eQzcMImakic8Co3lee5OsgO2fek3UuEsWLOoBunsQZ1vHcN83wQAJ1lRShe51tM11ch86w7piRc9yWmDoH0VP4oTYpb6mV4MtNd7MiPFcVS0KDjVHfdGIMfzhAVGRWpwhBBwFV31JhIzwctJ8mE4B9nBs= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1558626906585244.8730652081539; Thu, 23 May 2019 08:55:06 -0700 (PDT) Received: from localhost ([127.0.0.1]:39470 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hTq3X-0008BC-MX for importer@patchew.org; Thu, 23 May 2019 11:54:59 -0400 Received: from eggs.gnu.org ([209.51.188.92]:53822) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hTpwU-00039y-7e for qemu-devel@nongnu.org; Thu, 23 May 2019 11:47:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hTpwT-0007vn-76 for qemu-devel@nongnu.org; Thu, 23 May 2019 11:47:42 -0400 Received: from relay.sw.ru ([185.231.240.75]:34912) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hTpwS-0007tY-NL; Thu, 23 May 2019 11:47:41 -0400 Received: from [10.94.3.0] (helo=kvm.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1hTpwL-0002xl-RG; Thu, 23 May 2019 18:47:33 +0300 From: Vladimir Sementsov-Ogievskiy To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Thu, 23 May 2019 18:47:32 +0300 Message-Id: <20190523154733.54944-3-vsementsov@virtuozzo.com> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20190523154733.54944-1-vsementsov@virtuozzo.com> References: <20190523154733.54944-1-vsementsov@virtuozzo.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 185.231.240.75 Subject: [Qemu-devel] [PATCH 2/3] block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps 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: fam@euphon.net, kwolf@redhat.com, vsementsov@virtuozzo.com, mreitz@redhat.com, den@openvz.org, jsnow@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Firstly, no reason to optimize failure path. Then, function name is ambiguous: it checks for readonly and similar things, but someone may think that it will ignore normal bitmaps which was just unchanged, and this is in bad relation with the fact that we should drop IN_USE flag for unchanged bitmaps in the image. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow --- include/block/dirty-bitmap.h | 1 - block/dirty-bitmap.c | 12 ------------ block/qcow2-bitmap.c | 23 +++++++++++++---------- 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 8044ace63e..816022972b 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -105,7 +105,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_get_persistence(BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap); -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs); BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp= ); diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 59e6ebb861..eca2eed0bf 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -775,18 +775,6 @@ bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBit= map *bitmap) return bitmap->inconsistent; } =20 -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs) -{ - BdrvDirtyBitmap *bm; - QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { - if (bm->persistent && !bm->readonly && !bm->migration) { - return true; - } - } - - return false; -} - BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) { diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 8a75366c92..2b84bfa007 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1457,16 +1457,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDrive= rState *bs, Error **errp) Qcow2Bitmap *bm; QSIMPLEQ_HEAD(, Qcow2BitmapTable) drop_tables; Qcow2BitmapTable *tb, *tb_next; - - if (!bdrv_has_changed_persistent_bitmaps(bs)) { - /* nothing to do */ - return; - } - - if (!can_write(bs)) { - error_setg(errp, "No write access"); - return; - } + bool need_write =3D false; =20 QSIMPLEQ_INIT(&drop_tables); =20 @@ -1494,6 +1485,8 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriver= State *bs, Error **errp) continue; } =20 + need_write =3D true; + if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) { error_prepend(errp, "Bitmap '%s' doesn't satisfy the constrain= ts: ", name); @@ -1532,6 +1525,15 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDrive= rState *bs, Error **errp) bm->dirty_bitmap =3D bitmap; } =20 + if (!need_write) { + goto success; + } + + if (!can_write(bs)) { + error_setg(errp, "No write access"); + goto fail; + } + /* allocate clusters and store bitmaps */ QSIMPLEQ_FOREACH(bm, bm_list, entry) { if (bm->dirty_bitmap =3D=3D NULL) { @@ -1573,6 +1575,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriver= State *bs, Error **errp) bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap); } =20 +success: bitmap_list_free(bm_list); return; =20 --=20 2.18.0 From nobody Mon May 6 15:12:16 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=virtuozzo.com ARC-Seal: i=1; a=rsa-sha256; t=1558626803; cv=none; d=zoho.com; s=zohoarc; b=GIXc+iyQMt0vPdVkssqGsZQubDb0IKrpHe08gwJS58I0uSMIXzaD1T2XYhiEADD8F1Mce3lSPLM4DTPFikAAkkSIoIbK3Szo5MLiUOrRpXZ8aw4mlGeiruU4V1/PZuYArs4ZD3b7PlLxDqc3KItl1HGKKwX/o5qKD2D9NQOvJqM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1558626803; h=Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=f1/sc84nFm33Sx+d6mJBWCbFN38tFRKS4/2BK5wF/uk=; b=Zi8W395a5A6TSE1zD1hqVdp4qqq+O/KOxu1ffDbmdFreJ7U7FZDLh9g5DVu+5AaNB0fKiGTMZ4H6f0FMRiBo39deGdxZ0SlPzK8wR7gaFss3cH73g0tLYJlQtb7so5NLxXAV2PtJr+Snxud4s/mhDhhvxPkVtOk5zwQplUSi3BY= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (209.51.188.17 [209.51.188.17]) by mx.zohomail.com with SMTPS id 1558626803033545.701191046942; Thu, 23 May 2019 08:53:23 -0700 (PDT) Received: from localhost ([127.0.0.1]:39364 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hTq1m-0006Se-FE for importer@patchew.org; Thu, 23 May 2019 11:53:10 -0400 Received: from eggs.gnu.org ([209.51.188.92]:53808) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hTpwT-00039P-J8 for qemu-devel@nongnu.org; Thu, 23 May 2019 11:47:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hTpwR-0007uO-EM for qemu-devel@nongnu.org; Thu, 23 May 2019 11:47:41 -0400 Received: from relay.sw.ru ([185.231.240.75]:34914) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hTpwR-0007tO-2I; Thu, 23 May 2019 11:47:39 -0400 Received: from [10.94.3.0] (helo=kvm.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1hTpwL-0002xl-W7; Thu, 23 May 2019 18:47:34 +0300 From: Vladimir Sementsov-Ogievskiy To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Thu, 23 May 2019 18:47:33 +0300 Message-Id: <20190523154733.54944-4-vsementsov@virtuozzo.com> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20190523154733.54944-1-vsementsov@virtuozzo.com> References: <20190523154733.54944-1-vsementsov@virtuozzo.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 185.231.240.75 Subject: [Qemu-devel] [PATCH 3/3] block/qcow2-bitmap: rewrite bitmap reopening logic 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: fam@euphon.net, kwolf@redhat.com, vsementsov@virtuozzo.com, mreitz@redhat.com, den@openvz.org, jsnow@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Current logic =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Reopen rw -> ro: Store bitmaps and release BdrvDirtyBitmap's. Reopen ro -> rw: Load bitmap list Skip bitmaps which for which we don't have BdrvDirtyBitmap [this is the worst thing] A kind of fail with error message if we see not readonly bitmap This leads to at least the following bug: Assume we have bitmap0. Create external snapshot bitmap0 is stored and therefore removed Commit snapshot now we have no bitmaps Do some writes from guest (*) they are not marked in bitmap Shutdown Start bitmap0 is loaded as valid, but it is actually broken! It misses writes (*) Incremental backup it will be inconsistent New logic =3D=3D=3D=3D=3D=3D=3D=3D=3D Reopen rw -> ro: Store bitmaps and don't remove them from RAM, only mark them readonly (the latter is already done, but didn't work properly because of removing in storing function) Reopen to rw (don't filter case with reopen rw -> rw, it is supported now in qcow2_reopen_bitmaps_rw) Load bitmap list Carefully consider all possible combinations of bitmaps in RAM and in the image, try to fix corruptions, print corresponding error messages. Also, call qcow2_reopen_bitmaps_rw after the whole reopen queue commited, as otherwise we can't write to the qcow2 file child on reopen ro -> rw. Also, add corresponding test-cases to 255. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/qcow2.h | 5 +- include/block/block_int.h | 2 +- block.c | 29 ++---- block/qcow2-bitmap.c | 197 ++++++++++++++++++++++++++----------- block/qcow2.c | 2 +- tests/qemu-iotests/255 | 2 + tests/qemu-iotests/255.out | 35 +++++++ 7 files changed, 193 insertions(+), 79 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 8d92ef1fee..5928306e62 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -725,9 +725,10 @@ Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockD= riverState *bs, Error **errp); int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_update= d, Error **errp); -int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp); +void qcow2_reopen_bitmaps_rw(BlockDriverState *bs); int qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp); -void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **er= rp); +void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, + bool release_stored, Error **err= p); int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, diff --git a/include/block/block_int.h b/include/block/block_int.h index 1eebc7c8f3..9a694f3da0 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -536,7 +536,7 @@ struct BlockDriver { * as rw. This handler should realize it. It also should unset readonly * field of BlockDirtyBitmap's in case of success. */ - int (*bdrv_reopen_bitmaps_rw)(BlockDriverState *bs, Error **errp); + void (*bdrv_reopen_bitmaps_rw)(BlockDriverState *bs); bool (*bdrv_can_store_new_dirty_bitmap)(BlockDriverState *bs, const char *name, uint32_t granularity, diff --git a/block.c b/block.c index cb11537029..db1ec0c673 100644 --- a/block.c +++ b/block.c @@ -3334,6 +3334,16 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue,= Error **errp) bdrv_reopen_commit(&bs_entry->state); } =20 + QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { + BlockDriverState *bs =3D bs_entry->state.bs; + + if (!bdrv_is_writable(bs) || !bs->drv->bdrv_reopen_bitmaps_rw) { + continue; + } + + bs->drv->bdrv_reopen_bitmaps_rw(bs); + } + ret =3D 0; cleanup_perm: QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { @@ -3770,16 +3780,12 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_sta= te) BlockDriver *drv; BlockDriverState *bs; BdrvChild *child; - bool old_can_write, new_can_write; =20 assert(reopen_state !=3D NULL); bs =3D reopen_state->bs; drv =3D bs->drv; assert(drv !=3D NULL); =20 - old_can_write =3D - !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); - /* If there are any driver level actions to take */ if (drv->bdrv_reopen_commit) { drv->bdrv_reopen_commit(reopen_state); @@ -3823,21 +3829,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_stat= e) } =20 bdrv_refresh_limits(bs, NULL); - - new_can_write =3D - !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); - if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) { - Error *local_err =3D NULL; - if (drv->bdrv_reopen_bitmaps_rw(bs, &local_err) < 0) { - /* This is not fatal, bitmaps just left read-only, so all foll= owing - * writes will fail. User can remove read-only bitmaps to unbl= ock - * writes. - */ - error_reportf_err(local_err, - "%s: Failed to make dirty bitmaps writable: = ", - bdrv_get_node_name(bs)); - } - } } =20 /* diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 2b84bfa007..4e565db11f 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -28,6 +28,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu/cutils.h" +#include "qemu/error-report.h" =20 #include "block/block_int.h" #include "qcow2.h" @@ -951,6 +952,12 @@ static void set_readonly_helper(gpointer bitmap, gpoin= ter value) bdrv_dirty_bitmap_set_readonly(bitmap, (bool)value); } =20 +/* for g_slist_foreach for GSList of const char* elements */ +static void error_report_helper(gpointer message, gpointer _unused) +{ + error_report("%s", (const char *)message); +} + /* qcow2_load_dirty_bitmaps() * Return value is a hint for caller: true means that the Qcow2 header was * updated. (false doesn't mean that the header should be updated by the @@ -1103,76 +1110,150 @@ Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(Bl= ockDriverState *bs, return list; } =20 -int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_update= d, - Error **errp) +/* + * qcow2_reopen_bitmaps_rw + * + * The function is called in bdrv_reopen_multiple after all calls to + * bdrv_reopen_commit, when final bs is writable. It is done so, as we need + * write access to child bs, and with current reopening architecture, when + * reopen ro -> rw it is possible only after all reopen commits. + * + * So, we can't fail here. On the other hand, we may face different kinds = of + * corruptions and/or just can't write IN_USE flags to the image due to EI= O. + * + * Try to handle as many cases as we can. + */ +void qcow2_reopen_bitmaps_rw(BlockDriverState *bs) { BDRVQcow2State *s =3D bs->opaque; Qcow2BitmapList *bm_list; Qcow2Bitmap *bm; - GSList *ro_dirty_bitmaps =3D NULL; + GSList *ro_dirty_bitmaps =3D NULL, *corrupted_bitmaps =3D NULL; + Error *local_err =3D NULL; int ret =3D 0; - - if (header_updated !=3D NULL) { - *header_updated =3D false; - } + bool need_update =3D false, updated_ok =3D false; =20 if (s->nb_bitmaps =3D=3D 0) { /* No bitmaps - nothing to do */ - return 0; - } - - if (!can_write(bs)) { - error_setg(errp, "Can't write to the image on reopening bitmaps rw= "); - return -EINVAL; + return; } =20 bm_list =3D bitmap_list_load(bs, s->bitmap_directory_offset, - s->bitmap_directory_size, errp); + s->bitmap_directory_size, &local_err); if (bm_list =3D=3D NULL) { - return -EINVAL; + error_reportf_err(local_err, "Failed to reopen bitmaps rw: " + "cannot load bitmap list: "); + return; } =20 QSIMPLEQ_FOREACH(bm, bm_list, entry) { BdrvDirtyBitmap *bitmap =3D bdrv_find_dirty_bitmap(bs, bm->name); - if (bitmap =3D=3D NULL) { - continue; - } + const char *corruption =3D NULL; =20 - if (!bdrv_dirty_bitmap_readonly(bitmap)) { - error_setg(errp, "Bitmap %s was loaded prior to rw-reopen, but= was " - "not marked as readonly. This is a bug, something w= ent " - "wrong. All of the bitmaps may be corrupted", bm->n= ame); - ret =3D -EINVAL; - goto out; - } + if (bm->flags & BME_FLAG_IN_USE) { + if (bitmap =3D=3D NULL) { + /* + * It's an unexpected inconsistent bitmap, + * it is safe to ignore it. + */ + continue; + } =20 - bm->flags |=3D BME_FLAG_IN_USE; - ro_dirty_bitmaps =3D g_slist_append(ro_dirty_bitmaps, bitmap); + /* + * It's either an inconsistent bitmap, or we are reopening rw = -> rw, + * or we just didn't save bitmap for some buggy reason. Still,= no + * reason to consider in-RAM bitmap inconsistent, than, mark i= t rw. + */ + bdrv_dirty_bitmap_set_readonly(bitmap, false); + } else { + /* + * In-image bitmap not marked IN_USE + * + * The only valid case is + * bitmap && bdrv_dirty_bitmap_readonly(bitmap) && + * !bdrv_dirty_bitmap_inconsistent(bitmap)) + * + * which means reopen ro -> rw of consistent bitmap. + * + * Other cases a different kinds of corruptions: + */ + if (!bitmap) { + corruption =3D + "Corruption: unexpected valid bitmap '%s' is found in = the " + "image '%s' on reopen rw. Will try to set IN_USE flag.= "; + + bitmap =3D load_bitmap(bs, bm, NULL); + if (!bitmap) { + bitmap =3D bdrv_create_dirty_bitmap( + bs, bdrv_get_default_bitmap_granularity(bs), + bm->name, NULL); + } + + if (bitmap) { + bdrv_dirty_bitmap_set_persistence(bitmap, true); + bdrv_dirty_bitmap_set_readonly(bitmap, true); + bdrv_dirty_bitmap_set_inconsistent(bitmap); + } + } else if (!bdrv_dirty_bitmap_readonly(bitmap)) { + corruption =3D + "Corruption: bitmap '%s' is not marked IN_USE in the " + "image '%s' and not marked readonly in RAM. Will try t= o " + "set IN_USE flag."; + + bdrv_dirty_bitmap_set_readonly(bitmap, true); + bdrv_dirty_bitmap_set_inconsistent(bitmap); + } else if (bdrv_dirty_bitmap_inconsistent(bitmap)) { + corruption =3D + "Corruption: bitmap '%s' is inconsistent but is not ma= rked " + "IN_USE in the image. Will try to set IN_USE flag."; + + bdrv_dirty_bitmap_set_readonly(bitmap, true); + } + + if (bitmap) { + ro_dirty_bitmaps =3D g_slist_append(ro_dirty_bitmaps, bitm= ap); + } + bm->flags |=3D BME_FLAG_IN_USE; + need_update =3D true; + if (corruption) { + error_report(corruption, bm->name, bs->filename); + corrupted_bitmaps =3D g_slist_append(corrupted_bitmaps, bm= ->name); + } + } } =20 - if (ro_dirty_bitmaps !=3D NULL) { + if (need_update) { + if (!can_write(bs->file->bs)) { + error_report("Failed to reopen bitmaps rw: cannot write to " + "the protocol file"); + goto handle_corrupted; + } + /* in_use flags must be updated */ ret =3D update_ext_header_and_dir_in_place(bs, bm_list); if (ret < 0) { - error_setg_errno(errp, -ret, "Can't update bitmap directory"); - goto out; - } - if (header_updated !=3D NULL) { - *header_updated =3D true; + error_report("Cannot update bitmap directory: %s", strerror(-r= et)); + goto handle_corrupted; } + updated_ok =3D true; g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false); } =20 -out: +handle_corrupted: + if (corrupted_bitmaps) { + if (updated_ok) { + error_report("Corrupted bitmaps in '%s' successfully marked " + "IN_USE", bs->filename); + } else { + error_report("Failed to mark IN_USE the following corrupted " + "bitmaps in '%s', DO NOT USE THEM:", bs->filename= ); + g_slist_foreach(corrupted_bitmaps, error_report_helper, NULL); + } + } + g_slist_free(ro_dirty_bitmaps); + g_slist_free(corrupted_bitmaps); bitmap_list_free(bm_list); - - return ret; -} - -int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) -{ - return qcow2_reopen_bitmaps_rw_hint(bs, NULL, errp); } =20 /* Checks to see if it's safe to resize bitmaps */ @@ -1446,7 +1527,8 @@ fail: bitmap_list_free(bm_list); } =20 -void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **er= rp) +void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, + bool release_stored, Error **err= p) { BdrvDirtyBitmap *bitmap; BDRVQcow2State *s =3D bs->opaque; @@ -1559,20 +1641,23 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriv= erState *bs, Error **errp) g_free(tb); } =20 - QSIMPLEQ_FOREACH(bm, bm_list, entry) { - /* For safety, we remove bitmap after storing. - * We may be here in two cases: - * 1. bdrv_close. It's ok to drop bitmap. - * 2. inactivation. It means migration without 'dirty-bitmaps' - * capability, so bitmaps are not marked with - * BdrvDirtyBitmap.migration flags. It's not bad to drop them t= oo, - * and reload on invalidation. - */ - if (bm->dirty_bitmap =3D=3D NULL) { - continue; - } + if (release_stored) { + QSIMPLEQ_FOREACH(bm, bm_list, entry) { + /* + * For safety, we remove bitmap after storing. + * We may be here in two cases: + * 1. bdrv_close. It's ok to drop bitmap. + * 2. inactivation. It means migration without 'dirty-bitmaps' + * capability, so bitmaps are not marked with + * BdrvDirtyBitmap.migration flags. It's not bad to drop th= em + * too, and reload on invalidation. + */ + if (bm->dirty_bitmap =3D=3D NULL) { + continue; + } =20 - bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap); + bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap); + } } =20 success: @@ -1600,7 +1685,7 @@ int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Err= or **errp) BdrvDirtyBitmap *bitmap; Error *local_err =3D NULL; =20 - qcow2_store_persistent_dirty_bitmaps(bs, &local_err); + qcow2_store_persistent_dirty_bitmaps(bs, false, &local_err); if (local_err !=3D NULL) { error_propagate(errp, local_err); return -EINVAL; diff --git a/block/qcow2.c b/block/qcow2.c index d39882785d..f0a8479874 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2273,7 +2273,7 @@ static int qcow2_inactivate(BlockDriverState *bs) int ret, result =3D 0; Error *local_err =3D NULL; =20 - qcow2_store_persistent_dirty_bitmaps(bs, &local_err); + qcow2_store_persistent_dirty_bitmaps(bs, true, &local_err); if (local_err !=3D NULL) { result =3D -EINVAL; error_reportf_err(local_err, "Lost persistent bitmaps during " diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255 index 36712689d3..e8b0c9d4bb 100755 --- a/tests/qemu-iotests/255 +++ b/tests/qemu-iotests/255 @@ -79,3 +79,5 @@ def test(persistent, restart): =20 =20 test(persistent=3DFalse, restart=3DFalse) +test(persistent=3DTrue, restart=3DFalse) +test(persistent=3DTrue, restart=3DTrue) diff --git a/tests/qemu-iotests/255.out b/tests/qemu-iotests/255.out index 2bffb486d2..46e2e3ad4e 100644 --- a/tests/qemu-iotests/255.out +++ b/tests/qemu-iotests/255.out @@ -15,3 +15,38 @@ check, that no bitmaps in snapshot: not found {"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "= type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microsecon= ds": "USECS", "seconds": "SECS"}} check merged bitmap: name=3Dbitmap0 dirty-clusters=3D2 check updated bitmap: name=3Dbitmap0 dirty-clusters=3D3 + +Testcase persistent without restart + +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "no= de": "drive0", "persistent": true}} +{"return": {}} +initial bitmap: name=3Dbitmap0 dirty-clusters=3D1 +{"execute": "blockdev-snapshot-sync", "arguments": {"device": "drive0", "f= ormat": "qcow2", "snapshot-file": "TEST_DIR/PID-top"}} +{"return": {}} +check, that no bitmaps in snapshot: not found +{"execute": "block-commit", "arguments": {"device": "drive0", "top": "TEST= _DIR/PID-top"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "= type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds":= "USECS", "seconds": "SECS"}} +{"execute": "block-job-complete", "arguments": {"device": "drive0"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "= type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microsecon= ds": "USECS", "seconds": "SECS"}} +check merged bitmap: name=3Dbitmap0 dirty-clusters=3D2 +check updated bitmap: name=3Dbitmap0 dirty-clusters=3D3 + +Testcase persistent with restart + +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "no= de": "drive0", "persistent": true}} +{"return": {}} +initial bitmap: name=3Dbitmap0 dirty-clusters=3D1 +{"execute": "blockdev-snapshot-sync", "arguments": {"device": "drive0", "f= ormat": "qcow2", "snapshot-file": "TEST_DIR/PID-top"}} +{"return": {}} +check, that no bitmaps in snapshot: not found +... Restart ... +{"execute": "block-commit", "arguments": {"device": "drive0", "top": "TEST= _DIR/PID-top"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "= type": "commit"}, "event": "BLOCK_JOB_READY", "timestamp": {"microseconds":= "USECS", "seconds": "SECS"}} +{"execute": "block-job-complete", "arguments": {"device": "drive0"}} +{"return": {}} +{"data": {"device": "drive0", "len": 65536, "offset": 65536, "speed": 0, "= type": "commit"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microsecon= ds": "USECS", "seconds": "SECS"}} +check merged bitmap: name=3Dbitmap0 dirty-clusters=3D2 +check updated bitmap: name=3Dbitmap0 dirty-clusters=3D3 --=20 2.18.0