Series comparison

-[Qemu-devel] [PULL 0/7] Block patches
+[PULL 0/1] Block patches
-The following changes since commit 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6:
+The following changes since commit 41192db338588051f21501abc13743e62b0a5605:
-  Merge remote-tracking branch 'remotes/cleber/tags/python-next-pull-request' into staging (2019-03-07 16:16:02 +0000)
+  Merge remote-tracking branch 'remotes/ehabkost-gl/tags/machine-next-pull-request' into staging (2021-01-01 22:57:15 +0000)
 are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 6ca206204fa773c8626d59caf2a5676d6cc35f52:
+for you to fetch changes up to 593621f36b716eb091c4ec791db72dd4461789da:
-  iothread: document about why we need explicit aio_poll() (2019-03-08 10:20:57 +0000)
+  readline: Fix possible array index out of bounds in readline_hist_add() (2021-01-04 11:13:39 +0000)
 ----------------------------------------------------------------
 Pull request
 ----------------------------------------------------------------
-Anastasiia Rusakova (1):
+Alex Chen (1):
-  hw/block/virtio-blk: Clean req->dev repetitions
+  readline: Fix possible array index out of bounds in
     readline_hist_add()
-Peter Xu (5):
+ util/readline.c | 3 +++
-  iothread: replace init_done_cond with a semaphore
+file changed, 3 insertions(+)
   iothread: create the gcontext unconditionally
   iothread: create main loop unconditionally
   iothread: push gcontext earlier in the thread_fn
   iothread: document about why we need explicit aio_poll()
 Stefan Hajnoczi (1):
   MAINTAINERS: add missing support status fields
  MAINTAINERS               |  3 ++
  include/sysemu/iothread.h |  5 +--
  hw/block/virtio-blk.c     | 16 ++++---
  iothread.c                | 90 +++++++++++++++++++--------------------
 files changed, 57 insertions(+), 57 deletions(-)
 --
-.20.1
+.29.2

-[Qemu-devel] [PULL 1/7] MAINTAINERS: add missing support status fields
+[PULL 1/1] readline: Fix possible array index out of bounds in readline_hist_add()
-This patch adds the "S:" line for areas of the codebase that currently
+From: Alex Chen <alex.chen@huawei.com>
 lack a support status field.
-Note that there are a few more areas that are more abstract and do not
+When the 'cmdline' is the last entry in 'rs->history' array, there is
-correspond to a specific set of files.  They have not been modified.
+no need to put this entry to the end of the array, partly because it is
 the last entry, and partly because the next operition will lead to array
 index out of bounds.
-Cc: Alex Bennée <alex.bennee@linaro.org>
+Reported-by: Euler Robot <euler.robot@huawei.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: Alex Chen <alex.chen@huawei.com>
-Reviewed-by: Thomas Huth <thuth@redhat.com>
+Message-id: 20201203135043.117072-1-alex.chen@huawei.com
 Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
 Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20190301163518.20702-1-stefanha@redhat.com
 Message-Id: <20190301163518.20702-1-stefanha@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- MAINTAINERS | 3 +++
+ util/readline.c | 3 +++
 file changed, 3 insertions(+)
-diff --git a/MAINTAINERS b/MAINTAINERS
+diff --git a/util/readline.c b/util/readline.c
 index XXXXXXX..XXXXXXX 100644
