Series comparison

-[Qemu-devel] [PULL 0/5] Block patches
+[PULL 00/11] Block patches
-The following changes since commit a0def594286d9110a6035e02eef558cf3cf5d847:
+The following changes since commit 88afdc92b644120e0182c8567e1b1d236e471b12:
-  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-01-30 10:23:20 +0000)
+  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-09-05 15:48:42 +0100)
-are available in the git repository at:
+are available in the Git repository at:
-  https://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to acf6e5f0962c4be670d4a93ede77423512521876:
+for you to fetch changes up to 9bd2788f49c331b02372cc257b11e4c984d39708:
-  sheepdog: reorganize check for overlapping requests (2017-02-01 00:17:20 -0500)
+  block/nvme: Only report VFIO error on failed retry (2021-09-07 09:08:24 +0100)
 ----------------------------------------------------------------
-Block patches
+Pull request
 Userspace NVMe driver patches.
 ----------------------------------------------------------------
-Paolo Bonzini (5):
+Philippe Mathieu-Daudé (11):
-  sheepdog: remove unused cancellation support
+  block/nvme: Use safer trace format string
-  sheepdog: reorganize coroutine flow
+  util/vfio-helpers: Let qemu_vfio_verify_mappings() use error_report()
-  sheepdog: do not use BlockAIOCB
+  util/vfio-helpers: Replace qemu_mutex_lock() calls with
-  sheepdog: simplify inflight_aio_head management
+    QEMU_LOCK_GUARD
-  sheepdog: reorganize check for overlapping requests
+  util/vfio-helpers: Remove unreachable code in qemu_vfio_dma_map()
   block/nvme: Have nvme_create_queue_pair() report errors consistently
   util/vfio-helpers: Pass Error handle to qemu_vfio_dma_map()
   util/vfio-helpers: Extract qemu_vfio_water_mark_reached()
   util/vfio-helpers: Use error_setg in qemu_vfio_find_[fixed/temp]_iova
   util/vfio-helpers: Simplify qemu_vfio_dma_map() returning directly
   util/vfio-helpers: Let qemu_vfio_do_mapping() propagate Error
   block/nvme: Only report VFIO error on failed retry
- block/sheepdog.c | 289 ++++++++++++++++---------------------------------------
+ include/qemu/vfio-helpers.h |  2 +-
-file changed, 84 insertions(+), 205 deletions(-)
+ block/nvme.c                | 29 +++++++----
  util/vfio-helpers.c         | 99 ++++++++++++++++++++-----------------
  block/trace-events          |  2 +-
 files changed, 76 insertions(+), 56 deletions(-)
 --
-.9.3
+.31.1

-New patch
+[PULL 01/11] block/nvme: Use safer trace format string
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
+Fix when building with -Wshorten-64-to-32:
+  warning: implicit conversion loses integer precision: 'unsigned long' to 'int' [-Wshorten-64-to-32]
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210902070025.197072-2-philmd@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ block/trace-events | 2 +-
+file changed, 1 insertion(+), 1 deletion(-)
+diff --git a/block/trace-events b/block/trace-events
+index XXXXXXX..XXXXXXX 100644
+--- a/block/trace-events
++++ b/block/trace-events
+@@ -XXX,XX +XXX,XX @@ nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset 0x%"PRIx64" byte
+ nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p offset 0x%"PRIx64" bytes %"PRId64" ret %d"
+ nvme_dma_map_flush(void *s) "s %p"
+ nvme_free_req_queue_wait(void *s, unsigned q_index) "s %p q #%u"
+-nvme_create_queue_pair(unsigned q_index, void *q, unsigned size, void *aio_context, int fd) "index %u q %p size %u aioctx %p fd %d"
++nvme_create_queue_pair(unsigned q_index, void *q, size_t size, void *aio_context, int fd) "index %u q %p size %zu aioctx %p fd %d"
+ nvme_free_queue_pair(unsigned q_index, void *q) "index %u q %p"
+ nvme_cmd_map_qiov(void *s, void *cmd, void *req, void *qiov, int entries) "s %p cmd %p req %p qiov %p entries %d"
+ nvme_cmd_map_qiov_pages(void *s, int i, uint64_t page) "s %p page[%d] 0x%"PRIx64
+--
+.31.1

-New patch
+[PULL 02/11] util/vfio-helpers: Let qemu_vfio_verify_mappings() use error_report()
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
+Instead of displaying the error on stderr, use error_report()
+which also report to the monitor.
+Reviewed-by: Fam Zheng <fam@euphon.net>
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210902070025.197072-3-philmd@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ util/vfio-helpers.c | 4 ++--
+file changed, 2 insertions(+), 2 deletions(-)
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/vfio-helpers.c
++++ b/util/vfio-helpers.c
+@@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)
+     if (QEMU_VFIO_DEBUG) {
+         for (i = 0; i < s->nr_mappings - 1; ++i) {
+             if (!(s->mappings[i].host < s->mappings[i + 1].host)) {
+-                fprintf(stderr, "item %d not sorted!\n", i);
++                error_report("item %d not sorted!", i);
+                 qemu_vfio_dump_mappings(s);
+                 return false;
+             }
+             if (!(s->mappings[i].host + s->mappings[i].size <=
+                   s->mappings[i + 1].host)) {
+-                fprintf(stderr, "item %d overlap with next!\n", i);
++                error_report("item %d overlap with next!", i);
+                 qemu_vfio_dump_mappings(s);
+                 return false;
+             }
+--
+.31.1

-New patch
+[PULL 03/11] util/vfio-helpers: Replace qemu_mutex_lock() calls with QEMU_LOCK_GUARD
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
+Simplify qemu_vfio_dma_[un]map() handlers by replacing a pair of
+qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
+macro.
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210902070025.197072-4-philmd@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ util/vfio-helpers.c | 9 +++------
+file changed, 3 insertions(+), 6 deletions(-)
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/vfio-helpers.c
++++ b/util/vfio-helpers.c
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
+     assert(QEMU_PTR_IS_ALIGNED(host, qemu_real_host_page_size));
+     assert(QEMU_IS_ALIGNED(size, qemu_real_host_page_size));
+     trace_qemu_vfio_dma_map(s, host, size, temporary, iova);
+-    qemu_mutex_lock(&s->lock);
++    QEMU_LOCK_GUARD(&s->lock);
+     mapping = qemu_vfio_find_mapping(s, host, &index);
+     if (mapping) {
+         iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
+         *iova = iova0;
+     }
+ out:
+-    qemu_mutex_unlock(&s->lock);
+     return ret;
+ }
+@@ -XXX,XX +XXX,XX @@ void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host)
+     }
+     trace_qemu_vfio_dma_unmap(s, host);
+-    qemu_mutex_lock(&s->lock);
++    QEMU_LOCK_GUARD(&s->lock);
+     m = qemu_vfio_find_mapping(s, host, &index);
+     if (!m) {
+-        goto out;
++        return;
+     }
+     qemu_vfio_undo_mapping(s, m, NULL);
+-out:
+-    qemu_mutex_unlock(&s->lock);
+ }
+ static void qemu_vfio_reset(QEMUVFIOState *s)
+--
+.31.1

-New patch
+[PULL 04/11] util/vfio-helpers: Remove unreachable code in qemu_vfio_dma_map()
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
+qemu_vfio_add_mapping() returns a pointer to an indexed entry
+in pre-allocated QEMUVFIOState::mappings[], thus can not be NULL.
+Remove the pointless check.
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210902070025.197072-5-philmd@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ util/vfio-helpers.c | 4 ----
+file changed, 4 deletions(-)
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/vfio-helpers.c
++++ b/util/vfio-helpers.c
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
+             }
+             mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0);
+-            if (!mapping) {
+-                ret = -ENOMEM;
+-                goto out;
+-            }
+             assert(qemu_vfio_verify_mappings(s));
+             ret = qemu_vfio_do_mapping(s, host, size, iova0);
+             if (ret) {
+--
+.31.1

-New patch
+[PULL 05/11] block/nvme: Have nvme_create_queue_pair() report errors consistently
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
+nvme_create_queue_pair() does not return a boolean value (indicating
+eventual error) but a pointer, and is inconsistent in how it fills the
+error handler. To fulfill callers expectations, always set an error
+message on failure.
+Reported-by: Auger Eric <eric.auger@redhat.com>
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210902070025.197072-6-philmd@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ block/nvme.c | 3 +++
+file changed, 3 insertions(+)
+diff --git a/block/nvme.c b/block/nvme.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/nvme.c
++++ b/block/nvme.c
+@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
+     q = g_try_new0(NVMeQueuePair, 1);
+     if (!q) {
++        error_setg(errp, "Cannot allocate queue pair");
+         return NULL;
+     }
+     trace_nvme_create_queue_pair(idx, q, size, aio_context,
+@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
+                           qemu_real_host_page_size);
+     q->prp_list_pages = qemu_try_memalign(qemu_real_host_page_size, bytes);
+     if (!q->prp_list_pages) {
++        error_setg(errp, "Cannot allocate PRP page list");
+         goto fail;
+     }
+     memset(q->prp_list_pages, 0, bytes);
+@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
+     r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, bytes,
+                           false, &prp_list_iova);
+     if (r) {
++        error_setg_errno(errp, -r, "Cannot map buffer for DMA");
+         goto fail;
+     }
+     q->free_req_head = -1;
+--
+.31.1

-[Qemu-devel] [PULL 5/5] sheepdog: reorganize check for overlapping requests
+[PULL 06/11] util/vfio-helpers: Pass Error handle to qemu_vfio_dma_map()
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-Wrap the code that was copied repeatedly in the two functions,
+Currently qemu_vfio_dma_map() displays errors on stderr.
-sd_aio_setup and sd_aio_complete.
+When using management interface, this information is simply
 lost. Pass qemu_vfio_dma_map() an Error** handle so it can
 propagate the error to callers.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Reviewed-by: Fam Zheng <fam@euphon.net>
-Message-id: 20161129113245.32724-6-pbonzini@redhat.com
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20210902070025.197072-7-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 66 ++++++++++++++++++++++++++------------------------------
+ include/qemu/vfio-helpers.h |  2 +-
-file changed, 30 insertions(+), 36 deletions(-)
+ block/nvme.c                | 22 +++++++++++-----------
  util/vfio-helpers.c         | 10 ++++++----
 files changed, 18 insertions(+), 16 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/include/qemu/vfio-helpers.h
-+++ b/block/sheepdog.c
++++ b/include/qemu/vfio-helpers.h
-@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
+@@ -XXX,XX +XXX,XX @@ typedef struct QEMUVFIOState QEMUVFIOState;
-     return aio_req;
+ QEMUVFIOState *qemu_vfio_open_pci(const char *device, Error **errp);
  void qemu_vfio_close(QEMUVFIOState *s);
  int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
 -                      bool temporary, uint64_t *iova_list);
 +                      bool temporary, uint64_t *iova_list, Error **errp);
  int qemu_vfio_dma_reset_temporary(QEMUVFIOState *s);
  void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host);
  void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,
 diff --git a/block/nvme.c b/block/nvme.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/nvme.c
 +++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static bool nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q,
          return false;
      }
      memset(q->queue, 0, bytes);
 -    r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova);
 +    r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova, errp);
      if (r) {
 -        error_setg(errp, "Cannot map queue");
 -        return false;
 +        error_prepend(errp, "Cannot map queue: ");
      }
 -    return true;
 +    return r == 0;
  }
