Series comparison

 [Qemu-devel] [PULL 0/4] Block patches
-The following changes since commit d992f2f1368ceb92e6bfd8efece174110f4236ff:
+The following changes since commit 2b483739791b33c46e6084b51edcf62107058ae1:
-  Merge remote-tracking branch 'remotes/artyom/tags/pull-sun4v-20170226' into staging (2017-02-26 22:40:23 +0000)
+  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170904-2' into staging (2017-09-04 17:21:24 +0100)
 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 b461151ff31c7925f271c297e8abed20231ac7d3:
-  tests-aio-multithread: use atomic_read properly (2017-02-27 14:00:53 +0000)
+  block: document semantics of bdrv_co_preadv|pwritev (2017-09-05 11:07:02 +0100)
 ----------------------------------------------------------------
 ----------------------------------------------------------------
-Paolo Bonzini (4):
+Daniel P. Berrange (1):
-  curl: do not use aio_context_acquire/release
+  block: document semantics of bdrv_co_preadv|pwritev
   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 ++++++++-----
+Stefan Hajnoczi (3):
- block/iscsi.c                | 83 ++++++++++++++++++++++++++++++++++----------
+  qemu.py: make VM() a context manager
- block/nfs.c                  | 23 +++++++++---
+  iotests.py: add FilePath context manager
- tests/test-aio-multithread.c |  4 +--
+  qemu-iotests: use context managers for resource cleanup in 194
-files changed, 100 insertions(+), 34 deletions(-)
  include/block/block_int.h     |  31 +++++++++++
  scripts/qemu.py               |  16 +++++-
  tests/qemu-iotests/194        | 117 +++++++++++++++++++++---------------------
  tests/qemu-iotests/iotests.py |  26 ++++++++++
 files changed, 130 insertions(+), 60 deletions(-)
 --
-.9.3
+.13.5

-[Qemu-devel] [PULL 4/4] tests-aio-multithread: use atomic_read properly
+[Qemu-devel] [PULL 1/4] qemu.py: make VM() a context manager
-From: Paolo Bonzini <pbonzini@redhat.com>
+There are a number of ways to ensure that the QEMU process is shut down
 when the test ends, including atexit.register(), try: finally:, or
 unittest.teardown() methods.  All of these require extra code and the
 programmer must remember to add vm.shutdown().
-nodes[id].next is written by other threads.  If atomic_read is not used
+A nice solution is context managers:
 (matching atomic_set in mcs_mutex_lock!) the compiler can optimize the
 whole "if" away!
-Reported-by: Alex Bennée <alex.bennee@linaro.org>
+  with VM(binary) as vm:
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+      ...
-Tested-by: Greg Kurz <groug@kaod.org>
+  # vm is guaranteed to be shut down here
-Message-id: 20170227111726.9237-1-pbonzini@redhat.com
 Cc: Eduardo Habkost <ehabkost@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
 Message-id: 20170824072202.26818-2-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- tests/test-aio-multithread.c | 4 ++--
+ scripts/qemu.py | 16 +++++++++++++++-
-file changed, 2 insertions(+), 2 deletions(-)
+file changed, 15 insertions(+), 1 deletion(-)
-diff --git a/tests/test-aio-multithread.c b/tests/test-aio-multithread.c
+diff --git a/scripts/qemu.py b/scripts/qemu.py
 index XXXXXXX..XXXXXXX 100644
