Series comparison

 [Qemu-devel] [PULL 0/4] Block patches
-The following changes since commit d992f2f1368ceb92e6bfd8efece174110f4236ff:
+The following changes since commit 20d6c7312f1b812bb9c750f4087f69ac8485cc90:
-  Merge remote-tracking branch 'remotes/artyom/tags/pull-sun4v-20170226' into staging (2017-02-26 22:40:23 +0000)
+  Merge remote-tracking branch 'remotes/palmer/tags/riscv-for-master-3.2-part1' into staging (2019-01-03 13:26:30 +0000)
-are available in the git repository at:
+are available in the Git repository at:
   git://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 1ab17f9f5c63c2798d707aeb22588e4fcc17b2cd:
+for you to fetch changes up to 39a0408e768cd00142f5b57d27ab234282bf4df5:
-  tests-aio-multithread: use atomic_read properly (2017-02-27 14:00:53 +0000)
+  dmg: don't skip zero chunk (2019-01-04 11:15:09 +0000)
 ----------------------------------------------------------------
 Pull request
 Bug fixes for the .dmg image file format.
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Julio Faracco (1):
   dmg: Fixing wrong dmg block type value for block terminator.
-Paolo Bonzini (4):
+yuchenlin (3):
-  curl: do not use aio_context_acquire/release
+  dmg: fix binary search
-  nfs: do not use aio_context_acquire/release
+  dmg: use enumeration type instead of hard coding number
-  iscsi: do not use aio_context_acquire/release
+  dmg: don't skip zero chunk
   tests-aio-multithread: use atomic_read properly
- block/curl.c                 | 24 ++++++++-----
+ block/dmg.c | 31 ++++++++++++++++++++-----------
- block/iscsi.c                | 83 ++++++++++++++++++++++++++++++++++----------
+file changed, 20 insertions(+), 11 deletions(-)
  block/nfs.c                  | 23 +++++++++---
  tests/test-aio-multithread.c |  4 +--
 files changed, 100 insertions(+), 34 deletions(-)
 --
-.9.3
+.20.1

-[Qemu-devel] [PULL 2/4] nfs: do not use aio_context_acquire/release
+[Qemu-devel] [PULL 1/4] dmg: Fixing wrong dmg block type value for block terminator.
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: Julio Faracco <jcfaracco@gmail.com>
-Now that all bottom halves and callbacks take care of taking the
+This is a trivial patch to fix a wrong value for block terminator.
-AioContext lock, we can migrate some users away from it and to a
+The old value was 0x7fffffff which is wrong. It was not affecting the
-specific QemuMutex or CoMutex.
+code because QEMU dmg block is not handling block terminator right now.
 Neverthless, it should be fixed.
-Protect libnfs calls with a QemuMutex.  Callbacks are invoked
+Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
-using bottom halves, so we don't even have to drop it around
+Reviewed-by: yuchenlin <yuchenlin@synology.com>
-callback invocations.
+Message-id: 20181228145055.18039-1-jcfaracco@gmail.com
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-id: 20170222180725.28611-3-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/nfs.c | 23 +++++++++++++++++++----
+ block/dmg.c | 2 +-
-file changed, 19 insertions(+), 4 deletions(-)
+file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/block/nfs.c b/block/nfs.c
+diff --git a/block/dmg.c b/block/dmg.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/nfs.c
+--- a/block/dmg.c
-+++ b/block/nfs.c
++++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ typedef struct NFSClient {
+@@ -XXX,XX +XXX,XX @@ enum {
-     int events;
+     UDBZ,
-     bool has_zero_init;
+     ULFO,
-     AioContext *aio_context;
+     UDCM = 0x7ffffffe, /* Comments */
-+    QemuMutex mutex;
+-    UDLE               /* Last Entry */
-     blkcnt_t st_blocks;
++    UDLE = 0xffffffff  /* Last Entry */
-     bool cache_used;
+ };
-     NFSServer *server;
-@@ -XXX,XX +XXX,XX @@ static void nfs_parse_filename(const char *filename, QDict *options,
+ static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
  static void nfs_process_read(void *arg);
  static void nfs_process_write(void *arg);
 +/* Called with QemuMutex held.  */
  static void nfs_set_events(NFSClient *client)
  {
      int ev = nfs_which_events(client->context);
@@ -XXX,XX +XXX,XX @@ static void nfs_process_read(void *arg)
  {
      NFSClient *client = arg;
 -    aio_context_acquire(client->aio_context);
 +    qemu_mutex_lock(&client->mutex);
      nfs_service(client->context, POLLIN);
      nfs_set_events(client);
 -    aio_context_release(client->aio_context);
 +    qemu_mutex_unlock(&client->mutex);
  }
  static void nfs_process_write(void *arg)
  {
      NFSClient *client = arg;
 -    aio_context_acquire(client->aio_context);
 +    qemu_mutex_lock(&client->mutex);
      nfs_service(client->context, POLLOUT);
      nfs_set_events(client);
 -    aio_context_release(client->aio_context);
 +    qemu_mutex_unlock(&client->mutex);
  }
  static void nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)
@@ -XXX,XX +XXX,XX @@ static void nfs_co_generic_bh_cb(void *opaque)
      aio_co_wake(task->co);
  }
 +/* Called (via nfs_service) with QemuMutex held.  */
  static void
  nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
                    void *private_data)
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
      nfs_co_init_task(bs, &task);
      task.iov = iov;
 +    qemu_mutex_lock(&client->mutex);
      if (nfs_pread_async(client->context, client->fh,
                          offset, bytes, nfs_co_generic_cb, &task) != 0) {
 +        qemu_mutex_unlock(&client->mutex);
          return -ENOMEM;
      }
      nfs_set_events(client);
 +    qemu_mutex_unlock(&client->mutex);
      while (!task.complete) {
          qemu_coroutine_yield();
      }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
          buf = iov->iov[0].iov_base;
      }
 +    qemu_mutex_lock(&client->mutex);
      if (nfs_pwrite_async(client->context, client->fh,
                           offset, bytes, buf,
                           nfs_co_generic_cb, &task) != 0) {
 +        qemu_mutex_unlock(&client->mutex);
          if (my_buffer) {
              g_free(buf);
          }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
      }
      nfs_set_events(client);
 +    qemu_mutex_unlock(&client->mutex);
      while (!task.complete) {
          qemu_coroutine_yield();
      }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_flush(BlockDriverState *bs)
      nfs_co_init_task(bs, &task);
 +    qemu_mutex_lock(&client->mutex);
      if (nfs_fsync_async(client->context, client->fh, nfs_co_generic_cb,
                          &task) != 0) {
 +        qemu_mutex_unlock(&client->mutex);
          return -ENOMEM;
      }
      nfs_set_events(client);
 +    qemu_mutex_unlock(&client->mutex);
      while (!task.complete) {
          qemu_coroutine_yield();
      }
@@ -XXX,XX +XXX,XX @@ static void nfs_file_close(BlockDriverState *bs)
  {
      NFSClient *client = bs->opaque;
      nfs_client_close(client);
 +    qemu_mutex_destroy(&client->mutex);
  }
  static NFSServer *nfs_config(QDict *options, Error **errp)
@@ -XXX,XX +XXX,XX @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
      if (ret < 0) {
          return ret;
      }
 +    qemu_mutex_init(&client->mutex);
      bs->total_sectors = ret;
      ret = 0;
      return ret;