---- a/MAINTAINERS
+--- a/util/readline.c
-+++ b/MAINTAINERS
++++ b/util/readline.c
-@@ -XXX,XX +XXX,XX @@ F: include/hw/tricore/
+@@ -XXX,XX +XXX,XX @@ static void readline_hist_add(ReadLineState *rs, const char *cmdline)
+         }
- Multiarch Linux User Tests
+         if (strcmp(hist_entry, cmdline) == 0) {
- M: Alex Bennée <alex.bennee@linaro.org>
+         same_entry:
-+S: Maintained
++            if (idx == READLINE_MAX_CMDS - 1) {
- F: tests/tcg/multiarch/
++                return;
++            }
- Guest CPU Cores (KVM):
+             new_entry = hist_entry;
-@@ -XXX,XX +XXX,XX @@ F: qemu.sasl
+             /* Put this entry at the end of history */
- Coroutines
+             memmove(&rs->history[idx], &rs->history[idx + 1],
  M: Stefan Hajnoczi <stefanha@redhat.com>
  M: Kevin Wolf <kwolf@redhat.com>
 +S: Maintained
  F: util/*coroutine*
  F: include/qemu/coroutine*
  F: tests/test-coroutine.c
@@ -XXX,XX +XXX,XX @@ F: .gitlab-ci.yml
  Guest Test Compilation Support
  M: Alex Bennée <alex.bennee@linaro.org>
  R: Philippe Mathieu-Daudé <f4bug@amsat.org>
 +S: Maintained
  F: tests/tcg/Makefile
  F: tests/tcg/Makefile.include
  L: qemu-devel@nongnu.org
 --
-.20.1
+.29.2

-[Qemu-devel] [PULL 2/7] hw/block/virtio-blk: Clean req->dev repetitions
+Deleted patch
-From: Anastasiia Rusakova <arusakova917@gmail.com>
-Some functions sometimes uses req->dev even though a local variable
-VirtIOBlock* s = req->dev has already been defined.
-Updated places to use s everywhere in the file.
-Signed-off-by: Anastasiia Rusakova <arusakova917@gmail.com>
-Message-id: 20190307161925.4158-1-rusakova.nastasia@icloud.com
-Message-Id: <20190307161925.4158-1-rusakova.nastasia@icloud.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- hw/block/virtio-blk.c | 16 +++++++++-------
-file changed, 9 insertions(+), 7 deletions(-)
-diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/block/virtio-blk.c
-+++ b/hw/block/virtio-blk.c
-@@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret)
-         }
-         if (ret) {
--            int p = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type);
-+            int p = virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type);
-             bool is_read = !(p & VIRTIO_BLK_T_OUT);
-             /* Note that memory may be dirtied on read failure.  If the
-              * virtio request is not completed here, as is the case for
-@@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret)
-         }
-         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
--        block_acct_done(blk_get_stats(req->dev->blk), &req->acct);
-+        block_acct_done(blk_get_stats(s->blk), &req->acct);
-         virtio_blk_free_request(req);
-     }
-     aio_context_release(blk_get_aio_context(s->conf.conf.blk));
-@@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
- {
-     int status = VIRTIO_BLK_S_OK;
-     struct virtio_scsi_inhdr *scsi = NULL;
--    VirtIODevice *vdev = VIRTIO_DEVICE(req->dev);
--    VirtQueueElement *elem = &req->elem;
-     VirtIOBlock *blk = req->dev;
-+    VirtIODevice *vdev = VIRTIO_DEVICE(blk);
-+    VirtQueueElement *elem = &req->elem;
- #ifdef __linux__
-     int i;
-@@ -XXX,XX +XXX,XX @@ static void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
- static void virtio_blk_handle_flush(VirtIOBlockReq *req, MultiReqBuffer *mrb)
- {
--    block_acct_start(blk_get_stats(req->dev->blk), &req->acct, 0,
-+    VirtIOBlock *s = req->dev;
-+
-+    block_acct_start(blk_get_stats(s->blk), &req->acct, 0,
-                      BLOCK_ACCT_FLUSH);
-     /*
-      * Make sure all outstanding writes are posted to the backing device.
-      */
-     if (mrb->is_write && mrb->num_reqs > 0) {
--        virtio_blk_submit_multireq(req->dev->blk, mrb);
-+        virtio_blk_submit_multireq(s->blk, mrb);
-     }
--    blk_aio_flush(req->dev->blk, virtio_blk_flush_complete, req);
-+    blk_aio_flush(s->blk, virtio_blk_flush_complete, req);
- }
- static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
---
-.20.1

-[Qemu-devel] [PULL 3/7] iothread: replace init_done_cond with a semaphore
+Deleted patch
-From: Peter Xu <peterx@redhat.com>
-Only sending an init-done message using lock+cond seems an overkill to
-me.  Replacing it with a simpler semaphore.
-Meanwhile, init the semaphore unconditionally, then we can destroy it
-unconditionally too in finalize which seems cleaner.
-Signed-off-by: Peter Xu <peterx@redhat.com>
-Message-id: 20190306115532.23025-2-peterx@redhat.com
-Message-Id: <20190306115532.23025-2-peterx@redhat.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- include/sysemu/iothread.h |  3 +--
- iothread.c                | 17 ++++-------------
-files changed, 5 insertions(+), 15 deletions(-)
-diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/sysemu/iothread.h
-+++ b/include/sysemu/iothread.h
-@@ -XXX,XX +XXX,XX @@ typedef struct {
-     GMainContext *worker_context;
-     GMainLoop *main_loop;
-     GOnce once;
--    QemuMutex init_done_lock;
--    QemuCond init_done_cond;    /* is thread initialization done? */
-+    QemuSemaphore init_done_sem; /* is thread init done? */
-     bool stopping;              /* has iothread_stop() been called? */
-     bool running;               /* should iothread_run() continue? */
-     int thread_id;
-diff --git a/iothread.c b/iothread.c
-index XXXXXXX..XXXXXXX 100644
---- a/iothread.c
-+++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
-     rcu_register_thread();
-     my_iothread = iothread;
--    qemu_mutex_lock(&iothread->init_done_lock);
-     iothread->thread_id = qemu_get_thread_id();
--    qemu_cond_signal(&iothread->init_done_cond);
--    qemu_mutex_unlock(&iothread->init_done_lock);
-+    qemu_sem_post(&iothread->init_done_sem);
-     while (iothread->running) {
-         aio_poll(iothread->ctx, true);
-@@ -XXX,XX +XXX,XX @@ static void iothread_instance_init(Object *obj)
-     iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT;
-     iothread->thread_id = -1;
-+    qemu_sem_init(&iothread->init_done_sem, 0);
- }
- static void iothread_instance_finalize(Object *obj)
-@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
-     iothread_stop(iothread);
--    if (iothread->thread_id != -1) {
--        qemu_cond_destroy(&iothread->init_done_cond);
--        qemu_mutex_destroy(&iothread->init_done_lock);
--    }
-     /*
-      * Before glib2 2.33.10, there is a glib2 bug that GSource context
-      * pointer may not be cleared even if the context has already been
-@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
-         g_main_context_unref(iothread->worker_context);
-         iothread->worker_context = NULL;
-     }
-+    qemu_sem_destroy(&iothread->init_done_sem);
- }
- static void iothread_complete(UserCreatable *obj, Error **errp)
-@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-         return;
-     }
--    qemu_mutex_init(&iothread->init_done_lock);
--    qemu_cond_init(&iothread->init_done_cond);
-     iothread->once = (GOnce) G_ONCE_INIT;
-     /* This assumes we are called from a thread with useful CPU affinity for us
-@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-     g_free(name);
-     /* Wait for initialization to complete */
--    qemu_mutex_lock(&iothread->init_done_lock);
-     while (iothread->thread_id == -1) {
--        qemu_cond_wait(&iothread->init_done_cond,
--                       &iothread->init_done_lock);
-+        qemu_sem_wait(&iothread->init_done_sem);
-     }
--    qemu_mutex_unlock(&iothread->init_done_lock);
- }
- typedef struct {
---
-.20.1

-[Qemu-devel] [PULL 4/7] iothread: create the gcontext unconditionally
+Deleted patch
-From: Peter Xu <peterx@redhat.com>
-In existing code we create the gcontext dynamically at the first
-access of the gcontext from caller.  That can bring some complexity
-and potential races during using iothread.  Since the context itself
-is not that big a resource, and we won't have millions of iothread,
-let's simply create the gcontext unconditionally.
-This will also be a preparation work further to move the thread
-context push operation earlier than before (now it's only pushed right
-before we want to start running the gmainloop).
-Removing the g_once since it's not necessary, while introducing a new
-run_gcontext boolean to show whether we want to run the gcontext.
-Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
-Signed-off-by: Peter Xu <peterx@redhat.com>
-Message-id: 20190306115532.23025-3-peterx@redhat.com
-Message-Id: <20190306115532.23025-3-peterx@redhat.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- include/sysemu/iothread.h |  2 +-
- iothread.c                | 43 +++++++++++++++++++--------------------
-files changed, 22 insertions(+), 23 deletions(-)
-diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/sysemu/iothread.h
-+++ b/include/sysemu/iothread.h
-@@ -XXX,XX +XXX,XX @@ typedef struct {
-     QemuThread thread;
-     AioContext *ctx;
-+    bool run_gcontext;          /* whether we should run gcontext */
-     GMainContext *worker_context;
-     GMainLoop *main_loop;
--    GOnce once;
-     QemuSemaphore init_done_sem; /* is thread init done? */
-     bool stopping;              /* has iothread_stop() been called? */
-     bool running;               /* should iothread_run() continue? */
-diff --git a/iothread.c b/iothread.c
-index XXXXXXX..XXXXXXX 100644
---- a/iothread.c
-+++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
-          * We must check the running state again in case it was
-          * changed in previous aio_poll()
-          */
--        if (iothread->running && atomic_read(&iothread->worker_context)) {
-+        if (iothread->running && atomic_read(&iothread->run_gcontext)) {
-             GMainLoop *loop;
-             g_main_context_push_thread_default(iothread->worker_context);
-@@ -XXX,XX +XXX,XX @@ static void iothread_instance_init(Object *obj)
-     iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT;
-     iothread->thread_id = -1;
-     qemu_sem_init(&iothread->init_done_sem, 0);
-+    /* By default, we don't run gcontext */
-+    atomic_set(&iothread->run_gcontext, 0);
- }
- static void iothread_instance_finalize(Object *obj)
-@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
-     qemu_sem_destroy(&iothread->init_done_sem);
- }
-+static void iothread_init_gcontext(IOThread *iothread)
-+{
-+    GSource *source;
-+
-+    iothread->worker_context = g_main_context_new();
-+    source = aio_get_g_source(iothread_get_aio_context(iothread));
-+    g_source_attach(source, iothread->worker_context);
-+    g_source_unref(source);
-+}
-+
- static void iothread_complete(UserCreatable *obj, Error **errp)
- {
-     Error *local_error = NULL;
-@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-         return;
-     }
-+    /*
-+     * Init one GMainContext for the iothread unconditionally, even if
-+     * it's not used
-+     */
-+    iothread_init_gcontext(iothread);
-+
-     aio_context_set_poll_params(iothread->ctx,
-                                 iothread->poll_max_ns,
-                                 iothread->poll_grow,
-@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
-         return;
-     }
--    iothread->once = (GOnce) G_ONCE_INIT;
--
-     /* This assumes we are called from a thread with useful CPU affinity for us
-      * to inherit.
-      */
-@@ -XXX,XX +XXX,XX @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)
-     return head;
- }
--static gpointer iothread_g_main_context_init(gpointer opaque)
--{
--    AioContext *ctx;
--    IOThread *iothread = opaque;
--    GSource *source;
--
--    iothread->worker_context = g_main_context_new();
--
--    ctx = iothread_get_aio_context(iothread);
--    source = aio_get_g_source(ctx);
--    g_source_attach(source, iothread->worker_context);
--    g_source_unref(source);
--
--    aio_notify(iothread->ctx);
--    return NULL;
--}
--
- GMainContext *iothread_get_g_main_context(IOThread *iothread)
- {
--    g_once(&iothread->once, iothread_g_main_context_init, iothread);
--
-+    atomic_set(&iothread->run_gcontext, 1);
-+    aio_notify(iothread->ctx);
-     return iothread->worker_context;
- }
---
-.20.1

-[Qemu-devel] [PULL 5/7] iothread: create main loop unconditionally
+Deleted patch
-From: Peter Xu <peterx@redhat.com>
-Since we've have the gcontext always there, create the main loop
-altogether.  The iothread_run() is even cleaner.
-Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
-Signed-off-by: Peter Xu <peterx@redhat.com>
-Message-id: 20190306115532.23025-4-peterx@redhat.com
-Message-Id: <20190306115532.23025-4-peterx@redhat.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- iothread.c | 12 +++---------
-file changed, 3 insertions(+), 9 deletions(-)
-diff --git a/iothread.c b/iothread.c
-index XXXXXXX..XXXXXXX 100644
---- a/iothread.c
-+++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
-          * changed in previous aio_poll()
-          */
-         if (iothread->running && atomic_read(&iothread->run_gcontext)) {
--            GMainLoop *loop;
--
-             g_main_context_push_thread_default(iothread->worker_context);
--            iothread->main_loop =
--                g_main_loop_new(iothread->worker_context, TRUE);
--            loop = iothread->main_loop;
--
-             g_main_loop_run(iothread->main_loop);
--            iothread->main_loop = NULL;
--            g_main_loop_unref(loop);
--
-             g_main_context_pop_thread_default(iothread->worker_context);
-         }
-     }
-@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
-     if (iothread->worker_context) {
-         g_main_context_unref(iothread->worker_context);
-         iothread->worker_context = NULL;
-+        g_main_loop_unref(iothread->main_loop);
-+        iothread->main_loop = NULL;
-     }
-     qemu_sem_destroy(&iothread->init_done_sem);
- }
-@@ -XXX,XX +XXX,XX @@ static void iothread_init_gcontext(IOThread *iothread)
-     source = aio_get_g_source(iothread_get_aio_context(iothread));
-     g_source_attach(source, iothread->worker_context);
-     g_source_unref(source);
-+    iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE);
- }
- static void iothread_complete(UserCreatable *obj, Error **errp)
---
-.20.1

