Series comparison

-[Qemu-devel] [PULL 0/1] Block patches
+[Qemu-devel] [PULL v2 0/3] Block patches
-The following changes since commit 64175afc695c0672876fbbfc31b299c86d562cb4:
+The following changes since commit bec9c64ef7be8063f1192608b83877bc5c9ea217:
-  arm_gicv3: Fix ICC_BPR1 reset value when EL3 not implemented (2017-06-07 17:21:44 +0100)
+  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-02-13 18:24:08 +0000)
-are available in the git repository at:
+are available in the Git repository at:
-  git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
+  git://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 56faeb9bb6872b3f926b3b3e0452a70beea10af2:
+for you to fetch changes up to d2f668b74907cbd96d9df0774971768ed06de2f0:
-  block/gluster.c: Handle qdict_array_entries() failure (2017-06-09 08:41:29 -0400)
+  misc: fix spelling (2018-02-15 09:39:49 +0000)
 ----------------------------------------------------------------
-Gluster patch
+Pull request
 v2:
  * Dropped Fam's git-publish series because there is still ongoing discussion
 ----------------------------------------------------------------
-Peter Maydell (1):
+Marc-André Lureau (1):
-  block/gluster.c: Handle qdict_array_entries() failure
+  misc: fix spelling
- block/gluster.c | 3 +--
+Stefan Hajnoczi (1):
-file changed, 1 insertion(+), 2 deletions(-)
+  vl: pause vcpus before stopping iothreads
 Wolfgang Bumiller (1):
   ratelimit: don't align wait time with slices
  include/qemu/ratelimit.h   | 11 +++++------
  util/qemu-coroutine-lock.c |  2 +-
  vl.c                       | 12 ++++++++++--
 files changed, 16 insertions(+), 9 deletions(-)
 --
-.9.3
+.14.3