---- a/tests/test-aio-multithread.c
+--- a/scripts/qemu.py
-+++ b/tests/test-aio-multithread.c
++++ b/scripts/qemu.py
-@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_lock(void)
+@@ -XXX,XX +XXX,XX @@ import qmp.qmp
- static void mcs_mutex_unlock(void)
- {
-     int next;
+ class QEMUMachine(object):
--    if (nodes[id].next == -1) {
+-    '''A QEMU VM'''
-+    if (atomic_read(&nodes[id].next) == -1) {
++    '''A QEMU VM
-         if (atomic_read(&mutex_head) == id &&
++
-             atomic_cmpxchg(&mutex_head, id, -1) == id) {
++    Use this object as a context manager to ensure the QEMU process terminates::
-             /* Last item in the list, exit.  */
++
-@@ -XXX,XX +XXX,XX @@ static void mcs_mutex_unlock(void)
++        with VM(binary) as vm:
-     }
++            ...
++        # vm is guaranteed to be shut down here
-     /* Wake up the next in line.  */
++    '''
--    next = nodes[id].next;
-+    next = atomic_read(&nodes[id].next);
+     def __init__(self, binary, args=[], wrapper=[], name=None, test_dir="/var/tmp",
-     nodes[next].locked = 0;
+                  monitor_address=None, socket_scm_helper=None, debug=False):
-     qemu_futex_wake(&nodes[next].locked, 1);
+@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):
- }
+         self._socket_scm_helper = socket_scm_helper
          self._debug = debug
 +    def __enter__(self):
 +        return self
 +
 +    def __exit__(self, exc_type, exc_val, exc_tb):
 +        self.shutdown()
 +        return False
 +
      # This can be used to add an unused monitor instance.
      def add_monitor_telnet(self, ip, port):
          args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
 --
-.9.3
+.13.5

-[Qemu-devel] [PULL 2/4] nfs: do not use aio_context_acquire/release
+[Qemu-devel] [PULL 2/4] iotests.py: add FilePath context manager
-From: Paolo Bonzini <pbonzini@redhat.com>
+The scratch/ (TEST_DIR) directory is not automatically cleaned up after
 test execution.  It is the responsibility of tests to remove any files
 they create.
-Now that all bottom halves and callbacks take care of taking the
+A nice way of doing this is to declare files at the beginning of the
-AioContext lock, we can migrate some users away from it and to a
+test and automatically remove them with a context manager:
 specific QemuMutex or CoMutex.
-Protect libnfs calls with a QemuMutex.  Callbacks are invoked
+  with iotests.FilePath('test.img') as img_path:
-using bottom halves, so we don't even have to drop it around
+      qemu_img(...)
-callback invocations.
+      qemu_io(...)
   # img_path is guaranteed to be deleted here
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Message-id: 20170222180725.28611-3-pbonzini@redhat.com
+Message-id: 20170824072202.26818-3-stefanha@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/nfs.c | 23 +++++++++++++++++++----
+ tests/qemu-iotests/iotests.py | 26 ++++++++++++++++++++++++++
-file changed, 19 insertions(+), 4 deletions(-)
+file changed, 26 insertions(+)
-diff --git a/block/nfs.c b/block/nfs.c
+diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
 index XXXXXXX..XXXXXXX 100644
---- a/block/nfs.c
+--- a/tests/qemu-iotests/iotests.py
-+++ b/block/nfs.c
++++ b/tests/qemu-iotests/iotests.py
-@@ -XXX,XX +XXX,XX @@ typedef struct NFSClient {
+@@ -XXX,XX +XXX,XX @@ class Timeout:
-     int events;
+     def timeout(self, signum, frame):
-     bool has_zero_init;
+         raise Exception(self.errmsg)
-     AioContext *aio_context;
-+    QemuMutex mutex;
++
-     blkcnt_t st_blocks;
++class FilePath(object):
-     bool cache_used;
++    '''An auto-generated filename that cleans itself up.
-     NFSServer *server;
++
-@@ -XXX,XX +XXX,XX @@ static void nfs_parse_filename(const char *filename, QDict *options,
++    Use this context manager to generate filenames and ensure that the file
- static void nfs_process_read(void *arg);
++    gets deleted::
- static void nfs_process_write(void *arg);
++
++        with TestFilePath('test.img') as img_path:
-+/* Called with QemuMutex held.  */
++            qemu_img('create', img_path, '1G')
- static void nfs_set_events(NFSClient *client)
++        # migration_sock_path is automatically deleted
- {
++    '''
-     int ev = nfs_which_events(client->context);
++    def __init__(self, name):
-@@ -XXX,XX +XXX,XX @@ static void nfs_process_read(void *arg)
++        filename = '{0}-{1}'.format(os.getpid(), name)
- {
++        self.path = os.path.join(test_dir, filename)
-     NFSClient *client = arg;
++
++    def __enter__(self):
--    aio_context_acquire(client->aio_context);
++        return self.path
-+    qemu_mutex_lock(&client->mutex);
++
-     nfs_service(client->context, POLLIN);
++    def __exit__(self, exc_type, exc_val, exc_tb):
-     nfs_set_events(client);
++        try:
--    aio_context_release(client->aio_context);
++            os.remove(self.path)
-+    qemu_mutex_unlock(&client->mutex);
++        except OSError:
- }
++            pass
++        return False
- static void nfs_process_write(void *arg)
++
- {
++
-     NFSClient *client = arg;
+ class VM(qtest.QEMUQtestMachine):
+     '''A QEMU VM'''
--    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
+.13.5

-[Qemu-devel] [PULL 1/4] curl: do not use aio_context_acquire/release
+[Qemu-devel] [PULL 3/4] qemu-iotests: use context managers for resource cleanup in 194
-From: Paolo Bonzini <pbonzini@redhat.com>
+Switch from atexit.register() to a more elegant idiom of declaring
 resources in a with statement:
-Now that all bottom halves and callbacks take care of taking the
+  with FilePath('monitor.sock') as monitor_path,
-AioContext lock, we can migrate some users away from it and to a
+       VM() as vm:
-specific QemuMutex or CoMutex.
+      ...
-Protect BDRVCURLState access with a QemuMutex.
+The files and VMs will be automatically cleaned up whether the test
 passes or fails.
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
+Message-id: 20170824072202.26818-4-stefanha@redhat.com
 Message-id: 20170222180725.28611-2-pbonzini@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/curl.c | 24 +++++++++++++++---------
+ tests/qemu-iotests/194 | 117 ++++++++++++++++++++++++-------------------------
-file changed, 15 insertions(+), 9 deletions(-)
+file changed, 58 insertions(+), 59 deletions(-)
-diff --git a/block/curl.c b/block/curl.c
+diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
-index XXXXXXX..XXXXXXX 100644
+index XXXXXXX..XXXXXXX 100755
---- a/block/curl.c
+--- a/tests/qemu-iotests/194
-+++ b/block/curl.c
++++ b/tests/qemu-iotests/194
-@@ -XXX,XX +XXX,XX @@ typedef struct BDRVCURLState {
+@@ -XXX,XX +XXX,XX @@
-     char *cookie;
+ #
-     bool accept_range;
+ # Non-shared storage migration test using NBD server and drive-mirror
-     AioContext *aio_context;
-+    QemuMutex mutex;
+-import os
-     char *username;
+-import atexit
-     char *password;
+ import iotests
-     char *proxyusername;
-@@ -XXX,XX +XXX,XX @@ static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,
+ iotests.verify_platform(['linux'])
-     return FIND_RET_NONE;
- }
+-img_size = '1G'
+-source_img_path = os.path.join(iotests.test_dir, 'source.img')
-+/* Called with s->mutex held.  */
+-dest_img_path = os.path.join(iotests.test_dir, 'dest.img')
- static void curl_multi_check_completion(BDRVCURLState *s)
+-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, source_img_path, img_size)
- {
+-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, dest_img_path, img_size)
-     int msgs_in_queue;
+-
-@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)
+-iotests.log('Launching VMs...')
-                         continue;
+-migration_sock_path = os.path.join(iotests.test_dir, 'migration.sock')
-                     }
+-nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock')
+-source_vm = iotests.VM('source').add_drive(source_img_path)
-+                    qemu_mutex_unlock(&s->mutex);
+-dest_vm = (iotests.VM('dest').add_drive(dest_img_path)
-                     acb->common.cb(acb->common.opaque, -EPROTO);
+-                             .add_incoming('unix:{0}'.format(migration_sock_path)))
-+                    qemu_mutex_lock(&s->mutex);
+-source_vm.launch()
-                     qemu_aio_unref(acb);
+-atexit.register(source_vm.shutdown)
-                     state->acb[i] = NULL;
+-dest_vm.launch()
-                 }
+-atexit.register(dest_vm.shutdown)
-@@ -XXX,XX +XXX,XX @@ static void curl_multi_check_completion(BDRVCURLState *s)
+-
-     }
+-iotests.log('Launching NBD server on destination...')
- }
+-iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))
+-iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
-+/* Called with s->mutex held.  */
+-
- static void curl_multi_do_locked(CURLState *s)
+-iotests.log('Starting `drive-mirror` on source...')
- {
+-iotests.log(source_vm.qmp(
-     CURLSocket *socket, *next_socket;
+-              'drive-mirror',
-@@ -XXX,XX +XXX,XX @@ static void curl_multi_do(void *arg)
+-              device='drive0',
- {
+-              target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),
-     CURLState *s = (CURLState *)arg;
+-              sync='full',
+-              format='raw', # always raw, the server handles the format
--    aio_context_acquire(s->s->aio_context);
+-              mode='existing',
-+    qemu_mutex_lock(&s->s->mutex);
+-              job_id='mirror-job0'))
-     curl_multi_do_locked(s);
+-
--    aio_context_release(s->s->aio_context);
+-iotests.log('Waiting for `drive-mirror` to complete...')
-+    qemu_mutex_unlock(&s->s->mutex);
+-iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),
- }
+-            filters=[iotests.filter_qmp_event])
+-
- static void curl_multi_read(void *arg)
+-iotests.log('Starting migration...')
- {
+-source_vm.qmp('migrate-set-capabilities',
-     CURLState *s = (CURLState *)arg;
+-              capabilities=[{'capability': 'events', 'state': True}])
+-dest_vm.qmp('migrate-set-capabilities',
--    aio_context_acquire(s->s->aio_context);
+-            capabilities=[{'capability': 'events', 'state': True}])
-+    qemu_mutex_lock(&s->s->mutex);
+-iotests.log(source_vm.qmp('migrate', uri='unix:{0}'.format(migration_sock_path)))
-     curl_multi_do_locked(s);
+-
-     curl_multi_check_completion(s->s);
+-while True:
--    aio_context_release(s->s->aio_context);
+-    event1 = source_vm.event_wait('MIGRATION')
-+    qemu_mutex_unlock(&s->s->mutex);
+-    iotests.log(event1, filters=[iotests.filter_qmp_event])
- }
+-    if event1['data']['status'] in ('completed', 'failed'):
+-        iotests.log('Gracefully ending the `drive-mirror` job on source...')
- static void curl_multi_timeout_do(void *arg)
+-        iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))
-@@ -XXX,XX +XXX,XX @@ static void curl_multi_timeout_do(void *arg)
+-        break
-         return;
+-
-     }
+-while True:
+-    event2 = source_vm.event_wait('BLOCK_JOB_COMPLETED')
--    aio_context_acquire(s->aio_context);
+-    iotests.log(event2, filters=[iotests.filter_qmp_event])
-+    qemu_mutex_lock(&s->mutex);
+-    if event2['event'] == 'BLOCK_JOB_COMPLETED':
-     curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
+-        iotests.log('Stopping the NBD server on destination...')
+-        iotests.log(dest_vm.qmp('nbd-server-stop'))
-     curl_multi_check_completion(s);
+-        break
--    aio_context_release(s->aio_context);
++with iotests.FilePath('source.img') as source_img_path, \
-+    qemu_mutex_unlock(&s->mutex);
++     iotests.FilePath('dest.img') as dest_img_path, \
- #else
++     iotests.FilePath('migration.sock') as migration_sock_path, \
-     abort();
++     iotests.FilePath('nbd.sock') as nbd_sock_path, \
- #endif
++     iotests.VM('source') as source_vm, \
-@@ -XXX,XX +XXX,XX @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
++     iotests.VM('dest') as dest_vm:
-     curl_easy_cleanup(state->curl);
++
-     state->curl = NULL;
++    img_size = '1G'
++    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, source_img_path, img_size)
-+    qemu_mutex_init(&s->mutex);
++    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, dest_img_path, img_size)
-     curl_attach_aio_context(bs, bdrv_get_aio_context(bs));
++
++    iotests.log('Launching VMs...')
-     qemu_opts_del(opts);
++    (source_vm.add_drive(source_img_path)
-@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)
++              .launch())
-     CURLAIOCB *acb = p;
++    (dest_vm.add_drive(dest_img_path)
-     BlockDriverState *bs = acb->common.bs;
++            .add_incoming('unix:{0}'.format(migration_sock_path))
-     BDRVCURLState *s = bs->opaque;
++            .launch())
--    AioContext *ctx = bdrv_get_aio_context(bs);
++
++    iotests.log('Launching NBD server on destination...')
-     size_t start = acb->sector_num * BDRV_SECTOR_SIZE;
++    iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))
-     size_t end;
++    iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
++
--    aio_context_acquire(ctx);
++    iotests.log('Starting `drive-mirror` on source...')
-+    qemu_mutex_lock(&s->mutex);
++    iotests.log(source_vm.qmp(
++                  'drive-mirror',
-     // In case we have the requested data already (e.g. read-ahead),
++                  device='drive0',
-     // we can just call the callback and be done.
++                  target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),
-@@ -XXX,XX +XXX,XX @@ static void curl_readv_bh_cb(void *p)
++                  sync='full',
-     curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
++                  format='raw', # always raw, the server handles the format
++                  mode='existing',
- out:
++                  job_id='mirror-job0'))
--    aio_context_release(ctx);
++
-+    qemu_mutex_unlock(&s->mutex);
++    iotests.log('Waiting for `drive-mirror` to complete...')
-     if (ret != -EINPROGRESS) {
++    iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),
-         acb->common.cb(acb->common.opaque, ret);
++                filters=[iotests.filter_qmp_event])
-         qemu_aio_unref(acb);
++
-@@ -XXX,XX +XXX,XX @@ static void curl_close(BlockDriverState *bs)
++    iotests.log('Starting migration...')
++    source_vm.qmp('migrate-set-capabilities',
-     DPRINTF("CURL: Close\n");
++                  capabilities=[{'capability': 'events', 'state': True}])
-     curl_detach_aio_context(bs);
++    dest_vm.qmp('migrate-set-capabilities',
-+    qemu_mutex_destroy(&s->mutex);
++                capabilities=[{'capability': 'events', 'state': True}])
++    iotests.log(source_vm.qmp('migrate', uri='unix:{0}'.format(migration_sock_path)))
-     g_free(s->cookie);
++
-     g_free(s->url);
++    while True:
 +        event1 = source_vm.event_wait('MIGRATION')
 +        iotests.log(event1, filters=[iotests.filter_qmp_event])
 +        if event1['data']['status'] in ('completed', 'failed'):
 +            iotests.log('Gracefully ending the `drive-mirror` job on source...')
 +            iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))
 +            break
 +
 +    while True:
 +        event2 = source_vm.event_wait('BLOCK_JOB_COMPLETED')
 +        iotests.log(event2, filters=[iotests.filter_qmp_event])
 +        if event2['event'] == 'BLOCK_JOB_COMPLETED':
 +            iotests.log('Stopping the NBD server on destination...')
 +            iotests.log(dest_vm.qmp('nbd-server-stop'))
 +            break
 --
