[Qemu-devel] [PATCH] virtio_error: don't invoke status callbacks

Michael S. Tsirkin posted 1 patch 52 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1513195345-9506-1-git-send-email-mst@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
hw/virtio/virtio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

[Qemu-devel] [PATCH] virtio_error: don't invoke status callbacks

Posted by Michael S. Tsirkin 52 weeks ago
Backends don't need to know what frontend requested a reset,
and notifying then from virtio_error is messy because
virtio_error itself might be invoked from backend.

Let's just set the status directly.

Reported-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Ilya this should fix the crash you are seeing, but
the error itself still shows there's something wrong.
So I'd like to defer applying that patch until we
figure out what corrupted guest index.

If you know pls let me know!


 hw/virtio/virtio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ad564b0..d6002ee 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2469,7 +2469,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...)
     va_end(ap);
 
     if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
-        virtio_set_status(vdev, vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET);
+        vdev->status = vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET;
         virtio_notify_config(vdev);
     }
 
-- 
MST

Re: [Qemu-devel] [PATCH] virtio_error: don't invoke status callbacks

Posted by Ilya Maximets 52 weeks ago
On 13.12.2017 23:03, Michael S. Tsirkin wrote:
> Backends don't need to know what frontend requested a reset,
> and notifying then from virtio_error is messy because
> virtio_error itself might be invoked from backend.
> 
> Let's just set the status directly.
> 
> Reported-by: Ilya Maximets <i.maximets@samsung.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Ilya this should fix the crash you are seeing
I tested it and it really fixes the QEMU crash.
Thanks. 

> but
> the error itself still shows there's something wrong.
> So I'd like to defer applying that patch until we
> figure out what corrupted guest index.
> 
> If you know pls let me know!

It looks like virtio driver crash caused by the same issue that was fixed in recent
patches from Maxime Coquelin:

  2ae39a113af3 ("vhost: restore avail index from vring used index on disconnection")
  2d4ba6cc741d ("virtio: Add queue interface to restore avail index from vring used index")

Applying above patches on top of 2.10.1 fixes virtio driver's crash.

> 
>  hw/virtio/virtio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ad564b0..d6002ee 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2469,7 +2469,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...)
>      va_end(ap);
>  
>      if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> -        virtio_set_status(vdev, vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET);
> +        vdev->status = vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET;
>          virtio_notify_config(vdev);
>      }
>  
>