[Qemu-devel] [PULL 0/2] More fixes for QEMU 4.1.0-rc2

[Qemu-devel] [PULL 0/2] More fixes for QEMU 4.1.0-rc2

[Paolo Bonzini]

The following changes since commit 23da9e297b4120ca9702cabec91599a44255fe96:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190722' into staging (2019-07-22 15:16:48 +0100)

are available in the Git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 20b25d239ab7a94bb8bff3d0f13a9527ee75cf10:

  i386/kvm: Do not sync nested state during runtime (2019-07-22 18:16:05 +0200)

----------------------------------------------------------------
Two more bugfix patches that came in over the weekend.

----------------------------------------------------------------
Jan Kiszka (1):
      i386/kvm: Do not sync nested state during runtime

Zhengui li (1):
      virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk

 hw/scsi/virtio-scsi.c |  6 ++++--
 target/i386/kvm.c     | 10 +++++-----
 2 files changed, 9 insertions(+), 7 deletions(-)
-- 
2.21.0

[Qemu-devel] [PULL 1/2] virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk

[Paolo Bonzini]

From: Zhengui li <lizhengui@huawei.com>

commit a6f230c move blockbackend back to main AioContext on unplug. It set the AioContext of
SCSIDevice to the main AioContex, but s->ctx is still the iothread AioContex（if the scsi controller
is configure with iothread）. So if there are having in-flight requests during unplug, a failing assertion
happend. The bt is below:
(gdb) bt
#0  0x0000ffff86aacbd0 in raise () from /lib64/libc.so.6
#1  0x0000ffff86aadf7c in abort () from /lib64/libc.so.6
#2  0x0000ffff86aa6124 in __assert_fail_base () from /lib64/libc.so.6
#3  0x0000ffff86aa61a4 in __assert_fail () from /lib64/libc.so.6
#4  0x0000000000529118 in virtio_scsi_ctx_check (d=<optimized out>, s=<optimized out>, s=<optimized out>) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:246
#5  0x0000000000529ec4 in virtio_scsi_handle_cmd_req_prepare (s=0x2779ec00, req=0xffff740397d0) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:559
#6  0x000000000052a228 in virtio_scsi_handle_cmd_vq (s=0x2779ec00, vq=0xffff7c6d7110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:603
#7  0x000000000052afa8 in virtio_scsi_data_plane_handle_cmd (vdev=<optimized out>, vq=0xffff7c6d7110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi-dataplane.c:59
#8  0x000000000054d94c in virtio_queue_host_notifier_aio_poll (opaque=<optimized out>) at /home/qemu-4.0.0/hw/virtio/virtio.c:2452

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

To avoid assertion failed,  moving the "if" after qdev_simple_device_unplug_cb.

In addition, to avoid another qemu crash below, add aio_disable_external before
qdev_simple_device_unplug_cb, which disable the further processing of external clients
when doing qdev_simple_device_unplug_cb.
(gdb) bt
#0  scsi_req_unref (req=0xffff6802c6f0) at hw/scsi/scsi-bus.c:1283
#1  0x00000000005294a4 in virtio_scsi_handle_cmd_req_submit (req=<optimized out>,
    s=<optimized out>) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:589
#2  0x000000000052a2a8 in virtio_scsi_handle_cmd_vq (s=s@entry=0x9c90e90,
    vq=vq@entry=0xffff7c05f110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:625
#3  0x000000000052afd8 in virtio_scsi_data_plane_handle_cmd (vdev=<optimized out>,
    vq=0xffff7c05f110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi-dataplane.c:60
#4  0x000000000054d97c in virtio_queue_host_notifier_aio_poll (opaque=<optimized out>)
    at /home/qemu-4.0.0/hw/virtio/virtio.c:2447
#5  0x00000000009b204c in run_poll_handlers_once (ctx=ctx@entry=0x6efea40,
    timeout=timeout@entry=0xffff7d7f7308) at util/aio-posix.c:521
#6  0x00000000009b2b64 in run_poll_handlers (ctx=ctx@entry=0x6efea40,
    max_ns=max_ns@entry=4000, timeout=timeout@entry=0xffff7d7f7308) at util/aio-posix.c:559
#7  0x00000000009b2ca0 in try_poll_mode (ctx=ctx@entry=0x6efea40, timeout=0xffff7d7f7308,
    timeout@entry=0xffff7d7f7348) at util/aio-posix.c:594
#8  0x00000000009b31b8 in aio_poll (ctx=0x6efea40, blocking=blocking@entry=true)
    at util/aio-posix.c:636
#9  0x00000000006973cc in iothread_run (opaque=0x6ebd800) at iothread.c:75
#10 0x00000000009b592c in qemu_thread_start (args=0x6efef60) at util/qemu-thread-posix.c:502
#11 0x0000ffff8057f8bc in start_thread () from /lib64/libpthread.so.0
#12 0x0000ffff804e5f8c in thread_start () from /lib64/libc.so.6
(gdb) p bus
$1 = (SCSIBus *) 0x0