@@ -XXX,XX +XXX,XX @@ static int nfs_has_zero_init(BlockDriverState *bs)
      return client->has_zero_init;
  }
 +/* Called (via nfs_service) with QemuMutex held.  */
  static void
  nfs_get_allocated_file_size_cb(int ret, struct nfs_context *nfs, void *data,
                                 void *private_data)
 --
-.9.3
+.20.1

-[Qemu-devel] [PULL 3/4] iscsi: do not use aio_context_acquire/release
+[Qemu-devel] [PULL 2/4] dmg: fix binary search
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: yuchenlin <npes87184@gmail.com>
-Now that all bottom halves and callbacks take care of taking the
+There is a possible hang in original binary search implementation. That is
-AioContext lock, we can migrate some users away from it and to a
+if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
 specific QemuMutex or CoMutex.
-Protect libiscsi calls with a QemuMutex.  Callbacks are invoked
+The chunk1 will be still 4, and so on.
 using bottom halves, so we don't even have to drop it around
 callback invocations.
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: yuchenlin <npes87184@gmail.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Message-id: 20190103114700.9686-2-npes87184@gmail.com
 Message-id: 20170222180725.28611-4-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
+ block/dmg.c | 10 +++++++---
-file changed, 64 insertions(+), 19 deletions(-)
+file changed, 7 insertions(+), 3 deletions(-)
-diff --git a/block/iscsi.c b/block/iscsi.c
+diff --git a/block/dmg.c b/block/dmg.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/iscsi.c
+--- a/block/dmg.c
-+++ b/block/iscsi.c
++++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ typedef struct IscsiLun {
+@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
      int events;
      QEMUTimer *nop_timer;
      QEMUTimer *event_timer;
 +    QemuMutex mutex;
      struct scsi_inquiry_logical_block_provisioning lbp;
      struct scsi_inquiry_block_limits bl;
      unsigned char *zeroblock;
@@ -XXX,XX +XXX,XX @@ static int iscsi_translate_sense(struct scsi_sense *sense)
      return ret;
  }
 +/* Called (via iscsi_service) with QemuMutex held.  */
  static void
  iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
                          void *command_data, void *opaque)
@@ -XXX,XX +XXX,XX @@ static const AIOCBInfo iscsi_aiocb_info = {
  static void iscsi_process_read(void *arg);
  static void iscsi_process_write(void *arg);
 +/* Called with QemuMutex held.  */
  static void
  iscsi_set_events(IscsiLun *iscsilun)
  {
-@@ -XXX,XX +XXX,XX @@ iscsi_process_read(void *arg)
+     /* binary search */
-     IscsiLun *iscsilun = arg;
+     uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
-     struct iscsi_context *iscsi = iscsilun->iscsi;
+-    while (chunk1 != chunk2) {
++    while (chunk1 <= chunk2) {
--    aio_context_acquire(iscsilun->aio_context);
+         chunk3 = (chunk1 + chunk2) / 2;
-+    qemu_mutex_lock(&iscsilun->mutex);
+         if (s->sectors[chunk3] > sector_num) {
-     iscsi_service(iscsi, POLLIN);
+-            chunk2 = chunk3;
-     iscsi_set_events(iscsilun);
++            if (chunk3 == 0) {
--    aio_context_release(iscsilun->aio_context);
++                goto err;
-+    qemu_mutex_unlock(&iscsilun->mutex);
++            }
- }
++            chunk2 = chunk3 - 1;
+         } else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
- static void
+             return chunk3;
-@@ -XXX,XX +XXX,XX @@ iscsi_process_write(void *arg)
+         } else {
-     IscsiLun *iscsilun = arg;
+-            chunk1 = chunk3;
-     struct iscsi_context *iscsi = iscsilun->iscsi;
++            chunk1 = chunk3 + 1;
 -    aio_context_acquire(iscsilun->aio_context);
 +    qemu_mutex_lock(&iscsilun->mutex);
      iscsi_service(iscsi, POLLOUT);
      iscsi_set_events(iscsilun);
 -    aio_context_release(iscsilun->aio_context);
 +    qemu_mutex_unlock(&iscsilun->mutex);
  }
  static int64_t sector_lun2qemu(int64_t sector, IscsiLun *iscsilun)
@@ -XXX,XX +XXX,XX @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
      uint64_t lba;
      uint32_t num_sectors;
      bool fua = flags & BDRV_REQ_FUA;
 +    int r = 0;
      if (fua) {
          assert(iscsilun->dpofua);
@@ -XXX,XX +XXX,XX @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
      lba = sector_qemu2lun(sector_num, iscsilun);
      num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
      iscsi_co_init_iscsitask(iscsilun, &iTask);
 +    qemu_mutex_lock(&iscsilun->mutex);
  retry:
      if (iscsilun->use_16_for_rw) {
  #if LIBISCSI_API_VERSION >= (20160603)
@@ -XXX,XX +XXX,XX @@ retry:
  #endif
      while (!iTask.complete) {
          iscsi_set_events(iscsilun);
 +        qemu_mutex_unlock(&iscsilun->mutex);
          qemu_coroutine_yield();
 +        qemu_mutex_lock(&iscsilun->mutex);
      }
      if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
      if (iTask.status != SCSI_STATUS_GOOD) {
          iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors);
 -        return iTask.err_code;
 +        r = iTask.err_code;
 +        goto out_unlock;
      }
      iscsi_allocmap_set_allocated(iscsilun, sector_num, nb_sectors);
 -    return 0;
 +out_unlock:
 +    qemu_mutex_unlock(&iscsilun->mutex);
 +    return r;
  }
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
          goto out;
      }
 +    qemu_mutex_lock(&iscsilun->mutex);
  retry:
      if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
                                    sector_qemu2lun(sector_num, iscsilun),
 + 16, iscsi_co_generic_cb,
                                    &iTask) == NULL) {
          ret = -ENOMEM;
 -        goto out;
 +        goto out_unlock;
      }
      while (!iTask.complete) {
          iscsi_set_events(iscsilun);
 +        qemu_mutex_unlock(&iscsilun->mutex);
          qemu_coroutine_yield();
 +        qemu_mutex_lock(&iscsilun->mutex);
      }
      if (iTask.do_retry) {
@@ -XXX,XX +XXX,XX @@ retry:
           * because the device is busy or the cmd is not
           * supported) we pretend all blocks are allocated
           * for backwards compatibility */
 -        goto out;
 +        goto out_unlock;
      }
      lbas = scsi_datain_unmarshall(iTask.task);
      if (lbas == NULL) {
          ret = -EIO;
 -        goto out;
 +        goto out_unlock;
      }
      lbasd = &lbas->descriptors[0];
      if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
          ret = -EIO;
 -        goto out;
 +        goto out_unlock;
      }
      *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