-[Qemu-devel] [PULL 6/7] iothread: push gcontext earlier in the thread_fn
+Deleted patch
-From: Peter Xu <peterx@redhat.com>
-We were pushing the context until right before running the gmainloop.
-Now since we have everything unconditionally, we can move this
-earlier.
-One benefit is that now it's done even before init_done_sem, so as
-long as the iothread user calls iothread_create() and completes, we
-know that the thread stack is ready.
-Signed-off-by: Peter Xu <peterx@redhat.com>
-Message-id: 20190306115532.23025-5-peterx@redhat.com
-Message-Id: <20190306115532.23025-5-peterx@redhat.com>
-[Tweaked comment wording as discussed with Peter Xu.
---Stefan]
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- iothread.c | 9 ++++++---
-file changed, 6 insertions(+), 3 deletions(-)
-diff --git a/iothread.c b/iothread.c
-index XXXXXXX..XXXXXXX 100644
---- a/iothread.c
-+++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
-     IOThread *iothread = opaque;
-     rcu_register_thread();
--
-+    /*
-+     * g_main_context_push_thread_default() must be called before anything
-+     * in this new thread uses glib.
-+     */
-+    g_main_context_push_thread_default(iothread->worker_context);
-     my_iothread = iothread;
-     iothread->thread_id = qemu_get_thread_id();
-     qemu_sem_post(&iothread->init_done_sem);
-@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
-          * changed in previous aio_poll()
-          */
-         if (iothread->running && atomic_read(&iothread->run_gcontext)) {
--            g_main_context_push_thread_default(iothread->worker_context);
-             g_main_loop_run(iothread->main_loop);
--            g_main_context_pop_thread_default(iothread->worker_context);
-         }
-     }
-+    g_main_context_pop_thread_default(iothread->worker_context);
-     rcu_unregister_thread();
-     return NULL;
- }
---
-.20.1