-.9.3
+.13.5

-[Qemu-devel] [PULL 3/4] iscsi: do not use aio_context_acquire/release
+[Qemu-devel] [PULL 4/4] block: document semantics of bdrv_co_preadv|pwritev
-From: Paolo Bonzini <pbonzini@redhat.com>
+From: "Daniel P. Berrange" <berrange@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>
+Reviewed-by: Eric Blake <eblake@redhat.com>
-Message-id: 20170222180725.28611-4-pbonzini@redhat.com
+Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
 Message-id: 20170831105456.9558-1-berrange@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/iscsi.c | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------
+ include/block/block_int.h | 31 +++++++++++++++++++++++++++++++
-file changed, 64 insertions(+), 19 deletions(-)
+file changed, 31 insertions(+)
-diff --git a/block/iscsi.c b/block/iscsi.c
+diff --git a/include/block/block_int.h b/include/block/block_int.h
 index XXXXXXX..XXXXXXX 100644
---- a/block/iscsi.c
+--- a/include/block/block_int.h
-+++ b/block/iscsi.c
++++ b/include/block/block_int.h
-@@ -XXX,XX +XXX,XX @@ typedef struct IscsiLun {
+@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
-     int events;
-     QEMUTimer *nop_timer;
+     int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
-     QEMUTimer *event_timer;
+         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
-+    QemuMutex mutex;
++
-     struct scsi_inquiry_logical_block_provisioning lbp;
++    /**
-     struct scsi_inquiry_block_limits bl;
++     * @offset: position in bytes to read at
-     unsigned char *zeroblock;
++     * @bytes: number of bytes to read
-@@ -XXX,XX +XXX,XX @@ static int iscsi_translate_sense(struct scsi_sense *sense)
++     * @qiov: the buffers to fill with read data
-     return ret;
++     * @flags: currently unused, always 0
- }
++     *
++     * @offset and @bytes will be a multiple of 'request_alignment',
-+/* Called (via iscsi_service) with QemuMutex held.  */
++     * but the length of individual @qiov elements does not have to
- static void
++     * be a multiple.
- iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
++     *
-                         void *command_data, void *opaque)
++     * @bytes will always equal the total size of @qiov, and will be
-@@ -XXX,XX +XXX,XX @@ static const AIOCBInfo iscsi_aiocb_info = {
++     * no larger than 'max_transfer'.
- static void iscsi_process_read(void *arg);
++     *
- static void iscsi_process_write(void *arg);
++     * The buffer in @qiov may point directly to guest memory.
++     */
-+/* Called with QemuMutex held.  */
+     int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
- static void
+         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
- iscsi_set_events(IscsiLun *iscsilun)
+     int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
- {
+         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
-@@ -XXX,XX +XXX,XX @@ iscsi_process_read(void *arg)
+     int coroutine_fn (*bdrv_co_writev_flags)(BlockDriverState *bs,
-     IscsiLun *iscsilun = arg;
+         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
-     struct iscsi_context *iscsi = iscsilun->iscsi;
++    /**
++     * @offset: position in bytes to write at
--    aio_context_acquire(iscsilun->aio_context);
++     * @bytes: number of bytes to write
-+    qemu_mutex_lock(&iscsilun->mutex);
++     * @qiov: the buffers containing data to write
-     iscsi_service(iscsi, POLLIN);
++     * @flags: zero or more bits allowed by 'supported_write_flags'
-     iscsi_set_events(iscsilun);
++     *
--    aio_context_release(iscsilun->aio_context);
++     * @offset and @bytes will be a multiple of 'request_alignment',
-+    qemu_mutex_unlock(&iscsilun->mutex);
++     * but the length of individual @qiov elements does not have to
- }
++     * be a multiple.
++     *
- static void
++     * @bytes will always equal the total size of @qiov, and will be
-@@ -XXX,XX +XXX,XX @@ iscsi_process_write(void *arg)
++     * no larger than 'max_transfer'.
-     IscsiLun *iscsilun = arg;
++     *
-     struct iscsi_context *iscsi = iscsilun->iscsi;
++     * The buffer in @qiov may point directly to guest memory.
++     */
--    aio_context_acquire(iscsilun->aio_context);
+     int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
-+    qemu_mutex_lock(&iscsilun->mutex);
+         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
      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,
          }
      }
 +    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));
  }
 --