-+static void wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
+ static void nvme_free_queue_pair(NVMeQueuePair *q)
-+{
+@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
-+    SheepdogAIOCB *cb;
+     qemu_co_queue_init(&q->free_req_queue);
-+
+     q->completion_bh = aio_bh_new(aio_context, nvme_process_completion_bh, q);
-+retry:
+     r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, bytes,
-+    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
+-                          false, &prp_list_iova);
-+        if (AIOCBOverlapping(acb, cb)) {
++                          false, &prp_list_iova, errp);
-+            qemu_co_queue_wait(&s->overlapping_queue);
+     if (r) {
-+            goto retry;
+-        error_setg_errno(errp, -r, "Cannot map buffer for DMA");
-+        }
++        error_prepend(errp, "Cannot map buffer for DMA: ");
-+    }
+         goto fail;
-+}
+     }
-+
+     q->free_req_head = -1;
- static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
+@@ -XXX,XX +XXX,XX @@ static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
-                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
+         error_setg(errp, "Cannot allocate buffer for identify response");
-                          int type)
+         goto out;
-@@ -XXX,XX +XXX,XX @@ static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
+     }
-     acb->min_dirty_data_idx = UINT32_MAX;
+-    r = qemu_vfio_dma_map(s->vfio, id, id_size, true, &iova);
-     acb->max_dirty_data_idx = 0;
++    r = qemu_vfio_dma_map(s->vfio, id, id_size, true, &iova, errp);
-     acb->aiocb_type = type;
+     if (r) {
-+
+-        error_setg(errp, "Cannot map buffer for DMA");
-+    if (type == AIOCB_FLUSH_CACHE) {
++        error_prepend(errp, "Cannot map buffer for DMA: ");
-+        return;
+         goto out;
-+    }
+     }
-+
-+    wait_for_overlapping_aiocb(s, acb);
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd,
-+    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, acb, aiocb_siblings);
+ try_map:
- }
+         r = qemu_vfio_dma_map(s->vfio,
+                               qiov->iov[i].iov_base,
- /* Return -EIO in case of error, file descriptor on success */
+-                              len, true, &iova);
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
++                              len, true, &iova, NULL);
          if (r == -ENOSPC) {
              /*
               * In addition to the -ENOMEM error, the VFIO_IOMMU_MAP_DMA
@@ -XXX,XX +XXX,XX @@ static void nvme_aio_unplug(BlockDriverState *bs)
  static void nvme_register_buf(BlockDriverState *bs, void *host, size_t size)
  {
      int ret;
 +    Error *local_err = NULL;
      BDRVNVMeState *s = bs->opaque;
 -    ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL);
 +    ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL, &local_err);
      if (ret) {
          /* FIXME: we may run out of IOVA addresses after repeated
           * bdrv_register_buf/bdrv_unregister_buf, because nvme_vfio_dma_unmap
           * doesn't reclaim addresses for fixed mappings. */
 -        error_report("nvme_register_buf failed: %s", strerror(-ret));
 +        error_reportf_err(local_err, "nvme_register_buf failed: ");
      }
  }
--static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
-+static void sd_aio_complete(SheepdogAIOCB *acb)
+index XXXXXXX..XXXXXXX 100644
 --- a/util/vfio-helpers.c
 +++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ static void qemu_vfio_ram_block_added(RAMBlockNotifier *n, void *host,
                                        size_t size, size_t max_size)
  {
--    SheepdogAIOCB *cb;
+     QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier);
--
++    Error *local_err = NULL;
--    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
+     int ret;
--        if (AIOCBOverlapping(aiocb, cb)) {
--            return true;
+     trace_qemu_vfio_ram_block_added(s, host, max_size);
--        }
+-    ret = qemu_vfio_dma_map(s, host, max_size, false, NULL);
-+    if (acb->aiocb_type == AIOCB_FLUSH_CACHE) {
++    ret = qemu_vfio_dma_map(s, host, max_size, false, NULL, &local_err);
-+        return;
+     if (ret) {
 -        error_report("qemu_vfio_dma_map(%p, %zu) failed: %s", host, max_size,
 -                     strerror(-ret));
 +        error_reportf_err(local_err,
 +                          "qemu_vfio_dma_map(%p, %zu) failed: ",
 +                          host, max_size);
      }
--    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, aiocb, aiocb_siblings);
--    return false;
-+    QLIST_REMOVE(acb, aiocb_siblings);
-+    qemu_co_queue_restart_all(&acb->s->overlapping_queue);
  }
- static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
+@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
+  * mapping status within this area is not allowed).
-     }
+  */
+ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
-     sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
+-                      bool temporary, uint64_t *iova)
--
++                      bool temporary, uint64_t *iova, Error **errp)
--retry:
+ {
--    if (check_overlapping_aiocb(s, &acb)) {
+     int ret = 0;
--        qemu_co_queue_wait(&s->overlapping_queue);
+     int index;
 -        goto retry;
 -    }
 -
      sd_co_rw_vector(&acb);
      sd_write_done(&acb);
 +    sd_aio_complete(&acb);
 -    QLIST_REMOVE(&acb, aiocb_siblings);
 -    qemu_co_queue_restart_all(&s->overlapping_queue);
      return acb.ret;
  }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
      BDRVSheepdogState *s = bs->opaque;
      sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
 -
 -retry:
 -    if (check_overlapping_aiocb(s, &acb)) {
 -        qemu_co_queue_wait(&s->overlapping_queue);
 -        goto retry;
 -    }
 -
      sd_co_rw_vector(&acb);
 +    sd_aio_complete(&acb);
 -    QLIST_REMOVE(&acb, aiocb_siblings);
 -    qemu_co_queue_restart_all(&s->overlapping_queue);
      return acb.ret;
  }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
      if (--acb.nr_pending) {
          qemu_coroutine_yield();
      }
 +
 +    sd_aio_complete(&acb);
      return acb.ret;
  }
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
      }
      sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
                   count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
 -
 -retry:
 -    if (check_overlapping_aiocb(s, &acb)) {
 -        qemu_co_queue_wait(&s->overlapping_queue);
 -        goto retry;
 -    }
 -
      sd_co_rw_vector(&acb);
 +    sd_aio_complete(&acb);
 -    QLIST_REMOVE(&acb, aiocb_siblings);
 -    qemu_co_queue_restart_all(&s->overlapping_queue);
      return acb.ret;
  }
 --
-.9.3
+.31.1

-[Qemu-devel] [PULL 4/5] sheepdog: simplify inflight_aio_head management
+[PULL 07/11] util/vfio-helpers: Extract qemu_vfio_water_mark_reached()
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-Add to the list in add_aio_request and, indirectly, resend_aioreq.  Inline
+Extract qemu_vfio_water_mark_reached() for readability,
-free_aio_req in the caller, it does not simply undo alloc_aio_req's job.
+and have it provide an error hint it its Error* handle.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Suggested-by: Klaus Jensen <k.jensen@samsung.com>
-Message-id: 20161129113245.32724-5-pbonzini@redhat.com
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20210902070025.197072-8-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 23 ++++++-----------------
+ util/vfio-helpers.c | 17 ++++++++++++++++-
-file changed, 6 insertions(+), 17 deletions(-)
+file changed, 16 insertions(+), 1 deletion(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/util/vfio-helpers.c
-+++ b/block/sheepdog.c
++++ b/util/vfio-helpers.c
-@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
+@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
-     return aio_req;
+     return -ENOMEM;
  }
--static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
++/**
--{
++ * qemu_vfio_water_mark_reached:
--    SheepdogAIOCB *acb = aio_req->aiocb;
++ *
--
++ * Returns %true if high watermark has been reached, %false otherwise.
--    QLIST_REMOVE(aio_req, aio_siblings);
++ */
--    g_free(aio_req);
++static bool qemu_vfio_water_mark_reached(QEMUVFIOState *s, size_t size,
--
++                                         Error **errp)
--    acb->nr_pending--;
++{
--}
++    if (s->high_water_mark - s->low_water_mark + 1 < size) {
--
++        error_setg(errp, "iova exhausted (water mark reached)");
- static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
++        return true;
-                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
++    }
-                          int type)
++    return false;
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
++}
      while (!QLIST_EMPTY(&s->failed_aio_head)) {
          aio_req = QLIST_FIRST(&s->failed_aio_head);
          QLIST_REMOVE(aio_req, aio_siblings);
 -        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
          resend_aioreq(s, aio_req);
      }
  }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
      */
      s->co_recv = NULL;
 +    QLIST_REMOVE(aio_req, aio_siblings);
      switch (rsp.result) {
      case SD_RES_SUCCESS:
          break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
          break;
      }
 -    free_aio_req(s, aio_req);
 -    if (!acb->nr_pending) {
 +    g_free(aio_req);
 +
-+    if (!--acb->nr_pending) {
+ /* Map [host, host + size) area into a contiguous IOVA address space, and store
-         /*
+  * the result in @iova if not NULL. The caller need to make sure the area is
-          * We've finished all requests which belong to the AIOCB, so
+  * aligned to page size, and mustn't overlap with existing mapping areas (split
-          * we can switch back to sd_co_readv/writev now.
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
+     if (mapping) {
-     uint64_t old_oid = aio_req->base_oid;
+         iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);
-     bool create = aio_req->create;
+     } else {
+-        if (s->high_water_mark - s->low_water_mark + 1 < size) {
-+    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
++        if (qemu_vfio_water_mark_reached(s, size, errp)) {
-+
+             ret = -ENOMEM;
-     if (!nr_copies) {
+             goto out;
-         error_report("bug");
+         }
      }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
          iov.iov_len = sizeof(s->inode);
          aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                                  data_len, offset, 0, false, 0, offset);
 -        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
          add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
          if (--acb->nr_pending) {
              qemu_coroutine_yield();
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
                                  old_oid,
                                  acb->aiocb_type == AIOCB_DISCARD_OBJ ?
 : done);
 -        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
 -
          add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
                          acb->aiocb_type);
      done:
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
      acb.nr_pending++;
      aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
 , 0, 0, false, 0, 0);
 -    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
      add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
      if (--acb.nr_pending) {
 --
-.9.3
+.31.1

-[Qemu-devel] [PULL 2/5] sheepdog: reorganize coroutine flow
+[PULL 08/11] util/vfio-helpers: Use error_setg in qemu_vfio_find_[fixed/temp]_iova
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-Delimit co_recv's lifetime clearly in aio_read_response.
+Both qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova()
 return an errno which is unused (or overwritten). Have them propagate
 eventual errors to callers, returning a boolean (which is what the
 Error API recommends, see commit e3fe3988d78 "error: Document Error
 API usage rules" for rationale).
-Do a simple qemu_coroutine_enter in aio_read_response, letting
+Suggested-by: Klaus Jensen <k.jensen@samsung.com>
-sd_co_writev call sd_write_done.
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20210902070025.197072-9-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  util/vfio-helpers.c | 24 ++++++++++++++----------
 file changed, 14 insertions(+), 10 deletions(-)
-Handle nr_pending in the same way in sd_co_rw_vector,
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
 sd_write_done and sd_co_flush_to_disk.
 Remove sd_co_rw_vector's return value; just leave with no
 pending requests.
 [Jeff: added missing 'return' back, spotted by Paolo after
        series was applied.]
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  block/sheepdog.c | 115 ++++++++++++++++++++-----------------------------------
 file changed, 42 insertions(+), 73 deletions(-)
 diff --git a/block/sheepdog.c b/block/sheepdog.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/util/vfio-helpers.c
-+++ b/block/sheepdog.c
++++ b/util/vfio-helpers.c
-@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
+@@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)
-     enum AIOCBState aiocb_type;
+     return true;
      Coroutine *coroutine;
 -    void (*aio_done_func)(SheepdogAIOCB *);
 -
      int nr_pending;
      uint32_t min_affect_data_idx;
@@ -XXX,XX +XXX,XX @@ static const char * sd_strerror(int err)
   *
   * 1. In sd_co_rw_vector, we send the I/O requests to the server and
   *    link the requests to the inflight_list in the
 - *    BDRVSheepdogState.  The function exits without waiting for
 + *    BDRVSheepdogState.  The function yields while waiting for
   *    receiving the response.
   *
   * 2. We receive the response in aio_read_response, the fd handler to
 - *    the sheepdog connection.  If metadata update is needed, we send
 - *    the write request to the vdi object in sd_write_done, the write
 - *    completion function.  We switch back to sd_co_readv/writev after
 - *    all the requests belonging to the AIOCB are finished.
 + *    the sheepdog connection.  We switch back to sd_co_readv/sd_writev
 + *    after all the requests belonging to the AIOCB are finished.  If
 + *    needed, sd_co_writev will send another requests for the vdi object.
   */
  static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
      acb->nr_pending--;
  }
--static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
+-static int
--{
+-qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
--    qemu_coroutine_enter(acb->coroutine);
++static bool qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size,
--    qemu_aio_unref(acb);
++                                      uint64_t *iova, Error **errp)
--}
+ {
--
+     int i;
- static const AIOCBInfo sd_aiocb_info = {
-     .aiocb_size     = sizeof(SheepdogAIOCB),
+@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
- };
+             s->usable_iova_ranges[i].end - s->low_water_mark + 1 == 0) {
-@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
+             *iova = s->low_water_mark;
-     acb->sector_num = sector_num;
+             s->low_water_mark += size;
-     acb->nb_sectors = nb_sectors;
+-            return 0;
++            return true;
 -    acb->aio_done_func = NULL;
      acb->coroutine = qemu_coroutine_self();
      acb->ret = 0;
      acb->nr_pending = 0;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
      switch (acb->aiocb_type) {
      case AIOCB_WRITE_UDATA:
 -        /* this coroutine context is no longer suitable for co_recv
 -         * because we may send data to update vdi objects */
 -        s->co_recv = NULL;
          if (!is_data_obj(aio_req->oid)) {
              break;
          }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
          }
      }
