From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cAHyO-00054G-VE for qemu-devel@nongnu.org; Fri, 25 Nov 2016 09:59:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cAHyN-0008JB-H7 for qemu-devel@nongnu.org; Fri, 25 Nov 2016 09:59:33 -0500 From: Markus Armbruster References: <1476395910-8697-1-git-send-email-jsnow@redhat.com> <1476395910-8697-8-git-send-email-jsnow@redhat.com> Date: Fri, 25 Nov 2016 15:59:17 +0100 In-Reply-To: <1476395910-8697-8-git-send-email-jsnow@redhat.com> (John Snow's message of "Thu, 13 Oct 2016 17:58:27 -0400") Message-ID: <87vavba9mi.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v10 07/10] hbitmap: serialization List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: qemu-block@nongnu.org, vsementsov@virtuozzo.com, famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com John Snow writes: > From: Vladimir Sementsov-Ogievskiy > > Functions to serialize / deserialize(restore) HBitmap. HBitmap should be > saved to linear sequence of bits independently of endianness and bitmap > array element (unsigned long) size. Therefore Little Endian is chosen. > > These functions are appropriate for dirty bitmap migration, restoring > the bitmap in several steps is available. To save performance, every > step writes only the last level of the bitmap. All other levels are > restored by hbitmap_deserialize_finish() as a last step of restoring. > So, HBitmap is inconsistent while restoring. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > [Fix left shift operand to 1UL; add "finish" parameter. - Fam] > Signed-off-by: Fam Zheng > > Signed-off-by: John Snow [...] > diff --git a/util/hbitmap.c b/util/hbitmap.c > index f303975..5d1a21c 100644 > --- a/util/hbitmap.c > +++ b/util/hbitmap.c > @@ -397,6 +397,143 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item) > return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit) != 0; > } > > +uint64_t hbitmap_serialization_granularity(const HBitmap *hb) > +{ > + /* Require at least 64 bit granularity to be safe on both 64 bit and 32 bit > + * hosts. */ > + return 64 << hb->granularity; > +} Coverity reports: *** CID 1365378: Integer handling issues (OVERFLOW_BEFORE_WIDEN) /util/hbitmap.c: 404 in hbitmap_serialization_granularity() 398 } 399 400 uint64_t hbitmap_serialization_granularity(const HBitmap *hb) 401 { 402 /* Require at least 64 bit granularity to be safe on both 64 bit and 32 bit 403 * hosts. */ >>> CID 1365378: Integer handling issues (OVERFLOW_BEFORE_WIDEN) >>> Potentially overflowing expression "0x40 << hb->granularity" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "uint64_t" (64 bits, unsigned). 404 return 64 << hb->granularity; 405 } 406 407 /* Start should be aligned to serialization granularity, chunk size should be 408 * aligned to serialization granularity too, except for last chunk. 409 */ Use (uint64_t)64 << hb->granularity for an unsigned 64 bit shift. [...]