-.9.3
+.13.5

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

The following changes since commit 2b483739791b33c46e6084b51edcf62107058ae1:

Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170904-2' into staging (2017-09-04 17:21:24 +0100)

are available in the git repository at:

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

for you to fetch changes up to b461151ff31c7925f271c297e8abed20231ac7d3:

block: document semantics of bdrv_co_preadv|pwritev (2017-09-05 11:07:02 +0100)

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

Daniel P. Berrange (1):
  block: document semantics of bdrv_co_preadv|pwritev

Stefan Hajnoczi (3):
  qemu.py: make VM() a context manager
  iotests.py: add FilePath context manager
  qemu-iotests: use context managers for resource cleanup in 194

include/block/block_int.h     |  31 +++++++++++
 scripts/qemu.py               |  16 +++++-
 tests/qemu-iotests/194        | 117 +++++++++++++++++++++---------------------
 tests/qemu-iotests/iotests.py |  26 ++++++++++
 4 files changed, 130 insertions(+), 60 deletions(-)

-- 
2.13.5

There are a number of ways to ensure that the QEMU process is shut down
when the test ends, including atexit.register(), try: finally:, or
unittest.teardown() methods.  All of these require extra code and the
programmer must remember to add vm.shutdown().

A nice solution is context managers:

with VM(binary) as vm:
      ...
  # vm is guaranteed to be shut down here

Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Message-id: 20170824072202.26818-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 scripts/qemu.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -XXX,XX +XXX,XX @@ import qmp.qmp
 
 
 class QEMUMachine(object):
