Re: [Qemu-devel] bogus bdrv_check_request in bdrv_co_discard

[Olaf Hering <olaf@aepfle.de>]

On Wed, Mar 09, Olaf Hering wrote:

> On Wed, Mar 09, Kevin Wolf wrote:
> 
> > Removing integer overflow checks without removing the potentially
> > overflowing operation doesn't feel like a particularly good idea,
> > though.
> 
> Why does the code use signed ints anyway for sectors and offset?!

Until this underlying bug is fixed a change like this works for me:

diff --git a/block/io.c b/block/io.c
index a69bfc4..df1e383 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2464,7 +2464,7 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque)
     rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num, rwco->nb_sectors);
 }

-int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
+static int __bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
                                  int nb_sectors)
 {   
     BdrvTrackedRequest req;
@@ -2546,6 +2546,26 @@ out:
     return ret;
 }

+int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num,
+                                 int nb_sectors)
+{
+    int num, ret;
+    int limit = BDRV_REQUEST_MAX_SECTORS;
+    int remaining = nb_sectors;
+    int64_t sector_offset = sector_num;
+
+    do {
+        num = remaining > limit ? limit : remaining;
+        ret = __bdrv_co_discard(bs, sector_offset, num);
+        if (ret < 0)
+            break;
+        remaining -= num;
+        sector_offset += num;
+    } while (remaining > 0);
+
+    return ret;
+}
+
 int bdrv_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors)
 {   
     Coroutine *co;

Olaf