@@ -XXX,XX +XXX,XX @@ retry:
      if (*pnum > nb_sectors) {
          *pnum = nb_sectors;
      }
 +out_unlock:
 +    qemu_mutex_unlock(&iscsilun->mutex);
  out:
      if (iTask.task != NULL) {
          scsi_free_scsi_task(iTask.task);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
      num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
      iscsi_co_init_iscsitask(iscsilun, &iTask);
 +    qemu_mutex_lock(&iscsilun->mutex);
  retry:
      if (iscsilun->use_16_for_rw) {
  #if LIBISCSI_API_VERSION >= (20160603)
@@ -XXX,XX +XXX,XX @@ retry:
  #endif
      while (!iTask.complete) {
          iscsi_set_events(iscsilun);
 +        qemu_mutex_unlock(&iscsilun->mutex);
          qemu_coroutine_yield();
 +        qemu_mutex_lock(&iscsilun->mutex);
      }
      if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
          iTask.complete = 0;
          goto retry;
      }
 +    qemu_mutex_unlock(&iscsilun->mutex);
      if (iTask.status != SCSI_STATUS_GOOD) {
          return iTask.err_code;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_flush(BlockDriverState *bs)
      struct IscsiTask iTask;
      iscsi_co_init_iscsitask(iscsilun, &iTask);
 +    qemu_mutex_lock(&iscsilun->mutex);
  retry:
      if (iscsi_synchronizecache10_task(iscsilun->iscsi, iscsilun->lun, 0, 0, 0,
 , iscsi_co_generic_cb, &iTask) == NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
      while (!iTask.complete) {
          iscsi_set_events(iscsilun);
 +        qemu_mutex_unlock(&iscsilun->mutex);
          qemu_coroutine_yield();
 +        qemu_mutex_lock(&iscsilun->mutex);
      }
      if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
          iTask.complete = 0;
          goto retry;
      }
 +    qemu_mutex_unlock(&iscsilun->mutex);
      if (iTask.status != SCSI_STATUS_GOOD) {
          return iTask.err_code;
@@ -XXX,XX +XXX,XX @@ retry:
  }
  #ifdef __linux__
 +/* Called (via iscsi_service) with QemuMutex held.  */
  static void
  iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
                       void *command_data, void *opaque)
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
      acb->task->expxferlen = acb->ioh->dxfer_len;
      data.size = 0;
 +    qemu_mutex_lock(&iscsilun->mutex);
      if (acb->task->xfer_dir == SCSI_XFER_WRITE) {
          if (acb->ioh->iovec_count == 0) {
              data.data = acb->ioh->dxferp;
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
                                   iscsi_aio_ioctl_cb,
                                   (data.size > 0) ? &data : NULL,
                                   acb) != 0) {
 +        qemu_mutex_unlock(&iscsilun->mutex);
          scsi_free_scsi_task(acb->task);
          qemu_aio_unref(acb);
          return NULL;
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
      }
      iscsi_set_events(iscsilun);
 +    qemu_mutex_unlock(&iscsilun->mutex);
      return &acb->common;
  }
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
      IscsiLun *iscsilun = bs->opaque;
      struct IscsiTask iTask;
      struct unmap_list list;
 +    int r = 0;
      if (!is_byte_request_lun_aligned(offset, count, iscsilun)) {
          return -ENOTSUP;
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
      list.num = count / iscsilun->block_size;
      iscsi_co_init_iscsitask(iscsilun, &iTask);
 +    qemu_mutex_lock(&iscsilun->mutex);
  retry:
      if (iscsi_unmap_task(iscsilun->iscsi, iscsilun->lun, 0, 0, &list, 1,
                           iscsi_co_generic_cb, &iTask) == NULL) {
 -        return -ENOMEM;
 +        r = -ENOMEM;
 +        goto out_unlock;
      }
      while (!iTask.complete) {
          iscsi_set_events(iscsilun);
 +        qemu_mutex_unlock(&iscsilun->mutex);
          qemu_coroutine_yield();
 +        qemu_mutex_lock(&iscsilun->mutex);
      }
      if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
          /* the target might fail with a check condition if it
             is not happy with the alignment of the UNMAP request
             we silently fail in this case */
 -        return 0;
 +        goto out_unlock;
      }
      if (iTask.status != SCSI_STATUS_GOOD) {
 -        return iTask.err_code;
 +        r = iTask.err_code;
 +        goto out_unlock;
      }
      iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
                                 count >> BDRV_SECTOR_BITS);
 -    return 0;
 +out_unlock:
 +    qemu_mutex_unlock(&iscsilun->mutex);
 +    return r;
  }
  static int
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
      uint64_t lba;
      uint32_t nb_blocks;
      bool use_16_for_ws = iscsilun->use_16_for_rw;
 +    int r = 0;
      if (!is_byte_request_lun_aligned(offset, count, iscsilun)) {
          return -ENOTSUP;
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
          }
      }
++err:
-+    qemu_mutex_lock(&iscsilun->mutex);
+     return s->n_chunks; /* error */
      iscsi_co_init_iscsitask(iscsilun, &iTask);
  retry:
      if (use_16_for_ws) {
@@ -XXX,XX +XXX,XX @@ retry:
      while (!iTask.complete) {
          iscsi_set_events(iscsilun);
 +        qemu_mutex_unlock(&iscsilun->mutex);
          qemu_coroutine_yield();
 +        qemu_mutex_lock(&iscsilun->mutex);
      }
      if (iTask.status == SCSI_STATUS_CHECK_CONDITION &&
@@ -XXX,XX +XXX,XX @@ retry:
          /* WRITE SAME is not supported by the target */
          iscsilun->has_write_same = false;
          scsi_free_scsi_task(iTask.task);
 -        return -ENOTSUP;
 +        r = -ENOTSUP;
 +        goto out_unlock;
      }
      if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
      if (iTask.status != SCSI_STATUS_GOOD) {
          iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
                                     count >> BDRV_SECTOR_BITS);
 -        return iTask.err_code;
 +        r = iTask.err_code;
 +        goto out_unlock;
      }
      if (flags & BDRV_REQ_MAY_UNMAP) {
@@ -XXX,XX +XXX,XX @@ retry:
                                       count >> BDRV_SECTOR_BITS);
      }
 -    return 0;
 +out_unlock:
 +    qemu_mutex_unlock(&iscsilun->mutex);
 +    return r;
  }