-    '''A QEMU VM'''
+    '''A QEMU VM
+
+    Use this object as a context manager to ensure the QEMU process terminates::
+
+        with VM(binary) as vm:
+            ...
+        # vm is guaranteed to be shut down here
+    '''
 
     def __init__(self, binary, args=[], wrapper=[], name=None, test_dir="/var/tmp",
                  monitor_address=None, socket_scm_helper=None, debug=False):
@@ -XXX,XX +XXX,XX @@ class QEMUMachine(object):
         self._socket_scm_helper = socket_scm_helper
         self._debug = debug
 
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        self.shutdown()
+        return False
+
     # This can be used to add an unused monitor instance.
     def add_monitor_telnet(self, ip, port):
         args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
-- 
2.13.5

The scratch/ (TEST_DIR) directory is not automatically cleaned up after
test execution.  It is the responsibility of tests to remove any files
they create.

A nice way of doing this is to declare files at the beginning of the
test and automatically remove them with a context manager:

with iotests.FilePath('test.img') as img_path:
      qemu_img(...)
      qemu_io(...)
  # img_path is guaranteed to be deleted here

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20170824072202.26818-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/iotests.py | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -XXX,XX +XXX,XX @@ class Timeout:
     def timeout(self, signum, frame):
         raise Exception(self.errmsg)
 
