* [Qemu-devel] [PATCH v2] char: Fix removing wrong GSource that be found by fd_in_tag
@ 2017-04-17 11:22 zhanghailiang
2017-04-18 8:11 ` Marc-André Lureau
0 siblings, 1 reply; 3+ messages in thread
From: zhanghailiang @ 2017-04-17 11:22 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 b43dec, 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>
---
v2:
- Fix minor comments from 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..7f0169d 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->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..d781ad6 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->chr_gsource) {
+ io_remove_watch_poll(chr->chr_gsource);
+ chr->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..561926f 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->chr_gsource) {
+ chr->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..376a70b 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->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->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..9d050bc 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->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..e157f96 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 *chr_gsource;
DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
QTAILQ_ENTRY(Chardev) next;
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] char: Fix removing wrong GSource that be found by fd_in_tag
2017-04-17 11:22 [Qemu-devel] [PATCH v2] char: Fix removing wrong GSource that be found by fd_in_tag zhanghailiang
@ 2017-04-18 8:11 ` Marc-André Lureau
2017-04-19 0:42 ` Hailiang Zhang
0 siblings, 1 reply; 3+ messages in thread
From: Marc-André Lureau @ 2017-04-18 8:11 UTC (permalink / raw)
To: zhanghailiang; +Cc: pbonzini, qemu-devel, zhangchen fnst
Hi
----- Original Message -----
> 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 b43dec, 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.
You could simply name it "gsource" or "source" instead of "chr_gsource" to avoid redundancy in chr->chr_gsource.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
looks good to me:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> v2:
> - Fix minor comments from 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..7f0169d 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->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..d781ad6 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->chr_gsource) {
> + io_remove_watch_poll(chr->chr_gsource);
> + chr->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..561926f 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->chr_gsource) {
> + chr->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..376a70b 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->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->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..9d050bc 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->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..e157f96 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 *chr_gsource;
> DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
> QTAILQ_ENTRY(Chardev) next;
> };
> --
> 1.8.3.1
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] char: Fix removing wrong GSource that be found by fd_in_tag
2017-04-18 8:11 ` Marc-André Lureau
@ 2017-04-19 0:42 ` Hailiang Zhang
0 siblings, 0 replies; 3+ messages in thread
From: Hailiang Zhang @ 2017-04-19 0:42 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: xuquan8, pbonzini, qemu-devel, zhangchen fnst
On 2017/4/18 16:11, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
>> 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 b43dec, 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.
> You could simply name it "gsource" or "source" instead of "chr_gsource" to avoid redundancy in chr->chr_gsource.
OK, besides, i forgot to update the hash, I'll fix both of them in V3, thanks.
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> looks good to me:
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>
>> ---
>> v2:
>> - Fix minor comments from 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..7f0169d 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->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..d781ad6 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->chr_gsource) {
>> + io_remove_watch_poll(chr->chr_gsource);
>> + chr->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..561926f 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->chr_gsource) {
>> + chr->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..376a70b 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->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->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..9d050bc 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->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..e157f96 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 *chr_gsource;
>> DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST);
>> QTAILQ_ENTRY(Chardev) next;
>> };
>> --
>> 1.8.3.1
>>
>>
>>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-19 0:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-17 11:22 [Qemu-devel] [PATCH v2] char: Fix removing wrong GSource that be found by fd_in_tag zhanghailiang
2017-04-18 8:11 ` Marc-André Lureau
2017-04-19 0:42 ` Hailiang Zhang
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).