All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Johannes Berg" <johannes.berg@intel.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: [PULL 23/32] libvhost-user-glib: fix VugDev main fd cleanup
Date: Tue, 25 Feb 2020 10:14:20 -0500	[thread overview]
Message-ID: <20200225151210.647797-24-mst@redhat.com> (raw)
In-Reply-To: <20200225151210.647797-1-mst@redhat.com>

From: Johannes Berg <johannes.berg@intel.com>

If you try to make a device implementation that can handle multiple
connections and allow disconnections (which requires overriding the
VHOST_USER_NONE handling), then glib will warn that we remove a src
while it's still on the mainloop, and will poll() an FD that doesn't
exist anymore.

Fix this by making vug_source_new() require pairing with the new
vug_source_destroy() so we can keep the GSource referenced in the
meantime.

Note that this requires calling the new API in vhost-user-input.
vhost-user-gpu also uses vug_source_new(), but never seems to free
the result at all, so I haven't changed anything there.

Fixes: 8bb7ddb78a1c ("libvhost-user: add glib source helper")
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Message-Id: <20200123081708.7817-3-johannes@sipsolutions.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 contrib/libvhost-user/libvhost-user-glib.h |  1 +
 contrib/libvhost-user/libvhost-user-glib.c | 15 ++++++++++++---
 contrib/vhost-user-input/main.c            |  6 ++----
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/contrib/libvhost-user/libvhost-user-glib.h b/contrib/libvhost-user/libvhost-user-glib.h
index 64d539d93a..1a79a4916e 100644
--- a/contrib/libvhost-user/libvhost-user-glib.h
+++ b/contrib/libvhost-user/libvhost-user-glib.h
@@ -31,5 +31,6 @@ void vug_deinit(VugDev *dev);
 
 GSource *vug_source_new(VugDev *dev, int fd, GIOCondition cond,
                         vu_watch_cb vu_cb, gpointer data);
+void vug_source_destroy(GSource *src);
 
 #endif /* LIBVHOST_USER_GLIB_H */
diff --git a/contrib/libvhost-user/libvhost-user-glib.c b/contrib/libvhost-user/libvhost-user-glib.c
index 99edd2f3de..824c7780de 100644
--- a/contrib/libvhost-user/libvhost-user-glib.c
+++ b/contrib/libvhost-user/libvhost-user-glib.c
@@ -91,7 +91,6 @@ vug_source_new(VugDev *gdev, int fd, GIOCondition cond,
     g_source_add_poll(gsrc, &src->gfd);
     id = g_source_attach(gsrc, NULL);
     g_assert(id);
-    g_source_unref(gsrc);
 
     return gsrc;
 }
@@ -131,6 +130,16 @@ static void vug_watch(VuDev *dev, int condition, void *data)
     }
 }
 
+void vug_source_destroy(GSource *src)
+{
+    if (!src) {
+        return;
+    }
+
+    g_source_destroy(src);
+    g_source_unref(src);
+}
+
 bool
 vug_init(VugDev *dev, uint16_t max_queues, int socket,
          vu_panic_cb panic, const VuDevIface *iface)
@@ -144,7 +153,7 @@ vug_init(VugDev *dev, uint16_t max_queues, int socket,
     }
 
     dev->fdmap = g_hash_table_new_full(NULL, NULL, NULL,
-                                       (GDestroyNotify) g_source_destroy);
+                                       (GDestroyNotify) vug_source_destroy);
 
     dev->src = vug_source_new(dev, socket, G_IO_IN, vug_watch, NULL);
 
@@ -157,5 +166,5 @@ vug_deinit(VugDev *dev)
     g_assert(dev);
 
     g_hash_table_unref(dev->fdmap);
-    g_source_unref(dev->src);
+    vug_source_destroy(dev->src);
 }
diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
index ef4b7769f2..6020c6f33a 100644
--- a/contrib/vhost-user-input/main.c
+++ b/contrib/vhost-user-input/main.c
@@ -187,7 +187,7 @@ vi_queue_set_started(VuDev *dev, int qidx, bool started)
     }
 
     if (!started && vi->evsrc) {
-        g_source_destroy(vi->evsrc);
+        vug_source_destroy(vi->evsrc);
         vi->evsrc = NULL;
     }
 }
