debian/patches/extra/0036-kvm-nbd-make-it-thread-safe-fix-qcow2-over-nbd.patch

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: Eric Blake <eblake@redhat.com>
   3 Date: Sun, 11 Jun 2017 03:30:07 +0200
   4 Subject: [PATCH] nbd: make it thread-safe, fix qcow2 over nbd
   5
   6 RH-Author: Eric Blake <eblake@redhat.com>
   7 Message-id: <20170611033007.399-1-eblake@redhat.com>
   8 Patchwork-id: 75581
   9 O-Subject: [RHEV-7.4 qemu-kvm-rhev PATCH] nbd: make it thread-safe, fix qcow2 over nbd
  10 Bugzilla: 1454582
  11 RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
  12 RH-Acked-by: Max Reitz <mreitz@redhat.com>
  13 RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
  14
  15 From: Paolo Bonzini <pbonzini@redhat.com>
  16
  17 NBD is not thread safe, because it accesses s->in_flight without
  18 a CoMutex.  Fixing this will be required for multiqueue.
  19 CoQueue doesn't have spurious wakeups but, when another coroutine can
  20 run between qemu_co_queue_next's wakeup and qemu_co_queue_wait's
  21 re-locking of the mutex, the wait condition can become false and
  22 a loop is necessary.
  23
  24 In fact, it turns out that the loop is necessary even without this
  25 multi-threaded scenario.  A particular sequence of coroutine wakeups
  26 is happening ~80% of the time when starting a guest with qcow2 image
  27 served over NBD (i.e. qemu-nbd --format=raw, and QEMU's -drive option
  28 has -format=qcow2).  This patch fixes that issue too.
  29
  30 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  31 (cherry picked from commit 6bdcc018a6ed760b9dfe43539124e420aed83092)
  32 Signed-off-by: Eric Blake <eblake@redhat.com>
  33 Upstream-status: v6 pull request https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg01841.html
  34 Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
  35 ---
  36  block/nbd-client.c | 30 +++++++++---------------------
  37  1 file changed, 9 insertions(+), 21 deletions(-)
  38
  39 diff --git a/block/nbd-client.c b/block/nbd-client.c
  40 index 1e2952f..43e0292 100644
  41 --- a/block/nbd-client.c
  42 +++ b/block/nbd-client.c
  43 @@ -114,6 +114,10 @@ static int nbd_co_send_request(BlockDriverState *bs,
  44      int rc, ret, i;
  45
  46      qemu_co_mutex_lock(&s->send_mutex);
  47 +    while (s->in_flight == MAX_NBD_REQUESTS) {
  48 +        qemu_co_queue_wait(&s->free_sema, &s->send_mutex);
  49 +    }
  50 +    s->in_flight++;
  51
  52      for (i = 0; i < MAX_NBD_REQUESTS; i++) {
  53          if (s->recv_coroutine[i] == NULL) {
  54 @@ -176,20 +180,6 @@ static void nbd_co_receive_reply(NBDClientSession *s,
  55      }
  56  }
  57
  58 -static void nbd_coroutine_start(NBDClientSession *s,
  59 -                                NBDRequest *request)
  60 -{
  61 -    /* Poor man semaphore.  The free_sema is locked when no other request
  62 -     * can be accepted, and unlocked after receiving one reply.  */
  63 -    if (s->in_flight == MAX_NBD_REQUESTS) {
  64 -        qemu_co_queue_wait(&s->free_sema, NULL);
  65 -        assert(s->in_flight < MAX_NBD_REQUESTS);
  66 -    }
  67 -    s->in_flight++;
  68 -
  69 -    /* s->recv_coroutine[i] is set as soon as we get the send_lock.  */
  70 -}
  71 -
  72  static void nbd_coroutine_end(BlockDriverState *bs,
  73                                NBDRequest *request)
  74  {
  75 @@ -197,13 +187,16 @@ static void nbd_coroutine_end(BlockDriverState *bs,
  76      int i = HANDLE_TO_INDEX(s, request->handle);
  77
  78      s->recv_coroutine[i] = NULL;
  79 -    s->in_flight--;
  80 -    qemu_co_queue_next(&s->free_sema);
  81
  82      /* Kick the read_reply_co to get the next reply.  */
  83      if (s->read_reply_co) {
  84          aio_co_wake(s->read_reply_co);
  85      }
  86 +
  87 +    qemu_co_mutex_lock(&s->send_mutex);
  88 +    s->in_flight--;
  89 +    qemu_co_queue_next(&s->free_sema);
  90 +    qemu_co_mutex_unlock(&s->send_mutex);
  91  }
  92
  93  int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
  94 @@ -221,7 +214,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
  95      assert(bytes <= NBD_MAX_BUFFER_SIZE);
  96      assert(!flags);
  97
  98 -    nbd_coroutine_start(client, &request);
  99      ret = nbd_co_send_request(bs, &request, NULL);
 100      if (ret < 0) {
 101          reply.error = -ret;
 102 @@ -251,7 +243,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
 103
 104      assert(bytes <= NBD_MAX_BUFFER_SIZE);
 105
 106 -    nbd_coroutine_start(client, &request);
 107      ret = nbd_co_send_request(bs, &request, qiov);
 108      if (ret < 0) {
 109          reply.error = -ret;
 110 @@ -286,7 +277,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
 111          request.flags |= NBD_CMD_FLAG_NO_HOLE;
 112      }
 113
 114 -    nbd_coroutine_start(client, &request);
 115      ret = nbd_co_send_request(bs, &request, NULL);
 116      if (ret < 0) {
 117          reply.error = -ret;
 118 @@ -311,7 +301,6 @@ int nbd_client_co_flush(BlockDriverState *bs)
 119      request.from = 0;
 120      request.len = 0;
 121
 122 -    nbd_coroutine_start(client, &request);
 123      ret = nbd_co_send_request(bs, &request, NULL);
 124      if (ret < 0) {
 125          reply.error = -ret;
 126 @@ -337,7 +326,6 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
 127          return 0;
 128      }
 129
 130 -    nbd_coroutine_start(client, &request);
 131      ret = nbd_co_send_request(bs, &request, NULL);
 132      if (ret < 0) {
 133          reply.error = -ret;
 134 --
 135 1.8.3.1
 136