From nobody Mon May 20 19:42:19 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=1559938320; cv=none; d=zoho.com; s=zohoarc; b=PnBZHtMH1p5G4NEjgtVeqiFi1siFImPLfCnwYHjxfNGGJf/kIhhVWL6YluYH9VjHOWBvqf7fVB/+TDi1Uu6gsiY55Gf71ZdNXFh7PC7z2sBnV6AElm9ybqBcNHyozN4fRZqDe2LRGJYbJ/Uui+s2hDiIIq3nOIB2tuGYMXR7J7s= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1559938320; h=Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:Message-ID:Sender:Subject:To:ARC-Authentication-Results; bh=cnYAsWU6yr+d4JKGjgl6+tMSqe6a+8iicQHU5Bb3KGI=; b=FQft64vMXJL2L+kBfPjKV6oJhbKN8Vs30e9jG0tRDHkgOcOnnRiOLRdLfN0/Ox4BKGtjSSyRI3Tjliu5adU6ooQs9eIPWxi9rnYxDSW3lv6VDHZSAiIzDnERZTvffVCN3/pKni3Bb6qDrK8ga5/wYAaNgFBiGJQSrN0mnwv6/dg= 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 1559938320660754.8723195160937; Fri, 7 Jun 2019 13:12:00 -0700 (PDT) Received: from localhost ([::1]:52726 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hZLDQ-00028y-16 for importer@patchew.org; Fri, 07 Jun 2019 16:11:56 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56073) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hZJwM-0003bE-AC for qemu-devel@nongnu.org; Fri, 07 Jun 2019 14:50:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hZJwJ-0005hc-Vw for qemu-devel@nongnu.org; Fri, 07 Jun 2019 14:50:14 -0400 Received: from relay.sw.ru ([185.231.240.75]:50914) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hZJwH-0003N3-Vj; Fri, 07 Jun 2019 14:50:11 -0400 Received: from [10.94.3.0] (helo=kvm.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.91) (envelope-from ) id 1hZJuF-00018q-5Y; Fri, 07 Jun 2019 21:48:03 +0300 From: Vladimir Sementsov-Ogievskiy To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Fri, 7 Jun 2019 21:48:02 +0300 Message-Id: <20190607184802.100945-1-vsementsov@virtuozzo.com> X-Mailer: git-send-email 2.18.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 185.231.240.75 Subject: [Qemu-devel] [PATCH] qcow2-bitmaps: fix qcow2_can_store_new_dirty_bitmap X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, vsementsov@virtuozzo.com, jsnow@redhat.com, mreitz@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" qcow2_can_store_new_dirty_bitmap works wrong, as it considers only bitmaps already stored in the qcow2 image and ignores persistent BdrvDirtyBitmap objects. So, let's instead count persistent BdrvDirtyBitmaps. We load all qcow2 bitmaps on open, so there should not be any bitmap in the image for which we don't have BdrvDirtyBitmaps version. If it is - it's a kind of corruption, and no reason to check for corruptions here (open() and close() are better places for it). Signed-off-by: Vladimir Sementsov-Ogievskiy Reported-by: aihua liang --- Hi! Patch is better than discussing I thing, so here is my counter-suggestion f= or "[PATCH 0/5] block/dirty-bitmap: check number and size constraints against = queued bitmaps" by John. It's of course needs some additional refactoring, as first assert shows bad= design, I just wrote it in such manner to make minimum changes to fix the bug. Of course, Reported-by: aihua liang may be applied here (if I understood correctly), and I hope that Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=3D1712636 too. block/qcow2-bitmap.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index b2487101ed..7d1b3eeb2b 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1619,8 +1619,11 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverSta= te *bs, Error **errp) { BDRVQcow2State *s =3D bs->opaque; - bool found; - Qcow2BitmapList *bm_list; + BdrvDirtyBitmap *bitmap; + uint64_t bitmap_directory_size =3D 0; + uint32_t nb_bitmaps =3D 0; + + assert(!bdrv_find_dirty_bitmap(bs, name)); =20 if (s->qcow_version < 3) { /* Without autoclear_features, we would always have to assume @@ -1636,36 +1639,29 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverSt= ate *bs, goto fail; } =20 - if (s->nb_bitmaps =3D=3D 0) { - return true; + for (bitmap =3D bdrv_dirty_bitmap_next(bs, NULL); bitmap !=3D NULL; + bitmap =3D bdrv_dirty_bitmap_next(bs, bitmap)) + { + if (bdrv_dirty_bitmap_get_persistence(bitmap)) { + nb_bitmaps++; + bitmap_directory_size +=3D + calc_dir_entry_size(strlen(bdrv_dirty_bitmap_name(bitmap))= , 0); + } } + nb_bitmaps++; + bitmap_directory_size +=3D calc_dir_entry_size(strlen(name), 0); =20 - if (s->nb_bitmaps >=3D QCOW2_MAX_BITMAPS) { + if (nb_bitmaps > QCOW2_MAX_BITMAPS) { error_setg(errp, "Maximum number of persistent bitmaps is already reache= d"); goto fail; } =20 - if (s->bitmap_directory_size + calc_dir_entry_size(strlen(name), 0) > - QCOW2_MAX_BITMAP_DIRECTORY_SIZE) - { + if (bitmap_directory_size > QCOW2_MAX_BITMAP_DIRECTORY_SIZE) { error_setg(errp, "Not enough space in the bitmap directory"); goto fail; } =20 - bm_list =3D bitmap_list_load(bs, s->bitmap_directory_offset, - s->bitmap_directory_size, errp); - if (bm_list =3D=3D NULL) { - goto fail; - } - - found =3D find_bitmap_by_name(bm_list, name); - bitmap_list_free(bm_list); - if (found) { - error_setg(errp, "Bitmap with the same name is already stored"); - goto fail; - } - return true; =20 fail: --=20 2.18.0