+-    return -ENOMEM;
-+    /* No more data for this aio_req (reload_inode below uses its own file
++    error_setg(errp, "fixed iova range not found");
 +     * descriptor handler which doesn't use co_recv).
 +    */
 +    s->co_recv = NULL;
 +
-     switch (rsp.result) {
++    return false;
      case SD_RES_SUCCESS:
          break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
              aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
          }
          resend_aioreq(s, aio_req);
 -        goto out;
 +        return;
      default:
          acb->ret = -EIO;
          error_report("%s", sd_strerror(rsp.result));
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
           * We've finished all requests which belong to the AIOCB, so
           * we can switch back to sd_co_readv/writev now.
           */
 -        acb->aio_done_func(acb);
 +        qemu_coroutine_enter(acb->coroutine);
      }
 -out:
 -    s->co_recv = NULL;
 +
      return;
 +
  err:
 -    s->co_recv = NULL;
      reconnect_to_sdog(opaque);
  }
-@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
+-static int
- /*
+-qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
-  * This function is called after writing data objects.  If we need to
++static bool qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size,
-  * update metadata, this sends a write request to the vdi object.
++                                     uint64_t *iova, Error **errp)
 - * Otherwise, this switches back to sd_co_readv/writev.
   */
  static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
  {
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
+     int i;
-     mx = acb->max_dirty_data_idx;
-     if (mn <= mx) {
+@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
-         /* we need to update the vdi object. */
+             s->high_water_mark - s->usable_iova_ranges[i].start + 1 == 0) {
-+        ++acb->nr_pending;
+             *iova = s->high_water_mark - size;
-         offset = sizeof(s->inode) - sizeof(s->inode.data_vdi_id) +
+             s->high_water_mark = *iova;
-             mn * sizeof(s->inode.data_vdi_id[0]);
+-            return 0;
-         data_len = (mx - mn + 1) * sizeof(s->inode.data_vdi_id[0]);
++            return true;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
                                  data_len, offset, 0, false, 0, offset);
          QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
          add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
 -
 -        acb->aio_done_func = sd_finish_aiocb;
 -        acb->aiocb_type = AIOCB_WRITE_UDATA;
 -        return;
 +        if (--acb->nr_pending) {
 +            qemu_coroutine_yield();
 +        }
      }
 -
 -    sd_finish_aiocb(acb);
  }
  /* Delete current working VDI on the snapshot chain */
@@ -XXX,XX +XXX,XX @@ out:
   * Returns 1 when we need to wait a response, 0 when there is no sent
   * request and -errno in error cases.
   */
 -static int coroutine_fn sd_co_rw_vector(void *p)
 +static void coroutine_fn sd_co_rw_vector(void *p)
  {
      SheepdogAIOCB *acb = p;
      int ret = 0;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
          ret = sd_create_branch(s);
          if (ret) {
              acb->ret = -EIO;
 -            goto out;
 +            return;
          }
      }
+-    return -ENOMEM;
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
++    error_setg(errp, "temporary iova range not found");
-         idx++;
++
-         done += len;
++    return false;
      }
 -out:
 -    if (!--acb->nr_pending) {
 -        return acb->ret;
 +    if (--acb->nr_pending) {
 +        qemu_coroutine_yield();
      }
 -    return 1;
  }