- static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
-@@ -XXX,XX +XXX,XX @@ static void iscsi_nop_timed_event(void *opaque)
- {
-     IscsiLun *iscsilun = opaque;
--    aio_context_acquire(iscsilun->aio_context);
-+    qemu_mutex_lock(&iscsilun->mutex);
-     if (iscsi_get_nops_in_flight(iscsilun->iscsi) >= MAX_NOP_FAILURES) {
-         error_report("iSCSI: NOP timeout. Reconnecting...");
-         iscsilun->request_timed_out = true;
-@@ -XXX,XX +XXX,XX @@ static void iscsi_nop_timed_event(void *opaque)
-     iscsi_set_events(iscsilun);
- out:
--    aio_context_release(iscsilun->aio_context);
-+    qemu_mutex_unlock(&iscsilun->mutex);
- }
- static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
-@@ -XXX,XX +XXX,XX @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
-     scsi_free_scsi_task(task);
-     task = NULL;
-+    qemu_mutex_init(&iscsilun->mutex);
-     iscsi_attach_aio_context(bs, iscsilun->aio_context);
-     /* Guess the internal cluster (page) size of the iscsi target by the means
-@@ -XXX,XX +XXX,XX @@ static void iscsi_close(BlockDriverState *bs)
-     iscsi_destroy_context(iscsi);
-     g_free(iscsilun->zeroblock);
-     iscsi_allocmap_free(iscsilun);
-+    qemu_mutex_destroy(&iscsilun->mutex);
-     memset(iscsilun, 0, sizeof(IscsiLun));
- }
 --
-.9.3
+.20.1

-[Qemu-devel] [PULL 4/4] tests-aio-multithread: use atomic_read properly
+[Qemu-devel] [PULL 3/4] dmg: use enumeration type instead of hard coding number
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: yuchenlin <npes87184@gmail.com>
-nodes[id].next is written by other threads.  If atomic_read is not used
+Signed-off-by: yuchenlin <npes87184@gmail.com>
-(matching atomic_set in mcs_mutex_lock!) the compiler can optimize the
+Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
-whole "if" away!
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Message-id: 20190103114700.9686-3-npes87184@gmail.com
 Reported-by: Alex Bennée <alex.bennee@linaro.org>
 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 Tested-by: Greg Kurz <groug@kaod.org>
 Message-id: 20170227111726.9237-1-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/test-aio-multithread.c | 4 ++--
+ block/dmg.c | 4 ++--
 file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/tests/test-aio-multithread.c b/tests/test-aio-multithread.c
+diff --git a/block/dmg.c b/block/dmg.c
 index XXXXXXX..XXXXXXX 100644
---- a/tests/test-aio-multithread.c
+--- a/block/dmg.c
-+++ b/tests/test-aio-multithread.c
++++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_lock(void)
+@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
- static void mcs_mutex_unlock(void)
- {
+         /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
-     int next;
+          * therefore be unbounded. */
--    if (nodes[id].next == -1) {
+-        if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-+    if (atomic_read(&nodes[id].next) == -1) {
++        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-         if (atomic_read(&mutex_head) == id &&
+             error_report("sector count %" PRIu64 " for chunk %" PRIu32
-             atomic_cmpxchg(&mutex_head, id, -1) == id) {
+                          " is larger than max (%u)",
-             /* Last item in the list, exit.  */
+                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
-@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_unlock(void)
+@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-     }
+         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
+          * s->uncompressed_chunk may be too small to cover the large all-zeroes
-     /* Wake up the next in line.  */
+          * section. dmg_read_chunk is called to find s->current_chunk */
--    next = nodes[id].next;
+-        if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
-+    next = atomic_read(&nodes[id].next);
++        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
-     nodes[next].locked = 0;
+             qemu_iovec_memset(qiov, i * 512, 0, 512);
-     qemu_futex_wake(&nodes[next].locked, 1);
+             continue;
- }
+         }
 --
-.9.3
+.20.1

-[Qemu-devel] [PULL 1/4] curl: do not use aio_context_acquire/release
+[Qemu-devel] [PULL 4/4] dmg: don't skip zero chunk
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: yuchenlin <npes87184@gmail.com>
-Now that all bottom halves and callbacks take care of taking the
+The dmg file has many tables which describe: "start from sector XXX to
-AioContext lock, we can migrate some users away from it and to a
+sector XXX, the compression method is XXX and where the compressed data
-specific QemuMutex or CoMutex.
+resides on".
-Protect BDRVCURLState access with a QemuMutex.
+Each sector in the expanded file should be covered by a table. The table
 will describe the offset of compressed data (or raw depends on the type)
 in the dmg.
+For example:
+[-----------The expanded file------------]
+[---bzip table ---]/* zeros */[---zlib---]
+    ^
+    | if we want to read this sector.
+we will find bzip table which contains this sector, and get the
+compressed data offset, read it from dmg, uncompress it, finally write to
+expanded file.
+If we skip zero chunk (table), some sector cannot find the table which
+will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
+and finally causing dmg_co_preadv() return EIO.
+See:
+[-----------The expanded file------------]
+[---bzip table ---]/* zeros */[---zlib---]
+                    ^
+                    | if we want to read this sector.
+Oops, we cannot find the table contains it...
+In the original implementation, we don't have zero table. When we try to
+read sector inside the zero chunk. We will get EIO, and skip reading.
+After this patch, we treat zero chunk the same as ignore chunk, it will
+directly write zero and avoid some sector may not find the table.
+After this patch:
+[-----------The expanded file------------]
+[---bzip table ---][--zeros--][---zlib---]
+Signed-off-by: yuchenlin <npes87184@gmail.com>
+Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Message-id: 20190103114700.9686-4-npes87184@gmail.com
 Message-id: 20170222180725.28611-2-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/curl.c | 24 +++++++++++++++---------
+ block/dmg.c | 19 ++++++++++++-------
-file changed, 15 insertions(+), 9 deletions(-)
+file changed, 12 insertions(+), 7 deletions(-)
-diff --git a/block/curl.c b/block/curl.c
+diff --git a/block/dmg.c b/block/dmg.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/curl.c
+--- a/block/dmg.c
-+++ b/block/curl.c
++++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ typedef struct BDRVCURLState {
+@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
-     char *cookie;
+     case UDRW: /* copy */
-     bool accept_range;
+         uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
-     AioContext *aio_context;
+         break;
-+    QemuMutex mutex;
+-    case UDIG: /* zero */
-     char *username;
++    case UDZE: /* zero */
-     char *password;
++    case UDIG: /* ignore */
-     char *proxyusername;
+         /* as the all-zeroes block may be large, it is treated specially: the
-@@ -XXX,XX +XXX,XX @@ static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,
+          * sector is not copied from a large buffer, a simple memset is used
-     return FIND_RET_NONE;
+          * instead. Therefore uncompressed_sectors does not need to be set. */
- }
+@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {
+ static bool dmg_is_known_block_type(uint32_t entry_type)
 +/* Called with s->mutex held.  */
  static void curl_multi_check_completion(BDRVCURLState *s)
  {
-     int msgs_in_queue;
+     switch (entry_type) {
-@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)
++    case UDZE:    /* zeros */
-                         continue;
+     case UDRW:    /* uncompressed */
-                     }
+-    case UDIG:    /* zeroes */
++    case UDIG:    /* ignore */
-+                    qemu_mutex_unlock(&s->mutex);
+     case UDZO:    /* zlib */
-                     acb->common.cb(acb->common.opaque, -EPROTO);
+         return true;
-+                    qemu_mutex_lock(&s->mutex);
+     case UDBZ:    /* bzip2 */
-                     qemu_aio_unref(acb);
+@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
-                     state->acb[i] = NULL;
+         /* sector count */
-                 }
+         s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
-@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)
-     }
+-        /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
- }
+-         * therefore be unbounded. */
+-        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-+/* Called with s->mutex held.  */
++        /* all-zeroes sector (type UDZE and UDIG) does not need to be
- static void curl_multi_do_locked(CURLState *s)
++         * "uncompressed" and can therefore be unbounded. */
- {
++        if (s->types[i] != UDZE && s->types[i] != UDIG
-     CURLSocket *socket, *next_socket;
++            && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-@@ -XXX,XX +XXX,XX @@ static void curl_multi_do(void *arg)
+             error_report("sector count %" PRIu64 " for chunk %" PRIu32
- {
+                          " is larger than max (%u)",
-     CURLState *s = (CURLState *)arg;
+                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
+@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
--    aio_context_acquire(s->s->aio_context);
+                 return -1;
-+    qemu_mutex_lock(&s->s->mutex);
+             }
-     curl_multi_do_locked(s);
+             break;
--    aio_context_release(s->s->aio_context);
+-        case UDIG: /* zero */
-+    qemu_mutex_unlock(&s->s->mutex);
++        case UDZE: /* zeros */
- }
++        case UDIG: /* ignore */
+             /* see dmg_read, it is treated specially. No buffer needs to be
- static void curl_multi_read(void *arg)
+              * pre-filled, the zeroes can be set directly. */
- {
+             break;
-     CURLState *s = (CURLState *)arg;
+@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
--    aio_context_acquire(s->s->aio_context);
+          * s->uncompressed_chunk may be too small to cover the large all-zeroes
-+    qemu_mutex_lock(&s->s->mutex);
+          * section. dmg_read_chunk is called to find s->current_chunk */
-     curl_multi_do_locked(s);
+-        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
-     curl_multi_check_completion(s->s);
++        if (s->types[s->current_chunk] == UDZE
--    aio_context_release(s->s->aio_context);
++            || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
-+    qemu_mutex_unlock(&s->s->mutex);
+             qemu_iovec_memset(qiov, i * 512, 0, 512);
- }
+             continue;
+         }
  static void curl_multi_timeout_do(void *arg)
@@ -XXX,XX +XXX,XX @@ static void curl_multi_timeout_do(void *arg)
          return;
      }
 -    aio_context_acquire(s->aio_context);
 +    qemu_mutex_lock(&s->mutex);
      curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
      curl_multi_check_completion(s);
 -    aio_context_release(s->aio_context);
 +    qemu_mutex_unlock(&s->mutex);
  #else
      abort();
  #endif
@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
      curl_easy_cleanup(state->curl);
      state->curl = NULL;
 +    qemu_mutex_init(&s->mutex);
      curl_attach_aio_context(bs, bdrv_get_aio_context(bs));
      qemu_opts_del(opts);
@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)
      CURLAIOCB *acb = p;
      BlockDriverState *bs = acb->common.bs;
      BDRVCURLState *s = bs->opaque;
 -    AioContext *ctx = bdrv_get_aio_context(bs);
      size_t start = acb->sector_num * BDRV_SECTOR_SIZE;
      size_t end;
 -    aio_context_acquire(ctx);
 +    qemu_mutex_lock(&s->mutex);
      // In case we have the requested data already (e.g. read-ahead),
      // we can just call the callback and be done.