Signed-off-by: Zhengui li <lizhengui@huawei.com>
Message-Id: <1563696502-7972-1-git-send-email-lizhengui@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index d0bdbff090..aa33819d59 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -841,14 +841,16 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
         virtio_scsi_release(s);
     }
 
+    aio_disable_external(s->ctx);
+    qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
+    aio_enable_external(s->ctx);
+
     if (s->ctx) {
         virtio_scsi_acquire(s);
         /* If other users keep the BlockBackend in the iothread, that's ok */
         blk_set_aio_context(sd->conf.blk, qemu_get_aio_context(), NULL);
         virtio_scsi_release(s);
     }
-
-    qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
 }
 
 static struct SCSIBusInfo virtio_scsi_scsi_info = {
-- 
2.21.0

[Qemu-devel] [PULL 2/2] i386/kvm: Do not sync nested state during runtime

[Paolo Bonzini]

From: Jan Kiszka <jan.kiszka@siemens.com>

Writing the nested state e.g. after a vmport access can invalidate
important parts of the kernel-internal state, and it is not needed as
well. So leave this out from KVM_PUT_RUNTIME_STATE.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Message-Id: <bdd53f40-4e60-f3ae-7ec6-162198214953@siemens.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ada89d27cc..dbbb13772a 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -3563,12 +3563,12 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
 
     assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu));
 
-    ret = kvm_put_nested_state(x86_cpu);
-    if (ret < 0) {
-        return ret;
-    }
-
     if (level >= KVM_PUT_RESET_STATE) {
+        ret = kvm_put_nested_state(x86_cpu);
+        if (ret < 0) {
+            return ret;
+        }
+
         ret = kvm_put_msr_feature_control(x86_cpu);
         if (ret < 0) {
             return ret;
-- 
2.21.0

Re: [Qemu-devel] [PULL 1/2] virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk

[Philippe Mathieu-Daudé]

Hi Paolo,

On 7/22/19 6:16 PM, Paolo Bonzini wrote:
> From: Zhengui li <lizhengui@huawei.com>
> 
> commit a6f230c move blockbackend back to main AioContext on unplug. It set the AioContext of
> SCSIDevice to the main AioContex, but s->ctx is still the iothread AioContex（if the scsi controller
> is configure with iothread）. So if there are having in-flight requests during unplug, a failing assertion
> happend. The bt is below:

Patch v2 uses some weird utf-8 parenthesis instead of （ ）.

Re: [Qemu-devel] [PULL 0/2] More fixes for QEMU 4.1.0-rc2

[Philippe Mathieu-Daudé]

On 7/22/19 6:16 PM, Paolo Bonzini wrote:
> The following changes since commit 23da9e297b4120ca9702cabec91599a44255fe96:
> 
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190722' into staging (2019-07-22 15:16:48 +0100)
> 
> are available in the Git repository at:
> 
>   git://github.com/bonzini/qemu.git tags/for-upstream
> 
> for you to fetch changes up to 20b25d239ab7a94bb8bff3d0f13a9527ee75cf10:
> 
>   i386/kvm: Do not sync nested state during runtime (2019-07-22 18:16:05 +0200)
> 
> ----------------------------------------------------------------
> Two more bugfix patches that came in over the weekend.
> 
> ----------------------------------------------------------------
> Jan Kiszka (1):
>       i386/kvm: Do not sync nested state during runtime
> 
> Zhengui li (1):
>       virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk
> 
>  hw/scsi/virtio-scsi.c |  6 ++++--
>  target/i386/kvm.c     | 10 +++++-----
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 

Replying to cover for Peter, not sure it is worthwhile another pull
request (if Paolo has the time for...), but some parenthesis got
mojibaked in patch #1.

Re: [Qemu-devel] [PULL 0/2] More fixes for QEMU 4.1.0-rc2

[Peter Maydell]

On Mon, 22 Jul 2019 at 17:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit 23da9e297b4120ca9702cabec91599a44255fe96:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20190722' into staging (2019-07-22 15:16:48 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 20b25d239ab7a94bb8bff3d0f13a9527ee75cf10:
>
>   i386/kvm: Do not sync nested state during runtime (2019-07-22 18:16:05 +0200)
>
> ----------------------------------------------------------------
> Two more bugfix patches that came in over the weekend.
>
> ----------------------------------------------------------------
> Jan Kiszka (1):
>       i386/kvm: Do not sync nested state during runtime
>
> Zhengui li (1):
>       virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk

Hi; this fails "make check" (all hosts):

MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=arm-softmmu/qemu-system-arm QTEST_QEMU_IMG=qemu-img
tests/qos-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="qos-test"
PASS 1 qos-test /arm/imx25-pdk/imx.i2c/i2c-bus/ds1338/ds1338-tests/tx-rx
PASS 2 qos-test /arm/imx25-pdk/imx.i2c/i2c-bus/pca9552/pca9552-tests/tx-rx
PASS 3 qos-test /arm/imx25-pdk/imx.i2c/i2c-bus/pca9552/pca9552-tests/rx-autoinc
[...]
PASS 35 qos-test
/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/large_tx/uint_max
PASS 36 qos-test
/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/large_tx/net_bufsize
PASS 37 qos-test
/arm/virt/virtio-mmio/virtio-bus/virtio-rng-device/virtio/virtio-tests/nop
Broken pipe
/home/linux1/qemu/tests/libqtest.c:145: kill_qemu() detected QEMU
death from signal 11 (Segmentation fault) (core dumped)
Aborted (core dumped)
ERROR - too few tests run (expected 44, got 37)
/home/linux1/qemu/tests/Makefile.include:899: recipe for target
'check-qtest-arm' failed

Segfault seems to be on the test
/arm/virt/virtio-mmio/virtio-bus/virtio-scsi-device/virtio-scsi/virtio-scsi-tests/hotplug

Here's a backtrace (on an s390 box, which happened to be the
easiest to investigate on):

Thread 1 "qemu-system-arm" received signal SIGSEGV, Segmentation fault.
0x000000010043feda in aio_disable_external (ctx=0x0) at
/home/linux1/qemu/include/block/aio.h:493
493         atomic_inc(&ctx->external_disable_cnt);
(gdb) bt
#0  0x000000010043feda in aio_disable_external (ctx=0x0) at
/home/linux1/qemu/include/block/aio.h:493
#1  virtio_scsi_hotunplug (hotplug_dev=0x101b19040, dev=0x101c00c00,
errp=0x3ffffffe798)
    at /home/linux1/qemu/hw/scsi/virtio-scsi.c:844