- static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
+ /**
-@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
-     }
+             goto out;
+         }
-     acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
+         if (!temporary) {
--    acb->aio_done_func = sd_write_done;
+-            if (qemu_vfio_find_fixed_iova(s, size, &iova0)) {
-     acb->aiocb_type = AIOCB_WRITE_UDATA;
++            if (!qemu_vfio_find_fixed_iova(s, size, &iova0, errp)) {
+                 ret = -ENOMEM;
- retry:
+                 goto out;
-@@ -XXX,XX +XXX,XX @@ retry:
+             }
-         goto retry;
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
-     }
+             }
+             qemu_vfio_dump_mappings(s);
--    ret = sd_co_rw_vector(acb);
+         } else {
--    if (ret <= 0) {
+-            if (qemu_vfio_find_temp_iova(s, size, &iova0)) {
--        QLIST_REMOVE(acb, aiocb_siblings);
++            if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
--        qemu_co_queue_restart_all(&s->overlapping_queue);
+                 ret = -ENOMEM;
--        qemu_aio_unref(acb);
+                 goto out;
--        return ret;
+             }
 -    }
 -
 -    qemu_coroutine_yield();
 +    sd_co_rw_vector(acb);
 +    sd_write_done(acb);
      QLIST_REMOVE(acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -
 -    return acb->ret;
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
      acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
      acb->aiocb_type = AIOCB_READ_UDATA;
 -    acb->aio_done_func = sd_finish_aiocb;
  retry:
      if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
          goto retry;
      }
 -    ret = sd_co_rw_vector(acb);
 -    if (ret <= 0) {
 -        QLIST_REMOVE(acb, aiocb_siblings);
 -        qemu_co_queue_restart_all(&s->overlapping_queue);
 -        qemu_aio_unref(acb);
 -        return ret;
 -    }
 -
 -    qemu_coroutine_yield();
 +    sd_co_rw_vector(acb);
      QLIST_REMOVE(acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    return acb->ret;
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
  {
      BDRVSheepdogState *s = bs->opaque;
      SheepdogAIOCB *acb;
 +    int ret;
      AIOReq *aio_req;
      if (s->cache_flags != SD_FLAG_CMD_CACHE) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
      acb = sd_aio_setup(bs, NULL, 0, 0);
      acb->aiocb_type = AIOCB_FLUSH_CACHE;
 -    acb->aio_done_func = sd_finish_aiocb;
 +    acb->nr_pending++;
      aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
 , 0, 0, false, 0, 0);
      QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
      add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
 -    qemu_coroutine_yield();
 -    return acb->ret;
 +    if (--acb->nr_pending) {
 +        qemu_coroutine_yield();
 +    }
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
      acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
                         count >> BDRV_SECTOR_BITS);
      acb->aiocb_type = AIOCB_DISCARD_OBJ;
 -    acb->aio_done_func = sd_finish_aiocb;
  retry:
      if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
          goto retry;
      }
 -    ret = sd_co_rw_vector(acb);
 -    if (ret <= 0) {
 -        QLIST_REMOVE(acb, aiocb_siblings);
 -        qemu_co_queue_restart_all(&s->overlapping_queue);
 -        qemu_aio_unref(acb);
 -        return ret;
 -    }
 -
 -    qemu_coroutine_yield();
 +    sd_co_rw_vector(acb);
      QLIST_REMOVE(acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -
 -    return acb->ret;
 +    ret = acb->ret;
 +    qemu_aio_unref(acb);
 +    return ret;
  }
  static coroutine_fn int64_t
 --
-.9.3
+.31.1

-[Qemu-devel] [PULL 3/5] sheepdog: do not use BlockAIOCB
+[PULL 09/11] util/vfio-helpers: Simplify qemu_vfio_dma_map() returning directly
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-Sheepdog's AIOCB are completely internal entities for a group of
+To simplify qemu_vfio_dma_map():
-requests and do not need dynamic allocation.
+- reduce 'ret' (returned value) scope by returning errno directly,
 - remove the goto 'out' label.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
-Message-id: 20161129113245.32724-4-pbonzini@redhat.com
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Message-id: 20210902070025.197072-10-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 99 ++++++++++++++++++++++----------------------------------
+ util/vfio-helpers.c | 23 ++++++++++-------------
-file changed, 39 insertions(+), 60 deletions(-)
+file changed, 10 insertions(+), 13 deletions(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/util/vfio-helpers.c
-+++ b/block/sheepdog.c
++++ b/util/vfio-helpers.c
-@@ -XXX,XX +XXX,XX @@ static inline size_t count_data_objs(const struct SheepdogInode *inode)
+@@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_water_mark_reached(QEMUVFIOState *s, size_t size,
-     } while (0)
+ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
+                       bool temporary, uint64_t *iova, Error **errp)
  typedef struct SheepdogAIOCB SheepdogAIOCB;
 +typedef struct BDRVSheepdogState BDRVSheepdogState;
  typedef struct AIOReq {
      SheepdogAIOCB *aiocb;
@@ -XXX,XX +XXX,XX @@ enum AIOCBState {
         || y->max_affect_data_idx < x->min_affect_data_idx))
  struct SheepdogAIOCB {
 -    BlockAIOCB common;
 +    BDRVSheepdogState *s;
      QEMUIOVector *qiov;
@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
      QLIST_ENTRY(SheepdogAIOCB) aiocb_siblings;
  };
 -typedef struct BDRVSheepdogState {
 +struct BDRVSheepdogState {
      BlockDriverState *bs;
      AioContext *aio_context;
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVSheepdogState {
      CoQueue overlapping_queue;
      QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head;
 -} BDRVSheepdogState;
 +};
  typedef struct BDRVSheepdogReopenState {
      int fd;
@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
      acb->nr_pending--;
  }
 -static const AIOCBInfo sd_aiocb_info = {
 -    .aiocb_size     = sizeof(SheepdogAIOCB),
 -};
 -
 -static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
 -                                   int64_t sector_num, int nb_sectors)
 +static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
 +                         QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
 +                         int type)
  {
--    SheepdogAIOCB *acb;
+-    int ret = 0;
-     uint32_t object_size;
+     int index;
--    BDRVSheepdogState *s = bs->opaque;
+     IOVAMapping *mapping;
+     uint64_t iova0;
-     object_size = (UINT32_C(1) << s->inode.block_size_shift);
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
+     if (mapping) {
--    acb = qemu_aio_get(&sd_aiocb_info, bs, NULL, NULL);
+         iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);
-+    acb->s = s;
+     } else {
++        int ret;
-     acb->qiov = qiov;
++
+         if (qemu_vfio_water_mark_reached(s, size, errp)) {
-@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
+-            ret = -ENOMEM;
+-            goto out;
-     acb->min_dirty_data_idx = UINT32_MAX;
++            return -ENOMEM;
-     acb->max_dirty_data_idx = 0;
+         }
--
+         if (!temporary) {
--    return acb;
+             if (!qemu_vfio_find_fixed_iova(s, size, &iova0, errp)) {
-+    acb->aiocb_type = type;
+-                ret = -ENOMEM;
- }
+-                goto out;
++                return -ENOMEM;
- /* Return -EIO in case of error, file descriptor on success */
+             }
-@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
-  */
+             mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0);
- static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
+             assert(qemu_vfio_verify_mappings(s));
- {
+             ret = qemu_vfio_do_mapping(s, host, size, iova0);
--    BDRVSheepdogState *s = acb->common.bs->opaque;
+-            if (ret) {
-+    BDRVSheepdogState *s = acb->s;
++            if (ret < 0) {
-     struct iovec iov;
+                 qemu_vfio_undo_mapping(s, mapping, NULL);
-     AIOReq *aio_req;
+-                goto out;
-     uint32_t offset, data_len, mn, mx;
++                return ret;
-@@ -XXX,XX +XXX,XX @@ out:
+             }
-  * Returns 1 when we need to wait a response, 0 when there is no sent
+             qemu_vfio_dump_mappings(s);
-  * request and -errno in error cases.
+         } else {
-  */
+             if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
--static void coroutine_fn sd_co_rw_vector(void *p)
+-                ret = -ENOMEM;
-+static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
+-                goto out;
- {
++                return -ENOMEM;
--    SheepdogAIOCB *acb = p;
+             }
-     int ret = 0;
+             ret = qemu_vfio_do_mapping(s, host, size, iova0);
-     unsigned long len, done = 0, total = acb->nb_sectors * BDRV_SECTOR_SIZE;
+-            if (ret) {
-     unsigned long idx;
+-                goto out;
-     uint32_t object_size;
++            if (ret < 0) {
-     uint64_t oid;
++                return ret;
-     uint64_t offset;
+             }
 -    BDRVSheepdogState *s = acb->common.bs->opaque;
 +    BDRVSheepdogState *s = acb->s;
      SheepdogInode *inode = &s->inode;
      AIOReq *aio_req;
@@ -XXX,XX +XXX,XX @@ static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
  static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
                          int nb_sectors, QEMUIOVector *qiov)
  {
 -    SheepdogAIOCB *acb;
 +    SheepdogAIOCB acb;
      int ret;
      int64_t offset = (sector_num + nb_sectors) * BDRV_SECTOR_SIZE;
      BDRVSheepdogState *s = bs->opaque;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
          }
      }
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
--    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
+     if (iova) {
--    acb->aiocb_type = AIOCB_WRITE_UDATA;
+         *iova = iova0;
 +    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
  retry:
 -    if (check_overlapping_aiocb(s, acb)) {
 +    if (check_overlapping_aiocb(s, &acb)) {
          qemu_co_queue_wait(&s->overlapping_queue);
          goto retry;
      }
+-out:
 -    sd_co_rw_vector(acb);
 -    sd_write_done(acb);
 +    sd_co_rw_vector(&acb);
 +    sd_write_done(&acb);
 -    QLIST_REMOVE(acb, aiocb_siblings);
 +    QLIST_REMOVE(&acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
-+    return acb.ret;
++    return 0;
  }
- static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
+ /* Reset the high watermark and free all "temporary" mappings. */
                         int nb_sectors, QEMUIOVector *qiov)
  {
 -    SheepdogAIOCB *acb;
 -    int ret;
 +    SheepdogAIOCB acb;
      BDRVSheepdogState *s = bs->opaque;
 -    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
 -    acb->aiocb_type = AIOCB_READ_UDATA;
 +    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
  retry:
 -    if (check_overlapping_aiocb(s, acb)) {
 +    if (check_overlapping_aiocb(s, &acb)) {
          qemu_co_queue_wait(&s->overlapping_queue);
          goto retry;
      }
 -    sd_co_rw_vector(acb);
 +    sd_co_rw_vector(&acb);
 -    QLIST_REMOVE(acb, aiocb_siblings);
 +    QLIST_REMOVE(&acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
 +    return acb.ret;
  }
  static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
  {
      BDRVSheepdogState *s = bs->opaque;
 -    SheepdogAIOCB *acb;
 -    int ret;
 +    SheepdogAIOCB acb;
      AIOReq *aio_req;
      if (s->cache_flags != SD_FLAG_CMD_CACHE) {
          return 0;
      }
 -    acb = sd_aio_setup(bs, NULL, 0, 0);
 -    acb->aiocb_type = AIOCB_FLUSH_CACHE;
 +    sd_aio_setup(&acb, s, NULL, 0, 0, AIOCB_FLUSH_CACHE);
 -    acb->nr_pending++;
 -    aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
 +    acb.nr_pending++;
 +    aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
 , 0, 0, false, 0, 0);
      QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
 -    add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
 +    add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
 -    if (--acb->nr_pending) {
 +    if (--acb.nr_pending) {
          qemu_coroutine_yield();
      }
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
 +    return acb.ret;
  }
  static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
  static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
                                        int count)
  {
 -    SheepdogAIOCB *acb;
 +    SheepdogAIOCB acb;
      BDRVSheepdogState *s = bs->opaque;
 -    int ret;
      QEMUIOVector discard_iov;
      struct iovec iov;
      uint32_t zero = 0;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
      if (!QEMU_IS_ALIGNED(offset | count, BDRV_SECTOR_SIZE)) {
          return -ENOTSUP;
      }
 -    acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
 -                       count >> BDRV_SECTOR_BITS);
 -    acb->aiocb_type = AIOCB_DISCARD_OBJ;
 +    sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
 +                 count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
  retry:
 -    if (check_overlapping_aiocb(s, acb)) {
 +    if (check_overlapping_aiocb(s, &acb)) {
          qemu_co_queue_wait(&s->overlapping_queue);
          goto retry;
      }
 -    sd_co_rw_vector(acb);
 +    sd_co_rw_vector(&acb);
 -    QLIST_REMOVE(acb, aiocb_siblings);
 +    QLIST_REMOVE(&acb, aiocb_siblings);
      qemu_co_queue_restart_all(&s->overlapping_queue);
 -    ret = acb->ret;
 -    qemu_aio_unref(acb);
 -    return ret;
 +    return acb.ret;
  }
  static coroutine_fn int64_t
 --
-.9.3
+.31.1

-New patch
+[PULL 10/11] util/vfio-helpers: Let qemu_vfio_do_mapping() propagate Error
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
+Pass qemu_vfio_do_mapping() an Error* argument so it can propagate
+any error to callers. Replace error_report() which only report
+to the monitor by the more generic error_setg_errno().
+Reviewed-by: Fam Zheng <fam@euphon.net>
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210902070025.197072-11-philmd@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ util/vfio-helpers.c | 8 ++++----
+file changed, 4 insertions(+), 4 deletions(-)
+diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/vfio-helpers.c
++++ b/util/vfio-helpers.c
+@@ -XXX,XX +XXX,XX @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState *s,
+ /* Do the DMA mapping with VFIO. */
+ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size,
+-                                uint64_t iova)
++                                uint64_t iova, Error **errp)
+ {
+     struct vfio_iommu_type1_dma_map dma_map = {
+         .argsz = sizeof(dma_map),
+@@ -XXX,XX +XXX,XX @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size,
+     trace_qemu_vfio_do_mapping(s, host, iova, size);
+     if (ioctl(s->container, VFIO_IOMMU_MAP_DMA, &dma_map)) {
+-        error_report("VFIO_MAP_DMA failed: %s", strerror(errno));
++        error_setg_errno(errp, errno, "VFIO_MAP_DMA failed");
+         return -errno;
+     }
+     return 0;
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
+             mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0);
+             assert(qemu_vfio_verify_mappings(s));
+-            ret = qemu_vfio_do_mapping(s, host, size, iova0);
++            ret = qemu_vfio_do_mapping(s, host, size, iova0, errp);
+             if (ret < 0) {
+                 qemu_vfio_undo_mapping(s, mapping, NULL);
+                 return ret;
+@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
+             if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
+                 return -ENOMEM;
+             }
+-            ret = qemu_vfio_do_mapping(s, host, size, iova0);
++            ret = qemu_vfio_do_mapping(s, host, size, iova0, errp);
+             if (ret < 0) {
+                 return ret;
+             }
+--
+.31.1

-[Qemu-devel] [PULL 1/5] sheepdog: remove unused cancellation support
+[PULL 11/11] block/nvme: Only report VFIO error on failed retry
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-SheepdogAIOCB is internal to sheepdog.c, hence it is never canceled.
+We expect the first qemu_vfio_dma_map() to fail (indicating
 DMA mappings exhaustion, see commit 15a730e7a3a). Do not
 report the first failure as error, since we are going to
 flush the mappings and retry.
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+This removes spurious error message displayed on the monitor:
-Message-id: 20161129113245.32724-2-pbonzini@redhat.com
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+  (qemu) c
   (qemu) qemu-kvm: VFIO_MAP_DMA failed: No space left on device
   (qemu) info status
   VM status: running
 Reported-by: Tingting Mao <timao@redhat.com>
 Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20210902070025.197072-12-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/sheepdog.c | 52 ----------------------------------------------------
+ block/nvme.c | 8 +++++++-
-file changed, 52 deletions(-)
+file changed, 7 insertions(+), 1 deletion(-)
-diff --git a/block/sheepdog.c b/block/sheepdog.c
+diff --git a/block/nvme.c b/block/nvme.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/sheepdog.c
+--- a/block/nvme.c
-+++ b/block/sheepdog.c
++++ b/block/nvme.c
-@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd,
-     Coroutine *coroutine;
+     uint64_t *pagelist = req->prp_list_page;
-     void (*aio_done_func)(SheepdogAIOCB *);
+     int i, j, r;
+     int entries = 0;
--    bool cancelable;
++    Error *local_err = NULL, **errp = NULL;
-     int nr_pending;
+     assert(qiov->size);
-     uint32_t min_affect_data_idx;
+     assert(QEMU_IS_ALIGNED(qiov->size, s->page_size));
-@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
+@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd,
- {
+ try_map:
-     SheepdogAIOCB *acb = aio_req->aiocb;
+         r = qemu_vfio_dma_map(s->vfio,
+                               qiov->iov[i].iov_base,
--    acb->cancelable = false;
+-                              len, true, &iova, NULL);
-     QLIST_REMOVE(aio_req, aio_siblings);
++                              len, true, &iova, errp);
-     g_free(aio_req);
+         if (r == -ENOSPC) {
+             /*
-@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
+              * In addition to the -ENOMEM error, the VFIO_IOMMU_MAP_DMA
-     qemu_aio_unref(acb);
+@@ -XXX,XX +XXX,XX @@ try_map:
                      goto fail;
                  }
              }
 +            errp = &local_err;
 +
              goto try_map;
          }
          if (r) {
@@ -XXX,XX +XXX,XX @@ fail:
       * because they are already mapped before calling this function; for
       * temporary mappings, a later nvme_cmd_(un)map_qiov will reclaim by
       * calling qemu_vfio_dma_reset_temporary when necessary. */
 +    if (local_err) {
 +        error_reportf_err(local_err, "Cannot map buffer for DMA: ");
 +    }
      return r;
  }
--/*
-- * Check whether the specified acb can be canceled
-- *
-- * We can cancel aio when any request belonging to the acb is:
-- *  - Not processed by the sheepdog server.
-- *  - Not linked to the inflight queue.
-- */
--static bool sd_acb_cancelable(const SheepdogAIOCB *acb)
--{
--    BDRVSheepdogState *s = acb->common.bs->opaque;
--    AIOReq *aioreq;
--
--    if (!acb->cancelable) {
--        return false;
--    }
--
--    QLIST_FOREACH(aioreq, &s->inflight_aio_head, aio_siblings) {
--        if (aioreq->aiocb == acb) {
--            return false;
--        }
--    }
--
--    return true;
--}
--
--static void sd_aio_cancel(BlockAIOCB *blockacb)
--{
--    SheepdogAIOCB *acb = (SheepdogAIOCB *)blockacb;
--    BDRVSheepdogState *s = acb->common.bs->opaque;
--    AIOReq *aioreq, *next;
--
--    if (sd_acb_cancelable(acb)) {
--        /* Remove outstanding requests from failed queue.  */
--        QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings,
--                           next) {
--            if (aioreq->aiocb == acb) {
--                free_aio_req(s, aioreq);
--            }
--        }
--
--        assert(acb->nr_pending == 0);
--        if (acb->common.cb) {
--            acb->common.cb(acb->common.opaque, -ECANCELED);
--        }
--        sd_finish_aiocb(acb);
--    }
--}
--
- static const AIOCBInfo sd_aiocb_info = {
-     .aiocb_size     = sizeof(SheepdogAIOCB),
--    .cancel_async   = sd_aio_cancel,
- };
- static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
-@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
-     acb->nb_sectors = nb_sectors;
-     acb->aio_done_func = NULL;
--    acb->cancelable = true;
-     acb->coroutine = qemu_coroutine_self();
-     acb->ret = 0;
-     acb->nr_pending = 0;
 --
-.9.3
+.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

SheepdogAIOCB is internal to sheepdog.c, hence it is never canceled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-2-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 52 ----------------------------------------------------
 1 file changed, 52 deletions(-)

From: Paolo Bonzini <pbonzini@redhat.com>

Delimit co_recv's lifetime clearly in aio_read_response.

Do a simple qemu_coroutine_enter in aio_read_response, letting
sd_co_writev call sd_write_done.

Handle nr_pending in the same way in sd_co_rw_vector,
sd_write_done and sd_co_flush_to_disk.

Remove sd_co_rw_vector's return value; just leave with no
pending requests.

[Jeff: added missing 'return' back, spotted by Paolo after
       series was applied.]

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 115 ++++++++++++++++++++-----------------------------------
 1 file changed, 42 insertions(+), 73 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
     enum AIOCBState aiocb_type;
 
     Coroutine *coroutine;
-    void (*aio_done_func)(SheepdogAIOCB *);
-
     int nr_pending;
 
     uint32_t min_affect_data_idx;
@@ -XXX,XX +XXX,XX @@ static const char * sd_strerror(int err)
  *
  * 1. In sd_co_rw_vector, we send the I/O requests to the server and
  *    link the requests to the inflight_list in the
- *    BDRVSheepdogState.  The function exits without waiting for
+ *    BDRVSheepdogState.  The function yields while waiting for
  *    receiving the response.
  *
  * 2. We receive the response in aio_read_response, the fd handler to
- *    the sheepdog connection.  If metadata update is needed, we send
- *    the write request to the vdi object in sd_write_done, the write
- *    completion function.  We switch back to sd_co_readv/writev after
- *    all the requests belonging to the AIOCB are finished.
+ *    the sheepdog connection.  We switch back to sd_co_readv/sd_writev
+ *    after all the requests belonging to the AIOCB are finished.  If
+ *    needed, sd_co_writev will send another requests for the vdi object.
  */
 
 static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
     acb->nr_pending--;
 }
 
