| 1 | The following changes since commit ecc1f5adeec4e3324d1b695a7c54e3967c526949: | 1 | The following changes since commit 834eddf22ec762839b724538c7be1d1d3b2d9d3b: |
|---|---|---|---|
| 2 | 2 | ||
| 3 | maintainers: Add myself as linux-user reviewer (2017-05-11 13:31:11 -0400) | 3 | Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2018-03-13 10:49:02 +0000) |
| 4 | 4 | ||
| 5 | are available in the git repository at: | 5 | are available in the git repository at: |
| 6 | 6 | ||
| 7 | git://github.com/stefanha/qemu.git tags/block-pull-request | 7 | git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request |
| 8 | 8 | ||
| 9 | for you to fetch changes up to 321d1dba8bef9676a77e9399484e3cd8bf2cf16a: | 9 | for you to fetch changes up to 44acd46f60ce6f16d369cd443e77949deca56a2c: |
| 10 | 10 | ||
| 11 | aio: add missing aio_notify() to aio_enable_external() (2017-05-12 10:36:46 -0400) | 11 | block: include original filename when reporting invalid URIs (2018-03-13 08:06:55 -0400) |
| 12 | 12 | ||
| 13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
| 14 | 14 | Block patch | |
| 15 | ---------------------------------------------------------------- | 15 | ---------------------------------------------------------------- |
| 16 | 16 | ||
| 17 | Daniel P. Berrange (1): | 17 | Daniel P. Berrangé (1): |
| 18 | coroutine: remove GThread implementation | 18 | block: include original filename when reporting invalid URIs |
| 19 | 19 | ||
| 20 | Eric Blake (1): | 20 | block/gluster.c | 2 +- |
| 21 | block: Simplify BDRV_BLOCK_RAW recursion | 21 | block/sheepdog.c | 2 +- |
| 22 | 22 | 2 files changed, 2 insertions(+), 2 deletions(-) | |
| 23 | Stefan Hajnoczi (1): | ||
| 24 | aio: add missing aio_notify() to aio_enable_external() | ||
| 25 | |||
| 26 | configure | 19 ++--- | ||
| 27 | include/block/aio.h | 10 ++- | ||
| 28 | block/io.c | 4 +- | ||
| 29 | util/coroutine-gthread.c | 198 ----------------------------------------------- | ||
| 30 | .travis.yml | 5 +- | ||
| 31 | 5 files changed, 16 insertions(+), 220 deletions(-) | ||
| 32 | delete mode 100644 util/coroutine-gthread.c | ||
| 33 | 23 | ||
| 34 | -- | 24 | -- |
| 35 | 2.9.3 | 25 | 2.13.6 |
| 36 | 26 | ||
| 37 | 27 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
| 2 | 1 | ||
| 3 | The GThread implementation is not functional enough to actually | ||
| 4 | run QEMU reliably. While it was potentially useful for debugging, | ||
| 5 | we have a scripts/qemugdb/coroutine.py to enable tracing of | ||
| 6 | ucontext coroutines in GDB, so that removes the only reason for | ||
| 7 | GThread to exist. | ||
| 8 | |||
| 9 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
| 10 | Acked-by: Alex Bennée <alex.bennee@linaro.org> | ||
| 11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 12 | --- | ||
| 13 | configure | 19 ++--- | ||
| 14 | util/coroutine-gthread.c | 198 ----------------------------------------------- | ||
| 15 | .travis.yml | 5 +- | ||
| 16 | 3 files changed, 6 insertions(+), 216 deletions(-) | ||
| 17 | delete mode 100644 util/coroutine-gthread.c | ||
| 18 | |||
| 19 | diff --git a/configure b/configure | ||
| 20 | index XXXXXXX..XXXXXXX 100755 | ||
| 21 | --- a/configure | ||
| 22 | +++ b/configure | ||
| 23 | @@ -XXX,XX +XXX,XX @@ Advanced options (experts only): | ||
| 24 | --oss-lib path to OSS library | ||
| 25 | --cpu=CPU Build for host CPU [$cpu] | ||
| 26 | --with-coroutine=BACKEND coroutine backend. Supported options: | ||
| 27 | - gthread, ucontext, sigaltstack, windows | ||
| 28 | + ucontext, sigaltstack, windows | ||
| 29 | --enable-gcov enable test coverage analysis with gcov | ||
| 30 | --gcov=GCOV use specified gcov [$gcov_tool] | ||
| 31 | --disable-blobs disable installing provided firmware blobs | ||
| 32 | @@ -XXX,XX +XXX,XX @@ fi | ||
| 33 | # check and set a backend for coroutine | ||
| 34 | |||
| 35 | # We prefer ucontext, but it's not always possible. The fallback | ||
| 36 | -# is sigcontext. gthread is not selectable except explicitly, because | ||
| 37 | -# it is not functional enough to run QEMU proper. (It is occasionally | ||
| 38 | -# useful for debugging purposes.) On Windows the only valid backend | ||
| 39 | -# is the Windows-specific one. | ||
| 40 | +# is sigcontext. On Windows the only valid backend is the Windows | ||
| 41 | +# specific one. | ||
| 42 | |||
| 43 | ucontext_works=no | ||
| 44 | if test "$darwin" != "yes"; then | ||
| 45 | @@ -XXX,XX +XXX,XX @@ else | ||
| 46 | feature_not_found "ucontext" | ||
| 47 | fi | ||
| 48 | ;; | ||
| 49 | - gthread|sigaltstack) | ||
| 50 | + sigaltstack) | ||
| 51 | if test "$mingw32" = "yes"; then | ||
| 52 | error_exit "only the 'windows' coroutine backend is valid for Windows" | ||
| 53 | fi | ||
| 54 | @@ -XXX,XX +XXX,XX @@ else | ||
| 55 | fi | ||
| 56 | |||
| 57 | if test "$coroutine_pool" = ""; then | ||
| 58 | - if test "$coroutine" = "gthread"; then | ||
| 59 | - coroutine_pool=no | ||
| 60 | - else | ||
| 61 | - coroutine_pool=yes | ||
| 62 | - fi | ||
| 63 | -fi | ||
| 64 | -if test "$coroutine" = "gthread" -a "$coroutine_pool" = "yes"; then | ||
| 65 | - error_exit "'gthread' coroutine backend does not support pool (use --disable-coroutine-pool)" | ||
| 66 | + coroutine_pool=yes | ||
| 67 | fi | ||
| 68 | |||
| 69 | if test "$debug_stack_usage" = "yes"; then | ||
| 70 | diff --git a/util/coroutine-gthread.c b/util/coroutine-gthread.c | ||
| 71 | deleted file mode 100644 | ||
| 72 | index XXXXXXX..XXXXXXX | ||
| 73 | --- a/util/coroutine-gthread.c | ||
| 74 | +++ /dev/null | ||
| 75 | @@ -XXX,XX +XXX,XX @@ | ||
| 76 | -/* | ||
| 77 | - * GThread coroutine initialization code | ||
| 78 | - * | ||
| 79 | - * Copyright (C) 2006 Anthony Liguori <anthony@codemonkey.ws> | ||
| 80 | - * Copyright (C) 2011 Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> | ||
| 81 | - * | ||
| 82 | - * This library is free software; you can redistribute it and/or | ||
| 83 | - * modify it under the terms of the GNU Lesser General Public | ||
| 84 | - * License as published by the Free Software Foundation; either | ||
| 85 | - * version 2.0 of the License, or (at your option) any later version. | ||
| 86 | - * | ||
| 87 | - * This library is distributed in the hope that it will be useful, | ||
| 88 | - * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| 89 | - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU | ||
| 90 | - * Lesser General Public License for more details. | ||
| 91 | - * | ||
| 92 | - * You should have received a copy of the GNU Lesser General Public | ||
| 93 | - * License along with this library; if not, see <http://www.gnu.org/licenses/>. | ||
| 94 | - */ | ||
| 95 | - | ||
| 96 | -#include "qemu/osdep.h" | ||
| 97 | -#include "qemu-common.h" | ||
| 98 | -#include "qemu/coroutine_int.h" | ||
| 99 | - | ||
| 100 | -typedef struct { | ||
| 101 | - Coroutine base; | ||
| 102 | - GThread *thread; | ||
| 103 | - bool runnable; | ||
| 104 | - bool free_on_thread_exit; | ||
| 105 | - CoroutineAction action; | ||
| 106 | -} CoroutineGThread; | ||
| 107 | - | ||
| 108 | -static CompatGMutex coroutine_lock; | ||
| 109 | -static CompatGCond coroutine_cond; | ||
| 110 | - | ||
| 111 | -/* GLib 2.31 and beyond deprecated various parts of the thread API, | ||
| 112 | - * but the new interfaces are not available in older GLib versions | ||
| 113 | - * so we have to cope with both. | ||
| 114 | - */ | ||
| 115 | -#if GLIB_CHECK_VERSION(2, 31, 0) | ||
| 116 | -/* Awkwardly, the GPrivate API doesn't provide a way to update the | ||
| 117 | - * GDestroyNotify handler for the coroutine key dynamically. So instead | ||
| 118 | - * we track whether or not the CoroutineGThread should be freed on | ||
| 119 | - * thread exit / coroutine key update using the free_on_thread_exit | ||
| 120 | - * field. | ||
| 121 | - */ | ||
| 122 | -static void coroutine_destroy_notify(gpointer data) | ||
| 123 | -{ | ||
| 124 | - CoroutineGThread *co = data; | ||
| 125 | - if (co && co->free_on_thread_exit) { | ||
| 126 | - g_free(co); | ||
| 127 | - } | ||
| 128 | -} | ||
| 129 | - | ||
| 130 | -static GPrivate coroutine_key = G_PRIVATE_INIT(coroutine_destroy_notify); | ||
| 131 | - | ||
| 132 | -static inline CoroutineGThread *get_coroutine_key(void) | ||
| 133 | -{ | ||
| 134 | - return g_private_get(&coroutine_key); | ||
| 135 | -} | ||
| 136 | - | ||
| 137 | -static inline void set_coroutine_key(CoroutineGThread *co, | ||
| 138 | - bool free_on_thread_exit) | ||
| 139 | -{ | ||
| 140 | - /* Unlike g_static_private_set() this does not call the GDestroyNotify | ||
| 141 | - * if the previous value of the key was NULL. Fortunately we only need | ||
| 142 | - * the GDestroyNotify in the non-NULL key case. | ||
| 143 | - */ | ||
| 144 | - co->free_on_thread_exit = free_on_thread_exit; | ||
| 145 | - g_private_replace(&coroutine_key, co); | ||
| 146 | -} | ||
| 147 | - | ||
| 148 | -static inline GThread *create_thread(GThreadFunc func, gpointer data) | ||
| 149 | -{ | ||
| 150 | - return g_thread_new("coroutine", func, data); | ||
| 151 | -} | ||
| 152 | - | ||
| 153 | -#else | ||
| 154 | - | ||
| 155 | -/* Handle older GLib versions */ | ||
| 156 | - | ||
| 157 | -static GStaticPrivate coroutine_key = G_STATIC_PRIVATE_INIT; | ||
| 158 | - | ||
| 159 | -static inline CoroutineGThread *get_coroutine_key(void) | ||
| 160 | -{ | ||
| 161 | - return g_static_private_get(&coroutine_key); | ||
| 162 | -} | ||
| 163 | - | ||
| 164 | -static inline void set_coroutine_key(CoroutineGThread *co, | ||
| 165 | - bool free_on_thread_exit) | ||
| 166 | -{ | ||
| 167 | - g_static_private_set(&coroutine_key, co, | ||
| 168 | - free_on_thread_exit ? (GDestroyNotify)g_free : NULL); | ||
| 169 | -} | ||
| 170 | - | ||
| 171 | -static inline GThread *create_thread(GThreadFunc func, gpointer data) | ||
| 172 | -{ | ||
| 173 | - return g_thread_create_full(func, data, 0, TRUE, TRUE, | ||
| 174 | - G_THREAD_PRIORITY_NORMAL, NULL); | ||
| 175 | -} | ||
| 176 | - | ||
| 177 | -#endif | ||
| 178 | - | ||
| 179 | - | ||
| 180 | -static void __attribute__((constructor)) coroutine_init(void) | ||
| 181 | -{ | ||
| 182 | -#if !GLIB_CHECK_VERSION(2, 31, 0) | ||
| 183 | - if (!g_thread_supported()) { | ||
| 184 | - g_thread_init(NULL); | ||
| 185 | - } | ||
| 186 | -#endif | ||
| 187 | -} | ||
| 188 | - | ||
| 189 | -static void coroutine_wait_runnable_locked(CoroutineGThread *co) | ||
| 190 | -{ | ||
| 191 | - while (!co->runnable) { | ||
| 192 | - g_cond_wait(&coroutine_cond, &coroutine_lock); | ||
| 193 | - } | ||
| 194 | -} | ||
| 195 | - | ||
| 196 | -static void coroutine_wait_runnable(CoroutineGThread *co) | ||
| 197 | -{ | ||
| 198 | - g_mutex_lock(&coroutine_lock); | ||
| 199 | - coroutine_wait_runnable_locked(co); | ||
| 200 | - g_mutex_unlock(&coroutine_lock); | ||
| 201 | -} | ||
| 202 | - | ||
| 203 | -static gpointer coroutine_thread(gpointer opaque) | ||
| 204 | -{ | ||
| 205 | - CoroutineGThread *co = opaque; | ||
| 206 | - | ||
| 207 | - set_coroutine_key(co, false); | ||
| 208 | - coroutine_wait_runnable(co); | ||
| 209 | - co->base.entry(co->base.entry_arg); | ||
| 210 | - qemu_coroutine_switch(&co->base, co->base.caller, COROUTINE_TERMINATE); | ||
| 211 | - return NULL; | ||
| 212 | -} | ||
| 213 | - | ||
| 214 | -Coroutine *qemu_coroutine_new(void) | ||
| 215 | -{ | ||
| 216 | - CoroutineGThread *co; | ||
| 217 | - | ||
| 218 | - co = g_malloc0(sizeof(*co)); | ||
| 219 | - co->thread = create_thread(coroutine_thread, co); | ||
| 220 | - if (!co->thread) { | ||
| 221 | - g_free(co); | ||
| 222 | - return NULL; | ||
| 223 | - } | ||
| 224 | - return &co->base; | ||
| 225 | -} | ||
| 226 | - | ||
| 227 | -void qemu_coroutine_delete(Coroutine *co_) | ||
| 228 | -{ | ||
| 229 | - CoroutineGThread *co = DO_UPCAST(CoroutineGThread, base, co_); | ||
| 230 | - | ||
| 231 | - g_thread_join(co->thread); | ||
| 232 | - g_free(co); | ||
| 233 | -} | ||
| 234 | - | ||
| 235 | -CoroutineAction qemu_coroutine_switch(Coroutine *from_, | ||
| 236 | - Coroutine *to_, | ||
| 237 | - CoroutineAction action) | ||
| 238 | -{ | ||
| 239 | - CoroutineGThread *from = DO_UPCAST(CoroutineGThread, base, from_); | ||
| 240 | - CoroutineGThread *to = DO_UPCAST(CoroutineGThread, base, to_); | ||
| 241 | - | ||
| 242 | - g_mutex_lock(&coroutine_lock); | ||
| 243 | - from->runnable = false; | ||
| 244 | - from->action = action; | ||
| 245 | - to->runnable = true; | ||
| 246 | - to->action = action; | ||
| 247 | - g_cond_broadcast(&coroutine_cond); | ||
| 248 | - | ||
| 249 | - if (action != COROUTINE_TERMINATE) { | ||
| 250 | - coroutine_wait_runnable_locked(from); | ||
| 251 | - } | ||
| 252 | - g_mutex_unlock(&coroutine_lock); | ||
| 253 | - return from->action; | ||
| 254 | -} | ||
| 255 | - | ||
| 256 | -Coroutine *qemu_coroutine_self(void) | ||
| 257 | -{ | ||
| 258 | - CoroutineGThread *co = get_coroutine_key(); | ||
| 259 | - if (!co) { | ||
| 260 | - co = g_malloc0(sizeof(*co)); | ||
| 261 | - co->runnable = true; | ||
| 262 | - set_coroutine_key(co, true); | ||
| 263 | - } | ||
| 264 | - | ||
| 265 | - return &co->base; | ||
| 266 | -} | ||
| 267 | - | ||
| 268 | -bool qemu_in_coroutine(void) | ||
| 269 | -{ | ||
| 270 | - CoroutineGThread *co = get_coroutine_key(); | ||
| 271 | - | ||
| 272 | - return co && co->base.caller; | ||
| 273 | -} | ||
| 274 | diff --git a/.travis.yml b/.travis.yml | ||
| 275 | index XXXXXXX..XXXXXXX 100644 | ||
| 276 | --- a/.travis.yml | ||
| 277 | +++ b/.travis.yml | ||
| 278 | @@ -XXX,XX +XXX,XX @@ matrix: | ||
| 279 | - env: CONFIG="--enable-trace-backends=ust" | ||
| 280 | TEST_CMD="" | ||
| 281 | compiler: gcc | ||
| 282 | - - env: CONFIG="--with-coroutine=gthread" | ||
| 283 | - TEST_CMD="" | ||
| 284 | - compiler: gcc | ||
| 285 | - env: CONFIG="" | ||
| 286 | os: osx | ||
| 287 | compiler: clang | ||
| 288 | @@ -XXX,XX +XXX,XX @@ matrix: | ||
| 289 | compiler: none | ||
| 290 | env: | ||
| 291 | - COMPILER_NAME=gcc CXX=g++-5 CC=gcc-5 | ||
| 292 | - - CONFIG="--cc=gcc-5 --cxx=g++-5 --disable-pie --disable-linux-user --with-coroutine=gthread" | ||
| 293 | + - CONFIG="--cc=gcc-5 --cxx=g++-5 --disable-pie --disable-linux-user" | ||
| 294 | - TEST_CMD="" | ||
| 295 | before_script: | ||
| 296 | - ./configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread -fuse-ld=gold" || cat config.log | ||
| 297 | -- | ||
| 298 | 2.9.3 | ||
| 299 | |||
| 300 | diff view generated by jsdifflib |
| 1 | From: Eric Blake <eblake@redhat.com> | 1 | From: Daniel P. Berrangé <berrange@redhat.com> |
|---|---|---|---|
| 2 | 2 | ||
| 3 | Since we are already in coroutine context during the body of | 3 | Consider passing a JSON based block driver to "qemu-img commit" |
| 4 | bdrv_co_get_block_status(), we can shave off a few layers of | ||
| 5 | wrappers when recursing to query the protocol when a format driver | ||
| 6 | returned BDRV_BLOCK_RAW. | ||
| 7 | 4 | ||
| 8 | Note that we are already using the correct recursion later on in | 5 | $ qemu-img commit 'json:{"driver":"qcow2","file":{"driver":"gluster",\ |
| 9 | the same function, when probing whether the protocol layer is sparse | 6 | "volume":"gv0","path":"sn1.qcow2", |
| 10 | in order to find out if we can add BDRV_BLOCK_ZERO to an existing | 7 | "server":[{"type":\ |
| 11 | BDRV_BLOCK_DATA|BDRV_BLOCK_OFFSET_VALID. | 8 | "tcp","host":"10.73.199.197","port":"24007"}]},}' |
| 12 | 9 | ||
| 13 | Signed-off-by: Eric Blake <eblake@redhat.com> | 10 | Currently it will commit the content and then report an incredibly |
| 14 | Reviewed-by: Max Reitz <mreitz@redhat.com> | 11 | useless error message when trying to re-open the committed image: |
| 15 | Reviewed-by: Fam Zheng <famz@redhat.com> | 12 | |
| 16 | Message-id: 20170504173745.27414-1-eblake@redhat.com | 13 | qemu-img: invalid URI |
| 17 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 14 | Usage: file=gluster[+transport]://[host[:port]]volume/path[?socket=...][,file.debug=N][,file.logfile=/path/filename.log] |
| 15 | |||
| 16 | With this fix we get: | ||
| 17 | |||
| 18 | qemu-img: invalid URI json:{"server.0.host": "10.73.199.197", | ||
| 19 | "driver": "gluster", "path": "luks.qcow2", "server.0.type": | ||
| 20 | "tcp", "server.0.port": "24007", "volume": "gv0"} | ||
| 21 | |||
| 22 | Of course the root cause problem still exists, but now we know | ||
| 23 | what actually needs fixing. | ||
| 24 | |||
| 25 | Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> | ||
| 26 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
| 27 | Message-id: 20180206105204.14817-1-berrange@redhat.com | ||
| 28 | Signed-off-by: Jeff Cody <jcody@redhat.com> | ||
| 18 | --- | 29 | --- |
| 19 | block/io.c | 4 ++-- | 30 | block/gluster.c | 2 +- |
| 20 | 1 file changed, 2 insertions(+), 2 deletions(-) | 31 | block/sheepdog.c | 2 +- |
| 32 | 2 files changed, 2 insertions(+), 2 deletions(-) | ||
| 21 | 33 | ||
| 22 | diff --git a/block/io.c b/block/io.c | 34 | diff --git a/block/gluster.c b/block/gluster.c |
| 23 | index XXXXXXX..XXXXXXX 100644 | 35 | index XXXXXXX..XXXXXXX 100644 |
| 24 | --- a/block/io.c | 36 | --- a/block/gluster.c |
| 25 | +++ b/block/io.c | 37 | +++ b/block/gluster.c |
| 26 | @@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, | 38 | @@ -XXX,XX +XXX,XX @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf, |
| 27 | 39 | if (filename) { | |
| 28 | if (ret & BDRV_BLOCK_RAW) { | 40 | ret = qemu_gluster_parse_uri(gconf, filename); |
| 29 | assert(ret & BDRV_BLOCK_OFFSET_VALID); | 41 | if (ret < 0) { |
| 30 | - ret = bdrv_get_block_status(*file, ret >> BDRV_SECTOR_BITS, | 42 | - error_setg(errp, "invalid URI"); |
| 31 | - *pnum, pnum, file); | 43 | + error_setg(errp, "invalid URI %s", filename); |
| 32 | + ret = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS, | 44 | error_append_hint(errp, "Usage: file=gluster[+transport]://" |
| 33 | + *pnum, pnum, file); | 45 | "[host[:port]]volume/path[?socket=...]" |
| 46 | "[,file.debug=N]" | ||
| 47 | diff --git a/block/sheepdog.c b/block/sheepdog.c | ||
| 48 | index XXXXXXX..XXXXXXX 100644 | ||
| 49 | --- a/block/sheepdog.c | ||
| 50 | +++ b/block/sheepdog.c | ||
| 51 | @@ -XXX,XX +XXX,XX @@ static void sd_parse_uri(SheepdogConfig *cfg, const char *filename, | ||
| 52 | |||
| 53 | cfg->uri = uri = uri_parse(filename); | ||
| 54 | if (!uri) { | ||
| 55 | - error_setg(&err, "invalid URI"); | ||
| 56 | + error_setg(&err, "invalid URI '%s'", filename); | ||
| 34 | goto out; | 57 | goto out; |
| 35 | } | 58 | } |
| 36 | 59 | ||
| 37 | -- | 60 | -- |
| 38 | 2.9.3 | 61 | 2.13.6 |
| 39 | 62 | ||
| 40 | 63 | diff view generated by jsdifflib |
Deleted patch | |||
|---|---|---|---|
| 1 | The main loop uses aio_disable_external()/aio_enable_external() to | ||
| 2 | temporarily disable processing of external AioContext clients like | ||
| 3 | device emulation. | ||
| 4 | 1 | ||
| 5 | This allows monitor commands to quiesce I/O and prevent the guest from | ||
| 6 | submitting new requests while a monitor command is in progress. | ||
| 7 | |||
| 8 | The aio_enable_external() API is currently broken when an IOThread is in | ||
| 9 | aio_poll() waiting for fd activity when the main loop re-enables | ||
| 10 | external clients. Incrementing ctx->external_disable_cnt does not wake | ||
| 11 | the IOThread from ppoll(2) so fd processing remains suspended and leads | ||
| 12 | to unresponsive emulated devices. | ||
| 13 | |||
| 14 | This patch adds an aio_notify() call to aio_enable_external() so the | ||
| 15 | IOThread is kicked out of ppoll(2) and will re-arm the file descriptors. | ||
| 16 | |||
| 17 | The bug can be reproduced as follows: | ||
| 18 | |||
| 19 | $ qemu -M accel=kvm -m 1024 \ | ||
| 20 | -object iothread,id=iothread0 \ | ||
| 21 | -device virtio-scsi-pci,iothread=iothread0,id=virtio-scsi-pci0 \ | ||
| 22 | -drive if=none,id=drive0,aio=native,cache=none,format=raw,file=test.img \ | ||
| 23 | -device scsi-hd,id=scsi-hd0,drive=drive0 \ | ||
| 24 | -qmp tcp::5555,server,nowait | ||
| 25 | |||
| 26 | $ scripts/qmp/qmp-shell localhost:5555 | ||
| 27 | (qemu) blockdev-snapshot-sync device=drive0 snapshot-file=sn1.qcow2 | ||
| 28 | mode=absolute-paths format=qcow2 | ||
| 29 | |||
| 30 | After blockdev-snapshot-sync completes the SCSI disk will be | ||
| 31 | unresponsive. This leads to request timeouts inside the guest. | ||
| 32 | |||
| 33 | Reported-by: Qianqian Zhu <qizhu@redhat.com> | ||
| 34 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
| 35 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 36 | Message-id: 20170508180705.20609-1-stefanha@redhat.com | ||
| 37 | Suggested-by: Fam Zheng <famz@redhat.com> | ||
| 38 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 39 | --- | ||
| 40 | include/block/aio.h | 10 ++++++++-- | ||
| 41 | 1 file changed, 8 insertions(+), 2 deletions(-) | ||
| 42 | |||
| 43 | diff --git a/include/block/aio.h b/include/block/aio.h | ||
| 44 | index XXXXXXX..XXXXXXX 100644 | ||
| 45 | --- a/include/block/aio.h | ||
| 46 | +++ b/include/block/aio.h | ||
| 47 | @@ -XXX,XX +XXX,XX @@ static inline void aio_disable_external(AioContext *ctx) | ||
| 48 | */ | ||
| 49 | static inline void aio_enable_external(AioContext *ctx) | ||
| 50 | { | ||
| 51 | - assert(ctx->external_disable_cnt > 0); | ||
| 52 | - atomic_dec(&ctx->external_disable_cnt); | ||
| 53 | + int old; | ||
| 54 | + | ||
| 55 | + old = atomic_fetch_dec(&ctx->external_disable_cnt); | ||
| 56 | + assert(old > 0); | ||
| 57 | + if (old == 1) { | ||
| 58 | + /* Kick event loop so it re-arms file descriptors */ | ||
| 59 | + aio_notify(ctx); | ||
| 60 | + } | ||
| 61 | } | ||
| 62 | |||
| 63 | /** | ||
| 64 | -- | ||
| 65 | 2.9.3 | ||
| 66 | |||
| 67 | diff view generated by jsdifflib |