-[Qemu-devel] [PULL 7/7] iothread: document about why we need explicit aio_poll()
+Deleted patch
-From: Peter Xu <peterx@redhat.com>
-After consulting Paolo I know why we'd better keep the explicit
-aio_poll() in iothread_run().  Document it directly into the code so
-that future readers will know the answer from day one.
-Signed-off-by: Peter Xu <peterx@redhat.com>
-Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
-Message-id: 20190306115532.23025-6-peterx@redhat.com
-Message-Id: <20190306115532.23025-6-peterx@redhat.com>
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- iothread.c | 9 +++++++++
-file changed, 9 insertions(+)
-diff --git a/iothread.c b/iothread.c
-index XXXXXXX..XXXXXXX 100644
---- a/iothread.c
-+++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
-     qemu_sem_post(&iothread->init_done_sem);
-     while (iothread->running) {
-+        /*
-+         * Note: from functional-wise the g_main_loop_run() below can
-+         * already cover the aio_poll() events, but we can't run the
-+         * main loop unconditionally because explicit aio_poll() here
-+         * is faster than g_main_loop_run() when we do not need the
-+         * gcontext at all (e.g., pure block layer iothreads).  In
-+         * other words, when we want to run the gcontext with the
-+         * iothread we need to pay some performance for functionality.
-+         */
-         aio_poll(iothread->ctx, true);
-         /*
---
-.20.1

The following changes since commit 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6:

Merge remote-tracking branch 'remotes/cleber/tags/python-next-pull-request' into staging (2019-03-07 16:16:02 +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 6ca206204fa773c8626d59caf2a5676d6cc35f52:

iothread: document about why we need explicit aio_poll() (2019-03-08 10:20:57 +0000)

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

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

Anastasiia Rusakova (1):
  hw/block/virtio-blk: Clean req->dev repetitions

Peter Xu (5):
  iothread: replace init_done_cond with a semaphore
  iothread: create the gcontext unconditionally
  iothread: create main loop unconditionally
  iothread: push gcontext earlier in the thread_fn
  iothread: document about why we need explicit aio_poll()

Stefan Hajnoczi (1):
  MAINTAINERS: add missing support status fields

MAINTAINERS               |  3 ++
 include/sysemu/iothread.h |  5 +--
 hw/block/virtio-blk.c     | 16 ++++---
 iothread.c                | 90 +++++++++++++++++++--------------------
 4 files changed, 57 insertions(+), 57 deletions(-)

-- 
2.20.1

This patch adds the "S:" line for areas of the codebase that currently
lack a support status field.

Note that there are a few more areas that are more abstract and do not
correspond to a specific set of files.  They have not been modified.

Cc: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190301163518.20702-1-stefanha@redhat.com
Message-Id: <20190301163518.20702-1-stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index XXXXXXX..XXXXXXX 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -XXX,XX +XXX,XX @@ F: include/hw/tricore/
 
 Multiarch Linux User Tests
 M: Alex Bennée <alex.bennee@linaro.org>
+S: Maintained
 F: tests/tcg/multiarch/
 
 Guest CPU Cores (KVM):
@@ -XXX,XX +XXX,XX @@ F: qemu.sasl
 Coroutines
 M: Stefan Hajnoczi <stefanha@redhat.com>
 M: Kevin Wolf <kwolf@redhat.com>
+S: Maintained
 F: util/*coroutine*
 F: include/qemu/coroutine*
 F: tests/test-coroutine.c
@@ -XXX,XX +XXX,XX @@ F: .gitlab-ci.yml
 Guest Test Compilation Support
 M: Alex Bennée <alex.bennee@linaro.org>
 R: Philippe Mathieu-Daudé <f4bug@amsat.org>
+S: Maintained
 F: tests/tcg/Makefile
 F: tests/tcg/Makefile.include
 L: qemu-devel@nongnu.org
-- 
2.20.1

From: Anastasiia Rusakova <arusakova917@gmail.com>

Some functions sometimes uses req->dev even though a local variable
VirtIOBlock* s = req->dev has already been defined.
Updated places to use s everywhere in the file.

Signed-off-by: Anastasiia Rusakova <arusakova917@gmail.com>
Message-id: 20190307161925.4158-1-rusakova.nastasia@icloud.com
Message-Id: <20190307161925.4158-1-rusakova.nastasia@icloud.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret)
         }
 
         if (ret) {
-            int p = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type);
+            int p = virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type);
             bool is_read = !(p & VIRTIO_BLK_T_OUT);
             /* Note that memory may be dirtied on read failure.  If the
              * virtio request is not completed here, as is the case for
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret)
         }
 
         virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
-        block_acct_done(blk_get_stats(req->dev->blk), &req->acct);
+        block_acct_done(blk_get_stats(s->blk), &req->acct);
         virtio_blk_free_request(req);
     }
     aio_context_release(blk_get_aio_context(s->conf.conf.blk));
@@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req)
 {
     int status = VIRTIO_BLK_S_OK;
     struct virtio_scsi_inhdr *scsi = NULL;
-    VirtIODevice *vdev = VIRTIO_DEVICE(req->dev);
-    VirtQueueElement *elem = &req->elem;
     VirtIOBlock *blk = req->dev;
+    VirtIODevice *vdev = VIRTIO_DEVICE(blk);
+    VirtQueueElement *elem = &req->elem;
 
 #ifdef __linux__
     int i;
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb)
 
 static void virtio_blk_handle_flush(VirtIOBlockReq *req, MultiReqBuffer *mrb)
 {
-    block_acct_start(blk_get_stats(req->dev->blk), &req->acct, 0,
+    VirtIOBlock *s = req->dev;
+
+    block_acct_start(blk_get_stats(s->blk), &req->acct, 0,
                      BLOCK_ACCT_FLUSH);
 
     /*
      * Make sure all outstanding writes are posted to the backing device.
      */
     if (mrb->is_write && mrb->num_reqs > 0) {
-        virtio_blk_submit_multireq(req->dev->blk, mrb);
+        virtio_blk_submit_multireq(s->blk, mrb);
     }
-    blk_aio_flush(req->dev->blk, virtio_blk_flush_complete, req);
+    blk_aio_flush(s->blk, virtio_blk_flush_complete, req);
 }
 
 static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