-static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
-{
-    qemu_coroutine_enter(acb->coroutine);
-    qemu_aio_unref(acb);
-}
-
 static const AIOCBInfo sd_aiocb_info = {
     .aiocb_size     = sizeof(SheepdogAIOCB),
 };
@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
     acb->sector_num = sector_num;
     acb->nb_sectors = nb_sectors;
 
-    acb->aio_done_func = NULL;
     acb->coroutine = qemu_coroutine_self();
     acb->ret = 0;
     acb->nr_pending = 0;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
 
     switch (acb->aiocb_type) {
     case AIOCB_WRITE_UDATA:
-        /* this coroutine context is no longer suitable for co_recv
-         * because we may send data to update vdi objects */
-        s->co_recv = NULL;
         if (!is_data_obj(aio_req->oid)) {
             break;
         }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
         }
     }
 
+    /* No more data for this aio_req (reload_inode below uses its own file
+     * descriptor handler which doesn't use co_recv).
+    */
+    s->co_recv = NULL;
+
     switch (rsp.result) {
     case SD_RES_SUCCESS:
         break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
             aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
         }
         resend_aioreq(s, aio_req);
-        goto out;
+        return;
     default:
         acb->ret = -EIO;
         error_report("%s", sd_strerror(rsp.result));
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
          * We've finished all requests which belong to the AIOCB, so
          * we can switch back to sd_co_readv/writev now.
          */
-        acb->aio_done_func(acb);
+        qemu_coroutine_enter(acb->coroutine);
     }
-out:
-    s->co_recv = NULL;
+
     return;
+
 err:
-    s->co_recv = NULL;
     reconnect_to_sdog(opaque);
 }
 
@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
 /*
  * This function is called after writing data objects.  If we need to
  * update metadata, this sends a write request to the vdi object.
- * Otherwise, this switches back to sd_co_readv/writev.
  */
 static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
 {
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
     mx = acb->max_dirty_data_idx;
     if (mn <= mx) {
         /* we need to update the vdi object. */
+        ++acb->nr_pending;
         offset = sizeof(s->inode) - sizeof(s->inode.data_vdi_id) +
             mn * sizeof(s->inode.data_vdi_id[0]);
         data_len = (mx - mn + 1) * sizeof(s->inode.data_vdi_id[0]);
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
                                 data_len, offset, 0, false, 0, offset);
         QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
         add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
-
-        acb->aio_done_func = sd_finish_aiocb;
-        acb->aiocb_type = AIOCB_WRITE_UDATA;
-        return;
+        if (--acb->nr_pending) {
+            qemu_coroutine_yield();
+        }
     }
-
-    sd_finish_aiocb(acb);
 }
 
 /* Delete current working VDI on the snapshot chain */
@@ -XXX,XX +XXX,XX @@ out:
  * Returns 1 when we need to wait a response, 0 when there is no sent
  * request and -errno in error cases.
  */
-static int coroutine_fn sd_co_rw_vector(void *p)
+static void coroutine_fn sd_co_rw_vector(void *p)
 {
     SheepdogAIOCB *acb = p;
     int ret = 0;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
         ret = sd_create_branch(s);
         if (ret) {
             acb->ret = -EIO;
-            goto out;
+            return;
         }
     }
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_rw_vector(void *p)
         idx++;
         done += len;
     }
-out:
-    if (!--acb->nr_pending) {
-        return acb->ret;
+    if (--acb->nr_pending) {
+        qemu_coroutine_yield();
     }
-    return 1;
 }
 
 static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
     }
 
     acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
-    acb->aio_done_func = sd_write_done;
     acb->aiocb_type = AIOCB_WRITE_UDATA;
 
 retry:
@@ -XXX,XX +XXX,XX @@ retry:
         goto retry;
     }
 
-    ret = sd_co_rw_vector(acb);
-    if (ret <= 0) {
-        QLIST_REMOVE(acb, aiocb_siblings);
-        qemu_co_queue_restart_all(&s->overlapping_queue);
-        qemu_aio_unref(acb);
-        return ret;
-    }
-
-    qemu_coroutine_yield();
+    sd_co_rw_vector(acb);
+    sd_write_done(acb);
 
     QLIST_REMOVE(acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-
-    return acb->ret;
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
 
     acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
     acb->aiocb_type = AIOCB_READ_UDATA;
-    acb->aio_done_func = sd_finish_aiocb;
 
 retry:
     if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
         goto retry;
     }
 
-    ret = sd_co_rw_vector(acb);
-    if (ret <= 0) {
-        QLIST_REMOVE(acb, aiocb_siblings);
-        qemu_co_queue_restart_all(&s->overlapping_queue);
-        qemu_aio_unref(acb);
-        return ret;
-    }
-
-    qemu_coroutine_yield();
+    sd_co_rw_vector(acb);
 
     QLIST_REMOVE(acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    return acb->ret;
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 {
     BDRVSheepdogState *s = bs->opaque;
     SheepdogAIOCB *acb;
+    int ret;
     AIOReq *aio_req;
 
     if (s->cache_flags != SD_FLAG_CMD_CACHE) {
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 
     acb = sd_aio_setup(bs, NULL, 0, 0);
     acb->aiocb_type = AIOCB_FLUSH_CACHE;
-    acb->aio_done_func = sd_finish_aiocb;
 
+    acb->nr_pending++;
     aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                             0, 0, 0, false, 0, 0);
     QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
     add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
 
-    qemu_coroutine_yield();
-    return acb->ret;
+    if (--acb->nr_pending) {
+        qemu_coroutine_yield();
+    }
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
     acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
                        count >> BDRV_SECTOR_BITS);
     acb->aiocb_type = AIOCB_DISCARD_OBJ;
-    acb->aio_done_func = sd_finish_aiocb;
 
 retry:
     if (check_overlapping_aiocb(s, acb)) {
@@ -XXX,XX +XXX,XX @@ retry:
         goto retry;
     }
 
-    ret = sd_co_rw_vector(acb);
-    if (ret <= 0) {
-        QLIST_REMOVE(acb, aiocb_siblings);
-        qemu_co_queue_restart_all(&s->overlapping_queue);
-        qemu_aio_unref(acb);
-        return ret;
-    }
-
-    qemu_coroutine_yield();
+    sd_co_rw_vector(acb);
 
     QLIST_REMOVE(acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-
-    return acb->ret;
+    ret = acb->ret;
+    qemu_aio_unref(acb);
+    return ret;
 }
 
 static coroutine_fn int64_t
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Sheepdog's AIOCB are completely internal entities for a group of
requests and do not need dynamic allocation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-4-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 99 ++++++++++++++++++++++----------------------------------
 1 file changed, 39 insertions(+), 60 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static inline size_t count_data_objs(const struct SheepdogInode *inode)
     } while (0)
 
 typedef struct SheepdogAIOCB SheepdogAIOCB;
