Series comparison

-[Qemu-devel] [PULL 0/2] Block patches
+[PULL 0/1] Block patches
-The following changes since commit 285278ca785f5fa9a570927e1c0958a2ca2b2150:
+The following changes since commit 98b2e3c9ab3abfe476a2b02f8f51813edb90e72d:
-  Merge remote-tracking branch 'remotes/famz/tags/testing-pull-request' into staging (2018-10-27 19:55:08 +0100)
+  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-10-08 16:08:35 +0100)
 are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to bd54b11062c4baa7d2e4efadcf71b8cfd55311fd:
+for you to fetch changes up to 69de48445a0d6169f1e2a6c5bfab994e1c810e33:
-  nvdimm: Add docs hint for Linux driver name (2018-10-29 13:35:22 +0000)
+  test-bdrv-drain: fix iothread_join() hang (2019-10-14 09:48:01 +0100)
 ----------------------------------------------------------------
 Pull request
-No changelog-worthy entries, just small tweaks.
 ----------------------------------------------------------------
-Kees Cook (1):
+Stefan Hajnoczi (1):
-  nvdimm: Add docs hint for Linux driver name
+  test-bdrv-drain: fix iothread_join() hang
-Li Qiang (1):
+ tests/iothread.c | 10 ++++++++--
-  util: aio-posix: fix a typo
+file changed, 8 insertions(+), 2 deletions(-)
  docs/nvdimm.txt  | 5 +++--
  util/aio-posix.c | 2 +-
 files changed, 4 insertions(+), 3 deletions(-)
 --
-.17.2
+.21.0

-[Qemu-devel] [PULL 1/2] util: aio-posix: fix a typo
+Deleted patch
-From: Li Qiang <liq3ea@gmail.com>
-Cc: qemu-trivial@nongnu.org
-Signed-off-by: Li Qiang <liq3ea@gmail.com>
-Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
-Reviewed-by: Fam Zheng <famz@redhat.com>
-Message-id: 1538964972-3223-1-git-send-email-liq3ea@gmail.com
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- util/aio-posix.c | 2 +-
-file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/util/aio-posix.c b/util/aio-posix.c
-index XXXXXXX..XXXXXXX 100644
---- a/util/aio-posix.c
-+++ b/util/aio-posix.c
-@@ -XXX,XX +XXX,XX @@ struct AioHandler
- #ifdef CONFIG_EPOLL_CREATE1
--/* The fd number threashold to switch to epoll */
-+/* The fd number threshold to switch to epoll */
- #define EPOLL_ENABLE_THRESHOLD 64
- static void aio_epoll_disable(AioContext *ctx)
---
-.17.2