-- 
2.20.1

From: Peter Xu <peterx@redhat.com>

Only sending an init-done message using lock+cond seems an overkill to
me.  Replacing it with a simpler semaphore.

Meanwhile, init the semaphore unconditionally, then we can destroy it
unconditionally too in finalize which seems cleaner.

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20190306115532.23025-2-peterx@redhat.com
Message-Id: <20190306115532.23025-2-peterx@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/iothread.h |  3 +--
 iothread.c                | 17 ++++-------------
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
     GMainContext *worker_context;
     GMainLoop *main_loop;
     GOnce once;
-    QemuMutex init_done_lock;
-    QemuCond init_done_cond;    /* is thread initialization done? */
+    QemuSemaphore init_done_sem; /* is thread init done? */
     bool stopping;              /* has iothread_stop() been called? */
     bool running;               /* should iothread_run() continue? */
     int thread_id;
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
     rcu_register_thread();
 
     my_iothread = iothread;
-    qemu_mutex_lock(&iothread->init_done_lock);
     iothread->thread_id = qemu_get_thread_id();
-    qemu_cond_signal(&iothread->init_done_cond);
-    qemu_mutex_unlock(&iothread->init_done_lock);
+    qemu_sem_post(&iothread->init_done_sem);
 
     while (iothread->running) {
         aio_poll(iothread->ctx, true);
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_init(Object *obj)
 
     iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT;
     iothread->thread_id = -1;
+    qemu_sem_init(&iothread->init_done_sem, 0);
 }
 
 static void iothread_instance_finalize(Object *obj)
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
 
     iothread_stop(iothread);
 
