From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58476) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TX6Ss-0000RD-LF for qemu-devel@nongnu.org; Sat, 10 Nov 2012 03:30:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TX6Sp-0004ya-JE for qemu-devel@nongnu.org; Sat, 10 Nov 2012 03:30:54 -0500 Received: from chello084112167138.7.11.vie.surfer.at ([84.112.167.138]:40300 helo=wiesinger.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TX6Sp-0004yV-4U for qemu-devel@nongnu.org; Sat, 10 Nov 2012 03:30:51 -0500 Message-ID: <509E10AD.2030602@wiesinger.com> Date: Sat, 10 Nov 2012 09:30:37 +0100 From: Gerhard Wiesinger MIME-Version: 1.0 References: <509CBE3A.4040103@redhat.com> In-Reply-To: <509CBE3A.4040103@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block.c, block/vmdk.c: Fixed major bug in VMDK WRITE and READ handling - FIXES DATA CORRUPTION List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: "qemu-devel@nongnu.org" On 09.11.2012 09:26, Paolo Bonzini wrote: > Il 08/11/2012 20:05, Gerhard Wiesinger ha scritto: >> Fixed a MAJOR BUG in VMDK files on file boundaries on reads >> and ALSO ON WRITES WHICH MIGHT CORRUPT THE IMAGE AND DATA!!!!!! >> >> Triggered for example with the following VMDK file (partly listed): >> # Extent description >> RW 4193792 FLAT "XP-W1-f001.vmdk" 0 >> RW 2097664 FLAT "XP-W1-f002.vmdk" 0 >> RW 4193792 FLAT "XP-W1-f003.vmdk" 0 >> RW 512 FLAT "XP-W1-f004.vmdk" 0 >> RW 4193792 FLAT "XP-W1-f005.vmdk" 0 >> RW 2097664 FLAT "XP-W1-f006.vmdk" 0 >> RW 4193792 FLAT "XP-W1-f007.vmdk" 0 >> RW 512 FLAT "XP-W1-f008.vmdk" 0 >> >> Patch includes: >> 1.) Patch fixes wrong calculation on extent boundaries. Especially it >> fixes the relativeness of the sector number to the current extent. > Please just fix _this_ part. Everything else is not necessary for example > for distributions to fix this. It's an important bug, so we actually want > to make that as simple as this. Sent. > >> 2.) Added debug code to block.c and to block/vmdk.c to verify correctness > Same here. Also, please use the tracing infrastructure---a lot of the debug > messages you're adding, though not all, are in fact already available (not > saying the others aren't useful!) Any chance that the patch with debug code only (after some cleaning) would be accepted (other modules do debug logging, too)? I don't like to do useless work. Tracing infrastructure is quite limited to function calls only (as far as I saw). > >> 3.) Also optimized code which avoids multiplication and uses shifts. > The compiler can do this for you. > > Most importantly, making it more complex for reviewers to find only the > "interesting" part. > > Please check that the attached patch still works. > Made some tests and gcc is really clever in optimizing. Even other multipliers (513, 514, ...) than 512 will be optimized to shifts and adds until a limit of 512+8 (I think it was that) was reached. :-) Bugfix only resent. Ciao, Gerhard