virtio_error: don't invoke status callbacks

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

Posted by Michael S. Tsirkin 6 years, 4 months 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 6 years, 4 months 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);
>      }
>  
>