-    if (iothread->thread_id != -1) {
-        qemu_cond_destroy(&iothread->init_done_cond);
-        qemu_mutex_destroy(&iothread->init_done_lock);
-    }
     /*
      * Before glib2 2.33.10, there is a glib2 bug that GSource context
      * pointer may not be cleared even if the context has already been
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
         g_main_context_unref(iothread->worker_context);
         iothread->worker_context = NULL;
     }
+    qemu_sem_destroy(&iothread->init_done_sem);
 }
 
 static void iothread_complete(UserCreatable *obj, Error **errp)
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
         return;
     }
 
-    qemu_mutex_init(&iothread->init_done_lock);
-    qemu_cond_init(&iothread->init_done_cond);
     iothread->once = (GOnce) G_ONCE_INIT;
 
     /* This assumes we are called from a thread with useful CPU affinity for us
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
     g_free(name);
 
     /* Wait for initialization to complete */
-    qemu_mutex_lock(&iothread->init_done_lock);
     while (iothread->thread_id == -1) {
-        qemu_cond_wait(&iothread->init_done_cond,
-                       &iothread->init_done_lock);
+        qemu_sem_wait(&iothread->init_done_sem);
     }
-    qemu_mutex_unlock(&iothread->init_done_lock);
 }
 
 typedef struct {
-- 
2.20.1

From: Peter Xu <peterx@redhat.com>

In existing code we create the gcontext dynamically at the first
access of the gcontext from caller.  That can bring some complexity
and potential races during using iothread.  Since the context itself
is not that big a resource, and we won't have millions of iothread,
let's simply create the gcontext unconditionally.

This will also be a preparation work further to move the thread
context push operation earlier than before (now it's only pushed right
before we want to start running the gmainloop).

Removing the g_once since it's not necessary, while introducing a new
run_gcontext boolean to show whether we want to run the gcontext.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20190306115532.23025-3-peterx@redhat.com
Message-Id: <20190306115532.23025-3-peterx@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/sysemu/iothread.h |  2 +-
 iothread.c                | 43 +++++++++++++++++++--------------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
index XXXXXXX..XXXXXXX 100644
--- a/include/sysemu/iothread.h
+++ b/include/sysemu/iothread.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
 
     QemuThread thread;
     AioContext *ctx;
+    bool run_gcontext;          /* whether we should run gcontext */
     GMainContext *worker_context;
     GMainLoop *main_loop;
-    GOnce once;
     QemuSemaphore init_done_sem; /* is thread init done? */
     bool stopping;              /* has iothread_stop() been called? */
     bool running;               /* should iothread_run() continue? */
diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
          * We must check the running state again in case it was
          * changed in previous aio_poll()
          */
-        if (iothread->running && atomic_read(&iothread->worker_context)) {
+        if (iothread->running && atomic_read(&iothread->run_gcontext)) {
             GMainLoop *loop;
 
             g_main_context_push_thread_default(iothread->worker_context);
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_init(Object *obj)
     iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT;
     iothread->thread_id = -1;
     qemu_sem_init(&iothread->init_done_sem, 0);
+    /* By default, we don't run gcontext */
+    atomic_set(&iothread->run_gcontext, 0);
 }
 
 static void iothread_instance_finalize(Object *obj)
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
     qemu_sem_destroy(&iothread->init_done_sem);
 }
 
+static void iothread_init_gcontext(IOThread *iothread)
+{
+    GSource *source;
+
+    iothread->worker_context = g_main_context_new();
+    source = aio_get_g_source(iothread_get_aio_context(iothread));
+    g_source_attach(source, iothread->worker_context);
+    g_source_unref(source);
+}
+
 static void iothread_complete(UserCreatable *obj, Error **errp)
 {
     Error *local_error = NULL;
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
         return;
     }
 
+    /*
+     * Init one GMainContext for the iothread unconditionally, even if
+     * it's not used
+     */
+    iothread_init_gcontext(iothread);
+
     aio_context_set_poll_params(iothread->ctx,
                                 iothread->poll_max_ns,
                                 iothread->poll_grow,
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
         return;
     }
 
-    iothread->once = (GOnce) G_ONCE_INIT;
-
     /* This assumes we are called from a thread with useful CPU affinity for us
      * to inherit.
      */
@@ -XXX,XX +XXX,XX @@ IOThreadInfoList *qmp_query_iothreads(Error **errp)
     return head;
 }
 
