qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH] block: fix sector comparism in multiwrite_req_c


From: Michael Tokarev
Subject: [Qemu-devel] Re: [PATCH] block: fix sector comparism in multiwrite_req_compare
Date: Wed, 19 May 2010 23:26:58 +0400
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1.9) Gecko/20100411 Icedove/3.0.4

19.05.2010 22:53, Christoph Hellwig wrote:
The difference between the start sectors of two requests can be larger
than the size of the "int" type, which can lead to a not correctly
sorted multiwrite array and thus spurious I/O errors and filesystem
corruption due to incorrect request merges.

So instead of doing the cute sector arithmetics trick spell out the
exact comparisms.

Spotted by Kevin Wolf based on a testcase from Michael Tokarev.

Signed-off-by: Christoph Hellwig<address@hidden>

Index: qemu/block.c
===================================================================
--- qemu.orig/block.c   2010-05-19 17:08:24.970255636 +0200
+++ qemu/block.c        2010-05-19 17:17:34.227006021 +0200
@@ -1933,7 +1933,19 @@ static void multiwrite_cb(void *opaque,

  static int multiwrite_req_compare(const void *a, const void *b)
  {
-    return (((BlockRequest*) a)->sector - ((BlockRequest*) b)->sector);
+    const BlockRequest *req1 = a, *req2 = b;
+
+    /*
+     * Note that we can't simply subtract req2->sector from req1->sector
+     * here as that could overflow the return value.
+     */
+    if (req1->sector>  req2->sector) {
+        return 1;
+    } else if (req1->sector<  req2->sector) {
+        return -1;
+    } else {
+        return 0;
+    }
  }

Fantastic.

I ran numerous mkfs'es of my 1.5Tb image, -- every single
run before resulted in at least one "fun" comparison there.

There is NO MORE OVERLAPPING REQUESTS.  At least I can't
trigger one.

Fantastic!

Thanks guys!

Now the question is if the previous patch by Avi is actually
worth to apply -- I mean this one:
 [Qemu-devel] [PATCH +stable] block: don't attempt to merge overlapping requests

But it looks like all 3 should be applied:

 [Qemu-devel] [PATCH] block: fix sector comparism in multiwrite_req_compare
 [Qemu-devel] [PATCH +stable] block: don't attempt to merge overlapping requests
 [Qemu-devel] [PATCH] virtio-blk: fix barrier support

..to -stable.

I wonder how many similar "funny" cases are still around... ;)

/mjt



reply via email to

[Prev in Thread] Current Thread [Next in Thread]