+
+class FilePath(object):
+    '''An auto-generated filename that cleans itself up.
+
+    Use this context manager to generate filenames and ensure that the file
+    gets deleted::
+
+        with TestFilePath('test.img') as img_path:
+            qemu_img('create', img_path, '1G')
+        # migration_sock_path is automatically deleted
+    '''
+    def __init__(self, name):
+        filename = '{0}-{1}'.format(os.getpid(), name)
+        self.path = os.path.join(test_dir, filename)
+
+    def __enter__(self):
+        return self.path
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        try:
+            os.remove(self.path)
+        except OSError:
+            pass
+        return False
+
+
 class VM(qtest.QEMUQtestMachine):
     '''A QEMU VM'''
 
-- 
2.13.5

Switch from atexit.register() to a more elegant idiom of declaring
resources in a with statement:

with FilePath('monitor.sock') as monitor_path,
       VM() as vm:
      ...

The files and VMs will be automatically cleaned up whether the test
passes or fails.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20170824072202.26818-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/194 | 117 ++++++++++++++++++++++++-------------------------
 1 file changed, 58 insertions(+), 59 deletions(-)

diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index XXXXXXX..XXXXXXX 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -XXX,XX +XXX,XX @@
 #
 # Non-shared storage migration test using NBD server and drive-mirror
 