+typedef struct BDRVSheepdogState BDRVSheepdogState;
 
 typedef struct AIOReq {
     SheepdogAIOCB *aiocb;
@@ -XXX,XX +XXX,XX @@ enum AIOCBState {
        || y->max_affect_data_idx < x->min_affect_data_idx))
 
 struct SheepdogAIOCB {
-    BlockAIOCB common;
+    BDRVSheepdogState *s;
 
     QEMUIOVector *qiov;
 
@@ -XXX,XX +XXX,XX @@ struct SheepdogAIOCB {
     QLIST_ENTRY(SheepdogAIOCB) aiocb_siblings;
 };
 
-typedef struct BDRVSheepdogState {
+struct BDRVSheepdogState {
     BlockDriverState *bs;
     AioContext *aio_context;
 
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVSheepdogState {
 
     CoQueue overlapping_queue;
     QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head;
-} BDRVSheepdogState;
+};
 
 typedef struct BDRVSheepdogReopenState {
     int fd;
@@ -XXX,XX +XXX,XX @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
     acb->nr_pending--;
 }
 
-static const AIOCBInfo sd_aiocb_info = {
-    .aiocb_size     = sizeof(SheepdogAIOCB),
-};
-
-static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
-                                   int64_t sector_num, int nb_sectors)
+static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
+                         QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
+                         int type)
 {
-    SheepdogAIOCB *acb;
     uint32_t object_size;
-    BDRVSheepdogState *s = bs->opaque;
 
     object_size = (UINT32_C(1) << s->inode.block_size_shift);
 
-    acb = qemu_aio_get(&sd_aiocb_info, bs, NULL, NULL);
+    acb->s = s;
 
     acb->qiov = qiov;
 
@@ -XXX,XX +XXX,XX @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
 
     acb->min_dirty_data_idx = UINT32_MAX;
     acb->max_dirty_data_idx = 0;
-
-    return acb;
+    acb->aiocb_type = type;
 }
 
 /* Return -EIO in case of error, file descriptor on success */
@@ -XXX,XX +XXX,XX @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
  */
 static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
 {
-    BDRVSheepdogState *s = acb->common.bs->opaque;
+    BDRVSheepdogState *s = acb->s;
     struct iovec iov;
     AIOReq *aio_req;
     uint32_t offset, data_len, mn, mx;
@@ -XXX,XX +XXX,XX @@ out:
  * Returns 1 when we need to wait a response, 0 when there is no sent
  * request and -errno in error cases.
  */
-static void coroutine_fn sd_co_rw_vector(void *p)
+static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
 {
-    SheepdogAIOCB *acb = p;
     int ret = 0;
     unsigned long len, done = 0, total = acb->nb_sectors * BDRV_SECTOR_SIZE;
     unsigned long idx;
     uint32_t object_size;
     uint64_t oid;
     uint64_t offset;
-    BDRVSheepdogState *s = acb->common.bs->opaque;
+    BDRVSheepdogState *s = acb->s;
     SheepdogInode *inode = &s->inode;
     AIOReq *aio_req;
 
@@ -XXX,XX +XXX,XX @@ static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
 static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
                         int nb_sectors, QEMUIOVector *qiov)
 {
-    SheepdogAIOCB *acb;
+    SheepdogAIOCB acb;
     int ret;
     int64_t offset = (sector_num + nb_sectors) * BDRV_SECTOR_SIZE;
     BDRVSheepdogState *s = bs->opaque;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
         }
     }
 
-    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
-    acb->aiocb_type = AIOCB_WRITE_UDATA;
+    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
 
 retry:
-    if (check_overlapping_aiocb(s, acb)) {
+    if (check_overlapping_aiocb(s, &acb)) {
         qemu_co_queue_wait(&s->overlapping_queue);
         goto retry;
     }
 
-    sd_co_rw_vector(acb);
-    sd_write_done(acb);
+    sd_co_rw_vector(&acb);
+    sd_write_done(&acb);
 
-    QLIST_REMOVE(acb, aiocb_siblings);
+    QLIST_REMOVE(&acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
                        int nb_sectors, QEMUIOVector *qiov)
 {
-    SheepdogAIOCB *acb;
-    int ret;
+    SheepdogAIOCB acb;
     BDRVSheepdogState *s = bs->opaque;
 
-    acb = sd_aio_setup(bs, qiov, sector_num, nb_sectors);
-    acb->aiocb_type = AIOCB_READ_UDATA;
+    sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
 
 retry:
-    if (check_overlapping_aiocb(s, acb)) {
+    if (check_overlapping_aiocb(s, &acb)) {
         qemu_co_queue_wait(&s->overlapping_queue);
         goto retry;
     }
 
-    sd_co_rw_vector(acb);
+    sd_co_rw_vector(&acb);
 
-    QLIST_REMOVE(acb, aiocb_siblings);
+    QLIST_REMOVE(&acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
 {
     BDRVSheepdogState *s = bs->opaque;
-    SheepdogAIOCB *acb;
-    int ret;
+    SheepdogAIOCB acb;
     AIOReq *aio_req;
 
     if (s->cache_flags != SD_FLAG_CMD_CACHE) {
         return 0;
     }
 
-    acb = sd_aio_setup(bs, NULL, 0, 0);
-    acb->aiocb_type = AIOCB_FLUSH_CACHE;
+    sd_aio_setup(&acb, s, NULL, 0, 0, AIOCB_FLUSH_CACHE);
 
-    acb->nr_pending++;
-    aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
+    acb.nr_pending++;
+    aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
                             0, 0, 0, false, 0, 0);
     QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
-    add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
+    add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
 
-    if (--acb->nr_pending) {
+    if (--acb.nr_pending) {
         qemu_coroutine_yield();
     }
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
@@ -XXX,XX +XXX,XX @@ static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
 static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
                                       int count)
 {
-    SheepdogAIOCB *acb;
+    SheepdogAIOCB acb;
     BDRVSheepdogState *s = bs->opaque;
-    int ret;
     QEMUIOVector discard_iov;
     struct iovec iov;
     uint32_t zero = 0;
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
     if (!QEMU_IS_ALIGNED(offset | count, BDRV_SECTOR_SIZE)) {
         return -ENOTSUP;
     }
-    acb = sd_aio_setup(bs, &discard_iov, offset >> BDRV_SECTOR_BITS,
-                       count >> BDRV_SECTOR_BITS);
-    acb->aiocb_type = AIOCB_DISCARD_OBJ;
+    sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
+                 count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
 
 retry:
-    if (check_overlapping_aiocb(s, acb)) {
+    if (check_overlapping_aiocb(s, &acb)) {
         qemu_co_queue_wait(&s->overlapping_queue);
         goto retry;
     }
 
-    sd_co_rw_vector(acb);
+    sd_co_rw_vector(&acb);
 
-    QLIST_REMOVE(acb, aiocb_siblings);
+    QLIST_REMOVE(&acb, aiocb_siblings);
     qemu_co_queue_restart_all(&s->overlapping_queue);
-    ret = acb->ret;
-    qemu_aio_unref(acb);
-    return ret;
+    return acb.ret;
 }
 
 static coroutine_fn int64_t
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Add to the list in add_aio_request and, indirectly, resend_aioreq.  Inline
free_aio_req in the caller, it does not simply undo alloc_aio_req's job.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-5-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
     return aio_req;
 }
 
-static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
-{
-    SheepdogAIOCB *acb = aio_req->aiocb;
-
-    QLIST_REMOVE(aio_req, aio_siblings);
-    g_free(aio_req);
-
-    acb->nr_pending--;
-}
-
 static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
                          int type)
@@ -XXX,XX +XXX,XX @@ static coroutine_fn void reconnect_to_sdog(void *opaque)
     while (!QLIST_EMPTY(&s->failed_aio_head)) {
         aio_req = QLIST_FIRST(&s->failed_aio_head);
         QLIST_REMOVE(aio_req, aio_siblings);
-        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
         resend_aioreq(s, aio_req);
     }
 }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
     */
     s->co_recv = NULL;
 
+    QLIST_REMOVE(aio_req, aio_siblings);
     switch (rsp.result) {
     case SD_RES_SUCCESS:
         break;
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn aio_read_response(void *opaque)
         break;
     }
 
-    free_aio_req(s, aio_req);
-    if (!acb->nr_pending) {
+    g_free(aio_req);
+
+    if (!--acb->nr_pending) {
         /*
          * We've finished all requests which belong to the AIOCB, so
          * we can switch back to sd_co_readv/writev now.
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
     uint64_t old_oid = aio_req->base_oid;
     bool create = aio_req->create;
 
+    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
+
     if (!nr_copies) {
         error_report("bug");
     }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
         iov.iov_len = sizeof(s->inode);
         aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                                 data_len, offset, 0, false, 0, offset);
-        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
         add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
         if (--acb->nr_pending) {
             qemu_coroutine_yield();
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
                                 old_oid,
                                 acb->aiocb_type == AIOCB_DISCARD_OBJ ?
                                 0 : done);
-        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
-
         add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
                         acb->aiocb_type);
     done:
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
     acb.nr_pending++;
     aio_req = alloc_aio_req(s, &acb, vid_to_vdi_oid(s->inode.vdi_id),
                             0, 0, 0, false, 0, 0);
-    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
     add_aio_request(s, aio_req, NULL, 0, acb.aiocb_type);
 
     if (--acb.nr_pending) {
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Wrap the code that was copied repeatedly in the two functions,
sd_aio_setup and sd_aio_complete.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161129113245.32724-6-pbonzini@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/sheepdog.c | 66 ++++++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 36 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index XXXXXXX..XXXXXXX 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -XXX,XX +XXX,XX @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
     return aio_req;
 }
 
+static void wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
+{
+    SheepdogAIOCB *cb;
+
+retry:
+    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
+        if (AIOCBOverlapping(acb, cb)) {
+            qemu_co_queue_wait(&s->overlapping_queue);
+            goto retry;
+        }
+    }
+}
+
 static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
                          int type)
@@ -XXX,XX +XXX,XX @@ static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
     acb->min_dirty_data_idx = UINT32_MAX;
     acb->max_dirty_data_idx = 0;
     acb->aiocb_type = type;
+
+    if (type == AIOCB_FLUSH_CACHE) {
+        return;
+    }
+
+    wait_for_overlapping_aiocb(s, acb);
+    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, acb, aiocb_siblings);
 }
 
 /* Return -EIO in case of error, file descriptor on success */
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
     }
 }
 
-static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb)
+static void sd_aio_complete(SheepdogAIOCB *acb)
 {
-    SheepdogAIOCB *cb;
-
-    QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) {
-        if (AIOCBOverlapping(aiocb, cb)) {
-            return true;
-        }
+    if (acb->aiocb_type == AIOCB_FLUSH_CACHE) {
+        return;
     }
 
-    QLIST_INSERT_HEAD(&s->inflight_aiocb_head, aiocb, aiocb_siblings);
-    return false;
+    QLIST_REMOVE(acb, aiocb_siblings);
+    qemu_co_queue_restart_all(&acb->s->overlapping_queue);
 }
 
 static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
     }
 
     sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_WRITE_UDATA);