-[Qemu-devel] [PULL 2/2] nvdimm: Add docs hint for Linux driver name
+[PULL 1/1] test-bdrv-drain: fix iothread_join() hang
-From: Kees Cook <keescook@chromium.org>
+tests/test-bdrv-drain can hang in tests/iothread.c:iothread_run():
-I spent way too much time trying to figure out why the emulated NVDIMM
+  while (!atomic_read(&iothread->stopping)) {
-was missing under Linux. In an effort to help others who might be looking
+      aio_poll(iothread->ctx, true);
-for these kinds of things in the future, include a hint.
+  }
-Signed-off-by: Kees Cook <keescook@chromium.org>
+The iothread_join() function works as follows:
-Message-id: 20181018201351.GA25286@beast
   void iothread_join(IOThread *iothread)
   {
       iothread->stopping = true;
       aio_notify(iothread->ctx);
       qemu_thread_join(&iothread->thread);
 If iothread_run() checks iothread->stopping before the iothread_join()
 thread sets stopping to true, then aio_notify() may be optimized away
 and iothread_run() hangs forever in aio_poll().
 The correct way to change iothread->stopping is from a BH that executes
 within iothread_run().  This ensures that iothread->stopping is checked
 after we set it to true.
 This was already fixed for ./iothread.c (note this is a different source
 file!) by commit 2362a28ea11c145e1a13ae79342d76dc118a72a6 ("iothread:
 fix iothread_stop() race condition"), but not for tests/iothread.c.
 Fixes: 0c330a734b51c177ab8488932ac3b0c4d63a718a
        ("aio: introduce aio_co_schedule and aio_co_wake")
 Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
 Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
 Message-Id: <20191003100103.331-1-stefanha@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- docs/nvdimm.txt | 5 +++--
+ tests/iothread.c | 10 ++++++++--
-file changed, 3 insertions(+), 2 deletions(-)
+file changed, 8 insertions(+), 2 deletions(-)
-diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
+diff --git a/tests/iothread.c b/tests/iothread.c
 index XXXXXXX..XXXXXXX 100644
---- a/docs/nvdimm.txt
+--- a/tests/iothread.c
-+++ b/docs/nvdimm.txt
++++ b/tests/iothread.c
-@@ -XXX,XX +XXX,XX @@ Multiple vNVDIMM devices can be created if multiple pairs of "-object"
+@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
- and "-device" are provided.
+     return NULL;
+ }
- For above command line options, if the guest OS has the proper NVDIMM
--driver, it should be able to detect a NVDIMM device which is in the
+-void iothread_join(IOThread *iothread)
--persistent memory mode and whose size is $NVDIMM_SIZE.
++static void iothread_stop_bh(void *opaque)
-+driver (e.g. "CONFIG_ACPI_NFIT=y" under Linux), it should be able to
+ {
-+detect a NVDIMM device which is in the persistent memory mode and whose
++    IOThread *iothread = opaque;
-+size is $NVDIMM_SIZE.
++
+     iothread->stopping = true;
- Note:
+-    aio_notify(iothread->ctx);
++}
 +
 +void iothread_join(IOThread *iothread)
 +{
 +    aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
      qemu_thread_join(&iothread->thread);
      qemu_cond_destroy(&iothread->init_done_cond);
      qemu_mutex_destroy(&iothread->init_done_lock);
 --
-.17.2
+.21.0

From: Kees Cook <keescook@chromium.org>

I spent way too much time trying to figure out why the emulated NVDIMM
was missing under Linux. In an effort to help others who might be looking
for these kinds of things in the future, include a hint.

Signed-off-by: Kees Cook <keescook@chromium.org>
Message-id: 20181018201351.GA25286@beast
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 docs/nvdimm.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
index XXXXXXX..XXXXXXX 100644
--- a/docs/nvdimm.txt
+++ b/docs/nvdimm.txt
@@ -XXX,XX +XXX,XX @@ Multiple vNVDIMM devices can be created if multiple pairs of "-object"
 and "-device" are provided.
 
 For above command line options, if the guest OS has the proper NVDIMM
-driver, it should be able to detect a NVDIMM device which is in the
-persistent memory mode and whose size is $NVDIMM_SIZE.
+driver (e.g. "CONFIG_ACPI_NFIT=y" under Linux), it should be able to
+detect a NVDIMM device which is in the persistent memory mode and whose
+size is $NVDIMM_SIZE.
 
 Note:
 
-- 
2.17.2

tests/test-bdrv-drain can hang in tests/iothread.c:iothread_run():

while (!atomic_read(&iothread->stopping)) {
      aio_poll(iothread->ctx, true);
  }

The iothread_join() function works as follows:

void iothread_join(IOThread *iothread)
  {
      iothread->stopping = true;
      aio_notify(iothread->ctx);
      qemu_thread_join(&iothread->thread);

If iothread_run() checks iothread->stopping before the iothread_join()
thread sets stopping to true, then aio_notify() may be optimized away
and iothread_run() hangs forever in aio_poll().

The correct way to change iothread->stopping is from a BH that executes
within iothread_run().  This ensures that iothread->stopping is checked
after we set it to true.

This was already fixed for ./iothread.c (note this is a different source
file!) by commit 2362a28ea11c145e1a13ae79342d76dc118a72a6 ("iothread:
fix iothread_stop() race condition"), but not for tests/iothread.c.

Fixes: 0c330a734b51c177ab8488932ac3b0c4d63a718a
       ("aio: introduce aio_co_schedule and aio_co_wake")
Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20191003100103.331-1-stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/iothread.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/iothread.c b/tests/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/iothread.c
+++ b/tests/iothread.c
@@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque)
     return NULL;
 }
 
-void iothread_join(IOThread *iothread)
+static void iothread_stop_bh(void *opaque)
 {
+    IOThread *iothread = opaque;
+
     iothread->stopping = true;
-    aio_notify(iothread->ctx);
+}
+
+void iothread_join(IOThread *iothread)
+{
+    aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
     qemu_thread_join(&iothread->thread);
     qemu_cond_destroy(&iothread->init_done_cond);
     qemu_mutex_destroy(&iothread->init_done_lock);
-- 
2.21.0