@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)
      curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
  out:
 -    aio_context_release(ctx);
 +    qemu_mutex_unlock(&s->mutex);
      if (ret != -EINPROGRESS) {
          acb->common.cb(acb->common.opaque, ret);
          qemu_aio_unref(acb);
@@ -XXX,XX +XXX,XX @@ static void curl_close(BlockDriverState *bs)
      DPRINTF("CURL: Close\n");
      curl_detach_aio_context(bs);
 +    qemu_mutex_destroy(&s->mutex);
      g_free(s->cookie);
      g_free(s->url);
 --
-.9.3
+.20.1

The following changes since commit d992f2f1368ceb92e6bfd8efece174110f4236ff:

Merge remote-tracking branch 'remotes/artyom/tags/pull-sun4v-20170226' into staging (2017-02-26 22:40:23 +0000)

are available in the git repository at:

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

for you to fetch changes up to 1ab17f9f5c63c2798d707aeb22588e4fcc17b2cd:

tests-aio-multithread: use atomic_read properly (2017-02-27 14:00:53 +0000)

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

Paolo Bonzini (4):
  curl: do not use aio_context_acquire/release
  nfs: do not use aio_context_acquire/release
  iscsi: do not use aio_context_acquire/release
  tests-aio-multithread: use atomic_read properly

block/curl.c                 | 24 ++++++++-----
 block/iscsi.c                | 83 ++++++++++++++++++++++++++++++++++----------
 block/nfs.c                  | 23 +++++++++---
 tests/test-aio-multithread.c |  4 +--
 4 files changed, 100 insertions(+), 34 deletions(-)

-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Now that all bottom halves and callbacks take care of taking the
AioContext lock, we can migrate some users away from it and to a
specific QemuMutex or CoMutex.

Protect BDRVCURLState access with a QemuMutex.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170222180725.28611-2-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/curl.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index XXXXXXX..XXXXXXX 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -XXX,XX +XXX,XX @@ typedef struct BDRVCURLState {
     char *cookie;
     bool accept_range;
     AioContext *aio_context;
+    QemuMutex mutex;
     char *username;
     char *password;
     char *proxyusername;
@@ -XXX,XX +XXX,XX @@ static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,
     return FIND_RET_NONE;
 }
 
+/* Called with s->mutex held.  */
 static void curl_multi_check_completion(BDRVCURLState *s)
 {
     int msgs_in_queue;
@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)
                         continue;
                     }
 
+                    qemu_mutex_unlock(&s->mutex);
                     acb->common.cb(acb->common.opaque, -EPROTO);
+                    qemu_mutex_lock(&s->mutex);
                     qemu_aio_unref(acb);
                     state->acb[i] = NULL;
                 }
@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)
     }
 }
 
+/* Called with s->mutex held.  */
 static void curl_multi_do_locked(CURLState *s)
 {
     CURLSocket *socket, *next_socket;
@@ -XXX,XX +XXX,XX @@ static void curl_multi_do(void *arg)
 {
     CURLState *s = (CURLState *)arg;
 
-    aio_context_acquire(s->s->aio_context);
+    qemu_mutex_lock(&s->s->mutex);
     curl_multi_do_locked(s);
-    aio_context_release(s->s->aio_context);
+    qemu_mutex_unlock(&s->s->mutex);
 }
 
 static void curl_multi_read(void *arg)
 {
     CURLState *s = (CURLState *)arg;
 
-    aio_context_acquire(s->s->aio_context);
+    qemu_mutex_lock(&s->s->mutex);
     curl_multi_do_locked(s);
     curl_multi_check_completion(s->s);
-    aio_context_release(s->s->aio_context);
+    qemu_mutex_unlock(&s->s->mutex);
 }
 
 static void curl_multi_timeout_do(void *arg)
@@ -XXX,XX +XXX,XX @@ static void curl_multi_timeout_do(void *arg)
         return;
     }
 
-    aio_context_acquire(s->aio_context);
+    qemu_mutex_lock(&s->mutex);
     curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
 
     curl_multi_check_completion(s);
-    aio_context_release(s->aio_context);
+    qemu_mutex_unlock(&s->mutex);
 #else
     abort();
 #endif
@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
     curl_easy_cleanup(state->curl);
     state->curl = NULL;
 
+    qemu_mutex_init(&s->mutex);
     curl_attach_aio_context(bs, bdrv_get_aio_context(bs));
 
     qemu_opts_del(opts);
@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)
     CURLAIOCB *acb = p;
     BlockDriverState *bs = acb->common.bs;
     BDRVCURLState *s = bs->opaque;
-    AioContext *ctx = bdrv_get_aio_context(bs);
 
     size_t start = acb->sector_num * BDRV_SECTOR_SIZE;
     size_t end;
 
-    aio_context_acquire(ctx);
+    qemu_mutex_lock(&s->mutex);
 
     // In case we have the requested data already (e.g. read-ahead),
     // we can just call the callback and be done.
@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)
     curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
 
 out:
-    aio_context_release(ctx);
+    qemu_mutex_unlock(&s->mutex);
     if (ret != -EINPROGRESS) {
         acb->common.cb(acb->common.opaque, ret);
         qemu_aio_unref(acb);
@@ -XXX,XX +XXX,XX @@ static void curl_close(BlockDriverState *bs)
 
     DPRINTF("CURL: Close\n");
     curl_detach_aio_context(bs);
+    qemu_mutex_destroy(&s->mutex);
 
     g_free(s->cookie);
     g_free(s->url);
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Now that all bottom halves and callbacks take care of taking the
AioContext lock, we can migrate some users away from it and to a
specific QemuMutex or CoMutex.

Protect libnfs calls with a QemuMutex.  Callbacks are invoked
using bottom halves, so we don't even have to drop it around
callback invocations.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170222180725.28611-3-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nfs.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index XXXXXXX..XXXXXXX 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -XXX,XX +XXX,XX @@ typedef struct NFSClient {
     int events;
     bool has_zero_init;
     AioContext *aio_context;
+    QemuMutex mutex;
     blkcnt_t st_blocks;
     bool cache_used;
     NFSServer *server;
@@ -XXX,XX +XXX,XX @@ static void nfs_parse_filename(const char *filename, QDict *options,
 static void nfs_process_read(void *arg);
 static void nfs_process_write(void *arg);
 
+/* Called with QemuMutex held.  */
 static void nfs_set_events(NFSClient *client)
 {
     int ev = nfs_which_events(client->context);
@@ -XXX,XX +XXX,XX @@ static void nfs_process_read(void *arg)
 {
     NFSClient *client = arg;
 
-    aio_context_acquire(client->aio_context);
+    qemu_mutex_lock(&client->mutex);
     nfs_service(client->context, POLLIN);
     nfs_set_events(client);
-    aio_context_release(client->aio_context);
+    qemu_mutex_unlock(&client->mutex);
 }
 
 static void nfs_process_write(void *arg)
 {
     NFSClient *client = arg;
 
-    aio_context_acquire(client->aio_context);
+    qemu_mutex_lock(&client->mutex);
     nfs_service(client->context, POLLOUT);
     nfs_set_events(client);
-    aio_context_release(client->aio_context);
+    qemu_mutex_unlock(&client->mutex);
 }
 
 static void nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)
@@ -XXX,XX +XXX,XX @@ static void nfs_co_generic_bh_cb(void *opaque)
     aio_co_wake(task->co);
 }
 
+/* Called (via nfs_service) with QemuMutex held.  */
 static void
 nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
                   void *private_data)
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
     nfs_co_init_task(bs, &task);
     task.iov = iov;
 
+    qemu_mutex_lock(&client->mutex);
     if (nfs_pread_async(client->context, client->fh,
                         offset, bytes, nfs_co_generic_cb, &task) != 0) {
+        qemu_mutex_unlock(&client->mutex);
         return -ENOMEM;
     }
 
     nfs_set_events(client);
+    qemu_mutex_unlock(&client->mutex);
     while (!task.complete) {
         qemu_coroutine_yield();
     }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
         buf = iov->iov[0].iov_base;
     }
 
+    qemu_mutex_lock(&client->mutex);
     if (nfs_pwrite_async(client->context, client->fh,
                          offset, bytes, buf,
                          nfs_co_generic_cb, &task) != 0) {
+        qemu_mutex_unlock(&client->mutex);
         if (my_buffer) {
             g_free(buf);
         }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
     }
 
     nfs_set_events(client);
+    qemu_mutex_unlock(&client->mutex);
     while (!task.complete) {
         qemu_coroutine_yield();
     }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn nfs_co_flush(BlockDriverState *bs)
 
     nfs_co_init_task(bs, &task);
 
+    qemu_mutex_lock(&client->mutex);
     if (nfs_fsync_async(client->context, client->fh, nfs_co_generic_cb,
                         &task) != 0) {
+        qemu_mutex_unlock(&client->mutex);
         return -ENOMEM;
     }
 
     nfs_set_events(client);
+    qemu_mutex_unlock(&client->mutex);
     while (!task.complete) {
         qemu_coroutine_yield();
     }
@@ -XXX,XX +XXX,XX @@ static void nfs_file_close(BlockDriverState *bs)
 {
     NFSClient *client = bs->opaque;
     nfs_client_close(client);
+    qemu_mutex_destroy(&client->mutex);
 }
 
 static NFSServer *nfs_config(QDict *options, Error **errp)
@@ -XXX,XX +XXX,XX @@ static int nfs_file_open(BlockDriverState *bs, QDict *options, int flags,
     if (ret < 0) {
         return ret;
     }
+    qemu_mutex_init(&client->mutex);
     bs->total_sectors = ret;
     ret = 0;
     return ret;
@@ -XXX,XX +XXX,XX @@ static int nfs_has_zero_init(BlockDriverState *bs)
     return client->has_zero_init;
 }
 
+/* Called (via nfs_service) with QemuMutex held.  */
 static void
 nfs_get_allocated_file_size_cb(int ret, struct nfs_context *nfs, void *data,
                                void *private_data)
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

Now that all bottom halves and callbacks take care of taking the
AioContext lock, we can migrate some users away from it and to a
specific QemuMutex or CoMutex.

Protect libiscsi calls with a QemuMutex.  Callbacks are invoked
using bottom halves, so we don't even have to drop it around
callback invocations.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170222180725.28611-4-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 19 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index XXXXXXX..XXXXXXX 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -XXX,XX +XXX,XX @@ typedef struct IscsiLun {
     int events;
     QEMUTimer *nop_timer;
     QEMUTimer *event_timer;
+    QemuMutex mutex;
     struct scsi_inquiry_logical_block_provisioning lbp;
     struct scsi_inquiry_block_limits bl;
     unsigned char *zeroblock;
@@ -XXX,XX +XXX,XX @@ static int iscsi_translate_sense(struct scsi_sense *sense)
     return ret;
 }
 
+/* Called (via iscsi_service) with QemuMutex held.  */
 static void
 iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
                         void *command_data, void *opaque)
@@ -XXX,XX +XXX,XX @@ static const AIOCBInfo iscsi_aiocb_info = {
 static void iscsi_process_read(void *arg);
 static void iscsi_process_write(void *arg);
 
+/* Called with QemuMutex held.  */
 static void
 iscsi_set_events(IscsiLun *iscsilun)
 {
@@ -XXX,XX +XXX,XX @@ iscsi_process_read(void *arg)
     IscsiLun *iscsilun = arg;
     struct iscsi_context *iscsi = iscsilun->iscsi;
 
-    aio_context_acquire(iscsilun->aio_context);
+    qemu_mutex_lock(&iscsilun->mutex);
     iscsi_service(iscsi, POLLIN);
     iscsi_set_events(iscsilun);
-    aio_context_release(iscsilun->aio_context);
+    qemu_mutex_unlock(&iscsilun->mutex);
 }
 
 static void
@@ -XXX,XX +XXX,XX @@ iscsi_process_write(void *arg)
     IscsiLun *iscsilun = arg;
     struct iscsi_context *iscsi = iscsilun->iscsi;
 
-    aio_context_acquire(iscsilun->aio_context);
+    qemu_mutex_lock(&iscsilun->mutex);
     iscsi_service(iscsi, POLLOUT);
     iscsi_set_events(iscsilun);
-    aio_context_release(iscsilun->aio_context);
+    qemu_mutex_unlock(&iscsilun->mutex);
 }
 
 static int64_t sector_lun2qemu(int64_t sector, IscsiLun *iscsilun)
@@ -XXX,XX +XXX,XX @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     uint64_t lba;
     uint32_t num_sectors;
     bool fua = flags & BDRV_REQ_FUA;
+    int r = 0;
 
     if (fua) {
         assert(iscsilun->dpofua);
@@ -XXX,XX +XXX,XX @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     lba = sector_qemu2lun(sector_num, iscsilun);
     num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
     iscsi_co_init_iscsitask(iscsilun, &iTask);
+    qemu_mutex_lock(&iscsilun->mutex);
 retry:
     if (iscsilun->use_16_for_rw) {
 #if LIBISCSI_API_VERSION >= (20160603)
@@ -XXX,XX +XXX,XX @@ retry:
 #endif
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
+        qemu_mutex_unlock(&iscsilun->mutex);
         qemu_coroutine_yield();
+        qemu_mutex_lock(&iscsilun->mutex);
     }
 
     if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
 
     if (iTask.status != SCSI_STATUS_GOOD) {
         iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors);
-        return iTask.err_code;
+        r = iTask.err_code;
+        goto out_unlock;
     }
 
     iscsi_allocmap_set_allocated(iscsilun, sector_num, nb_sectors);
 