-
-retry:
-    if (check_overlapping_aiocb(s, &acb)) {
-        qemu_co_queue_wait(&s->overlapping_queue);
-        goto retry;
-    }
-
     sd_co_rw_vector(&acb);
     sd_write_done(&acb);
+    sd_aio_complete(&acb);
 
-    QLIST_REMOVE(&acb, aiocb_siblings);
-    qemu_co_queue_restart_all(&s->overlapping_queue);
     return acb.ret;
 }
 
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
     BDRVSheepdogState *s = bs->opaque;
 
     sd_aio_setup(&acb, s, qiov, sector_num, nb_sectors, AIOCB_READ_UDATA);
-
-retry:
-    if (check_overlapping_aiocb(s, &acb)) {
-        qemu_co_queue_wait(&s->overlapping_queue);
-        goto retry;
-    }
-
     sd_co_rw_vector(&acb);
+    sd_aio_complete(&acb);
 
-    QLIST_REMOVE(&acb, aiocb_siblings);
-    qemu_co_queue_restart_all(&s->overlapping_queue);
     return acb.ret;
 }
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
     if (--acb.nr_pending) {
         qemu_coroutine_yield();
     }
+
+    sd_aio_complete(&acb);
     return acb.ret;
 }
 
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int sd_co_pdiscard(BlockDriverState *bs, int64_t offset,
     }
     sd_aio_setup(&acb, s, &discard_iov, offset >> BDRV_SECTOR_BITS,
                  count >> BDRV_SECTOR_BITS, AIOCB_DISCARD_OBJ);
-
-retry:
-    if (check_overlapping_aiocb(s, &acb)) {
-        qemu_co_queue_wait(&s->overlapping_queue);
-        goto retry;
-    }
-
     sd_co_rw_vector(&acb);
+    sd_aio_complete(&acb);
 
-    QLIST_REMOVE(&acb, aiocb_siblings);
-    qemu_co_queue_restart_all(&s->overlapping_queue);
     return acb.ret;
 }
 
-- 
2.9.3

The following changes since commit 88afdc92b644120e0182c8567e1b1d236e471b12:

Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2021-09-05 15:48:42 +0100)

are available in the Git repository at:

https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 9bd2788f49c331b02372cc257b11e4c984d39708:

block/nvme: Only report VFIO error on failed retry (2021-09-07 09:08:24 +0100)

----------------------------------------------------------------
Pull request

Userspace NVMe driver patches.

----------------------------------------------------------------

Philippe Mathieu-Daudé (11):
  block/nvme: Use safer trace format string
  util/vfio-helpers: Let qemu_vfio_verify_mappings() use error_report()
  util/vfio-helpers: Replace qemu_mutex_lock() calls with
    QEMU_LOCK_GUARD
  util/vfio-helpers: Remove unreachable code in qemu_vfio_dma_map()
  block/nvme: Have nvme_create_queue_pair() report errors consistently
  util/vfio-helpers: Pass Error handle to qemu_vfio_dma_map()
  util/vfio-helpers: Extract qemu_vfio_water_mark_reached()
  util/vfio-helpers: Use error_setg in qemu_vfio_find_[fixed/temp]_iova
  util/vfio-helpers: Simplify qemu_vfio_dma_map() returning directly
  util/vfio-helpers: Let qemu_vfio_do_mapping() propagate Error
  block/nvme: Only report VFIO error on failed retry

include/qemu/vfio-helpers.h |  2 +-
 block/nvme.c                | 29 +++++++----
 util/vfio-helpers.c         | 99 ++++++++++++++++++++-----------------
 block/trace-events          |  2 +-
 4 files changed, 76 insertions(+), 56 deletions(-)

-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Fix when building with -Wshorten-64-to-32:

warning: implicit conversion loses integer precision: 'unsigned long' to 'int' [-Wshorten-64-to-32]

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-2-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/trace-events | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/trace-events b/block/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -XXX,XX +XXX,XX @@ nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset 0x%"PRIx64" byte
 nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p offset 0x%"PRIx64" bytes %"PRId64" ret %d"
 nvme_dma_map_flush(void *s) "s %p"
 nvme_free_req_queue_wait(void *s, unsigned q_index) "s %p q #%u"
-nvme_create_queue_pair(unsigned q_index, void *q, unsigned size, void *aio_context, int fd) "index %u q %p size %u aioctx %p fd %d"
+nvme_create_queue_pair(unsigned q_index, void *q, size_t size, void *aio_context, int fd) "index %u q %p size %zu aioctx %p fd %d"
 nvme_free_queue_pair(unsigned q_index, void *q) "index %u q %p"
 nvme_cmd_map_qiov(void *s, void *cmd, void *req, void *qiov, int entries) "s %p cmd %p req %p qiov %p entries %d"
 nvme_cmd_map_qiov_pages(void *s, int i, uint64_t page) "s %p page[%d] 0x%"PRIx64
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Instead of displaying the error on stderr, use error_report()
which also report to the monitor.

Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-3-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/vfio-helpers.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)
     if (QEMU_VFIO_DEBUG) {
         for (i = 0; i < s->nr_mappings - 1; ++i) {
             if (!(s->mappings[i].host < s->mappings[i + 1].host)) {
-                fprintf(stderr, "item %d not sorted!\n", i);
+                error_report("item %d not sorted!", i);
                 qemu_vfio_dump_mappings(s);
                 return false;
             }
             if (!(s->mappings[i].host + s->mappings[i].size <=
                   s->mappings[i + 1].host)) {
-                fprintf(stderr, "item %d overlap with next!\n", i);
+                error_report("item %d overlap with next!", i);
                 qemu_vfio_dump_mappings(s);
                 return false;
             }
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Simplify qemu_vfio_dma_[un]map() handlers by replacing a pair of
qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
macro.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-4-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/vfio-helpers.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
     assert(QEMU_PTR_IS_ALIGNED(host, qemu_real_host_page_size));
     assert(QEMU_IS_ALIGNED(size, qemu_real_host_page_size));
     trace_qemu_vfio_dma_map(s, host, size, temporary, iova);
-    qemu_mutex_lock(&s->lock);
+    QEMU_LOCK_GUARD(&s->lock);
     mapping = qemu_vfio_find_mapping(s, host, &index);
     if (mapping) {
         iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
         *iova = iova0;
     }
 out:
-    qemu_mutex_unlock(&s->lock);
     return ret;
 }
 
@@ -XXX,XX +XXX,XX @@ void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host)
     }
 
     trace_qemu_vfio_dma_unmap(s, host);
-    qemu_mutex_lock(&s->lock);
+    QEMU_LOCK_GUARD(&s->lock);
     m = qemu_vfio_find_mapping(s, host, &index);
     if (!m) {
-        goto out;
+        return;
     }
     qemu_vfio_undo_mapping(s, m, NULL);
-out:
-    qemu_mutex_unlock(&s->lock);
 }
 
 static void qemu_vfio_reset(QEMUVFIOState *s)
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

qemu_vfio_add_mapping() returns a pointer to an indexed entry
in pre-allocated QEMUVFIOState::mappings[], thus can not be NULL.
Remove the pointless check.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-5-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/vfio-helpers.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
             }
 
             mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0);
-            if (!mapping) {
-                ret = -ENOMEM;
-                goto out;
-            }
             assert(qemu_vfio_verify_mappings(s));
             ret = qemu_vfio_do_mapping(s, host, size, iova0);
             if (ret) {
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

nvme_create_queue_pair() does not return a boolean value (indicating
eventual error) but a pointer, and is inconsistent in how it fills the
error handler. To fulfill callers expectations, always set an error
message on failure.

Reported-by: Auger Eric <eric.auger@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-6-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
 
     q = g_try_new0(NVMeQueuePair, 1);
     if (!q) {
+        error_setg(errp, "Cannot allocate queue pair");
         return NULL;
     }
     trace_nvme_create_queue_pair(idx, q, size, aio_context,
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
                           qemu_real_host_page_size);
     q->prp_list_pages = qemu_try_memalign(qemu_real_host_page_size, bytes);
     if (!q->prp_list_pages) {
+        error_setg(errp, "Cannot allocate PRP page list");
         goto fail;
     }
     memset(q->prp_list_pages, 0, bytes);
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
     r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, bytes,
                           false, &prp_list_iova);
     if (r) {
+        error_setg_errno(errp, -r, "Cannot map buffer for DMA");
         goto fail;
     }
     q->free_req_head = -1;
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Currently qemu_vfio_dma_map() displays errors on stderr.
When using management interface, this information is simply
lost. Pass qemu_vfio_dma_map() an Error** handle so it can
propagate the error to callers.

Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-7-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/vfio-helpers.h |  2 +-
 block/nvme.c                | 22 +++++++++++-----------
 util/vfio-helpers.c         | 10 ++++++----
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/include/qemu/vfio-helpers.h b/include/qemu/vfio-helpers.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/vfio-helpers.h
+++ b/include/qemu/vfio-helpers.h
@@ -XXX,XX +XXX,XX @@ typedef struct QEMUVFIOState QEMUVFIOState;
 QEMUVFIOState *qemu_vfio_open_pci(const char *device, Error **errp);
 void qemu_vfio_close(QEMUVFIOState *s);
 int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
-                      bool temporary, uint64_t *iova_list);
+                      bool temporary, uint64_t *iova_list, Error **errp);
 int qemu_vfio_dma_reset_temporary(QEMUVFIOState *s);
 void qemu_vfio_dma_unmap(QEMUVFIOState *s, void *host);
 void *qemu_vfio_pci_map_bar(QEMUVFIOState *s, int index,
diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static bool nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q,
         return false;
     }
     memset(q->queue, 0, bytes);
-    r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova);
+    r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova, errp);
     if (r) {
-        error_setg(errp, "Cannot map queue");
-        return false;
+        error_prepend(errp, "Cannot map queue: ");
     }
-    return true;
+    return r == 0;
 }
 
 static void nvme_free_queue_pair(NVMeQueuePair *q)
@@ -XXX,XX +XXX,XX @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s,
     qemu_co_queue_init(&q->free_req_queue);
     q->completion_bh = aio_bh_new(aio_context, nvme_process_completion_bh, q);
     r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, bytes,
-                          false, &prp_list_iova);
+                          false, &prp_list_iova, errp);
     if (r) {
-        error_setg_errno(errp, -r, "Cannot map buffer for DMA");
+        error_prepend(errp, "Cannot map buffer for DMA: ");
         goto fail;
     }
     q->free_req_head = -1;
@@ -XXX,XX +XXX,XX @@ static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp)
         error_setg(errp, "Cannot allocate buffer for identify response");
         goto out;
     }
