qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3] char: Fix removing wrong GSource that be found by fd_in_tag
@ 2017-04-19  1:15 zhanghailiang
  2017-04-21 10:05 ` Paolo Bonzini
  0 siblings, 1 reply; 2+ messages in thread
From: zhanghailiang @ 2017-04-19  1:15 UTC (permalink / raw)
  To: pbonzini, marcandre.lureau; +Cc: qemu-devel, zhangchen.fnst, zhanghailiang

We use fd_in_tag to find a GSource, fd_in_tag is return value of
g_source_attach(GSource *source, GMainContext *context), the return
value is unique only in the same context, so we may get the same
values with different 'context' parameters.

It is no problem to find the right fd_in_tag by using
 g_main_context_find_source_by_id(GMainContext *context, guint source_id)
while there is only one default main context.

But colo-compare tries to create/use its own context, and if we pass wrong
'context' parameter with right fd_in_tag, we will find a wrong GSource to handle.
We tried to fix the related codes in commit b43decb015a6efeb9e3cdbdb80f6547ad7248a4c,
but it didn't fix the bug completely, because we still have some codes didn't pass
*right* context parameter for remove_fd_in_watch().

Let's fix it by record the GSource directly instead of fd_in_tag.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
v3:
 - Use the full hash (Marc-André Lureau)
 - Use a simply name 'gsource' instead of 'chr_gsource' (Marc-André Lureau)
---
 chardev/char-fd.c     |  8 ++++----
 chardev/char-io.c     | 23 ++++++++---------------
 chardev/char-io.h     |  4 ++--
 chardev/char-pty.c    |  6 +++---
 chardev/char-socket.c |  8 ++++----
 chardev/char-udp.c    |  8 ++++----
 chardev/char.c        |  2 +-
 include/sysemu/char.h |  2 +-
 8 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index 548dd4c..0b182c5 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -58,7 +58,7 @@ static gboolean fd_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
     ret = qio_channel_read(
         chan, (gchar *)buf, len, NULL);
     if (ret == 0) {
-        remove_fd_in_watch(chr, NULL);
+        remove_fd_in_watch(chr);
         qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
         return FALSE;
     }
@@ -89,9 +89,9 @@ static void fd_chr_update_read_handler(Chardev *chr,
 {
     FDChardev *s = FD_CHARDEV(chr);
 
-    remove_fd_in_watch(chr, NULL);
+    remove_fd_in_watch(chr);
     if (s->ioc_in) {
-        chr->fd_in_tag = io_add_watch_poll(chr, s->ioc_in,
+        chr->gsource = io_add_watch_poll(chr, s->ioc_in,
                                            fd_chr_read_poll,
                                            fd_chr_read, chr,
                                            context);
@@ -103,7 +103,7 @@ static void char_fd_finalize(Object *obj)
     Chardev *chr = CHARDEV(obj);
     FDChardev *s = FD_CHARDEV(obj);
 
-    remove_fd_in_watch(chr, NULL);
+    remove_fd_in_watch(chr);
     if (s->ioc_in) {
         object_unref(OBJECT(s->ioc_in));
     }
diff --git a/chardev/char-io.c b/chardev/char-io.c
index b4bb094..b5708ee 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -98,7 +98,7 @@ static GSourceFuncs io_watch_poll_funcs = {
     .finalize = io_watch_poll_finalize,
 };
 
-guint io_add_watch_poll(Chardev *chr,
+GSource *io_add_watch_poll(Chardev *chr,
                         QIOChannel *ioc,
                         IOCanReadHandler *fd_can_read,
                         QIOChannelFunc fd_read,
@@ -106,7 +106,6 @@ guint io_add_watch_poll(Chardev *chr,
                         GMainContext *context)
 {
     IOWatchPoll *iwp;
-    int tag;
     char *name;
 
     iwp = (IOWatchPoll *) g_source_new(&io_watch_poll_funcs,
@@ -122,21 +121,15 @@ guint io_add_watch_poll(Chardev *chr,
     g_source_set_name((GSource *)iwp, name);
     g_free(name);
 
-    tag = g_source_attach(&iwp->parent, context);
+    g_source_attach(&iwp->parent, context);
     g_source_unref(&iwp->parent);
-    return tag;
+    return (GSource *)iwp;
 }
 
-static void io_remove_watch_poll(guint tag, GMainContext *context)
+static void io_remove_watch_poll(GSource *source)
 {
-    GSource *source;
     IOWatchPoll *iwp;
 
-    g_return_if_fail(tag > 0);
-
-    source = g_main_context_find_source_by_id(context, tag);
-    g_return_if_fail(source != NULL);
-
     iwp = io_watch_poll_from_source(source);
     if (iwp->src) {
         g_source_destroy(iwp->src);
@@ -146,11 +139,11 @@ static void io_remove_watch_poll(guint tag, GMainContext *context)
     g_source_destroy(&iwp->parent);
 }
 
-void remove_fd_in_watch(Chardev *chr, GMainContext *context)
+void remove_fd_in_watch(Chardev *chr)
 {
-    if (chr->fd_in_tag) {
-        io_remove_watch_poll(chr->fd_in_tag, context);
-        chr->fd_in_tag = 0;
+    if (chr->gsource) {
+        io_remove_watch_poll(chr->gsource);
+        chr->gsource = NULL;
     }
 }
 
diff --git a/chardev/char-io.h b/chardev/char-io.h
index 842be56..55973a7 100644
--- a/chardev/char-io.h
+++ b/chardev/char-io.h
@@ -29,14 +29,14 @@
 #include "sysemu/char.h"
 
 /* Can only be used for read */
-guint io_add_watch_poll(Chardev *chr,
+GSource *io_add_watch_poll(Chardev *chr,
                         QIOChannel *ioc,
                         IOCanReadHandler *fd_can_read,
                         QIOChannelFunc fd_read,
                         gpointer user_data,
                         GMainContext *context);
 
-void remove_fd_in_watch(Chardev *chr, GMainContext *context);
+void remove_fd_in_watch(Chardev *chr);
 
 int io_channel_send(QIOChannel *ioc, const void *buf, size_t len);
 
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index a6337be..581ab34 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -199,7 +199,7 @@ static void pty_chr_state(Chardev *chr, int connected)
             g_source_remove(s->open_tag);
             s->open_tag = 0;
         }
-        remove_fd_in_watch(chr, NULL);
+        remove_fd_in_watch(chr);
         s->connected = 0;
         /* (re-)connect poll interval for idle guests: once per second.
          * We check more frequently in case the guests sends data to
@@ -215,8 +215,8 @@ static void pty_chr_state(Chardev *chr, int connected)
             s->connected = 1;
             s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr);
         }
-        if (!chr->fd_in_tag) {
-            chr->fd_in_tag = io_add_watch_poll(chr, s->ioc,
+        if (!chr->gsource) {
+            chr->gsource = io_add_watch_poll(chr, s->ioc,
                                                pty_chr_read_poll,
                                                pty_chr_read,
                                                chr, NULL);
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 36ab0d6..d8de051 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -327,7 +327,7 @@ static void tcp_chr_free_connection(Chardev *chr)
     }
 
     tcp_set_msgfds(chr, NULL, 0);
-    remove_fd_in_watch(chr, NULL);
+    remove_fd_in_watch(chr);
     object_unref(OBJECT(s->sioc));
     s->sioc = NULL;
     object_unref(OBJECT(s->ioc));
@@ -484,7 +484,7 @@ static void tcp_chr_connect(void *opaque)
 
     s->connected = 1;
     if (s->ioc) {
-        chr->fd_in_tag = io_add_watch_poll(chr, s->ioc,
+        chr->gsource = io_add_watch_poll(chr, s->ioc,
                                            tcp_chr_read_poll,
                                            tcp_chr_read,
                                            chr, NULL);
@@ -501,9 +501,9 @@ static void tcp_chr_update_read_handler(Chardev *chr,
         return;
     }
 
-    remove_fd_in_watch(chr, NULL);
+    remove_fd_in_watch(chr);
     if (s->ioc) {
-        chr->fd_in_tag = io_add_watch_poll(chr, s->ioc,
+        chr->gsource = io_add_watch_poll(chr, s->ioc,
                                            tcp_chr_read_poll,
                                            tcp_chr_read, chr,
                                            context);
diff --git a/chardev/char-udp.c b/chardev/char-udp.c
index 804bd22..12240e5 100644
--- a/chardev/char-udp.c
+++ b/chardev/char-udp.c
@@ -81,7 +81,7 @@ static gboolean udp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
     ret = qio_channel_read(
         s->ioc, (char *)s->buf, sizeof(s->buf), NULL);
     if (ret <= 0) {
-        remove_fd_in_watch(chr, NULL);
+        remove_fd_in_watch(chr);
         return FALSE;
     }
     s->bufcnt = ret;
@@ -101,9 +101,9 @@ static void udp_chr_update_read_handler(Chardev *chr,
 {
     UdpChardev *s = UDP_CHARDEV(chr);
 
-    remove_fd_in_watch(chr, NULL);
+    remove_fd_in_watch(chr);
     if (s->ioc) {
-        chr->fd_in_tag = io_add_watch_poll(chr, s->ioc,
+        chr->gsource = io_add_watch_poll(chr, s->ioc,
                                            udp_chr_read_poll,
                                            udp_chr_read, chr,
                                            context);
@@ -115,7 +115,7 @@ static void char_udp_finalize(Object *obj)
     Chardev *chr = CHARDEV(obj);
     UdpChardev *s = UDP_CHARDEV(obj);
 
-    remove_fd_in_watch(chr, NULL);
+    remove_fd_in_watch(chr);
     if (s->ioc) {
         object_unref(OBJECT(s->ioc));
     }
diff --git a/chardev/char.c b/chardev/char.c
index 3df1163..54cd5f4 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -560,7 +560,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
     cc = CHARDEV_GET_CLASS(s);
     if (!opaque && !fd_can_read && !fd_read && !fd_event) {
         fe_open = 0;
-        remove_fd_in_watch(s, context);
+        remove_fd_in_watch(s);
     } else {
         fe_open = 1;
     }
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 450881d..84f5c23 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -93,7 +93,7 @@ struct Chardev {
     char *filename;
     int logfd;
     int be_open;
-    guint fd_in_tag;
+    GSource *gsource;
     DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
     QTAILQ_ENTRY(Chardev) next;
 };
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-04-21 10:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-19  1:15 [Qemu-devel] [PATCH v3] char: Fix removing wrong GSource that be found by fd_in_tag zhanghailiang
2017-04-21 10:05 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).