From 7ba9addc165b37b764baa08c02518b15b2361707 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 28 Aug 2013 15:18:29 +0530 Subject: char: move backends' io watch tag to CharDriverState All the backends implement an io watcher tag for callbacks. Move it to CharDriverState from each backend's struct to make accessing the tag from backend-neutral functions easier. This will be used later to cancel a callback on chardev detach from a frontend. CC: Reviewed-by: Gerd Hoffmann Signed-off-by: Amit Shah --- qemu-char.c | 77 +++++++++++++++++++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 38 deletions(-) (limited to 'qemu-char.c') diff --git a/qemu-char.c b/qemu-char.c index 62594965bd..99f2b949f4 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -798,7 +798,6 @@ static int io_channel_send(GIOChannel *fd, const void *buf, size_t len) typedef struct FDCharDriver { CharDriverState *chr; GIOChannel *fd_in, *fd_out; - guint fd_in_tag; int max_size; QTAILQ_ENTRY(FDCharDriver) node; } FDCharDriver; @@ -830,9 +829,9 @@ static gboolean fd_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) status = g_io_channel_read_chars(chan, (gchar *)buf, len, &bytes_read, NULL); if (status == G_IO_STATUS_EOF) { - if (s->fd_in_tag) { - io_remove_watch_poll(s->fd_in_tag); - s->fd_in_tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } qemu_chr_be_event(chr, CHR_EVENT_CLOSED); return FALSE; @@ -863,13 +862,14 @@ static void fd_chr_update_read_handler(CharDriverState *chr) { FDCharDriver *s = chr->opaque; - if (s->fd_in_tag) { - io_remove_watch_poll(s->fd_in_tag); - s->fd_in_tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } if (s->fd_in) { - s->fd_in_tag = io_add_watch_poll(s->fd_in, fd_chr_read_poll, fd_chr_read, chr); + chr->fd_in_tag = io_add_watch_poll(s->fd_in, fd_chr_read_poll, + fd_chr_read, chr); } } @@ -877,9 +877,9 @@ static void fd_chr_close(struct CharDriverState *chr) { FDCharDriver *s = chr->opaque; - if (s->fd_in_tag) { - io_remove_watch_poll(s->fd_in_tag); - s->fd_in_tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } if (s->fd_in) { @@ -1012,7 +1012,6 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts) typedef struct { GIOChannel *fd; - guint fd_tag; int connected; int read_bytes; guint timer_tag; @@ -1127,9 +1126,9 @@ static void pty_chr_state(CharDriverState *chr, int connected) PtyCharDriver *s = chr->opaque; if (!connected) { - if (s->fd_tag) { - io_remove_watch_poll(s->fd_tag); - s->fd_tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } s->connected = 0; /* (re-)connect poll interval for idle guests: once per second. @@ -1144,7 +1143,8 @@ static void pty_chr_state(CharDriverState *chr, int connected) if (!s->connected) { s->connected = 1; qemu_chr_be_generic_open(chr); - s->fd_tag = io_add_watch_poll(s->fd, pty_chr_read_poll, pty_chr_read, chr); + chr->fd_in_tag = io_add_watch_poll(s->fd, pty_chr_read_poll, + pty_chr_read, chr); } } } @@ -1155,9 +1155,9 @@ static void pty_chr_close(struct CharDriverState *chr) PtyCharDriver *s = chr->opaque; int fd; - if (s->fd_tag) { - io_remove_watch_poll(s->fd_tag); - s->fd_tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } fd = g_io_channel_unix_get_fd(s->fd); g_io_channel_unref(s->fd); @@ -2165,7 +2165,6 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts) typedef struct { int fd; GIOChannel *chan; - guint tag; uint8_t buf[READ_BUF_LEN]; int bufcnt; int bufptr; @@ -2221,9 +2220,9 @@ static gboolean udp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) s->bufcnt = bytes_read; s->bufptr = s->bufcnt; if (status != G_IO_STATUS_NORMAL) { - if (s->tag) { - io_remove_watch_poll(s->tag); - s->tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } return FALSE; } @@ -2242,22 +2241,23 @@ static void udp_chr_update_read_handler(CharDriverState *chr) { NetCharDriver *s = chr->opaque; - if (s->tag) { - io_remove_watch_poll(s->tag); - s->tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } if (s->chan) { - s->tag = io_add_watch_poll(s->chan, udp_chr_read_poll, udp_chr_read, chr); + chr->fd_in_tag = io_add_watch_poll(s->chan, udp_chr_read_poll, + udp_chr_read, chr); } } static void udp_chr_close(CharDriverState *chr) { NetCharDriver *s = chr->opaque; - if (s->tag) { - io_remove_watch_poll(s->tag); - s->tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } if (s->chan) { g_io_channel_unref(s->chan); @@ -2308,7 +2308,7 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts) typedef struct { GIOChannel *chan, *listen_chan; - guint tag, listen_tag; + guint listen_tag; int fd, listen_fd; int connected; int max_size; @@ -2493,9 +2493,9 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) if (s->listen_chan) { s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, tcp_chr_accept, chr); } - if (s->tag) { - io_remove_watch_poll(s->tag); - s->tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } g_io_channel_unref(s->chan); s->chan = NULL; @@ -2526,7 +2526,8 @@ static void tcp_chr_connect(void *opaque) s->connected = 1; if (s->chan) { - s->tag = io_add_watch_poll(s->chan, tcp_chr_read_poll, tcp_chr_read, chr); + chr->fd_in_tag = io_add_watch_poll(s->chan, tcp_chr_read_poll, + tcp_chr_read, chr); } qemu_chr_be_generic_open(chr); } @@ -2609,9 +2610,9 @@ static void tcp_chr_close(CharDriverState *chr) { TCPCharDriver *s = chr->opaque; if (s->fd >= 0) { - if (s->tag) { - io_remove_watch_poll(s->tag); - s->tag = 0; + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; } if (s->chan) { g_io_channel_unref(s->chan); -- cgit v1.2.1 From 26da70c72524eb22c946ab19ec98a217b8252f7e Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 28 Aug 2013 15:23:37 +0530 Subject: char: use common function to disable callbacks on chardev close This deduplicates code used a lot of times. CC: Reviewed-by: Gerd Hoffmann Signed-off-by: Amit Shah --- qemu-char.c | 62 +++++++++++++++++++------------------------------------------ 1 file changed, 19 insertions(+), 43 deletions(-) (limited to 'qemu-char.c') diff --git a/qemu-char.c b/qemu-char.c index 99f2b949f4..0a0833f77b 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -725,6 +725,14 @@ static void io_remove_watch_poll(guint tag) g_source_destroy(&iwp->parent); } +static void remove_fd_in_watch(CharDriverState *chr) +{ + if (chr->fd_in_tag) { + io_remove_watch_poll(chr->fd_in_tag); + chr->fd_in_tag = 0; + } +} + #ifndef _WIN32 static GIOChannel *io_channel_from_fd(int fd) { @@ -829,10 +837,7 @@ static gboolean fd_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) status = g_io_channel_read_chars(chan, (gchar *)buf, len, &bytes_read, NULL); if (status == G_IO_STATUS_EOF) { - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } + remove_fd_in_watch(chr); qemu_chr_be_event(chr, CHR_EVENT_CLOSED); return FALSE; } @@ -862,11 +867,7 @@ static void fd_chr_update_read_handler(CharDriverState *chr) { FDCharDriver *s = chr->opaque; - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } - + remove_fd_in_watch(chr); if (s->fd_in) { chr->fd_in_tag = io_add_watch_poll(s->fd_in, fd_chr_read_poll, fd_chr_read, chr); @@ -877,11 +878,7 @@ static void fd_chr_close(struct CharDriverState *chr) { FDCharDriver *s = chr->opaque; - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } - + remove_fd_in_watch(chr); if (s->fd_in) { g_io_channel_unref(s->fd_in); } @@ -1126,10 +1123,7 @@ static void pty_chr_state(CharDriverState *chr, int connected) PtyCharDriver *s = chr->opaque; if (!connected) { - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } + 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 @@ -1155,10 +1149,7 @@ static void pty_chr_close(struct CharDriverState *chr) PtyCharDriver *s = chr->opaque; int fd; - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } + remove_fd_in_watch(chr); fd = g_io_channel_unix_get_fd(s->fd); g_io_channel_unref(s->fd); close(fd); @@ -2220,10 +2211,7 @@ static gboolean udp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) s->bufcnt = bytes_read; s->bufptr = s->bufcnt; if (status != G_IO_STATUS_NORMAL) { - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } + remove_fd_in_watch(chr); return FALSE; } @@ -2241,11 +2229,7 @@ static void udp_chr_update_read_handler(CharDriverState *chr) { NetCharDriver *s = chr->opaque; - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } - + remove_fd_in_watch(chr); if (s->chan) { chr->fd_in_tag = io_add_watch_poll(s->chan, udp_chr_read_poll, udp_chr_read, chr); @@ -2255,10 +2239,8 @@ static void udp_chr_update_read_handler(CharDriverState *chr) static void udp_chr_close(CharDriverState *chr) { NetCharDriver *s = chr->opaque; - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } + + remove_fd_in_watch(chr); if (s->chan) { g_io_channel_unref(s->chan); closesocket(s->fd); @@ -2493,10 +2475,7 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) if (s->listen_chan) { s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, tcp_chr_accept, chr); } - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } + remove_fd_in_watch(chr); g_io_channel_unref(s->chan); s->chan = NULL; closesocket(s->fd); @@ -2610,10 +2589,7 @@ static void tcp_chr_close(CharDriverState *chr) { TCPCharDriver *s = chr->opaque; if (s->fd >= 0) { - if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); - chr->fd_in_tag = 0; - } + remove_fd_in_watch(chr); if (s->chan) { g_io_channel_unref(s->chan); } -- cgit v1.2.1 From 386a5a1e0057e220f79c48fe3689e3dfb17f1b09 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 28 Aug 2013 15:24:05 +0530 Subject: char: remove watch callback on chardev detach from frontend If a frontend device releases the chardev (via unplug), the chr handlers are set to NULL via qdev's exit callbacks invoking qemu_chr_add_handlers(). If the chardev had a pending operation, a callback will be invoked, which will try to access data in the just-released frontend, causing a segfault. Ensure the callbacks are disabled when frontends release chardevs. This was seen when a virtio-serial port was unplugged when heavy guest->host IO was in progress (causing a callback to be registered). In the window in which the throttling was active, unplugging ports caused a qemu segfault. https://bugzilla.redhat.com/show_bug.cgi?id=985205 CC: Reported-by: Sibiao Luo Reviewed-by: Gerd Hoffmann Signed-off-by: Amit Shah --- qemu-char.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'qemu-char.c') diff --git a/qemu-char.c b/qemu-char.c index 0a0833f77b..6f111abfbe 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -193,6 +193,8 @@ void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...) va_end(ap); } +static void remove_fd_in_watch(CharDriverState *chr); + void qemu_chr_add_handlers(CharDriverState *s, IOCanReadHandler *fd_can_read, IOReadHandler *fd_read, @@ -203,6 +205,7 @@ void qemu_chr_add_handlers(CharDriverState *s, if (!opaque && !fd_can_read && !fd_read && !fd_event) { fe_open = 0; + remove_fd_in_watch(s); } else { fe_open = 1; } -- cgit v1.2.1