-    return 0;
+out_unlock:
+    qemu_mutex_unlock(&iscsilun->mutex);
+    return r;
 }
 
 
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
         goto out;
     }
 
+    qemu_mutex_lock(&iscsilun->mutex);
 retry:
     if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
                                   sector_qemu2lun(sector_num, iscsilun),
                                   8 + 16, iscsi_co_generic_cb,
                                   &iTask) == NULL) {
         ret = -ENOMEM;
-        goto out;
+        goto out_unlock;
     }
 
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
+        qemu_mutex_unlock(&iscsilun->mutex);
         qemu_coroutine_yield();
+        qemu_mutex_lock(&iscsilun->mutex);
     }
 
     if (iTask.do_retry) {
@@ -XXX,XX +XXX,XX @@ retry:
          * because the device is busy or the cmd is not
          * supported) we pretend all blocks are allocated
          * for backwards compatibility */
-        goto out;
+        goto out_unlock;
     }
 
     lbas = scsi_datain_unmarshall(iTask.task);
     if (lbas == NULL) {
         ret = -EIO;
-        goto out;
+        goto out_unlock;
     }
 
     lbasd = &lbas->descriptors[0];
 
     if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
         ret = -EIO;
-        goto out;
+        goto out_unlock;
     }
 
     *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
@@ -XXX,XX +XXX,XX @@ retry:
     if (*pnum > nb_sectors) {
         *pnum = nb_sectors;
     }
+out_unlock:
+    qemu_mutex_unlock(&iscsilun->mutex);
 out:
     if (iTask.task != NULL) {
         scsi_free_scsi_task(iTask.task);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs,
     num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
 
     iscsi_co_init_iscsitask(iscsilun, &iTask);
+    qemu_mutex_lock(&iscsilun->mutex);
 retry:
     if (iscsilun->use_16_for_rw) {
 #if LIBISCSI_API_VERSION >= (20160603)
@@ -XXX,XX +XXX,XX @@ retry:
 #endif
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
+        qemu_mutex_unlock(&iscsilun->mutex);
         qemu_coroutine_yield();
+        qemu_mutex_lock(&iscsilun->mutex);
     }
 
     if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
         iTask.complete = 0;
         goto retry;
     }
+    qemu_mutex_unlock(&iscsilun->mutex);
 
     if (iTask.status != SCSI_STATUS_GOOD) {
         return iTask.err_code;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_flush(BlockDriverState *bs)
     struct IscsiTask iTask;
 
     iscsi_co_init_iscsitask(iscsilun, &iTask);
+    qemu_mutex_lock(&iscsilun->mutex);
 retry:
     if (iscsi_synchronizecache10_task(iscsilun->iscsi, iscsilun->lun, 0, 0, 0,
                                       0, iscsi_co_generic_cb, &iTask) == NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
 
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
+        qemu_mutex_unlock(&iscsilun->mutex);
         qemu_coroutine_yield();
+        qemu_mutex_lock(&iscsilun->mutex);
     }
 
     if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
         iTask.complete = 0;
         goto retry;
     }
+    qemu_mutex_unlock(&iscsilun->mutex);
 
     if (iTask.status != SCSI_STATUS_GOOD) {
         return iTask.err_code;
@@ -XXX,XX +XXX,XX @@ retry:
 }
 
 #ifdef __linux__
+/* Called (via iscsi_service) with QemuMutex held.  */
 static void
 iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
                      void *command_data, void *opaque)
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     acb->task->expxferlen = acb->ioh->dxfer_len;
 
     data.size = 0;
+    qemu_mutex_lock(&iscsilun->mutex);
     if (acb->task->xfer_dir == SCSI_XFER_WRITE) {
         if (acb->ioh->iovec_count == 0) {
             data.data = acb->ioh->dxferp;
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
                                  iscsi_aio_ioctl_cb,
                                  (data.size > 0) ? &data : NULL,
                                  acb) != 0) {
+        qemu_mutex_unlock(&iscsilun->mutex);
         scsi_free_scsi_task(acb->task);
         qemu_aio_unref(acb);
         return NULL;
@@ -XXX,XX +XXX,XX @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     }
 
     iscsi_set_events(iscsilun);
+    qemu_mutex_unlock(&iscsilun->mutex);
 
     return &acb->common;
 }
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
     IscsiLun *iscsilun = bs->opaque;
     struct IscsiTask iTask;
     struct unmap_list list;
+    int r = 0;
 
     if (!is_byte_request_lun_aligned(offset, count, iscsilun)) {
         return -ENOTSUP;
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
     list.num = count / iscsilun->block_size;
 
     iscsi_co_init_iscsitask(iscsilun, &iTask);
+    qemu_mutex_lock(&iscsilun->mutex);
 retry:
     if (iscsi_unmap_task(iscsilun->iscsi, iscsilun->lun, 0, 0, &list, 1,
                          iscsi_co_generic_cb, &iTask) == NULL) {
-        return -ENOMEM;
+        r = -ENOMEM;
+        goto out_unlock;
     }
 
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
+        qemu_mutex_unlock(&iscsilun->mutex);
         qemu_coroutine_yield();
+        qemu_mutex_lock(&iscsilun->mutex);
     }
 
     if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
         /* the target might fail with a check condition if it
            is not happy with the alignment of the UNMAP request
            we silently fail in this case */
-        return 0;
+        goto out_unlock;
     }
 
     if (iTask.status != SCSI_STATUS_GOOD) {
-        return iTask.err_code;
+        r = iTask.err_code;
+        goto out_unlock;
     }
 
     iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
                                count >> BDRV_SECTOR_BITS);
 
-    return 0;
+out_unlock:
+    qemu_mutex_unlock(&iscsilun->mutex);
+    return r;
 }
 
 static int
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
     uint64_t lba;
     uint32_t nb_blocks;
     bool use_16_for_ws = iscsilun->use_16_for_rw;
+    int r = 0;
 
     if (!is_byte_request_lun_aligned(offset, count, iscsilun)) {
         return -ENOTSUP;
@@ -XXX,XX +XXX,XX @@ coroutine_fn iscsi_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
         }
     }
 
+    qemu_mutex_lock(&iscsilun->mutex);
     iscsi_co_init_iscsitask(iscsilun, &iTask);
 retry:
     if (use_16_for_ws) {
@@ -XXX,XX +XXX,XX @@ retry:
 
     while (!iTask.complete) {
         iscsi_set_events(iscsilun);
+        qemu_mutex_unlock(&iscsilun->mutex);
         qemu_coroutine_yield();
+        qemu_mutex_lock(&iscsilun->mutex);
     }
 
     if (iTask.status == SCSI_STATUS_CHECK_CONDITION &&
@@ -XXX,XX +XXX,XX @@ retry:
         /* WRITE SAME is not supported by the target */
         iscsilun->has_write_same = false;
         scsi_free_scsi_task(iTask.task);
-        return -ENOTSUP;
+        r = -ENOTSUP;
+        goto out_unlock;
     }
 
     if (iTask.task != NULL) {
@@ -XXX,XX +XXX,XX @@ retry:
     if (iTask.status != SCSI_STATUS_GOOD) {
         iscsi_allocmap_set_invalid(iscsilun, offset >> BDRV_SECTOR_BITS,
                                    count >> BDRV_SECTOR_BITS);
-        return iTask.err_code;
+        r = iTask.err_code;
+        goto out_unlock;
     }
 
     if (flags & BDRV_REQ_MAY_UNMAP) {
@@ -XXX,XX +XXX,XX @@ retry:
                                      count >> BDRV_SECTOR_BITS);
     }
 
-    return 0;
+out_unlock:
+    qemu_mutex_unlock(&iscsilun->mutex);
+    return r;
 }
 
 static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