-    r = qemu_vfio_dma_map(s->vfio, id, id_size, true, &iova);
+    r = qemu_vfio_dma_map(s->vfio, id, id_size, true, &iova, errp);
     if (r) {
-        error_setg(errp, "Cannot map buffer for DMA");
+        error_prepend(errp, "Cannot map buffer for DMA: ");
         goto out;
     }
 
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd,
 try_map:
         r = qemu_vfio_dma_map(s->vfio,
                               qiov->iov[i].iov_base,
-                              len, true, &iova);
+                              len, true, &iova, NULL);
         if (r == -ENOSPC) {
             /*
              * In addition to the -ENOMEM error, the VFIO_IOMMU_MAP_DMA
@@ -XXX,XX +XXX,XX @@ static void nvme_aio_unplug(BlockDriverState *bs)
 static void nvme_register_buf(BlockDriverState *bs, void *host, size_t size)
 {
     int ret;
+    Error *local_err = NULL;
     BDRVNVMeState *s = bs->opaque;
 
-    ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL);
+    ret = qemu_vfio_dma_map(s->vfio, host, size, false, NULL, &local_err);
     if (ret) {
         /* FIXME: we may run out of IOVA addresses after repeated
          * bdrv_register_buf/bdrv_unregister_buf, because nvme_vfio_dma_unmap
          * doesn't reclaim addresses for fixed mappings. */
-        error_report("nvme_register_buf failed: %s", strerror(-ret));
+        error_reportf_err(local_err, "nvme_register_buf failed: ");
     }
 }
 
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ static void qemu_vfio_ram_block_added(RAMBlockNotifier *n, void *host,
                                       size_t size, size_t max_size)
 {
     QEMUVFIOState *s = container_of(n, QEMUVFIOState, ram_notifier);
+    Error *local_err = NULL;
     int ret;
 
     trace_qemu_vfio_ram_block_added(s, host, max_size);
-    ret = qemu_vfio_dma_map(s, host, max_size, false, NULL);
+    ret = qemu_vfio_dma_map(s, host, max_size, false, NULL, &local_err);
     if (ret) {
-        error_report("qemu_vfio_dma_map(%p, %zu) failed: %s", host, max_size,
-                     strerror(-ret));
+        error_reportf_err(local_err,
+                          "qemu_vfio_dma_map(%p, %zu) failed: ",
+                          host, max_size);
     }
 }
 
@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
  * mapping status within this area is not allowed).
  */
 int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
-                      bool temporary, uint64_t *iova)
+                      bool temporary, uint64_t *iova, Error **errp)
 {
     int ret = 0;
     int index;
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Extract qemu_vfio_water_mark_reached() for readability,
and have it provide an error hint it its Error* handle.

Suggested-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-8-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/vfio-helpers.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
     return -ENOMEM;
 }
 
+/**
+ * qemu_vfio_water_mark_reached:
+ *
+ * Returns %true if high watermark has been reached, %false otherwise.
+ */
+static bool qemu_vfio_water_mark_reached(QEMUVFIOState *s, size_t size,
+                                         Error **errp)
+{
+    if (s->high_water_mark - s->low_water_mark + 1 < size) {
+        error_setg(errp, "iova exhausted (water mark reached)");
+        return true;
+    }
+    return false;
+}
+
 /* Map [host, host + size) area into a contiguous IOVA address space, and store
  * the result in @iova if not NULL. The caller need to make sure the area is
  * aligned to page size, and mustn't overlap with existing mapping areas (split
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
     if (mapping) {
         iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);
     } else {
-        if (s->high_water_mark - s->low_water_mark + 1 < size) {
+        if (qemu_vfio_water_mark_reached(s, size, errp)) {
             ret = -ENOMEM;
             goto out;
         }
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Both qemu_vfio_find_fixed_iova() and qemu_vfio_find_temp_iova()
return an errno which is unused (or overwritten). Have them propagate
eventual errors to callers, returning a boolean (which is what the
Error API recommends, see commit e3fe3988d78 "error: Document Error
API usage rules" for rationale).

Suggested-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-9-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/vfio-helpers.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_verify_mappings(QEMUVFIOState *s)
     return true;
 }
 
-static int
-qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
+static bool qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size,
+                                      uint64_t *iova, Error **errp)
 {
     int i;
 
@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_fixed_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
             s->usable_iova_ranges[i].end - s->low_water_mark + 1 == 0) {
             *iova = s->low_water_mark;
             s->low_water_mark += size;
-            return 0;
+            return true;
         }
     }
-    return -ENOMEM;
+    error_setg(errp, "fixed iova range not found");
+
+    return false;
 }
 
-static int
-qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
+static bool qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size,
+                                     uint64_t *iova, Error **errp)
 {
     int i;
 
@@ -XXX,XX +XXX,XX @@ qemu_vfio_find_temp_iova(QEMUVFIOState *s, size_t size, uint64_t *iova)
             s->high_water_mark - s->usable_iova_ranges[i].start + 1 == 0) {
             *iova = s->high_water_mark - size;
             s->high_water_mark = *iova;
-            return 0;
+            return true;
         }
     }
-    return -ENOMEM;
+    error_setg(errp, "temporary iova range not found");
+
+    return false;
 }
 
 /**
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
             goto out;
         }
         if (!temporary) {
-            if (qemu_vfio_find_fixed_iova(s, size, &iova0)) {
+            if (!qemu_vfio_find_fixed_iova(s, size, &iova0, errp)) {
                 ret = -ENOMEM;
                 goto out;
             }
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
             }
             qemu_vfio_dump_mappings(s);
         } else {
-            if (qemu_vfio_find_temp_iova(s, size, &iova0)) {
+            if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
                 ret = -ENOMEM;
                 goto out;
             }
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

To simplify qemu_vfio_dma_map():
- reduce 'ret' (returned value) scope by returning errno directly,
- remove the goto 'out' label.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-10-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/vfio-helpers.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ static bool qemu_vfio_water_mark_reached(QEMUVFIOState *s, size_t size,
 int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
                       bool temporary, uint64_t *iova, Error **errp)
 {
-    int ret = 0;
     int index;
     IOVAMapping *mapping;
     uint64_t iova0;
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
     if (mapping) {
         iova0 = mapping->iova + ((uint8_t *)host - (uint8_t *)mapping->host);
     } else {
+        int ret;
+
         if (qemu_vfio_water_mark_reached(s, size, errp)) {
-            ret = -ENOMEM;
-            goto out;
+            return -ENOMEM;
         }
         if (!temporary) {
             if (!qemu_vfio_find_fixed_iova(s, size, &iova0, errp)) {
-                ret = -ENOMEM;
-                goto out;
+                return -ENOMEM;
             }
 
             mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0);
             assert(qemu_vfio_verify_mappings(s));
             ret = qemu_vfio_do_mapping(s, host, size, iova0);
-            if (ret) {
+            if (ret < 0) {
                 qemu_vfio_undo_mapping(s, mapping, NULL);
-                goto out;
+                return ret;
             }
             qemu_vfio_dump_mappings(s);
         } else {
             if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
-                ret = -ENOMEM;
-                goto out;
+                return -ENOMEM;
             }
             ret = qemu_vfio_do_mapping(s, host, size, iova0);
-            if (ret) {
-                goto out;
+            if (ret < 0) {
+                return ret;
             }
         }
     }
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
     if (iova) {
         *iova = iova0;
     }
-out:
-    return ret;
+    return 0;
 }
 
 /* Reset the high watermark and free all "temporary" mappings. */
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Pass qemu_vfio_do_mapping() an Error* argument so it can propagate
any error to callers. Replace error_report() which only report
to the monitor by the more generic error_setg_errno().

Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-11-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/vfio-helpers.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index XXXXXXX..XXXXXXX 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -XXX,XX +XXX,XX @@ static IOVAMapping *qemu_vfio_add_mapping(QEMUVFIOState *s,
 
 /* Do the DMA mapping with VFIO. */
 static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size,
-                                uint64_t iova)
+                                uint64_t iova, Error **errp)
 {
     struct vfio_iommu_type1_dma_map dma_map = {
         .argsz = sizeof(dma_map),
@@ -XXX,XX +XXX,XX @@ static int qemu_vfio_do_mapping(QEMUVFIOState *s, void *host, size_t size,
     trace_qemu_vfio_do_mapping(s, host, iova, size);
 
     if (ioctl(s->container, VFIO_IOMMU_MAP_DMA, &dma_map)) {
-        error_report("VFIO_MAP_DMA failed: %s", strerror(errno));
+        error_setg_errno(errp, errno, "VFIO_MAP_DMA failed");
         return -errno;
     }
     return 0;
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
 
             mapping = qemu_vfio_add_mapping(s, host, size, index + 1, iova0);
             assert(qemu_vfio_verify_mappings(s));
-            ret = qemu_vfio_do_mapping(s, host, size, iova0);
+            ret = qemu_vfio_do_mapping(s, host, size, iova0, errp);
             if (ret < 0) {
                 qemu_vfio_undo_mapping(s, mapping, NULL);
                 return ret;
@@ -XXX,XX +XXX,XX @@ int qemu_vfio_dma_map(QEMUVFIOState *s, void *host, size_t size,
             if (!qemu_vfio_find_temp_iova(s, size, &iova0, errp)) {
                 return -ENOMEM;
             }
-            ret = qemu_vfio_do_mapping(s, host, size, iova0);
+            ret = qemu_vfio_do_mapping(s, host, size, iova0, errp);
             if (ret < 0) {
                 return ret;
             }
-- 
2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

We expect the first qemu_vfio_dma_map() to fail (indicating
DMA mappings exhaustion, see commit 15a730e7a3a). Do not
report the first failure as error, since we are going to
flush the mappings and retry.

This removes spurious error message displayed on the monitor:

(qemu) c
  (qemu) qemu-kvm: VFIO_MAP_DMA failed: No space left on device
  (qemu) info status
  VM status: running

Reported-by: Tingting Mao <timao@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210902070025.197072-12-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/nvme.c b/block/nvme.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd,
     uint64_t *pagelist = req->prp_list_page;
     int i, j, r;
     int entries = 0;
+    Error *local_err = NULL, **errp = NULL;
 
     assert(qiov->size);
     assert(QEMU_IS_ALIGNED(qiov->size, s->page_size));
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int nvme_cmd_map_qiov(BlockDriverState *bs, NvmeCmd *cmd,
 try_map:
         r = qemu_vfio_dma_map(s->vfio,
                               qiov->iov[i].iov_base,
-                              len, true, &iova, NULL);
+                              len, true, &iova, errp);
         if (r == -ENOSPC) {
             /*
              * In addition to the -ENOMEM error, the VFIO_IOMMU_MAP_DMA
@@ -XXX,XX +XXX,XX @@ try_map:
                     goto fail;
                 }
             }
+            errp = &local_err;
+
             goto try_map;
         }
         if (r) {
@@ -XXX,XX +XXX,XX @@ fail:
      * because they are already mapped before calling this function; for
      * temporary mappings, a later nvme_cmd_(un)map_qiov will reclaim by
      * calling qemu_vfio_dma_reset_temporary when necessary. */
+    if (local_err) {
+        error_reportf_err(local_err, "Cannot map buffer for DMA: ");
+    }
     return r;
 }
 
-- 
2.31.1