#2  0x000000010053a874 in qdev_unplug (dev=0x101c00c00,
errp=0x3ffffffe848) at /home/linux1/qemu/qdev-monitor.c:831
#3  0x0000000100746c3a in qmp_marshal_device_del (args=<optimized
out>, ret=<optimized out>, errp=0x3ffffffe918)
    at qapi/qapi-commands-qdev.c:99
#4  0x0000000100847f98 in do_qmp_dispatch (errp=0x3ffffffe910,
allow_oob=false, request=0x3ffdc004b00,
    cmds=<optimized out>) at /home/linux1/qemu/qapi/qmp-dispatch.c:131
#5  qmp_dispatch (cmds=<optimized out>, request=0x3ffdc004b00,
allow_oob=<optimized out>)
    at /home/linux1/qemu/qapi/qmp-dispatch.c:174
#6  0x00000001007176a8 in monitor_qmp_dispatch
(mon=mon@entry=0x10123eef0, req=<optimized out>)
    at /home/linux1/qemu/monitor/qmp.c:120
#7  0x0000000100717e92 in monitor_qmp_bh_dispatcher (data=<optimized
out>) at /home/linux1/qemu/monitor/qmp.c:209
#8  0x0000000100894a60 in aio_bh_call (bh=0x10115ec20) at
/home/linux1/qemu/util/async.c:89
#9  aio_bh_poll (ctx=<optimized out>) at /home/linux1/qemu/util/async.c:117
#10 0x0000000100898970 in aio_dispatch (ctx=0x10115e090) at
/home/linux1/qemu/util/aio-posix.c:459
#11 0x00000001008948f0 in aio_ctx_dispatch (source=<optimized out>,
callback=<optimized out>, user_data=<optimized out>)
    at /home/linux1/qemu/util/async.c:260
#12 0x000003fffcb5051e in g_main_context_dispatch () from
/lib/s390x-linux-gnu/libglib-2.0.so.0
#13 0x000000010089778a in glib_pollfds_poll () at
/home/linux1/qemu/util/main-loop.c:218
#14 os_host_main_loop_wait (timeout=<optimized out>) at
/home/linux1/qemu/util/main-loop.c:241
#15 main_loop_wait (nonblocking=<optimized out>) at
/home/linux1/qemu/util/main-loop.c:517
#16 0x000000010053f182 in main_loop () at /home/linux1/qemu/vl.c:1791



You might as well fix the commit message mojibake Philippe pointed
out when you do the respin...

thanks
-- PMM

Re: [Qemu-devel] [PULL 1/2] virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk

[Peter Maydell]