-static gpointer iothread_g_main_context_init(gpointer opaque)
-{
-    AioContext *ctx;
-    IOThread *iothread = opaque;
-    GSource *source;
-
-    iothread->worker_context = g_main_context_new();
-
-    ctx = iothread_get_aio_context(iothread);
-    source = aio_get_g_source(ctx);
-    g_source_attach(source, iothread->worker_context);
-    g_source_unref(source);
-
-    aio_notify(iothread->ctx);
-    return NULL;
-}
-
 GMainContext *iothread_get_g_main_context(IOThread *iothread)
 {
-    g_once(&iothread->once, iothread_g_main_context_init, iothread);
-
+    atomic_set(&iothread->run_gcontext, 1);
+    aio_notify(iothread->ctx);
     return iothread->worker_context;
 }
 
-- 
2.20.1

From: Peter Xu <peterx@redhat.com>

Since we've have the gcontext always there, create the main loop
altogether.  The iothread_run() is even cleaner.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20190306115532.23025-4-peterx@redhat.com
Message-Id: <20190306115532.23025-4-peterx@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
          * changed in previous aio_poll()
          */
         if (iothread->running && atomic_read(&iothread->run_gcontext)) {
-            GMainLoop *loop;
-
             g_main_context_push_thread_default(iothread->worker_context);
-            iothread->main_loop =
-                g_main_loop_new(iothread->worker_context, TRUE);
-            loop = iothread->main_loop;
-
             g_main_loop_run(iothread->main_loop);
-            iothread->main_loop = NULL;
-            g_main_loop_unref(loop);
-
             g_main_context_pop_thread_default(iothread->worker_context);
         }
     }
