| Message ID | 1492564532-91680-1-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/04/2017 03:15, zhanghailiang wrote: > 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; > }; > Queued for 2.10, thanks. Paolo
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; };