-import os
-import atexit
 import iotests
 
 iotests.verify_platform(['linux'])
 
-img_size = '1G'
-source_img_path = os.path.join(iotests.test_dir, 'source.img')
-dest_img_path = os.path.join(iotests.test_dir, 'dest.img')
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, source_img_path, img_size)
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, dest_img_path, img_size)
-
-iotests.log('Launching VMs...')
-migration_sock_path = os.path.join(iotests.test_dir, 'migration.sock')
-nbd_sock_path = os.path.join(iotests.test_dir, 'nbd.sock')
-source_vm = iotests.VM('source').add_drive(source_img_path)
-dest_vm = (iotests.VM('dest').add_drive(dest_img_path)
-                             .add_incoming('unix:{0}'.format(migration_sock_path)))
-source_vm.launch()
-atexit.register(source_vm.shutdown)
-dest_vm.launch()
-atexit.register(dest_vm.shutdown)
-
-iotests.log('Launching NBD server on destination...')
-iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))
-iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
-
-iotests.log('Starting `drive-mirror` on source...')
-iotests.log(source_vm.qmp(
-              'drive-mirror',
-              device='drive0',
-              target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),
-              sync='full',
-              format='raw', # always raw, the server handles the format
-              mode='existing',
-              job_id='mirror-job0'))
-
-iotests.log('Waiting for `drive-mirror` to complete...')
-iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),
-            filters=[iotests.filter_qmp_event])
-
-iotests.log('Starting migration...')
-source_vm.qmp('migrate-set-capabilities',
-              capabilities=[{'capability': 'events', 'state': True}])
-dest_vm.qmp('migrate-set-capabilities',
-            capabilities=[{'capability': 'events', 'state': True}])
-iotests.log(source_vm.qmp('migrate', uri='unix:{0}'.format(migration_sock_path)))
-
-while True:
-    event1 = source_vm.event_wait('MIGRATION')
-    iotests.log(event1, filters=[iotests.filter_qmp_event])
-    if event1['data']['status'] in ('completed', 'failed'):
-        iotests.log('Gracefully ending the `drive-mirror` job on source...')
-        iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))
-        break
-
-while True:
-    event2 = source_vm.event_wait('BLOCK_JOB_COMPLETED')
-    iotests.log(event2, filters=[iotests.filter_qmp_event])
-    if event2['event'] == 'BLOCK_JOB_COMPLETED':
-        iotests.log('Stopping the NBD server on destination...')
-        iotests.log(dest_vm.qmp('nbd-server-stop'))
-        break
+with iotests.FilePath('source.img') as source_img_path, \
+     iotests.FilePath('dest.img') as dest_img_path, \
+     iotests.FilePath('migration.sock') as migration_sock_path, \
+     iotests.FilePath('nbd.sock') as nbd_sock_path, \
+     iotests.VM('source') as source_vm, \
+     iotests.VM('dest') as dest_vm:
+
+    img_size = '1G'
+    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, source_img_path, img_size)
+    iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, dest_img_path, img_size)
+
+    iotests.log('Launching VMs...')
+    (source_vm.add_drive(source_img_path)
+              .launch())
+    (dest_vm.add_drive(dest_img_path)
+            .add_incoming('unix:{0}'.format(migration_sock_path))
+            .launch())
+
+    iotests.log('Launching NBD server on destination...')
+    iotests.log(dest_vm.qmp('nbd-server-start', addr={'type': 'unix', 'data': {'path': nbd_sock_path}}))
+    iotests.log(dest_vm.qmp('nbd-server-add', device='drive0', writable=True))
+
+    iotests.log('Starting `drive-mirror` on source...')
+    iotests.log(source_vm.qmp(
+                  'drive-mirror',
+                  device='drive0',
+                  target='nbd+unix:///drive0?socket={0}'.format(nbd_sock_path),
+                  sync='full',
+                  format='raw', # always raw, the server handles the format
+                  mode='existing',
+                  job_id='mirror-job0'))
+
+    iotests.log('Waiting for `drive-mirror` to complete...')
+    iotests.log(source_vm.event_wait('BLOCK_JOB_READY'),
+                filters=[iotests.filter_qmp_event])
+
+    iotests.log('Starting migration...')
+    source_vm.qmp('migrate-set-capabilities',
+                  capabilities=[{'capability': 'events', 'state': True}])
+    dest_vm.qmp('migrate-set-capabilities',
+                capabilities=[{'capability': 'events', 'state': True}])
+    iotests.log(source_vm.qmp('migrate', uri='unix:{0}'.format(migration_sock_path)))
+
+    while True:
+        event1 = source_vm.event_wait('MIGRATION')
+        iotests.log(event1, filters=[iotests.filter_qmp_event])
+        if event1['data']['status'] in ('completed', 'failed'):
+            iotests.log('Gracefully ending the `drive-mirror` job on source...')
+            iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0'))
+            break
+
+    while True:
+        event2 = source_vm.event_wait('BLOCK_JOB_COMPLETED')
+        iotests.log(event2, filters=[iotests.filter_qmp_event])
+        if event2['event'] == 'BLOCK_JOB_COMPLETED':
+            iotests.log('Stopping the NBD server on destination...')
+            iotests.log(dest_vm.qmp('nbd-server-stop'))
+            break
-- 
2.13.5