@@ -401,9 +401,7 @@ main(int argc, char *argv[])
 
     vug_deinit(&vi.dev);
 
-    if (vi.evsrc) {
-        g_source_unref(vi.evsrc);
-    }
+    vug_source_destroy(vi.evsrc);
     g_array_free(vi.config, TRUE);
     g_free(vi.queue);
     return 0;
-- 
MST



  parent reply	other threads:[~2020-02-25 15:26 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-25 15:12 [PULL 00/32] virtio, pc: fixes, features Michael S. Tsirkin
2020-02-25 15:12 ` [PULL 01/32] bios-tables-test: tell people how to update Michael S. Tsirkin
2020-02-25 15:12 ` [PULL 02/32] bios-tables-test: fix up DIFF generation Michael S. Tsirkin
2020-02-25 15:12 ` [PULL 03/32] bios-tables-test: default diff command Michael S. Tsirkin
2020-02-25 15:12 ` [PULL 04/32] rebuild-expected-aml.sh: remind about the process Michael S. Tsirkin
2020-02-25 15:12 ` [PULL 05/32] vhost-user-fs: do delete virtio_queues in unrealize Michael S. Tsirkin
2020-02-25 15:12 ` [PULL 06/32] vhost-user-fs: convert to the new virtio_delete_queue function Michael S. Tsirkin
2020-02-25 15:12 ` [PULL 07/32] virtio-pmem: do delete rq_vq in virtio_pmem_unrealize Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 08/32] virtio-crypto: do delete ctrl_vq in virtio_crypto_device_unrealize Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 09/32] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 10/32] vhost-user-blk: convert to new virtio_delete_queue Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 11/32] virtio: gracefully handle invalid region caches Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 12/32] virtio-iommu: Add skeleton Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 13/32] virtio-iommu: Decode the command payload Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 14/32] virtio-iommu: Implement attach/detach command Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 15/32] virtio-iommu: Implement map/unmap Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 16/32] virtio-iommu: Implement translate Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 17/32] virtio-iommu: Implement fault reporting Michael S. Tsirkin
2020-02-25 15:13 ` [PULL 18/32] virtio-iommu: Support migration Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 19/32] virtio-iommu-pci: Add virtio iommu pci support Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 20/32] hw/arm/virt: Add the virtio-iommu device tree mappings Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 21/32] MAINTAINERS: add virtio-iommu related files Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 22/32] libvhost-user: implement VHOST_USER_PROTOCOL_F_REPLY_ACK Michael S. Tsirkin
2020-02-25 15:14 ` Michael S. Tsirkin [this message]
2020-02-25 15:14 ` [PULL 24/32] libvhost-user-glib: use g_main_context_get_thread_default() Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 25/32] libvhost-user: handle NOFD flag in call/kick/err better Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 26/32] docs: vhost-user: add in-band kick/call messages Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 27/32] libvhost-user: implement in-band notifications Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 28/32] acpi: cpuhp: document CPHP_GET_CPU_ID_CMD command Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 29/32] vhost-user: only set slave channel for first vq Michael S. Tsirkin
2020-02-25 15:14 ` [PULL 30/32] tests/vhost-user-bridge: move to contrib/ Michael S. Tsirkin
2020-02-25 15:15 ` [PULL 31/32] virtiofsd: add it to the tools list Michael S. Tsirkin
2020-02-25 15:15 ` [PULL 32/32] Fixed assert in vhost_user_set_mem_table_postcopy Michael S. Tsirkin
2020-02-25 16:47 ` [PULL 00/32] virtio, pc: fixes, features Peter Maydell
2020-02-25 18:39   ` Michael S. Tsirkin
2020-02-25 21:57   ` Michael S. Tsirkin
2020-02-26  4:48 ` no-reply

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=20200225151210.647797-24-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=johannes.berg@intel.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=peter.maydell@linaro.org \
    --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.