[PATCH 04/13] block/preallocate: serialise the filter state

["Denis V. Lunev" <den@openvz.org>]

From: Denis V. Lunev <den@openvz.org>

The filter keeps the file length it has seen, the length it has
reserved and where the trailing zeroes start in plain fields, and
updates them with a read, a compare and a store. That was indivisible
while one node was driven by one AioContext. It is not any more:
virtio-blk with iothread-vq-mapping drives one node from several.

Two writers extending the file then both pass the compare, and the
smaller end lands last:

  writer A, [1G, 1G+64k)            writer B, [1G+64k, 1G+128k)
  --------------------------------  --------------------------------
  reads data_end 1G, 1G+64k > 1G
                                    reads data_end 1G, 1G+128k > 1G
                                    stores data_end = 1G+128k
  stores data_end = 1G+64k

Both writes are acknowledged, and data_end now sits below B's data.
Nothing raises it again except a later write ending beyond it, and
closing the image truncates the file to it, so B's data is cut off the
end. The last pair of extending writes before a shutdown is exactly
where this bites.

Take a mutex across handle_write() and preallocate_co_getlength(). The
remaining readers, apart from preallocate_co_truncate(), reach the
state from global state code with the node drained, where no request
is in flight.

Holding it cannot deadlock. A writer waiting for it has not reached
bs->file yet, so it owns nothing below the filter, and the only calls
made under it go to the child. preallocate_co_truncate() is left out
for that reason: growing the file waits for the requests in flight on
the child, and once the next patch issues the preallocation without
the mutex, one of those can be a preallocation that needs it back.

Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: qemu-stable@nongnu.org
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 block/preallocate.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/block/preallocate.c b/block/preallocate.c
index 8cd98684f4..442cdb0176 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -76,6 +76,8 @@ typedef struct BDRVPreallocateState {
      * BLK_PERM_WRITE permissions on file child.
      */
 
+    CoMutex lock;
+
     /* Gives up the resize permission on children when parents don't need it */
     QEMUBH *drop_resize_bh;
 } BDRVPreallocateState;
@@ -155,6 +157,7 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
      * For this to work, mark them invalid.
      */
     preallocate_set_state(s, -EINVAL);
+    qemu_co_mutex_init(&s->lock);
     s->drop_resize_bh = qemu_bh_new(preallocate_drop_resize_bh, bs);
 
     ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
@@ -324,8 +327,8 @@ static bool GRAPH_RDLOCK has_prealloc_perms(BlockDriverState *bs)
  * one bdrv_co_pwrite_zeroes() call.
  */
 static bool coroutine_fn GRAPH_RDLOCK
-handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
-             bool want_merge_zero)
+handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
+                    bool want_merge_zero)
 {
     BDRVPreallocateState *s = bs->opaque;
     int64_t end = offset + bytes;
@@ -404,6 +407,20 @@ handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
     return want_merge_zero;
 }
 
+static bool coroutine_fn GRAPH_RDLOCK
+handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
+             bool want_merge_zero)
+{
+    BDRVPreallocateState *s = bs->opaque;
+    bool ret;
+
+    qemu_co_mutex_lock(&s->lock);
+    ret = handle_write_locked(bs, offset, bytes, want_merge_zero);
+    qemu_co_mutex_unlock(&s->lock);
+
+    return ret;
+}
+
 static int coroutine_fn GRAPH_RDLOCK
 preallocate_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
                              int64_t bytes, BdrvRequestFlags flags)
@@ -506,8 +523,11 @@ preallocate_co_getlength(BlockDriverState *bs)
     int64_t ret;
     BDRVPreallocateState *s = bs->opaque;
 
+    qemu_co_mutex_lock(&s->lock);
     if (s->data_end >= 0) {
-        return s->data_end;
+        ret = s->data_end;
+        qemu_co_mutex_unlock(&s->lock);
+        return ret;
     }
 
     ret = bdrv_co_getlength(bs->file->bs);
@@ -515,6 +535,7 @@ preallocate_co_getlength(BlockDriverState *bs)
     if (has_prealloc_perms(bs)) {
         preallocate_set_state(s, ret);
     }
+    qemu_co_mutex_unlock(&s->lock);
 
     return ret;
 }
-- 
2.53.0