-New patch
+[Qemu-devel] [PULL v2 1/3] vl: pause vcpus before stopping iothreads
+Commit dce8921b2baaf95974af8176406881872067adfa ("iothread: Stop threads
+before main() quits") introduced iothread_stop_all() to avoid the
+following virtio-scsi assertion failure:
+  assert(blk_get_aio_context(d->conf.blk) == s->ctx);
+Back then the assertion failed because when bdrv_close_all() made
+d->conf.blk NULL, blk_get_aio_context() returned the global AioContext
+instead of s->ctx.
+The same assertion can still fail today when vcpus submit new I/O
+requests after iothread_stop_all() has moved the BDS to the global
+AioContext.
+This patch hardens the iothread_stop_all() approach by pausing vcpus
+before calling iothread_stop_all().
+Note that the assertion failure is a race condition.  It is not possible
+to reproduce it reliably.
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Message-id: 20180201110708.8080-1-stefanha@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ vl.c | 12 ++++++++++--
+file changed, 10 insertions(+), 2 deletions(-)
+diff --git a/vl.c b/vl.c
+index XXXXXXX..XXXXXXX 100644
+--- a/vl.c
++++ b/vl.c
+@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
+     main_loop();
+     replay_disable_events();
++
++    /* The ordering of the following is delicate.  Stop vcpus to prevent new
++     * I/O requests being queued by the guest.  Then stop IOThreads (this
++     * includes a drain operation and completes all request processing).  At
++     * this point emulated devices are still associated with their IOThreads
++     * (if any) but no longer have any work to do.  Only then can we close
++     * block devices safely because we know there is no more I/O coming.
++     */
++    pause_all_vcpus();
+     iothread_stop_all();
+-
+-    pause_all_vcpus();
+     bdrv_close_all();
++
+     res_free();
+     /* vhost-user must be cleaned up before chardevs.  */
+--
+.14.3

-New patch
+[Qemu-devel] [PULL v2 2/3] ratelimit: don't align wait time with slices
+From: Wolfgang Bumiller <w.bumiller@proxmox.com>
+It is possible for rate limited writes to keep overshooting a slice's
+quota by a tiny amount causing the slice-aligned waiting period to
+effectively halve the rate.
+Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
+Reviewed-by: Alberto Garcia <berto@igalia.com>
+Message-id: 20180207071758.6818-1-w.bumiller@proxmox.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ include/qemu/ratelimit.h | 11 +++++------
+file changed, 5 insertions(+), 6 deletions(-)
+diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/qemu/ratelimit.h
++++ b/include/qemu/ratelimit.h
+@@ -XXX,XX +XXX,XX @@ typedef struct {
+ static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
+ {
+     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+-    uint64_t delay_slices;
++    double delay_slices;
+     assert(limit->slice_quota && limit->slice_ns);
+@@ -XXX,XX +XXX,XX @@ static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
+         return 0;
+     }
+-    /* Quota exceeded. Calculate the next time slice we may start
+-     * sending data again. */
+-    delay_slices = (limit->dispatched + limit->slice_quota - 1) /
+-        limit->slice_quota;
++    /* Quota exceeded. Wait based on the excess amount and then start a new
++     * slice. */
++    delay_slices = (double)limit->dispatched / limit->slice_quota;
+     limit->slice_end_time = limit->slice_start_time +
+-        delay_slices * limit->slice_ns;
++        (uint64_t)(delay_slices * limit->slice_ns);
+     return limit->slice_end_time - now;
+ }
+--
+.14.3

-[Qemu-devel] [PULL 1/1] block/gluster.c: Handle qdict_array_entries() failure
+[Qemu-devel] [PULL v2 3/3] misc: fix spelling
-From: Peter Maydell <peter.maydell@linaro.org>
+From: Marc-André Lureau <marcandre.lureau@redhat.com>
-In qemu_gluster_parse_json(), the call to qdict_array_entries()
+s/pupulate/populate
 could return a negative error code, which we were ignoring
 because we assigned the result to an unsigned variable.
 Fix this by using the 'int' type instead, which matches the
 return type of qdict_array_entries() and also the type
 we use for the loop enumeration variable 'i'.
-(Spotted by Coverity, CID 1360960.)
+Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
 Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
 Message-id: 20180208162447.10851-1-marcandre.lureau@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  util/qemu-coroutine-lock.c | 2 +-
 file changed, 1 insertion(+), 1 deletion(-)
-Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
+diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Jeff Cody <jcody@redhat.com>
 Message-id: 1496682098-1540-1-git-send-email-peter.maydell@linaro.org
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
  block/gluster.c | 3 +--
 file changed, 1 insertion(+), 2 deletions(-)
 diff --git a/block/gluster.c b/block/gluster.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/gluster.c
+--- a/util/qemu-coroutine-lock.c
-+++ b/block/gluster.c
++++ b/util/qemu-coroutine-lock.c
-@@ -XXX,XX +XXX,XX @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
+@@ -XXX,XX +XXX,XX @@ void qemu_co_queue_run_restart(Coroutine *co)
-     Error *local_err = NULL;
+      * invalid memory.  Therefore, use a temporary queue and do not touch
-     char *str = NULL;
+      * the "co" coroutine as soon as you enter another one.
-     const char *ptr;
+      *
--    size_t num_servers;
+-     * In its turn resumed "co" can pupulate "co_queue_wakeup" queue with
--    int i, type;
++     * In its turn resumed "co" can populate "co_queue_wakeup" queue with
-+    int i, type, num_servers;
+      * new coroutines to be woken up.  The caller, who has resumed "co",
+      * will be responsible for traversing the same queue, which may cause
-     /* create opts info from runtime_json_opts list */
+      * a different wakeup order but not any missing wakeups.
      opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort);
 --
-.9.3
+.14.3

From: Peter Maydell <peter.maydell@linaro.org>

In qemu_gluster_parse_json(), the call to qdict_array_entries()
could return a negative error code, which we were ignoring
because we assigned the result to an unsigned variable.
Fix this by using the 'int' type instead, which matches the
return type of qdict_array_entries() and also the type
we use for the loop enumeration variable 'i'.

(Spotted by Coverity, CID 1360960.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 1496682098-1540-1-git-send-email-peter.maydell@linaro.org
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/gluster.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -XXX,XX +XXX,XX @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
     Error *local_err = NULL;
     char *str = NULL;
     const char *ptr;
-    size_t num_servers;
-    int i, type;
+    int i, type, num_servers;
 
     /* create opts info from runtime_json_opts list */
     opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort);
-- 
2.9.3

The following changes since commit bec9c64ef7be8063f1192608b83877bc5c9ea217:

Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-02-13 18:24:08 +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 d2f668b74907cbd96d9df0774971768ed06de2f0:

misc: fix spelling (2018-02-15 09:39:49 +0000)

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

v2:
 * Dropped Fam's git-publish series because there is still ongoing discussion

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

Marc-André Lureau (1):
  misc: fix spelling

Stefan Hajnoczi (1):
  vl: pause vcpus before stopping iothreads

Wolfgang Bumiller (1):
  ratelimit: don't align wait time with slices

include/qemu/ratelimit.h   | 11 +++++------
 util/qemu-coroutine-lock.c |  2 +-
 vl.c                       | 12 ++++++++++--
 3 files changed, 16 insertions(+), 9 deletions(-)

-- 
2.14.3

Commit dce8921b2baaf95974af8176406881872067adfa ("iothread: Stop threads
before main() quits") introduced iothread_stop_all() to avoid the
following virtio-scsi assertion failure:

assert(blk_get_aio_context(d->conf.blk) == s->ctx);

Back then the assertion failed because when bdrv_close_all() made
d->conf.blk NULL, blk_get_aio_context() returned the global AioContext
instead of s->ctx.

The same assertion can still fail today when vcpus submit new I/O
requests after iothread_stop_all() has moved the BDS to the global
AioContext.

This patch hardens the iothread_stop_all() approach by pausing vcpus
before calling iothread_stop_all().

Note that the assertion failure is a race condition.  It is not possible
to reproduce it reliably.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20180201110708.8080-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 vl.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index XXXXXXX..XXXXXXX 100644
--- a/vl.c
+++ b/vl.c
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
 
     main_loop();
     replay_disable_events();
+
+    /* The ordering of the following is delicate.  Stop vcpus to prevent new
+     * I/O requests being queued by the guest.  Then stop IOThreads (this
+     * includes a drain operation and completes all request processing).  At
+     * this point emulated devices are still associated with their IOThreads
+     * (if any) but no longer have any work to do.  Only then can we close
+     * block devices safely because we know there is no more I/O coming.
+     */
+    pause_all_vcpus();
     iothread_stop_all();
-
-    pause_all_vcpus();
     bdrv_close_all();
+
     res_free();
 
     /* vhost-user must be cleaned up before chardevs.  */
-- 
2.14.3

From: Wolfgang Bumiller <w.bumiller@proxmox.com>

It is possible for rate limited writes to keep overshooting a slice's
quota by a tiny amount causing the slice-aligned waiting period to
effectively halve the rate.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-id: 20180207071758.6818-1-w.bumiller@proxmox.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/ratelimit.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/ratelimit.h
+++ b/include/qemu/ratelimit.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
 static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
 {
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-    uint64_t delay_slices;
+    double delay_slices;
 
     assert(limit->slice_quota && limit->slice_ns);
 
@@ -XXX,XX +XXX,XX @@ static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
         return 0;
     }
 
-    /* Quota exceeded. Calculate the next time slice we may start
-     * sending data again. */
-    delay_slices = (limit->dispatched + limit->slice_quota - 1) /
-        limit->slice_quota;
+    /* Quota exceeded. Wait based on the excess amount and then start a new
+     * slice. */
+    delay_slices = (double)limit->dispatched / limit->slice_quota;
     limit->slice_end_time = limit->slice_start_time +
-        delay_slices * limit->slice_ns;
+        (uint64_t)(delay_slices * limit->slice_ns);
     return limit->slice_end_time - now;
 }
 
-- 
2.14.3

From: Marc-André Lureau <marcandre.lureau@redhat.com>

s/pupulate/populate

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20180208162447.10851-1-marcandre.lureau@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine-lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index XXXXXXX..XXXXXXX 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -XXX,XX +XXX,XX @@ void qemu_co_queue_run_restart(Coroutine *co)
      * invalid memory.  Therefore, use a temporary queue and do not touch
      * the "co" coroutine as soon as you enter another one.
      *
-     * In its turn resumed "co" can pupulate "co_queue_wakeup" queue with
+     * In its turn resumed "co" can populate "co_queue_wakeup" queue with
      * new coroutines to be woken up.  The caller, who has resumed "co",
      * will be responsible for traversing the same queue, which may cause
      * a different wakeup order but not any missing wakeups.
-- 
2.14.3