@@ -XXX,XX +XXX,XX @@ static void iscsi_nop_timed_event(void *opaque)
 {
     IscsiLun *iscsilun = opaque;
 
-    aio_context_acquire(iscsilun->aio_context);
+    qemu_mutex_lock(&iscsilun->mutex);
     if (iscsi_get_nops_in_flight(iscsilun->iscsi) >= MAX_NOP_FAILURES) {
         error_report("iSCSI: NOP timeout. Reconnecting...");
         iscsilun->request_timed_out = true;
@@ -XXX,XX +XXX,XX @@ static void iscsi_nop_timed_event(void *opaque)
     iscsi_set_events(iscsilun);
 
 out:
-    aio_context_release(iscsilun->aio_context);
+    qemu_mutex_unlock(&iscsilun->mutex);
 }
 
 static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **errp)
@@ -XXX,XX +XXX,XX @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
     scsi_free_scsi_task(task);
     task = NULL;
 
+    qemu_mutex_init(&iscsilun->mutex);
     iscsi_attach_aio_context(bs, iscsilun->aio_context);
 
     /* Guess the internal cluster (page) size of the iscsi target by the means
@@ -XXX,XX +XXX,XX @@ static void iscsi_close(BlockDriverState *bs)
     iscsi_destroy_context(iscsi);
     g_free(iscsilun->zeroblock);
     iscsi_allocmap_free(iscsilun);
+    qemu_mutex_destroy(&iscsilun->mutex);
     memset(iscsilun, 0, sizeof(IscsiLun));
 }
 
-- 
2.9.3

From: Paolo Bonzini <pbonzini@redhat.com>

nodes[id].next is written by other threads.  If atomic_read is not used
(matching atomic_set in mcs_mutex_lock!) the compiler can optimize the
whole "if" away!

Reported-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Greg Kurz <groug@kaod.org>
Message-id: 20170227111726.9237-1-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-aio-multithread.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test-aio-multithread.c b/tests/test-aio-multithread.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/test-aio-multithread.c
+++ b/tests/test-aio-multithread.c
@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_lock(void)
 static void mcs_mutex_unlock(void)
 {
     int next;
-    if (nodes[id].next == -1) {
+    if (atomic_read(&nodes[id].next) == -1) {
         if (atomic_read(&mutex_head) == id &&
             atomic_cmpxchg(&mutex_head, id, -1) == id) {
             /* Last item in the list, exit.  */
@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_unlock(void)
     }
 
     /* Wake up the next in line.  */
-    next = nodes[id].next;
+    next = atomic_read(&nodes[id].next);
     nodes[next].locked = 0;
     qemu_futex_wake(&nodes[next].locked, 1);
 }
-- 
2.9.3

From: yuchenlin <npes87184@gmail.com>

There is a possible hang in original binary search implementation. That is
if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.

The chunk1 will be still 4, and so on.

Signed-off-by: yuchenlin <npes87184@gmail.com>
Message-id: 20190103114700.9686-2-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
 {
     /* binary search */
     uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
-    while (chunk1 != chunk2) {
+    while (chunk1 <= chunk2) {
         chunk3 = (chunk1 + chunk2) / 2;
         if (s->sectors[chunk3] > sector_num) {
-            chunk2 = chunk3;
+            if (chunk3 == 0) {
+                goto err;
+            }
+            chunk2 = chunk3 - 1;
         } else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
             return chunk3;
         } else {
-            chunk1 = chunk3;
+            chunk1 = chunk3 + 1;
         }
     }
+err:
     return s->n_chunks; /* error */
 }
 
-- 
2.20.1

From: yuchenlin <npes87184@gmail.com>

Signed-off-by: yuchenlin <npes87184@gmail.com>
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190103114700.9686-3-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
 
         /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
          * therefore be unbounded. */
-        if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
             error_report("sector count %" PRIu64 " for chunk %" PRIu32
                          " is larger than max (%u)",
                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
          * s->uncompressed_chunk may be too small to cover the large all-zeroes
          * section. dmg_read_chunk is called to find s->current_chunk */
-        if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
+        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
             qemu_iovec_memset(qiov, i * 512, 0, 512);
             continue;
         }
-- 
2.20.1

From: yuchenlin <npes87184@gmail.com>

The dmg file has many tables which describe: "start from sector XXX to
sector XXX, the compression method is XXX and where the compressed data
resides on".

Each sector in the expanded file should be covered by a table. The table
will describe the offset of compressed data (or raw depends on the type)
in the dmg.

For example:

[-----------The expanded file------------]
[---bzip table ---]/* zeros */[---zlib---]
    ^
    | if we want to read this sector.

we will find bzip table which contains this sector, and get the
compressed data offset, read it from dmg, uncompress it, finally write to
expanded file.

If we skip zero chunk (table), some sector cannot find the table which
will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
and finally causing dmg_co_preadv() return EIO.

See:

[-----------The expanded file------------]
[---bzip table ---]/* zeros */[---zlib---]
                    ^
                    | if we want to read this sector.

Oops, we cannot find the table contains it...

In the original implementation, we don't have zero table. When we try to
read sector inside the zero chunk. We will get EIO, and skip reading.

After this patch, we treat zero chunk the same as ignore chunk, it will
directly write zero and avoid some sector may not find the table.

After this patch:

[-----------The expanded file------------]
[---bzip table ---][--zeros--][---zlib---]

Signed-off-by: yuchenlin <npes87184@gmail.com>
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190103114700.9686-4-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
     case UDRW: /* copy */
         uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
         break;
-    case UDIG: /* zero */
+    case UDZE: /* zero */
+    case UDIG: /* ignore */
         /* as the all-zeroes block may be large, it is treated specially: the
          * sector is not copied from a large buffer, a simple memset is used
          * instead. Therefore uncompressed_sectors does not need to be set. */
@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {
 static bool dmg_is_known_block_type(uint32_t entry_type)
 {
     switch (entry_type) {
+    case UDZE:    /* zeros */
     case UDRW:    /* uncompressed */
-    case UDIG:    /* zeroes */
+    case UDIG:    /* ignore */
     case UDZO:    /* zlib */
         return true;
     case UDBZ:    /* bzip2 */
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
         /* sector count */
         s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
 
-        /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
-         * therefore be unbounded. */
-        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+        /* all-zeroes sector (type UDZE and UDIG) does not need to be
+         * "uncompressed" and can therefore be unbounded. */
+        if (s->types[i] != UDZE && s->types[i] != UDIG
+            && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
             error_report("sector count %" PRIu64 " for chunk %" PRIu32
                          " is larger than max (%u)",
                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
                 return -1;
             }
             break;
-        case UDIG: /* zero */
+        case UDZE: /* zeros */
+        case UDIG: /* ignore */
             /* see dmg_read, it is treated specially. No buffer needs to be
              * pre-filled, the zeroes can be set directly. */
             break;
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
          * s->uncompressed_chunk may be too small to cover the large all-zeroes
          * section. dmg_read_chunk is called to find s->current_chunk */
-        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
+        if (s->types[s->current_chunk] == UDZE
+            || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
             qemu_iovec_memset(qiov, i * 512, 0, 512);
             continue;
         }
-- 
2.20.1