@@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj)
     if (iothread->worker_context) {
         g_main_context_unref(iothread->worker_context);
         iothread->worker_context = NULL;
+        g_main_loop_unref(iothread->main_loop);
+        iothread->main_loop = NULL;
     }
     qemu_sem_destroy(&iothread->init_done_sem);
 }
@@ -XXX,XX +XXX,XX @@ static void iothread_init_gcontext(IOThread *iothread)
     source = aio_get_g_source(iothread_get_aio_context(iothread));
     g_source_attach(source, iothread->worker_context);
     g_source_unref(source);
+    iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE);
 }
 
 static void iothread_complete(UserCreatable *obj, Error **errp)
-- 
2.20.1

From: Peter Xu <peterx@redhat.com>

We were pushing the context until right before running the gmainloop.
Now since we have everything unconditionally, we can move this
earlier.

One benefit is that now it's done even before init_done_sem, so as
long as the iothread user calls iothread_create() and completes, we
know that the thread stack is ready.

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-id: 20190306115532.23025-5-peterx@redhat.com
Message-Id: <20190306115532.23025-5-peterx@redhat.com>

[Tweaked comment wording as discussed with Peter Xu.
--Stefan]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
     IOThread *iothread = opaque;
 
     rcu_register_thread();
-
+    /*
+     * g_main_context_push_thread_default() must be called before anything
+     * in this new thread uses glib.
+     */
+    g_main_context_push_thread_default(iothread->worker_context);
     my_iothread = iothread;
     iothread->thread_id = qemu_get_thread_id();
     qemu_sem_post(&iothread->init_done_sem);
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
          * changed in previous aio_poll()
          */
         if (iothread->running && atomic_read(&iothread->run_gcontext)) {
-            g_main_context_push_thread_default(iothread->worker_context);
             g_main_loop_run(iothread->main_loop);
-            g_main_context_pop_thread_default(iothread->worker_context);
         }
     }
 
+    g_main_context_pop_thread_default(iothread->worker_context);
     rcu_unregister_thread();
     return NULL;
 }
-- 
2.20.1

From: Peter Xu <peterx@redhat.com>

After consulting Paolo I know why we'd better keep the explicit
aio_poll() in iothread_run().  Document it directly into the code so
that future readers will know the answer from day one.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20190306115532.23025-6-peterx@redhat.com
Message-Id: <20190306115532.23025-6-peterx@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
     qemu_sem_post(&iothread->init_done_sem);
 
     while (iothread->running) {
+        /*
+         * Note: from functional-wise the g_main_loop_run() below can
+         * already cover the aio_poll() events, but we can't run the
+         * main loop unconditionally because explicit aio_poll() here
+         * is faster than g_main_loop_run() when we do not need the
+         * gcontext at all (e.g., pure block layer iothreads).  In
+         * other words, when we want to run the gcontext with the
+         * iothread we need to pay some performance for functionality.
+         */
         aio_poll(iothread->ctx, true);
 
         /*
-- 
2.20.1

From: Alex Chen <alex.chen@huawei.com>

When the 'cmdline' is the last entry in 'rs->history' array, there is
no need to put this entry to the end of the array, partly because it is
the last entry, and partly because the next operition will lead to array
index out of bounds.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Message-id: 20201203135043.117072-1-alex.chen@huawei.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/readline.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/util/readline.c b/util/readline.c
index XXXXXXX..XXXXXXX 100644
--- a/util/readline.c
+++ b/util/readline.c
@@ -XXX,XX +XXX,XX @@ static void readline_hist_add(ReadLineState *rs, const char *cmdline)
         }
         if (strcmp(hist_entry, cmdline) == 0) {
         same_entry:
+            if (idx == READLINE_MAX_CMDS - 1) {
+                return;
+            }
             new_entry = hist_entry;
             /* Put this entry at the end of history */
             memmove(&rs->history[idx], &rs->history[idx + 1],
-- 
2.29.2