All of lore.kernel.org
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>,
	Halil Pasic <pasic@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH 1/1] virtio: fail device if set_event_notifier fails
Date: Thu,  2 Mar 2017 19:59:42 +0100	[thread overview]
Message-ID: <20170302185942.76255-1-pasic@linux.vnet.ibm.com> (raw)

The function virtio_notify_irqfd used to ignore the return code of
event_notifier_set. Let's fail the device should this occur.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>

---

This patch is most likely flawed because virtio_notify_irqfd
is probably supposed to be thread-safe and neither strerror
nor virtio_error are that. Anyway lets get the discussion started with this.

There was a suggestion by Michael to make event_notifier_set void
and assert inside. After some thinking, I settled at the opinion,
that neither doing nothing nor crashing the guest is a good idea
should this failure happen in production. So I came up with this.

Looking forward to your feedback.
---
 hw/virtio/virtio.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 23483c7..e05f3e5 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1555,6 +1555,8 @@ static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
 void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq)
 {
     bool should_notify;
+    int rc;
+
     rcu_read_lock();
     should_notify = virtio_should_notify(vdev, vq);
     rcu_read_unlock();
@@ -1581,7 +1583,11 @@ void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq)
      * to an atomic operation.
      */
     virtio_set_isr(vq->vdev, 0x1);
-    event_notifier_set(&vq->guest_notifier);
+    rc = event_notifier_set(&vq->guest_notifier);
+    if (unlikely(rc)) {
+        virtio_error(vdev, "guest notifier broken for vdev at %p (%s)", vdev,
+                     strerror(-rc));
+    }
 }
 
 static void virtio_irq(VirtQueue *vq)
-- 
2.8.4

             reply	other threads:[~2017-03-02 18:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-02 18:59 Halil Pasic [this message]
2017-03-03 12:21 ` [Qemu-devel] [PATCH 1/1] virtio: fail device if set_event_notifier fails Cornelia Huck
2017-03-03 12:43   ` Halil Pasic
2017-03-03 12:50     ` Cornelia Huck
2017-03-03 13:08       ` Halil Pasic
2017-03-06 14:56         ` Cornelia Huck
2017-03-06 15:21           ` Halil Pasic
2017-03-06 16:04             ` Cornelia Huck
2017-03-23 17:09               ` Michael S. Tsirkin
2017-03-29 11:12                 ` Halil Pasic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170302185942.76255-1-pasic@linux.vnet.ibm.com \
    --to=pasic@linux.vnet.ibm.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.