On Mon, 22 Jul 2019 at 17:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Zhengui li <lizhengui@huawei.com>
>
> commit a6f230c move blockbackend back to main AioContext on unplug. It set the AioContext of
> SCSIDevice to the main AioContex, but s->ctx is still the iothread AioContex（if the scsi controller
> is configure with iothread）. So if there are having in-flight requests during unplug, a failing assertion
> happend. The bt is below:
> (gdb) bt
> #0  0x0000ffff86aacbd0 in raise () from /lib64/libc.so.6
> #1  0x0000ffff86aadf7c in abort () from /lib64/libc.so.6
> #2  0x0000ffff86aa6124 in __assert_fail_base () from /lib64/libc.so.6
> #3  0x0000ffff86aa61a4 in __assert_fail () from /lib64/libc.so.6
> #4  0x0000000000529118 in virtio_scsi_ctx_check (d=<optimized out>, s=<optimized out>, s=<optimized out>) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:246
> #5  0x0000000000529ec4 in virtio_scsi_handle_cmd_req_prepare (s=0x2779ec00, req=0xffff740397d0) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:559
> #6  0x000000000052a228 in virtio_scsi_handle_cmd_vq (s=0x2779ec00, vq=0xffff7c6d7110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:603
> #7  0x000000000052afa8 in virtio_scsi_data_plane_handle_cmd (vdev=<optimized out>, vq=0xffff7c6d7110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi-dataplane.c:59
> #8  0x000000000054d94c in virtio_queue_host_notifier_aio_poll (opaque=<optimized out>) at /home/qemu-4.0.0/hw/virtio/virtio.c:2452
>
> assert(blk_get_aio_context(d->conf.blk) == s->ctx) failed.
>
> To avoid assertion failed,  moving the "if" after qdev_simple_device_unplug_cb.
>
> In addition, to avoid another qemu crash below, add aio_disable_external before
> qdev_simple_device_unplug_cb, which disable the further processing of external clients
> when doing qdev_simple_device_unplug_cb.
> (gdb) bt
> #0  scsi_req_unref (req=0xffff6802c6f0) at hw/scsi/scsi-bus.c:1283
> #1  0x00000000005294a4 in virtio_scsi_handle_cmd_req_submit (req=<optimized out>,
>     s=<optimized out>) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:589
> #2  0x000000000052a2a8 in virtio_scsi_handle_cmd_vq (s=s@entry=0x9c90e90,
>     vq=vq@entry=0xffff7c05f110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:625
> #3  0x000000000052afd8 in virtio_scsi_data_plane_handle_cmd (vdev=<optimized out>,
>     vq=0xffff7c05f110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi-dataplane.c:60
> #4  0x000000000054d97c in virtio_queue_host_notifier_aio_poll (opaque=<optimized out>)
>     at /home/qemu-4.0.0/hw/virtio/virtio.c:2447
> #5  0x00000000009b204c in run_poll_handlers_once (ctx=ctx@entry=0x6efea40,
>     timeout=timeout@entry=0xffff7d7f7308) at util/aio-posix.c:521
> #6  0x00000000009b2b64 in run_poll_handlers (ctx=ctx@entry=0x6efea40,
>     max_ns=max_ns@entry=4000, timeout=timeout@entry=0xffff7d7f7308) at util/aio-posix.c:559
> #7  0x00000000009b2ca0 in try_poll_mode (ctx=ctx@entry=0x6efea40, timeout=0xffff7d7f7308,
>     timeout@entry=0xffff7d7f7348) at util/aio-posix.c:594
> #8  0x00000000009b31b8 in aio_poll (ctx=0x6efea40, blocking=blocking@entry=true)
>     at util/aio-posix.c:636
> #9  0x00000000006973cc in iothread_run (opaque=0x6ebd800) at iothread.c:75
> #10 0x00000000009b592c in qemu_thread_start (args=0x6efef60) at util/qemu-thread-posix.c:502
> #11 0x0000ffff8057f8bc in start_thread () from /lib64/libpthread.so.0
> #12 0x0000ffff804e5f8c in thread_start () from /lib64/libc.so.6
> (gdb) p bus
> $1 = (SCSIBus *) 0x0
>
> Signed-off-by: Zhengui li <lizhengui@huawei.com>
> Message-Id: <1563696502-7972-1-git-send-email-lizhengui@huawei.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/scsi/virtio-scsi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index d0bdbff090..aa33819d59 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -841,14 +841,16 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>          virtio_scsi_release(s);
>      }
>
> +    aio_disable_external(s->ctx);
> +    qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
> +    aio_enable_external(s->ctx);

These new calls assume s->ctx is non-NULL...

> +
>      if (s->ctx) {

...but the existing code believes it might be NULL.
Coverity would have complained about this even if 'make check'
didn't catch the bug.

>          virtio_scsi_acquire(s);
>          /* If other users keep the BlockBackend in the iothread, that's ok */
>          blk_set_aio_context(sd->conf.blk, qemu_get_aio_context(), NULL);
>          virtio_scsi_release(s);
>      }
> -
> -    qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
>  }

thanks
-- PMM