From: "Daniel P. Berrange" <berrange@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 20170831105456.9558-1-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/block/block_int.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -XXX,XX +XXX,XX @@ struct BlockDriver {
 
     int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
+
+    /**
+     * @offset: position in bytes to read at
+     * @bytes: number of bytes to read
+     * @qiov: the buffers to fill with read data
+     * @flags: currently unused, always 0
+     *
+     * @offset and @bytes will be a multiple of 'request_alignment',
+     * but the length of individual @qiov elements does not have to
+     * be a multiple.
+     *
+     * @bytes will always equal the total size of @qiov, and will be
+     * no larger than 'max_transfer'.
+     *
+     * The buffer in @qiov may point directly to guest memory.
+     */
     int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
     int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
     int coroutine_fn (*bdrv_co_writev_flags)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
+    /**
+     * @offset: position in bytes to write at
+     * @bytes: number of bytes to write
+     * @qiov: the buffers containing data to write
+     * @flags: zero or more bits allowed by 'supported_write_flags'
+     *
+     * @offset and @bytes will be a multiple of 'request_alignment',
+     * but the length of individual @qiov elements does not have to
+     * be a multiple.
+     *
+     * @bytes will always equal the total size of @qiov, and will be
+     * no larger than 'max_transfer'.
+     *
+     * The buffer in @qiov may point directly to guest memory.
+     */
     int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
         uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
 
-- 
2.13.5