From e3fa109f29c42631c5f0f29e6f0167043fbd7a8d Mon Sep 17 00:00:00 2001

From: Kevin Wolf <kwolf@redhat.com>

Date: Tue, 14 Jan 2014 11:41:35 +0100

Subject: [PATCH 26/37] block: Assert serialisation assumptions in pwritev

Message-id: <1392117622-28812-27-git-send-email-kwolf@redhat.com>

Patchwork-id: 57191

O-Subject: [RHEL-7.0 qemu-kvm PATCH v2 26/37] block: Assert serialisation assumptions in pwritev

Bugzilla: 748906

RH-Acked-by: Laszlo Ersek <lersek@redhat.com>

RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

RH-Acked-by: Max Reitz <mreitz@redhat.com>

If a request calls wait_serialising_requests() and actually has to wait

in this function (i.e. a coroutine yield), other requests can run and

previously read data (like the head or tail buffer) could become

outdated. In this case, we would have to restart from the beginning to

read in the updated data.

However, we're lucky and don't actually need to do that: A request can

only wait in the first call of wait_serialising_requests() because we

mark it as serialising before that call, so any later requests would

wait. So as we don't wait in practice, we don't have to reload the data.

This is an important assumption that may not be broken or data

corruption will happen. Document it with some assertions.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

(cherry picked from commit 28de2dcd88de31f50bbd43d9c2fcb046c3a727cb)

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

 block.c | 16 ++++++++++++----

 1 file changed, 12 insertions(+), 4 deletions(-)

---

 block.c |   16 ++++++++++++----

 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c

index 9eeb072..76de7d2 100644

--- a/block.c

+++ b/block.c

@@ -2153,14 +2153,15 @@ static bool tracked_request_overlaps(BdrvTrackedRequest *req,

     return true;

 }

-static void coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)

+static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)

 {

     BlockDriverState *bs = self->bs;

     BdrvTrackedRequest *req;

     bool retry;

+    bool waited = false;

     if (!bs->serialising_in_flight) {

-        return;

+        return false;

     }

     do {

@@ -2186,11 +2187,14 @@ static void coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)

                     qemu_co_queue_wait(&req->wait_queue);

                     self->waiting_for = NULL;

                     retry = true;

+                    waited = true;

                     break;

                 }

             }

         }

     } while (retry);

+

+    return waited;

 }

 /*

@@ -3036,6 +3040,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,

     QEMUIOVector *qiov, int flags)

 {

     BlockDriver *drv = bs->drv;

+    bool waited;

     int ret;

     int64_t sector_num = offset >> BDRV_SECTOR_BITS;

@@ -3044,7 +3049,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,

     assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);

     assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);

-    wait_serialising_requests(req);

+    waited = wait_serialising_requests(req);

+    assert(!waited || !req->serialising);

     if (flags & BDRV_REQ_ZERO_WRITE) {

         ret = bdrv_co_do_write_zeroes(bs, sector_num, nb_sectors, flags);

@@ -3142,9 +3148,11 @@ static int coroutine_fn bdrv_co_do_pwritev(BlockDriverState *bs,

         QEMUIOVector tail_qiov;

         struct iovec tail_iov;

         size_t tail_bytes;

+        bool waited;

         mark_request_serialising(&req, align);

-        wait_serialising_requests(&req;;

+        waited = wait_serialising_requests(&req;;

+        assert(!waited || !use_local_qiov);

         tail_buf = qemu_blockalign(bs, align);

         tail_iov = (struct iovec) {

-- 

1.7.1