* [Qemu-devel] [PATCH 01/20] char-socket: fix error reporting
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 02/20] qemu-char: remove dead/confusing logic with nb_stdio_clients Amit Shah
` (19 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Right now the inet connect code tries all available addresses but until one
doesn't fail. It passes local_err each time without clearing it from the
previous failure. This can trigger an assert since the inet connect code
tries to set an error on an object != NULL.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
util/qemu-sockets.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 1350ccc..3f12296 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -373,6 +373,10 @@ int inet_connect_opts(QemuOpts *opts, Error **errp,
}
for (e = res; e != NULL; e = e->ai_next) {
+ if (error_is_set(errp)) {
+ error_free(*errp);
+ *errp = NULL;
+ }
if (connect_state != NULL) {
connect_state->current_addr = e;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 02/20] qemu-char: remove dead/confusing logic with nb_stdio_clients
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 01/20] char-socket: fix error reporting Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support Amit Shah
` (18 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
This code is very old dating back to 2007. What is puzzling is that
STDIO_MAX_CLIENTS was always #define to 1 meaning that all of the code to deal
with more than one client was unreachable.
Just remove the whole mess of it.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 136 ++++++++++++++----------------------------------------------
1 file changed, 30 insertions(+), 106 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 36295b1..0a74c10 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -539,9 +539,6 @@ int send_all(int fd, const void *_buf, int len1)
}
#endif /* !_WIN32 */
-#define STDIO_MAX_CLIENTS 1
-static int stdio_nb_clients;
-
#ifndef _WIN32
typedef struct {
@@ -594,11 +591,8 @@ static void fd_chr_update_read_handler(CharDriverState *chr)
FDCharDriver *s = chr->opaque;
if (s->fd_in >= 0) {
- if (display_type == DT_NOGRAPHIC && s->fd_in == 0) {
- } else {
- qemu_set_fd_handler2(s->fd_in, fd_chr_read_poll,
- fd_chr_read, NULL, chr);
- }
+ qemu_set_fd_handler2(s->fd_in, fd_chr_read_poll,
+ fd_chr_read, NULL, chr);
}
}
@@ -607,10 +601,7 @@ static void fd_chr_close(struct CharDriverState *chr)
FDCharDriver *s = chr->opaque;
if (s->fd_in >= 0) {
- if (display_type == DT_NOGRAPHIC && s->fd_in == 0) {
- } else {
- qemu_set_fd_handler2(s->fd_in, NULL, NULL, NULL, NULL);
- }
+ qemu_set_fd_handler2(s->fd_in, NULL, NULL, NULL, NULL);
}
g_free(s);
@@ -677,53 +668,6 @@ static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts)
return qemu_chr_open_fd(fd_in, fd_out);
}
-
-/* for STDIO, we handle the case where several clients use it
- (nographic mode) */
-
-#define TERM_FIFO_MAX_SIZE 1
-
-static uint8_t term_fifo[TERM_FIFO_MAX_SIZE];
-static int term_fifo_size;
-
-static int stdio_read_poll(void *opaque)
-{
- CharDriverState *chr = opaque;
-
- /* try to flush the queue if needed */
- if (term_fifo_size != 0 && qemu_chr_be_can_write(chr) > 0) {
- qemu_chr_be_write(chr, term_fifo, 1);
- term_fifo_size = 0;
- }
- /* see if we can absorb more chars */
- if (term_fifo_size == 0)
- return 1;
- else
- return 0;
-}
-
-static void stdio_read(void *opaque)
-{
- int size;
- uint8_t buf[1];
- CharDriverState *chr = opaque;
-
- size = read(0, buf, 1);
- if (size == 0) {
- /* stdin has been closed. Remove it from the active list. */
- qemu_set_fd_handler2(0, NULL, NULL, NULL, NULL);
- qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
- return;
- }
- if (size > 0) {
- if (qemu_chr_be_can_write(chr) > 0) {
- qemu_chr_be_write(chr, buf, 1);
- } else if (term_fifo_size == 0) {
- term_fifo[term_fifo_size++] = buf[0];
- }
- }
-}
-
/* init terminal so that we can grab keys */
static struct termios oldtty;
static int old_fd0_flags;
@@ -760,8 +704,6 @@ static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
static void qemu_chr_close_stdio(struct CharDriverState *chr)
{
term_exit();
- stdio_nb_clients--;
- qemu_set_fd_handler2(0, NULL, NULL, NULL, NULL);
fd_chr_close(chr);
}
@@ -769,25 +711,18 @@ static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts)
{
CharDriverState *chr;
- if (stdio_nb_clients >= STDIO_MAX_CLIENTS) {
- return NULL;
- }
if (is_daemonized()) {
error_report("cannot use stdio with -daemonize");
return NULL;
}
- if (stdio_nb_clients == 0) {
- old_fd0_flags = fcntl(0, F_GETFL);
- tcgetattr (0, &oldtty);
- fcntl(0, F_SETFL, O_NONBLOCK);
- atexit(term_exit);
- }
+ old_fd0_flags = fcntl(0, F_GETFL);
+ tcgetattr (0, &oldtty);
+ fcntl(0, F_SETFL, O_NONBLOCK);
+ atexit(term_exit);
chr = qemu_chr_open_fd(0, 1);
chr->chr_close = qemu_chr_close_stdio;
chr->chr_set_echo = qemu_chr_set_echo_stdio;
- qemu_set_fd_handler2(0, stdio_read_poll, stdio_read, NULL, chr);
- stdio_nb_clients++;
stdio_allow_signal = qemu_opt_get_bool(opts, "signal",
display_type != DT_NOGRAPHIC);
qemu_chr_fe_set_echo(chr, false);
@@ -1448,8 +1383,6 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd)
#else /* _WIN32 */
-static CharDriverState *stdio_clients[STDIO_MAX_CLIENTS];
-
typedef struct {
int max_size;
HANDLE hcom, hrecv, hsend;
@@ -1951,7 +1884,6 @@ static void win_stdio_close(CharDriverState *chr)
g_free(chr->opaque);
g_free(chr);
- stdio_nb_clients--;
}
static CharDriverState *qemu_chr_open_win_stdio(QemuOpts *opts)
@@ -1961,11 +1893,6 @@ static CharDriverState *qemu_chr_open_win_stdio(QemuOpts *opts)
DWORD dwMode;
int is_console = 0;
- if (stdio_nb_clients >= STDIO_MAX_CLIENTS
- || ((display_type != DT_NOGRAPHIC) && (stdio_nb_clients != 0))) {
- return NULL;
- }
-
chr = g_malloc0(sizeof(CharDriverState));
stdio = g_malloc0(sizeof(WinStdioCharState));
@@ -1981,37 +1908,34 @@ static CharDriverState *qemu_chr_open_win_stdio(QemuOpts *opts)
chr->chr_write = win_stdio_write;
chr->chr_close = win_stdio_close;
- if (stdio_nb_clients == 0) {
- if (is_console) {
- if (qemu_add_wait_object(stdio->hStdIn,
- win_stdio_wait_func, chr)) {
- fprintf(stderr, "qemu_add_wait_object: failed\n");
- }
- } else {
- DWORD dwId;
-
- stdio->hInputReadyEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
- stdio->hInputDoneEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
- stdio->hInputThread = CreateThread(NULL, 0, win_stdio_thread,
- chr, 0, &dwId);
-
- if (stdio->hInputThread == INVALID_HANDLE_VALUE
- || stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
- || stdio->hInputDoneEvent == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "cannot create stdio thread or event\n");
- exit(1);
- }
- if (qemu_add_wait_object(stdio->hInputReadyEvent,
- win_stdio_thread_wait_func, chr)) {
- fprintf(stderr, "qemu_add_wait_object: failed\n");
- }
+ if (is_console) {
+ if (qemu_add_wait_object(stdio->hStdIn,
+ win_stdio_wait_func, chr)) {
+ fprintf(stderr, "qemu_add_wait_object: failed\n");
+ }
+ } else {
+ DWORD dwId;
+
+ stdio->hInputReadyEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+ stdio->hInputDoneEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+ stdio->hInputThread = CreateThread(NULL, 0, win_stdio_thread,
+ chr, 0, &dwId);
+
+ if (stdio->hInputThread == INVALID_HANDLE_VALUE
+ || stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
+ || stdio->hInputDoneEvent == INVALID_HANDLE_VALUE) {
+ fprintf(stderr, "cannot create stdio thread or event\n");
+ exit(1);
+ }
+ if (qemu_add_wait_object(stdio->hInputReadyEvent,
+ win_stdio_thread_wait_func, chr)) {
+ fprintf(stderr, "qemu_add_wait_object: failed\n");
}
}
dwMode |= ENABLE_LINE_INPUT;
- stdio_clients[stdio_nb_clients++] = chr;
- if (stdio_nb_clients == 1 && is_console) {
+ if (is_console) {
/* set the terminal in raw mode */
/* ENABLE_QUICK_EDIT_MODE | ENABLE_EXTENDED_FLAGS */
dwMode |= ENABLE_PROCESSED_INPUT;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 01/20] char-socket: fix error reporting Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 02/20] qemu-char: remove dead/confusing logic with nb_stdio_clients Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-29 9:53 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 04/20] qemu-char: convert fd_chr to use a GIOChannel Amit Shah
` (17 subsequent siblings)
20 siblings, 1 reply; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
This is a special GSource that supports CharDriverState style
poll callbacks.
For reviewability and bisectability, this code is #if 0'd out in this
patch to avoid unused warnings since all of the functions are static.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index 0a74c10..8aa0b41 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -541,6 +541,146 @@ int send_all(int fd, const void *_buf, int len1)
#ifndef _WIN32
+#if 0
+typedef struct IOWatchPoll
+{
+ GSource *src;
+ int max_size;
+
+ IOCanReadHandler *fd_can_read;
+ void *opaque;
+
+ QTAILQ_ENTRY(IOWatchPoll) node;
+} IOWatchPoll;
+
+static QTAILQ_HEAD(, IOWatchPoll) io_watch_poll_list =
+ QTAILQ_HEAD_INITIALIZER(io_watch_poll_list);
+
+static IOWatchPoll *io_watch_poll_from_source(GSource *source)
+{
+ IOWatchPoll *i;
+
+ QTAILQ_FOREACH(i, &io_watch_poll_list, node) {
+ if (i->src == source) {
+ return i;
+ }
+ }
+
+ return NULL;
+}
+
+static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_)
+{
+ IOWatchPoll *iwp = io_watch_poll_from_source(source);
+
+ iwp->max_size = iwp->fd_can_read(iwp->opaque);
+ if (iwp->max_size == 0) {
+ return FALSE;
+ }
+
+ return g_io_watch_funcs.prepare(source, timeout_);
+}
+
+static gboolean io_watch_poll_check(GSource *source)
+{
+ IOWatchPoll *iwp = io_watch_poll_from_source(source);
+
+ if (iwp->max_size == 0) {
+ return FALSE;
+ }
+
+ return g_io_watch_funcs.check(source);
+}
+
+static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
+ gpointer user_data)
+{
+ return g_io_watch_funcs.dispatch(source, callback, user_data);
+}
+
+static void io_watch_poll_finalize(GSource *source)
+{
+ IOWatchPoll *iwp = io_watch_poll_from_source(source);
+ QTAILQ_REMOVE(&io_watch_poll_list, iwp, node);
+ g_io_watch_funcs.finalize(source);
+}
+
+static GSourceFuncs io_watch_poll_funcs = {
+ .prepare = io_watch_poll_prepare,
+ .check = io_watch_poll_check,
+ .dispatch = io_watch_poll_dispatch,
+ .finalize = io_watch_poll_finalize,
+};
+
+/* Can only be used for read */
+static guint io_add_watch_poll(GIOChannel *channel,
+ IOCanReadHandler *fd_can_read,
+ GIOFunc fd_read,
+ gpointer user_data)
+{
+ IOWatchPoll *iwp;
+ GSource *src;
+ guint tag;
+
+ src = g_io_create_watch(channel, G_IO_IN | G_IO_ERR | G_IO_HUP);
+ g_source_set_funcs(src, &io_watch_poll_funcs);
+ g_source_set_callback(src, (GSourceFunc)fd_read, user_data, NULL);
+ tag = g_source_attach(src, NULL);
+ g_source_unref(src);
+
+ iwp = g_malloc0(sizeof(*iwp));
+ iwp->src = src;
+ iwp->max_size = 0;
+ iwp->fd_can_read = fd_can_read;
+ iwp->opaque = user_data;
+
+ QTAILQ_INSERT_HEAD(&io_watch_poll_list, iwp, node);
+
+ return tag;
+}
+
+static GIOChannel *io_channel_from_fd(int fd)
+{
+ GIOChannel *chan;
+
+ if (fd == -1) {
+ return NULL;
+ }
+
+ chan = g_io_channel_unix_new(fd);
+
+ g_io_channel_set_encoding(chan, NULL, NULL);
+ g_io_channel_set_buffered(chan, FALSE);
+
+ return chan;
+}
+
+static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
+{
+ GIOStatus status;
+ gsize bytes_written;
+ int len;
+ const uint8_t *buf = _buf;
+
+ len = len1;
+ while (len > 0) {
+ status = g_io_channel_write_chars(fd, (const gchar *)buf, len,
+ &bytes_written, NULL);
+ if (status != G_IO_STATUS_NORMAL) {
+ if (status != G_IO_STATUS_AGAIN) {
+ return -1;
+ }
+ } else if (status == G_IO_STATUS_EOF) {
+ break;
+ } else {
+ buf += bytes_written;
+ len -= bytes_written;
+ }
+ }
+ return len1 - len;
+}
+#endif
+
typedef struct {
int fd_in, fd_out;
int max_size;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support
2013-03-05 17:51 ` [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support Amit Shah
@ 2013-03-29 9:53 ` Amit Shah
2013-03-29 12:24 ` Anthony Liguori
0 siblings, 1 reply; 32+ messages in thread
From: Amit Shah @ 2013-03-29 9:53 UTC (permalink / raw)
To: qemu list; +Cc: Anthony Liguori, Anthony Liguori
On (Tue) 05 Mar 2013 [23:21:18], Amit Shah wrote:
> From: Anthony Liguori <aliguori@us.ibm.com>
>
> This is a special GSource that supports CharDriverState style
> poll callbacks.
>
> For reviewability and bisectability, this code is #if 0'd out in this
> patch to avoid unused warnings since all of the functions are static.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> +static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
> +{
> + GIOStatus status;
> + gsize bytes_written;
> + int len;
> + const uint8_t *buf = _buf;
> +
> + len = len1;
> + while (len > 0) {
> + status = g_io_channel_write_chars(fd, (const gchar *)buf, len,
> + &bytes_written, NULL);
> + if (status != G_IO_STATUS_NORMAL) {
> + if (status != G_IO_STATUS_AGAIN) {
> + return -1;
> + }
It's not quite right to return -1 here; previous iterations of the
while loop could have successfully written data, and (len1 - len)
could be +ve.
How to approach this? Convert all callers of qemu_chr_fe_write() to
also pass a bytes_written param to handle this case?
> + } else if (status == G_IO_STATUS_EOF) {
> + break;
> + } else {
> + buf += bytes_written;
> + len -= bytes_written;
> + }
> + }
> + return len1 - len;
> +}
> +#endif
> +
> typedef struct {
> int fd_in, fd_out;
> int max_size;
Amit
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support
2013-03-29 9:53 ` Amit Shah
@ 2013-03-29 12:24 ` Anthony Liguori
2013-03-29 12:42 ` Amit Shah
0 siblings, 1 reply; 32+ messages in thread
From: Anthony Liguori @ 2013-03-29 12:24 UTC (permalink / raw)
To: Amit Shah, qemu list
Amit Shah <amit.shah@redhat.com> writes:
> On (Tue) 05 Mar 2013 [23:21:18], Amit Shah wrote:
>> From: Anthony Liguori <aliguori@us.ibm.com>
>>
>> This is a special GSource that supports CharDriverState style
>> poll callbacks.
>>
>> For reviewability and bisectability, this code is #if 0'd out in this
>> patch to avoid unused warnings since all of the functions are static.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>
>
>> +static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
>> +{
>> + GIOStatus status;
>> + gsize bytes_written;
>> + int len;
>> + const uint8_t *buf = _buf;
>> +
>> + len = len1;
>> + while (len > 0) {
>> + status = g_io_channel_write_chars(fd, (const gchar *)buf, len,
>> + &bytes_written, NULL);
>> + if (status != G_IO_STATUS_NORMAL) {
>> + if (status != G_IO_STATUS_AGAIN) {
>> + return -1;
>> + }
>
> It's not quite right to return -1 here; previous iterations of the
> while loop could have successfully written data, and (len1 - len)
> could be +ve.
Once -1 is returned, it's a terminal error. It doesn't matter that we
may have written some data.
Regards,
Anthony Liguori
>
> How to approach this? Convert all callers of qemu_chr_fe_write() to
> also pass a bytes_written param to handle this case?
>
>> + } else if (status == G_IO_STATUS_EOF) {
>> + break;
>> + } else {
>> + buf += bytes_written;
>> + len -= bytes_written;
>> + }
>> + }
>> + return len1 - len;
>> +}
>> +#endif
>> +
>> typedef struct {
>> int fd_in, fd_out;
>> int max_size;
>
> Amit
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support
2013-03-29 12:24 ` Anthony Liguori
@ 2013-03-29 12:42 ` Amit Shah
2013-03-29 14:03 ` Anthony Liguori
0 siblings, 1 reply; 32+ messages in thread
From: Amit Shah @ 2013-03-29 12:42 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu list
On (Fri) 29 Mar 2013 [07:24:07], Anthony Liguori wrote:
> Amit Shah <amit.shah@redhat.com> writes:
>
> > On (Tue) 05 Mar 2013 [23:21:18], Amit Shah wrote:
> >> From: Anthony Liguori <aliguori@us.ibm.com>
> >>
> >> This is a special GSource that supports CharDriverState style
> >> poll callbacks.
> >>
> >> For reviewability and bisectability, this code is #if 0'd out in this
> >> patch to avoid unused warnings since all of the functions are static.
> >>
> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> >
> >
> >> +static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
> >> +{
> >> + GIOStatus status;
> >> + gsize bytes_written;
> >> + int len;
> >> + const uint8_t *buf = _buf;
> >> +
> >> + len = len1;
> >> + while (len > 0) {
> >> + status = g_io_channel_write_chars(fd, (const gchar *)buf, len,
> >> + &bytes_written, NULL);
> >> + if (status != G_IO_STATUS_NORMAL) {
> >> + if (status != G_IO_STATUS_AGAIN) {
> >> + return -1;
> >> + }
> >
> > It's not quite right to return -1 here; previous iterations of the
> > while loop could have successfully written data, and (len1 - len)
> > could be +ve.
>
> Once -1 is returned, it's a terminal error. It doesn't matter that we
> may have written some data.
Why do you say that?
Try this:
Start a guest with a virtio-serial channel.
In the guest, start writing something to the channel, say a 1M file.
In the host, open the chardev but don't read from it.
This will make the guest send some data, fill the vq and the char
buffers, and then make it stop.
The flow control code in virtio-console.c and virtio-serial-bus.c will
be triggered, and a watch will be registered for the pending data to
be sent once the chardev becomes writable.
If one starts reading from the chardev, we'll get duplicate data in
the current case (16 bytes were written, 20 not, but a retry for all
the 36 bytes will be tried in this case).
Instead, we only want to continue writing from the offset that was
last written.
Having a chardev open but not reading from it causing data to stop
flowing isn't a terminal error.
Amit
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support
2013-03-29 12:42 ` Amit Shah
@ 2013-03-29 14:03 ` Anthony Liguori
2013-03-29 16:08 ` Amit Shah
0 siblings, 1 reply; 32+ messages in thread
From: Anthony Liguori @ 2013-03-29 14:03 UTC (permalink / raw)
To: Amit Shah; +Cc: qemu list
Amit Shah <amit.shah@redhat.com> writes:
> On (Fri) 29 Mar 2013 [07:24:07], Anthony Liguori wrote:
>> Amit Shah <amit.shah@redhat.com> writes:
>>
>> > On (Tue) 05 Mar 2013 [23:21:18], Amit Shah wrote:
>> >> From: Anthony Liguori <aliguori@us.ibm.com>
>> >>
>> >> This is a special GSource that supports CharDriverState style
>> >> poll callbacks.
>> >>
>> >> For reviewability and bisectability, this code is #if 0'd out in this
>> >> patch to avoid unused warnings since all of the functions are static.
>> >>
>> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> >> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>> >
>> >
>> >> +static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
>> >> +{
>> >> + GIOStatus status;
>> >> + gsize bytes_written;
>> >> + int len;
>> >> + const uint8_t *buf = _buf;
>> >> +
>> >> + len = len1;
>> >> + while (len > 0) {
>> >> + status = g_io_channel_write_chars(fd, (const gchar *)buf, len,
>> >> + &bytes_written, NULL);
>> >> + if (status != G_IO_STATUS_NORMAL) {
>> >> + if (status != G_IO_STATUS_AGAIN) {
>> >> + return -1;
>> >> + }
>> >
>> > It's not quite right to return -1 here; previous iterations of the
>> > while loop could have successfully written data, and (len1 - len)
>> > could be +ve.
>>
>> Once -1 is returned, it's a terminal error. It doesn't matter that we
>> may have written some data.
>
> Why do you say that?
Because you're quoting the wrong patch :-) This bit is rewritten by a
later patch which is the source of your problem below. In the patch you
quote, we busy spin until all data is written. However, with:
commit 23673ca740e0eda66901ca801a5a901df378b063
Author: Anthony Liguori <aliguori@us.ibm.com>
Date: Tue Mar 5 23:21:23 2013 +0530
qemu-char: add watch support
We started to return EAGAIN even if we have a partially successful
write. I'm running a patch through testing right now that rewrites this
function to have sane semantics (return bytes written on partial write).
I'll post as soon as testing completes.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support
2013-03-29 14:03 ` Anthony Liguori
@ 2013-03-29 16:08 ` Amit Shah
0 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-29 16:08 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu list
On (Fri) 29 Mar 2013 [09:03:10], Anthony Liguori wrote:
> Amit Shah <amit.shah@redhat.com> writes:
>
> > On (Fri) 29 Mar 2013 [07:24:07], Anthony Liguori wrote:
> >> Amit Shah <amit.shah@redhat.com> writes:
> >>
> >> > On (Tue) 05 Mar 2013 [23:21:18], Amit Shah wrote:
> >> >> From: Anthony Liguori <aliguori@us.ibm.com>
> >> >>
> >> >> This is a special GSource that supports CharDriverState style
> >> >> poll callbacks.
> >> >>
> >> >> For reviewability and bisectability, this code is #if 0'd out in this
> >> >> patch to avoid unused warnings since all of the functions are static.
> >> >>
> >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> >> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> >> >
> >> >
> >> >> +static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
> >> >> +{
> >> >> + GIOStatus status;
> >> >> + gsize bytes_written;
> >> >> + int len;
> >> >> + const uint8_t *buf = _buf;
> >> >> +
> >> >> + len = len1;
> >> >> + while (len > 0) {
> >> >> + status = g_io_channel_write_chars(fd, (const gchar *)buf, len,
> >> >> + &bytes_written, NULL);
> >> >> + if (status != G_IO_STATUS_NORMAL) {
> >> >> + if (status != G_IO_STATUS_AGAIN) {
> >> >> + return -1;
> >> >> + }
> >> >
> >> > It's not quite right to return -1 here; previous iterations of the
> >> > while loop could have successfully written data, and (len1 - len)
> >> > could be +ve.
> >>
> >> Once -1 is returned, it's a terminal error. It doesn't matter that we
> >> may have written some data.
> >
> > Why do you say that?
>
> Because you're quoting the wrong patch :-)
Indeed.
> This bit is rewritten by a
> later patch which is the source of your problem below. In the patch you
> quote, we busy spin until all data is written. However, with:
>
> commit 23673ca740e0eda66901ca801a5a901df378b063
> Author: Anthony Liguori <aliguori@us.ibm.com>
> Date: Tue Mar 5 23:21:23 2013 +0530
>
> qemu-char: add watch support
>
> We started to return EAGAIN even if we have a partially successful
> write. I'm running a patch through testing right now that rewrites this
> function to have sane semantics (return bytes written on partial write).
Yes, that's where the problem is: EINTR and EAGAIN returns.
> I'll post as soon as testing completes.
Thanks!
Amit
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 04/20] qemu-char: convert fd_chr to use a GIOChannel
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (2 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 03/20] char: add IOWatchPoll support Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 05/20] qemu-char: convert pty to GIOChannel Amit Shah
` (16 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
This uses the newly introduced IOWatchPoll source.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 103 ++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 62 insertions(+), 41 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 8aa0b41..4c63ccb 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -541,7 +541,6 @@ int send_all(int fd, const void *_buf, int len1)
#ifndef _WIN32
-#if 0
typedef struct IOWatchPoll
{
GSource *src;
@@ -679,60 +678,71 @@ static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
}
return len1 - len;
}
-#endif
-typedef struct {
- int fd_in, fd_out;
+typedef struct FDCharDriver {
+ CharDriverState *chr;
+ GIOChannel *fd_in, *fd_out;
+ guint fd_in_tag;
int max_size;
+ QTAILQ_ENTRY(FDCharDriver) node;
} FDCharDriver;
-
static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
FDCharDriver *s = chr->opaque;
- return send_all(s->fd_out, buf, len);
+
+ return io_channel_send_all(s->fd_out, buf, len);
}
-static int fd_chr_read_poll(void *opaque)
+static gboolean fd_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
FDCharDriver *s = chr->opaque;
-
- s->max_size = qemu_chr_be_can_write(chr);
- return s->max_size;
-}
-
-static void fd_chr_read(void *opaque)
-{
- CharDriverState *chr = opaque;
- FDCharDriver *s = chr->opaque;
- int size, len;
+ int len;
uint8_t buf[READ_BUF_LEN];
+ GIOStatus status;
+ gsize bytes_read;
len = sizeof(buf);
- if (len > s->max_size)
+ if (len > s->max_size) {
len = s->max_size;
- if (len == 0)
- return;
- size = read(s->fd_in, buf, len);
- if (size == 0) {
- /* FD has been closed. Remove it from the active list. */
- qemu_set_fd_handler2(s->fd_in, NULL, NULL, NULL, NULL);
+ }
+ if (len == 0) {
+ return FALSE;
+ }
+
+ status = g_io_channel_read_chars(chan, (gchar *)buf,
+ len, &bytes_read, NULL);
+ if (status == G_IO_STATUS_EOF) {
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
- return;
+ return FALSE;
}
- if (size > 0) {
- qemu_chr_be_write(chr, buf, size);
+ if (status == G_IO_STATUS_NORMAL) {
+ qemu_chr_be_write(chr, buf, bytes_read);
}
+
+ return TRUE;
+}
+
+static int fd_chr_read_poll(void *opaque)
+{
+ CharDriverState *chr = opaque;
+ FDCharDriver *s = chr->opaque;
+
+ s->max_size = qemu_chr_be_can_write(chr);
+ return s->max_size;
}
static void fd_chr_update_read_handler(CharDriverState *chr)
{
FDCharDriver *s = chr->opaque;
- if (s->fd_in >= 0) {
- qemu_set_fd_handler2(s->fd_in, fd_chr_read_poll,
- fd_chr_read, NULL, chr);
+ if (s->fd_in_tag) {
+ g_source_remove(s->fd_in_tag);
+ }
+
+ if (s->fd_in) {
+ s->fd_in_tag = io_add_watch_poll(s->fd_in, fd_chr_read_poll, fd_chr_read, chr);
}
}
@@ -740,8 +750,16 @@ static void fd_chr_close(struct CharDriverState *chr)
{
FDCharDriver *s = chr->opaque;
- if (s->fd_in >= 0) {
- qemu_set_fd_handler2(s->fd_in, NULL, NULL, NULL, NULL);
+ if (s->fd_in_tag) {
+ g_source_remove(s->fd_in_tag);
+ s->fd_in_tag = 0;
+ }
+
+ if (s->fd_in) {
+ g_io_channel_unref(s->fd_in);
+ }
+ if (s->fd_out) {
+ g_io_channel_unref(s->fd_out);
}
g_free(s);
@@ -756,8 +774,9 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
chr = g_malloc0(sizeof(CharDriverState));
s = g_malloc0(sizeof(FDCharDriver));
- s->fd_in = fd_in;
- s->fd_out = fd_out;
+ s->fd_in = io_channel_from_fd(fd_in);
+ s->fd_out = io_channel_from_fd(fd_out);
+ s->chr = chr;
chr->opaque = s;
chr->chr_write = fd_chr_write;
chr->chr_update_read_handler = fd_chr_update_read_handler;
@@ -1230,22 +1249,24 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
case CHR_IOCTL_SERIAL_SET_PARAMS:
{
QEMUSerialSetParams *ssp = arg;
- tty_serial_init(s->fd_in, ssp->speed, ssp->parity,
+ tty_serial_init(g_io_channel_unix_get_fd(s->fd_in),
+ ssp->speed, ssp->parity,
ssp->data_bits, ssp->stop_bits);
}
break;
case CHR_IOCTL_SERIAL_SET_BREAK:
{
int enable = *(int *)arg;
- if (enable)
- tcsendbreak(s->fd_in, 1);
+ if (enable) {
+ tcsendbreak(g_io_channel_unix_get_fd(s->fd_in), 1);
+ }
}
break;
case CHR_IOCTL_SERIAL_GET_TIOCM:
{
int sarg = 0;
int *targ = (int *)arg;
- ioctl(s->fd_in, TIOCMGET, &sarg);
+ ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMGET, &sarg);
*targ = 0;
if (sarg & TIOCM_CTS)
*targ |= CHR_TIOCM_CTS;
@@ -1265,7 +1286,7 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
{
int sarg = *(int *)arg;
int targ = 0;
- ioctl(s->fd_in, TIOCMGET, &targ);
+ ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMGET, &targ);
targ &= ~(CHR_TIOCM_CTS | CHR_TIOCM_CAR | CHR_TIOCM_DSR
| CHR_TIOCM_RI | CHR_TIOCM_DTR | CHR_TIOCM_RTS);
if (sarg & CHR_TIOCM_CTS)
@@ -1280,7 +1301,7 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
targ |= TIOCM_DTR;
if (sarg & CHR_TIOCM_RTS)
targ |= TIOCM_RTS;
- ioctl(s->fd_in, TIOCMSET, &targ);
+ ioctl(g_io_channel_unix_get_fd(s->fd_in), TIOCMSET, &targ);
}
break;
default:
@@ -1295,7 +1316,7 @@ static void qemu_chr_close_tty(CharDriverState *chr)
int fd = -1;
if (s) {
- fd = s->fd_in;
+ fd = g_io_channel_unix_get_fd(s->fd_in);
}
fd_chr_close(chr);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 05/20] qemu-char: convert pty to GIOChannel
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (3 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 04/20] qemu-char: convert fd_chr to use a GIOChannel Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 06/20] qemu-char: convert UDP " Amit Shah
` (15 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 44 +++++++++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 4c63ccb..64b95d7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -953,7 +953,8 @@ static void cfmakeraw (struct termios *termios_p)
#define HAVE_CHARDEV_TTY 1
typedef struct {
- int fd;
+ GIOChannel *fd;
+ guint fd_tag;
int connected;
int polling;
int read_bytes;
@@ -972,7 +973,7 @@ static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
pty_chr_update_read_handler(chr);
return 0;
}
- return send_all(s->fd, buf, len);
+ return io_channel_send_all(s->fd, buf, len);
}
static int pty_chr_read_poll(void *opaque)
@@ -984,36 +985,39 @@ static int pty_chr_read_poll(void *opaque)
return s->read_bytes;
}
-static void pty_chr_read(void *opaque)
+static gboolean pty_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
PtyCharDriver *s = chr->opaque;
- int size, len;
+ gsize size, len;
uint8_t buf[READ_BUF_LEN];
+ GIOStatus status;
len = sizeof(buf);
if (len > s->read_bytes)
len = s->read_bytes;
if (len == 0)
- return;
- size = read(s->fd, buf, len);
- if ((size == -1 && errno == EIO) ||
- (size == 0)) {
+ return FALSE;
+ status = g_io_channel_read_chars(s->fd, (gchar *)buf, len, &size, NULL);
+ if (status != G_IO_STATUS_NORMAL) {
pty_chr_state(chr, 0);
- return;
- }
- if (size > 0) {
+ return FALSE;
+ } else {
pty_chr_state(chr, 1);
qemu_chr_be_write(chr, buf, size);
}
+ return TRUE;
}
static void pty_chr_update_read_handler(CharDriverState *chr)
{
PtyCharDriver *s = chr->opaque;
- qemu_set_fd_handler2(s->fd, pty_chr_read_poll,
- pty_chr_read, NULL, chr);
+ if (s->fd_tag) {
+ g_source_remove(s->fd_tag);
+ }
+
+ s->fd_tag = io_add_watch_poll(s->fd, pty_chr_read_poll, pty_chr_read, chr);
s->polling = 1;
/*
* Short timeout here: just need wait long enougth that qemu makes
@@ -1031,7 +1035,8 @@ static void pty_chr_state(CharDriverState *chr, int connected)
PtyCharDriver *s = chr->opaque;
if (!connected) {
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+ g_source_remove(s->fd_tag);
+ s->fd_tag = 0;
s->connected = 0;
s->polling = 0;
/* (re-)connect poll interval for idle guests: once per second.
@@ -1066,9 +1071,14 @@ static void pty_chr_timer(void *opaque)
static void pty_chr_close(struct CharDriverState *chr)
{
PtyCharDriver *s = chr->opaque;
+ int fd;
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
- close(s->fd);
+ if (s->fd_tag) {
+ g_source_remove(s->fd_tag);
+ }
+ fd = g_io_channel_unix_get_fd(s->fd);
+ g_io_channel_unref(s->fd);
+ close(fd);
qemu_del_timer(s->timer);
qemu_free_timer(s->timer);
g_free(s);
@@ -1120,7 +1130,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
chr->chr_update_read_handler = pty_chr_update_read_handler;
chr->chr_close = pty_chr_close;
- s->fd = master_fd;
+ s->fd = io_channel_from_fd(master_fd);
s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr);
return chr;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 06/20] qemu-char: convert UDP to GIOChannel
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (4 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 05/20] qemu-char: convert pty to GIOChannel Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 07/20] qemu-char: tcp: make use GIOChannel Amit Shah
` (14 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 57 insertions(+), 11 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 64b95d7..94ff332 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -654,6 +654,26 @@ static GIOChannel *io_channel_from_fd(int fd)
return chan;
}
+static GIOChannel *io_channel_from_socket(int fd)
+{
+ GIOChannel *chan;
+
+ if (fd == -1) {
+ return NULL;
+ }
+
+#ifdef _WIN32
+ chan = g_io_channel_win32_new_socket(fd);
+#else
+ chan = g_io_channel_unix_new(fd);
+#endif
+
+ g_io_channel_set_encoding(chan, NULL, NULL);
+ g_io_channel_set_buffered(chan, FALSE);
+
+ return chan;
+}
+
static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
{
GIOStatus status;
@@ -2126,6 +2146,8 @@ static CharDriverState *qemu_chr_open_win_stdio(QemuOpts *opts)
typedef struct {
int fd;
+ GIOChannel *chan;
+ guint tag;
uint8_t buf[READ_BUF_LEN];
int bufcnt;
int bufptr;
@@ -2135,8 +2157,17 @@ typedef struct {
static int udp_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
NetCharDriver *s = chr->opaque;
+ gsize bytes_written;
+ GIOStatus status;
+
+ status = g_io_channel_write_chars(s->chan, (const gchar *)buf, len, &bytes_written, NULL);
+ if (status == G_IO_STATUS_EOF) {
+ return 0;
+ } else if (status != G_IO_STATUS_NORMAL) {
+ return -1;
+ }
- return send(s->fd, (const void *)buf, len, 0);
+ return bytes_written;
}
static int udp_chr_read_poll(void *opaque)
@@ -2157,17 +2188,22 @@ static int udp_chr_read_poll(void *opaque)
return s->max_size;
}
-static void udp_chr_read(void *opaque)
+static gboolean udp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
NetCharDriver *s = chr->opaque;
+ gsize bytes_read = 0;
+ GIOStatus status;
if (s->max_size == 0)
- return;
- s->bufcnt = qemu_recv(s->fd, s->buf, sizeof(s->buf), 0);
+ return FALSE;
+ status = g_io_channel_read_chars(s->chan, (gchar *)s->buf, sizeof(s->buf),
+ &bytes_read, NULL);
+ s->bufcnt = bytes_read;
s->bufptr = s->bufcnt;
- if (s->bufcnt <= 0)
- return;
+ if (status != G_IO_STATUS_NORMAL) {
+ return FALSE;
+ }
s->bufptr = 0;
while (s->max_size > 0 && s->bufptr < s->bufcnt) {
@@ -2175,23 +2211,32 @@ static void udp_chr_read(void *opaque)
s->bufptr++;
s->max_size = qemu_chr_be_can_write(chr);
}
+
+ return TRUE;
}
static void udp_chr_update_read_handler(CharDriverState *chr)
{
NetCharDriver *s = chr->opaque;
- if (s->fd >= 0) {
- qemu_set_fd_handler2(s->fd, udp_chr_read_poll,
- udp_chr_read, NULL, chr);
+ if (s->tag) {
+ g_source_remove(s->tag);
+ s->tag = 0;
+ }
+
+ if (s->chan) {
+ s->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->fd >= 0) {
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+ if (s->tag) {
+ g_source_remove(s->tag);
+ }
+ if (s->chan) {
+ g_io_channel_unref(s->chan);
closesocket(s->fd);
}
g_free(s);
@@ -2214,6 +2259,7 @@ static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
}
s->fd = fd;
+ s->chan = io_channel_from_socket(s->fd);
s->bufcnt = 0;
s->bufptr = 0;
chr->opaque = s;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 07/20] qemu-char: tcp: make use GIOChannel
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (5 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 06/20] qemu-char: convert UDP " Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 08/20] qemu-char: add watch support Amit Shah
` (13 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
I didn't bother switching to g_io_channel_read/write because we need to use
sendmsg on Unix. No problem though since we're using an unbuffered channel.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 60 ++++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 42 insertions(+), 18 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 94ff332..5a53491 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2285,6 +2285,9 @@ return_err:
/* TCP Net console */
typedef struct {
+
+ GIOChannel *chan, *listen_chan;
+ guint tag, listen_tag;
int fd, listen_fd;
int connected;
int max_size;
@@ -2294,13 +2297,13 @@ typedef struct {
int msgfd;
} TCPCharDriver;
-static void tcp_chr_accept(void *opaque);
+static gboolean tcp_chr_accept(GIOChannel *chan, GIOCondition cond, void *opaque);
static int tcp_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
TCPCharDriver *s = chr->opaque;
if (s->connected) {
- return send_all(s->fd, buf, len);
+ return io_channel_send_all(s->chan, buf, len);
} else {
/* XXX: indicate an error ? */
return len;
@@ -2440,15 +2443,16 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
}
#endif
-static void tcp_chr_read(void *opaque)
+static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr->opaque;
uint8_t buf[READ_BUF_LEN];
int len, size;
- if (!s->connected || s->max_size <= 0)
- return;
+ if (!s->connected || s->max_size <= 0) {
+ return FALSE;
+ }
len = sizeof(buf);
if (len > s->max_size)
len = s->max_size;
@@ -2456,10 +2460,13 @@ static void tcp_chr_read(void *opaque)
if (size == 0) {
/* connection closed */
s->connected = 0;
- if (s->listen_fd >= 0) {
- qemu_set_fd_handler2(s->listen_fd, NULL, tcp_chr_accept, NULL, chr);
+ if (s->listen_chan) {
+ s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, tcp_chr_accept, chr);
}
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+ g_source_remove(s->tag);
+ s->tag = 0;
+ g_io_channel_unref(s->chan);
+ s->chan = NULL;
closesocket(s->fd);
s->fd = -1;
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
@@ -2469,6 +2476,8 @@ static void tcp_chr_read(void *opaque)
if (size > 0)
qemu_chr_be_write(chr, buf, size);
}
+
+ return TRUE;
}
#ifndef _WIN32
@@ -2484,9 +2493,8 @@ static void tcp_chr_connect(void *opaque)
TCPCharDriver *s = chr->opaque;
s->connected = 1;
- if (s->fd >= 0) {
- qemu_set_fd_handler2(s->fd, tcp_chr_read_poll,
- tcp_chr_read, NULL, chr);
+ if (s->chan) {
+ s->tag = io_add_watch_poll(s->chan, tcp_chr_read_poll, tcp_chr_read, chr);
}
qemu_chr_generic_open(chr);
}
@@ -2516,13 +2524,15 @@ static int tcp_chr_add_client(CharDriverState *chr, int fd)
if (s->do_nodelay)
socket_set_nodelay(fd);
s->fd = fd;
- qemu_set_fd_handler2(s->listen_fd, NULL, NULL, NULL, NULL);
+ s->chan = io_channel_from_socket(fd);
+ g_source_remove(s->listen_tag);
+ s->listen_tag = 0;
tcp_chr_connect(chr);
return 0;
}
-static void tcp_chr_accept(void *opaque)
+static gboolean tcp_chr_accept(GIOChannel *channel, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
TCPCharDriver *s = chr->opaque;
@@ -2547,7 +2557,7 @@ static void tcp_chr_accept(void *opaque)
}
fd = qemu_accept(s->listen_fd, addr, &len);
if (fd < 0 && errno != EINTR) {
- return;
+ return FALSE;
} else if (fd >= 0) {
if (s->do_telnetopt)
tcp_chr_telnet_init(fd);
@@ -2556,17 +2566,29 @@ static void tcp_chr_accept(void *opaque)
}
if (tcp_chr_add_client(chr, fd) < 0)
close(fd);
+
+ return TRUE;
}
static void tcp_chr_close(CharDriverState *chr)
{
TCPCharDriver *s = chr->opaque;
if (s->fd >= 0) {
- qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+ if (s->tag) {
+ g_source_remove(s->tag);
+ }
+ if (s->chan) {
+ g_io_channel_unref(s->chan);
+ }
closesocket(s->fd);
}
if (s->listen_fd >= 0) {
- qemu_set_fd_handler2(s->listen_fd, NULL, NULL, NULL, NULL);
+ if (s->listen_tag) {
+ g_source_remove(s->listen_tag);
+ }
+ if (s->listen_chan) {
+ g_io_channel_unref(s->listen_chan);
+ }
closesocket(s->listen_fd);
}
g_free(s);
@@ -2632,7 +2654,8 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
if (is_listen) {
s->listen_fd = fd;
- qemu_set_fd_handler2(s->listen_fd, NULL, tcp_chr_accept, NULL, chr);
+ s->listen_chan = io_channel_from_socket(s->listen_fd);
+ s->listen_tag = g_io_add_watch(s->listen_chan, G_IO_IN, tcp_chr_accept, chr);
if (is_telnet) {
s->do_telnetopt = 1;
}
@@ -2640,13 +2663,14 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
s->connected = 1;
s->fd = fd;
socket_set_nodelay(fd);
+ s->chan = io_channel_from_socket(s->fd);
tcp_chr_connect(chr);
}
if (is_listen && is_waitconnect) {
printf("QEMU waiting for connection on: %s\n",
chr->filename);
- tcp_chr_accept(chr);
+ tcp_chr_accept(s->listen_chan, G_IO_IN, chr);
socket_set_nonblock(s->listen_fd);
}
return chr;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 08/20] qemu-char: add watch support
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (6 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 07/20] qemu-char: tcp: make use GIOChannel Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 09/20] qemu-char: add pty watch Amit Shah
` (12 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
This allows a front-end to request for a callback when the backend
is writable again.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
include/char/char.h | 4 ++++
qemu-char.c | 32 +++++++++++++++++++++++++++++++-
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/char/char.h b/include/char/char.h
index c91ce3c..09ac401 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -56,6 +56,7 @@ typedef void IOEventHandler(void *opaque, int event);
struct CharDriverState {
void (*init)(struct CharDriverState *s);
int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
+ GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond);
void (*chr_update_read_handler)(struct CharDriverState *s);
int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
int (*get_msgfd)(struct CharDriverState *s);
@@ -152,6 +153,9 @@ void qemu_chr_fe_close(struct CharDriverState *chr);
void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)
GCC_FMT_ATTR(2, 3);
+guint qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
+ GIOFunc func, void *user_data);
+
/**
* @qemu_chr_fe_write:
*
diff --git a/qemu-char.c b/qemu-char.c
index 5a53491..7091743 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -686,7 +686,11 @@ static int io_channel_send_all(GIOChannel *fd, const void *_buf, int len1)
status = g_io_channel_write_chars(fd, (const gchar *)buf, len,
&bytes_written, NULL);
if (status != G_IO_STATUS_NORMAL) {
- if (status != G_IO_STATUS_AGAIN) {
+ if (status == G_IO_STATUS_AGAIN) {
+ errno = EAGAIN;
+ return -1;
+ } else {
+ errno = EINVAL;
return -1;
}
} else if (status == G_IO_STATUS_EOF) {
@@ -753,6 +757,12 @@ static int fd_chr_read_poll(void *opaque)
return s->max_size;
}
+static GSource *fd_chr_add_watch(CharDriverState *chr, GIOCondition cond)
+{
+ FDCharDriver *s = chr->opaque;
+ return g_io_create_watch(s->fd_out, cond);
+}
+
static void fd_chr_update_read_handler(CharDriverState *chr)
{
FDCharDriver *s = chr->opaque;
@@ -796,8 +806,10 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
s = g_malloc0(sizeof(FDCharDriver));
s->fd_in = io_channel_from_fd(fd_in);
s->fd_out = io_channel_from_fd(fd_out);
+ fcntl(fd_out, F_SETFL, O_NONBLOCK);
s->chr = chr;
chr->opaque = s;
+ chr->chr_add_watch = fd_chr_add_watch;
chr->chr_write = fd_chr_write;
chr->chr_update_read_handler = fd_chr_update_read_handler;
chr->chr_close = fd_chr_close;
@@ -3279,6 +3291,24 @@ void qemu_chr_fe_close(struct CharDriverState *chr)
}
}
+guint qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
+ GIOFunc func, void *user_data)
+{
+ GSource *src;
+ guint tag;
+
+ if (s->chr_add_watch == NULL) {
+ return -ENOSYS;
+ }
+
+ src = s->chr_add_watch(s, cond);
+ g_source_set_callback(src, (GSourceFunc)func, user_data, NULL);
+ tag = g_source_attach(src, NULL);
+ g_source_unref(src);
+
+ return tag;
+}
+
void qemu_chr_delete(CharDriverState *chr)
{
QTAILQ_REMOVE(&chardevs, chr, next);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 09/20] qemu-char: add pty watch
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (7 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 08/20] qemu-char: add watch support Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 10/20] char: add gio watch fn for tcp backends Amit Shah
` (11 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
This lets ptys support adding front end watchs.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index 7091743..f1d089f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1008,6 +1008,12 @@ static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
return io_channel_send_all(s->fd, buf, len);
}
+static GSource *pty_chr_add_watch(CharDriverState *chr, GIOCondition cond)
+{
+ PtyCharDriver *s = chr->opaque;
+ return g_io_create_watch(s->fd, cond);
+}
+
static int pty_chr_read_poll(void *opaque)
{
CharDriverState *chr = opaque;
@@ -1161,6 +1167,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
chr->chr_write = pty_chr_write;
chr->chr_update_read_handler = pty_chr_update_read_handler;
chr->chr_close = pty_chr_close;
+ chr->chr_add_watch = pty_chr_add_watch;
s->fd = io_channel_from_fd(master_fd);
s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 10/20] char: add gio watch fn for tcp backends
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (8 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 09/20] qemu-char: add pty watch Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer Amit Shah
` (10 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index f1d089f..eb0ac81 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2462,6 +2462,12 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
}
#endif
+static GSource *tcp_chr_add_watch(CharDriverState *chr, GIOCondition cond)
+{
+ TCPCharDriver *s = chr->opaque;
+ return g_io_create_watch(s->chan, cond);
+}
+
static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque)
{
CharDriverState *chr = opaque;
@@ -2670,6 +2676,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
chr->chr_close = tcp_chr_close;
chr->get_msgfd = tcp_get_msgfd;
chr->chr_add_client = tcp_chr_add_client;
+ chr->chr_add_watch = tcp_chr_add_watch;
if (is_listen) {
s->listen_fd = fd;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (9 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 10/20] char: add gio watch fn for tcp backends Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-15 15:06 ` Laurent Desnogues
2013-03-05 17:51 ` [Qemu-devel] [PATCH 12/20] qemu-char: remove use of QEMUTimer in favor of glib idle function Amit Shah
` (9 subsequent siblings)
20 siblings, 1 reply; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 68 ++++++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index eb0ac81..6dba943 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -990,12 +990,50 @@ typedef struct {
int connected;
int polling;
int read_bytes;
- QEMUTimer *timer;
+ guint timer_tag;
} PtyCharDriver;
static void pty_chr_update_read_handler(CharDriverState *chr);
static void pty_chr_state(CharDriverState *chr, int connected);
+static gboolean pty_chr_timer(gpointer opaque)
+{
+ struct CharDriverState *chr = opaque;
+ PtyCharDriver *s = chr->opaque;
+
+ if (s->connected) {
+ goto out;
+ }
+ if (s->polling) {
+ /* If we arrive here without polling being cleared due
+ * read returning -EIO, then we are (re-)connected */
+ pty_chr_state(chr, 1);
+ goto out;
+ }
+
+ /* Next poll ... */
+ pty_chr_update_read_handler(chr);
+
+out:
+ return FALSE;
+}
+
+static void pty_chr_rearm_timer(CharDriverState *chr, int ms)
+{
+ PtyCharDriver *s = chr->opaque;
+
+ if (s->timer_tag) {
+ g_source_remove(s->timer_tag);
+ s->timer_tag = 0;
+ }
+
+ if (ms == 1000) {
+ s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
+ } else {
+ s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
+ }
+}
+
static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
{
PtyCharDriver *s = chr->opaque;
@@ -1065,7 +1103,7 @@ static void pty_chr_update_read_handler(CharDriverState *chr)
* timeout to the normal (much longer) poll interval before the
* timer triggers.
*/
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 10);
+ pty_chr_rearm_timer(chr, 10);
}
static void pty_chr_state(CharDriverState *chr, int connected)
@@ -1080,7 +1118,7 @@ static void pty_chr_state(CharDriverState *chr, int connected)
/* (re-)connect poll interval for idle guests: once per second.
* We check more frequently in case the guests sends data to
* the virtual device linked to our pty. */
- qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 1000);
+ pty_chr_rearm_timer(chr, 1000);
} else {
if (!s->connected)
qemu_chr_generic_open(chr);
@@ -1088,23 +1126,6 @@ static void pty_chr_state(CharDriverState *chr, int connected)
}
}
-static void pty_chr_timer(void *opaque)
-{
- struct CharDriverState *chr = opaque;
- PtyCharDriver *s = chr->opaque;
-
- if (s->connected)
- return;
- if (s->polling) {
- /* If we arrive here without polling being cleared due
- * read returning -EIO, then we are (re-)connected */
- pty_chr_state(chr, 1);
- return;
- }
-
- /* Next poll ... */
- pty_chr_update_read_handler(chr);
-}
static void pty_chr_close(struct CharDriverState *chr)
{
@@ -1117,8 +1138,9 @@ static void pty_chr_close(struct CharDriverState *chr)
fd = g_io_channel_unix_get_fd(s->fd);
g_io_channel_unref(s->fd);
close(fd);
- qemu_del_timer(s->timer);
- qemu_free_timer(s->timer);
+ if (s->timer_tag) {
+ g_source_remove(s->timer_tag);
+ }
g_free(s);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
@@ -1170,7 +1192,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
chr->chr_add_watch = pty_chr_add_watch;
s->fd = io_channel_from_fd(master_fd);
- s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr);
+ s->timer_tag = 0;
return chr;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer
2013-03-05 17:51 ` [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer Amit Shah
@ 2013-03-15 15:06 ` Laurent Desnogues
2013-03-15 15:44 ` Anthony Liguori
0 siblings, 1 reply; 32+ messages in thread
From: Laurent Desnogues @ 2013-03-15 15:06 UTC (permalink / raw)
To: Amit Shah; +Cc: Anthony Liguori, qemu list, Anthony Liguori
Hello,
On Tue, Mar 5, 2013 at 6:51 PM, Amit Shah <amit.shah@redhat.com> wrote:
> From: Anthony Liguori <aliguori@us.ibm.com>
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> qemu-char.c | 68 ++++++++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index eb0ac81..6dba943 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -990,12 +990,50 @@ typedef struct {
> int connected;
> int polling;
> int read_bytes;
> - QEMUTimer *timer;
> + guint timer_tag;
> } PtyCharDriver;
>
> static void pty_chr_update_read_handler(CharDriverState *chr);
> static void pty_chr_state(CharDriverState *chr, int connected);
>
> +static gboolean pty_chr_timer(gpointer opaque)
> +{
> + struct CharDriverState *chr = opaque;
> + PtyCharDriver *s = chr->opaque;
> +
> + if (s->connected) {
> + goto out;
> + }
> + if (s->polling) {
> + /* If we arrive here without polling being cleared due
> + * read returning -EIO, then we are (re-)connected */
> + pty_chr_state(chr, 1);
> + goto out;
> + }
> +
> + /* Next poll ... */
> + pty_chr_update_read_handler(chr);
> +
> +out:
> + return FALSE;
> +}
> +
> +static void pty_chr_rearm_timer(CharDriverState *chr, int ms)
> +{
> + PtyCharDriver *s = chr->opaque;
> +
> + if (s->timer_tag) {
> + g_source_remove(s->timer_tag);
> + s->timer_tag = 0;
> + }
> +
> + if (ms == 1000) {
> + s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
It looks like g_timeout_add_seconds isn't available for
poor people using some old distros (glib 2.12.3 here).
Thanks,
Laurent
> + } else {
> + s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
> + }
> +}
> +
> static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
> {
> PtyCharDriver *s = chr->opaque;
> @@ -1065,7 +1103,7 @@ static void pty_chr_update_read_handler(CharDriverState *chr)
> * timeout to the normal (much longer) poll interval before the
> * timer triggers.
> */
> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 10);
> + pty_chr_rearm_timer(chr, 10);
> }
>
> static void pty_chr_state(CharDriverState *chr, int connected)
> @@ -1080,7 +1118,7 @@ static void pty_chr_state(CharDriverState *chr, int connected)
> /* (re-)connect poll interval for idle guests: once per second.
> * We check more frequently in case the guests sends data to
> * the virtual device linked to our pty. */
> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 1000);
> + pty_chr_rearm_timer(chr, 1000);
> } else {
> if (!s->connected)
> qemu_chr_generic_open(chr);
> @@ -1088,23 +1126,6 @@ static void pty_chr_state(CharDriverState *chr, int connected)
> }
> }
>
> -static void pty_chr_timer(void *opaque)
> -{
> - struct CharDriverState *chr = opaque;
> - PtyCharDriver *s = chr->opaque;
> -
> - if (s->connected)
> - return;
> - if (s->polling) {
> - /* If we arrive here without polling being cleared due
> - * read returning -EIO, then we are (re-)connected */
> - pty_chr_state(chr, 1);
> - return;
> - }
> -
> - /* Next poll ... */
> - pty_chr_update_read_handler(chr);
> -}
>
> static void pty_chr_close(struct CharDriverState *chr)
> {
> @@ -1117,8 +1138,9 @@ static void pty_chr_close(struct CharDriverState *chr)
> fd = g_io_channel_unix_get_fd(s->fd);
> g_io_channel_unref(s->fd);
> close(fd);
> - qemu_del_timer(s->timer);
> - qemu_free_timer(s->timer);
> + if (s->timer_tag) {
> + g_source_remove(s->timer_tag);
> + }
> g_free(s);
> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
> }
> @@ -1170,7 +1192,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
> chr->chr_add_watch = pty_chr_add_watch;
>
> s->fd = io_channel_from_fd(master_fd);
> - s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr);
> + s->timer_tag = 0;
>
> return chr;
> }
> --
> 1.8.1.2
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer
2013-03-15 15:06 ` Laurent Desnogues
@ 2013-03-15 15:44 ` Anthony Liguori
2013-03-15 16:19 ` Laurent Desnogues
2013-03-25 9:38 ` Stefan Hajnoczi
0 siblings, 2 replies; 32+ messages in thread
From: Anthony Liguori @ 2013-03-15 15:44 UTC (permalink / raw)
To: Laurent Desnogues, Amit Shah; +Cc: qemu list
Laurent Desnogues <laurent.desnogues@gmail.com> writes:
> Hello,
>
> On Tue, Mar 5, 2013 at 6:51 PM, Amit Shah <amit.shah@redhat.com> wrote:
>> From: Anthony Liguori <aliguori@us.ibm.com>
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>> ---
>> qemu-char.c | 68 ++++++++++++++++++++++++++++++++++++++++---------------------
>> 1 file changed, 45 insertions(+), 23 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index eb0ac81..6dba943 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -990,12 +990,50 @@ typedef struct {
>> int connected;
>> int polling;
>> int read_bytes;
>> - QEMUTimer *timer;
>> + guint timer_tag;
>> } PtyCharDriver;
>>
>> static void pty_chr_update_read_handler(CharDriverState *chr);
>> static void pty_chr_state(CharDriverState *chr, int connected);
>>
>> +static gboolean pty_chr_timer(gpointer opaque)
>> +{
>> + struct CharDriverState *chr = opaque;
>> + PtyCharDriver *s = chr->opaque;
>> +
>> + if (s->connected) {
>> + goto out;
>> + }
>> + if (s->polling) {
>> + /* If we arrive here without polling being cleared due
>> + * read returning -EIO, then we are (re-)connected */
>> + pty_chr_state(chr, 1);
>> + goto out;
>> + }
>> +
>> + /* Next poll ... */
>> + pty_chr_update_read_handler(chr);
>> +
>> +out:
>> + return FALSE;
>> +}
>> +
>> +static void pty_chr_rearm_timer(CharDriverState *chr, int ms)
>> +{
>> + PtyCharDriver *s = chr->opaque;
>> +
>> + if (s->timer_tag) {
>> + g_source_remove(s->timer_tag);
>> + s->timer_tag = 0;
>> + }
>> +
>> + if (ms == 1000) {
>> + s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
>
> It looks like g_timeout_add_seconds isn't available for
> poor people using some old distros (glib 2.12.3 here).
Can you test adding:
#if !GLIB_CHECK_VERSION(2, 14, 0)
static guint g_timeout_add_seconds(guint interval, GSourceFunc function,
gpointer data)
{
return g_timeout_add(interval * 1000, function, data);
}
#endif
We probably should introduce a glib-compat to centralize work arounds
for older versions of glib...
Regards,
Anthony Liguori
>
> Thanks,
>
> Laurent
>
>> + } else {
>> + s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
>> + }
>> +}
>> +
>> static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>> {
>> PtyCharDriver *s = chr->opaque;
>> @@ -1065,7 +1103,7 @@ static void pty_chr_update_read_handler(CharDriverState *chr)
>> * timeout to the normal (much longer) poll interval before the
>> * timer triggers.
>> */
>> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 10);
>> + pty_chr_rearm_timer(chr, 10);
>> }
>>
>> static void pty_chr_state(CharDriverState *chr, int connected)
>> @@ -1080,7 +1118,7 @@ static void pty_chr_state(CharDriverState *chr, int connected)
>> /* (re-)connect poll interval for idle guests: once per second.
>> * We check more frequently in case the guests sends data to
>> * the virtual device linked to our pty. */
>> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 1000);
>> + pty_chr_rearm_timer(chr, 1000);
>> } else {
>> if (!s->connected)
>> qemu_chr_generic_open(chr);
>> @@ -1088,23 +1126,6 @@ static void pty_chr_state(CharDriverState *chr, int connected)
>> }
>> }
>>
>> -static void pty_chr_timer(void *opaque)
>> -{
>> - struct CharDriverState *chr = opaque;
>> - PtyCharDriver *s = chr->opaque;
>> -
>> - if (s->connected)
>> - return;
>> - if (s->polling) {
>> - /* If we arrive here without polling being cleared due
>> - * read returning -EIO, then we are (re-)connected */
>> - pty_chr_state(chr, 1);
>> - return;
>> - }
>> -
>> - /* Next poll ... */
>> - pty_chr_update_read_handler(chr);
>> -}
>>
>> static void pty_chr_close(struct CharDriverState *chr)
>> {
>> @@ -1117,8 +1138,9 @@ static void pty_chr_close(struct CharDriverState *chr)
>> fd = g_io_channel_unix_get_fd(s->fd);
>> g_io_channel_unref(s->fd);
>> close(fd);
>> - qemu_del_timer(s->timer);
>> - qemu_free_timer(s->timer);
>> + if (s->timer_tag) {
>> + g_source_remove(s->timer_tag);
>> + }
>> g_free(s);
>> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
>> }
>> @@ -1170,7 +1192,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
>> chr->chr_add_watch = pty_chr_add_watch;
>>
>> s->fd = io_channel_from_fd(master_fd);
>> - s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr);
>> + s->timer_tag = 0;
>>
>> return chr;
>> }
>> --
>> 1.8.1.2
>>
>>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer
2013-03-15 15:44 ` Anthony Liguori
@ 2013-03-15 16:19 ` Laurent Desnogues
2013-03-25 9:38 ` Stefan Hajnoczi
1 sibling, 0 replies; 32+ messages in thread
From: Laurent Desnogues @ 2013-03-15 16:19 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Amit Shah, qemu list
On Fri, Mar 15, 2013 at 4:44 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Laurent Desnogues <laurent.desnogues@gmail.com> writes:
>
>> Hello,
>>
>> On Tue, Mar 5, 2013 at 6:51 PM, Amit Shah <amit.shah@redhat.com> wrote:
>>> From: Anthony Liguori <aliguori@us.ibm.com>
>>>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>>> ---
>>> qemu-char.c | 68 ++++++++++++++++++++++++++++++++++++++++---------------------
>>> 1 file changed, 45 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/qemu-char.c b/qemu-char.c
>>> index eb0ac81..6dba943 100644
>>> --- a/qemu-char.c
>>> +++ b/qemu-char.c
>>> @@ -990,12 +990,50 @@ typedef struct {
>>> int connected;
>>> int polling;
>>> int read_bytes;
>>> - QEMUTimer *timer;
>>> + guint timer_tag;
>>> } PtyCharDriver;
>>>
>>> static void pty_chr_update_read_handler(CharDriverState *chr);
>>> static void pty_chr_state(CharDriverState *chr, int connected);
>>>
>>> +static gboolean pty_chr_timer(gpointer opaque)
>>> +{
>>> + struct CharDriverState *chr = opaque;
>>> + PtyCharDriver *s = chr->opaque;
>>> +
>>> + if (s->connected) {
>>> + goto out;
>>> + }
>>> + if (s->polling) {
>>> + /* If we arrive here without polling being cleared due
>>> + * read returning -EIO, then we are (re-)connected */
>>> + pty_chr_state(chr, 1);
>>> + goto out;
>>> + }
>>> +
>>> + /* Next poll ... */
>>> + pty_chr_update_read_handler(chr);
>>> +
>>> +out:
>>> + return FALSE;
>>> +}
>>> +
>>> +static void pty_chr_rearm_timer(CharDriverState *chr, int ms)
>>> +{
>>> + PtyCharDriver *s = chr->opaque;
>>> +
>>> + if (s->timer_tag) {
>>> + g_source_remove(s->timer_tag);
>>> + s->timer_tag = 0;
>>> + }
>>> +
>>> + if (ms == 1000) {
>>> + s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
>>
>> It looks like g_timeout_add_seconds isn't available for
>> poor people using some old distros (glib 2.12.3 here).
>
> Can you test adding:
>
> #if !GLIB_CHECK_VERSION(2, 14, 0)
> static guint g_timeout_add_seconds(guint interval, GSourceFunc function,
> gpointer data)
> {
> return g_timeout_add(interval * 1000, function, data);
> }
> #endif
That works fine, thanks for looking!
> We probably should introduce a glib-compat to centralize work arounds
> for older versions of glib...
Agreed.
Thanks again,
Laurent
> Regards,
>
> Anthony Liguori
>
>>
>> Thanks,
>>
>> Laurent
>>
>>> + } else {
>>> + s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
>>> + }
>>> +}
>>> +
>>> static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>>> {
>>> PtyCharDriver *s = chr->opaque;
>>> @@ -1065,7 +1103,7 @@ static void pty_chr_update_read_handler(CharDriverState *chr)
>>> * timeout to the normal (much longer) poll interval before the
>>> * timer triggers.
>>> */
>>> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 10);
>>> + pty_chr_rearm_timer(chr, 10);
>>> }
>>>
>>> static void pty_chr_state(CharDriverState *chr, int connected)
>>> @@ -1080,7 +1118,7 @@ static void pty_chr_state(CharDriverState *chr, int connected)
>>> /* (re-)connect poll interval for idle guests: once per second.
>>> * We check more frequently in case the guests sends data to
>>> * the virtual device linked to our pty. */
>>> - qemu_mod_timer(s->timer, qemu_get_clock_ms(rt_clock) + 1000);
>>> + pty_chr_rearm_timer(chr, 1000);
>>> } else {
>>> if (!s->connected)
>>> qemu_chr_generic_open(chr);
>>> @@ -1088,23 +1126,6 @@ static void pty_chr_state(CharDriverState *chr, int connected)
>>> }
>>> }
>>>
>>> -static void pty_chr_timer(void *opaque)
>>> -{
>>> - struct CharDriverState *chr = opaque;
>>> - PtyCharDriver *s = chr->opaque;
>>> -
>>> - if (s->connected)
>>> - return;
>>> - if (s->polling) {
>>> - /* If we arrive here without polling being cleared due
>>> - * read returning -EIO, then we are (re-)connected */
>>> - pty_chr_state(chr, 1);
>>> - return;
>>> - }
>>> -
>>> - /* Next poll ... */
>>> - pty_chr_update_read_handler(chr);
>>> -}
>>>
>>> static void pty_chr_close(struct CharDriverState *chr)
>>> {
>>> @@ -1117,8 +1138,9 @@ static void pty_chr_close(struct CharDriverState *chr)
>>> fd = g_io_channel_unix_get_fd(s->fd);
>>> g_io_channel_unref(s->fd);
>>> close(fd);
>>> - qemu_del_timer(s->timer);
>>> - qemu_free_timer(s->timer);
>>> + if (s->timer_tag) {
>>> + g_source_remove(s->timer_tag);
>>> + }
>>> g_free(s);
>>> qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
>>> }
>>> @@ -1170,7 +1192,7 @@ static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
>>> chr->chr_add_watch = pty_chr_add_watch;
>>>
>>> s->fd = io_channel_from_fd(master_fd);
>>> - s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr);
>>> + s->timer_tag = 0;
>>>
>>> return chr;
>>> }
>>> --
>>> 1.8.1.2
>>>
>>>
>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer
2013-03-15 15:44 ` Anthony Liguori
2013-03-15 16:19 ` Laurent Desnogues
@ 2013-03-25 9:38 ` Stefan Hajnoczi
1 sibling, 0 replies; 32+ messages in thread
From: Stefan Hajnoczi @ 2013-03-25 9:38 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Laurent Desnogues, Amit Shah, qemu list
On Fri, Mar 15, 2013 at 4:44 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Laurent Desnogues <laurent.desnogues@gmail.com> writes:
>
>> Hello,
>>
>> On Tue, Mar 5, 2013 at 6:51 PM, Amit Shah <amit.shah@redhat.com> wrote:
>>> From: Anthony Liguori <aliguori@us.ibm.com>
>>>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>> Signed-off-by: Amit Shah <amit.shah@redhat.com>
>>> ---
>>> qemu-char.c | 68 ++++++++++++++++++++++++++++++++++++++++---------------------
>>> 1 file changed, 45 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/qemu-char.c b/qemu-char.c
>>> index eb0ac81..6dba943 100644
>>> --- a/qemu-char.c
>>> +++ b/qemu-char.c
>>> @@ -990,12 +990,50 @@ typedef struct {
>>> int connected;
>>> int polling;
>>> int read_bytes;
>>> - QEMUTimer *timer;
>>> + guint timer_tag;
>>> } PtyCharDriver;
>>>
>>> static void pty_chr_update_read_handler(CharDriverState *chr);
>>> static void pty_chr_state(CharDriverState *chr, int connected);
>>>
>>> +static gboolean pty_chr_timer(gpointer opaque)
>>> +{
>>> + struct CharDriverState *chr = opaque;
>>> + PtyCharDriver *s = chr->opaque;
>>> +
>>> + if (s->connected) {
>>> + goto out;
>>> + }
>>> + if (s->polling) {
>>> + /* If we arrive here without polling being cleared due
>>> + * read returning -EIO, then we are (re-)connected */
>>> + pty_chr_state(chr, 1);
>>> + goto out;
>>> + }
>>> +
>>> + /* Next poll ... */
>>> + pty_chr_update_read_handler(chr);
>>> +
>>> +out:
>>> + return FALSE;
>>> +}
>>> +
>>> +static void pty_chr_rearm_timer(CharDriverState *chr, int ms)
>>> +{
>>> + PtyCharDriver *s = chr->opaque;
>>> +
>>> + if (s->timer_tag) {
>>> + g_source_remove(s->timer_tag);
>>> + s->timer_tag = 0;
>>> + }
>>> +
>>> + if (ms == 1000) {
>>> + s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
>>
>> It looks like g_timeout_add_seconds isn't available for
>> poor people using some old distros (glib 2.12.3 here).
>
> Can you test adding:
>
> #if !GLIB_CHECK_VERSION(2, 14, 0)
> static guint g_timeout_add_seconds(guint interval, GSourceFunc function,
> gpointer data)
> {
> return g_timeout_add(interval * 1000, function, data);
> }
> #endif
>
> We probably should introduce a glib-compat to centralize work arounds
> for older versions of glib...
Hi Anthony,
Are you sending a patch for g_timeout_add_seconds() compatibility?
The RHEL5 builds are failing because their glib is old:
http://buildbot.b1-systems.de/qemu/builders/default_x86_64_rhel5/builds/551/steps/compile/logs/stdio
Stefan
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 12/20] qemu-char: remove use of QEMUTimer in favor of glib idle function
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (10 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 11/20] qemu-char: use a glib timeout instead of qemu-timer Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 13/20] qemu-char: make char drivers dynamically registerable Amit Shah
` (8 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
qemu-char is now independent of the QEMU main loop.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
include/char/char.h | 2 +-
qemu-char.c | 12 +++++-------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/char/char.h b/include/char/char.h
index 09ac401..e8b781c 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -71,7 +71,7 @@ struct CharDriverState {
void (*chr_guest_open)(struct CharDriverState *chr);
void (*chr_guest_close)(struct CharDriverState *chr);
void *opaque;
- QEMUTimer *open_timer;
+ int idle_tag;
char *label;
char *filename;
int opened;
diff --git a/qemu-char.c b/qemu-char.c
index 6dba943..2c7c929 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -122,20 +122,18 @@ void qemu_chr_be_event(CharDriverState *s, int event)
s->chr_event(s->handler_opaque, event);
}
-static void qemu_chr_fire_open_event(void *opaque)
+static gboolean qemu_chr_generic_open_bh(gpointer opaque)
{
CharDriverState *s = opaque;
qemu_chr_be_event(s, CHR_EVENT_OPENED);
- qemu_free_timer(s->open_timer);
- s->open_timer = NULL;
+ s->idle_tag = 0;
+ return FALSE;
}
void qemu_chr_generic_open(CharDriverState *s)
{
- if (s->open_timer == NULL) {
- s->open_timer = qemu_new_timer_ms(rt_clock,
- qemu_chr_fire_open_event, s);
- qemu_mod_timer(s->open_timer, qemu_get_clock_ms(rt_clock) - 1);
+ if (s->idle_tag == 0) {
+ s->idle_tag = g_idle_add(qemu_chr_generic_open_bh, s);
}
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 13/20] qemu-char: make char drivers dynamically registerable
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (11 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 12/20] qemu-char: remove use of QEMUTimer in favor of glib idle function Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 14/20] qemu-char: move spice registration to spice-qemu-char.c Amit Shah
` (7 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
include/char/char.h | 2 +
qemu-char.c | 110 +++++++++++++++++++++++++++++++---------------------
2 files changed, 68 insertions(+), 44 deletions(-)
diff --git a/include/char/char.h b/include/char/char.h
index e8b781c..2e24270 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -244,6 +244,8 @@ CharDriverState *qemu_chr_find(const char *name);
QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
+void register_char_driver(const char *name, CharDriverState *(*open)(QemuOpts *));
+
/* add an eventfd to the qemu devices that are polled */
CharDriverState *qemu_chr_open_eventfd(int eventfd);
diff --git a/qemu-char.c b/qemu-char.c
index 2c7c929..9bcdf73 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3177,53 +3177,31 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
#endif
-static const struct {
+typedef struct CharDriver {
const char *name;
CharDriverState *(*open)(QemuOpts *opts);
-} backend_table[] = {
- { .name = "null", .open = qemu_chr_open_null },
- { .name = "socket", .open = qemu_chr_open_socket },
- { .name = "udp", .open = qemu_chr_open_udp },
- { .name = "msmouse", .open = qemu_chr_open_msmouse },
- { .name = "vc", .open = vc_init },
- { .name = "memory", .open = qemu_chr_open_ringbuf },
-#ifdef _WIN32
- { .name = "file", .open = qemu_chr_open_win_file_out },
- { .name = "pipe", .open = qemu_chr_open_win_pipe },
- { .name = "console", .open = qemu_chr_open_win_con },
- { .name = "serial", .open = qemu_chr_open_win },
- { .name = "stdio", .open = qemu_chr_open_win_stdio },
-#else
- { .name = "file", .open = qemu_chr_open_file_out },
- { .name = "pipe", .open = qemu_chr_open_pipe },
- { .name = "stdio", .open = qemu_chr_open_stdio },
-#endif
-#ifdef CONFIG_BRLAPI
- { .name = "braille", .open = chr_baum_init },
-#endif
-#ifdef HAVE_CHARDEV_TTY
- { .name = "tty", .open = qemu_chr_open_tty },
- { .name = "serial", .open = qemu_chr_open_tty },
- { .name = "pty", .open = qemu_chr_open_pty },
-#endif
-#ifdef HAVE_CHARDEV_PARPORT
- { .name = "parallel", .open = qemu_chr_open_pp },
- { .name = "parport", .open = qemu_chr_open_pp },
-#endif
-#ifdef CONFIG_SPICE
- { .name = "spicevmc", .open = qemu_chr_open_spice },
-#if SPICE_SERVER_VERSION >= 0x000c02
- { .name = "spiceport", .open = qemu_chr_open_spice_port },
-#endif
-#endif
-};
+} CharDriver;
+
+static GSList *backends;
+
+void register_char_driver(const char *name, CharDriverState *(*open)(QemuOpts *))
+{
+ CharDriver *s;
+
+ s = g_malloc0(sizeof(*s));
+ s->name = g_strdup(name);
+ s->open = open;
+
+ backends = g_slist_append(backends, s);
+}
CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
void (*init)(struct CharDriverState *s),
Error **errp)
{
+ CharDriver *cd;
CharDriverState *chr;
- int i;
+ GSList *i;
if (qemu_opts_id(opts) == NULL) {
error_setg(errp, "chardev: no id specified");
@@ -3235,17 +3213,20 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
qemu_opts_id(opts));
goto err;
}
- for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
- if (strcmp(backend_table[i].name, qemu_opt_get(opts, "backend")) == 0)
+ for (i = backends; i; i = i->next) {
+ cd = i->data;
+
+ if (strcmp(cd->name, qemu_opt_get(opts, "backend")) == 0) {
break;
+ }
}
- if (i == ARRAY_SIZE(backend_table)) {
+ if (i == NULL) {
error_setg(errp, "chardev: backend \"%s\" not found",
qemu_opt_get(opts, "backend"));
- goto err;
+ return NULL;
}
- chr = backend_table[i].open(opts);
+ chr = cd->open(opts);
if (!chr) {
error_setg(errp, "chardev: opening backend \"%s\" failed",
qemu_opt_get(opts, "backend"));
@@ -3677,3 +3658,44 @@ void qmp_chardev_remove(const char *id, Error **errp)
}
qemu_chr_delete(chr);
}
+
+static void register_types(void)
+{
+ register_char_driver("null", qemu_chr_open_null);
+ register_char_driver("socket", qemu_chr_open_socket);
+ register_char_driver("udp", qemu_chr_open_udp);
+ register_char_driver("msmouse", qemu_chr_open_msmouse);
+ register_char_driver("vc", vc_init);
+ register_char_driver("memory", qemu_chr_open_ringbuf);
+#ifdef _WIN32
+ register_char_driver("file", qemu_chr_open_win_file_out);
+ register_char_driver("pipe", qemu_chr_open_win_pipe);
+ register_char_driver("console", qemu_chr_open_win_con);
+ register_char_driver("serial", qemu_chr_open_win);
+ register_char_driver("stdio", qemu_chr_open_win_stdio);
+#else
+ register_char_driver("file", qemu_chr_open_file_out);
+ register_char_driver("pipe", qemu_chr_open_pipe);
+ register_char_driver("stdio", qemu_chr_open_stdio);
+#endif
+#ifdef CONFIG_BRLAPI
+ register_char_driver("braille", chr_baum_init);
+#endif
+#ifdef HAVE_CHARDEV_TTY
+ register_char_driver("tty", qemu_chr_open_tty);
+ register_char_driver("serial", qemu_chr_open_tty);
+ register_char_driver("pty", qemu_chr_open_pty);
+#endif
+#ifdef HAVE_CHARDEV_PARPORT
+ register_char_driver("parallel", qemu_chr_open_pp);
+ register_char_driver("parport", qemu_chr_open_pp);
+#endif
+#ifdef CONFIG_SPICE
+ register_char_driver("spicevmc", qemu_chr_open_spice);
+#if SPICE_SERVER_VERSION >= 0x000c02
+ register_char_driver("spiceport", qemu_chr_open_spice_port);
+#endif
+#endif
+}
+
+type_init(register_types);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 14/20] qemu-char: move spice registration to spice-qemu-char.c
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (12 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 13/20] qemu-char: make char drivers dynamically registerable Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 15/20] qemu-char: move baum registration to baum.c Amit Shah
` (6 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 6 ------
spice-qemu-char.c | 10 ++++++++++
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 9bcdf73..2e9f92e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3690,12 +3690,6 @@ static void register_types(void)
register_char_driver("parallel", qemu_chr_open_pp);
register_char_driver("parport", qemu_chr_open_pp);
#endif
-#ifdef CONFIG_SPICE
- register_char_driver("spicevmc", qemu_chr_open_spice);
-#if SPICE_SERVER_VERSION >= 0x000c02
- register_char_driver("spiceport", qemu_chr_open_spice_port);
-#endif
-#endif
}
type_init(register_types);
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index a4d7de8..aea3d24 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -307,3 +307,13 @@ void qemu_spice_register_ports(void)
}
}
#endif
+
+static void register_types(void)
+{
+ register_char_driver("spicevmc", qemu_chr_open_spice);
+#if SPICE_SERVER_VERSION >= 0x000c02
+ register_char_driver("spiceport", qemu_chr_open_spice_port);
+#endif
+}
+
+type_init(register_types);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 15/20] qemu-char: move baum registration to baum.c
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (13 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 14/20] qemu-char: move spice registration to spice-qemu-char.c Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 16/20] qemu-char: move msmouse registeration to msmouse.c Amit Shah
` (5 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/baum.c | 9 ++++++++-
hw/baum.h | 30 ------------------------------
qemu-char.c | 4 ----
vl.c | 1 -
4 files changed, 8 insertions(+), 36 deletions(-)
delete mode 100644 hw/baum.h
diff --git a/hw/baum.c b/hw/baum.c
index 09dcb9c..d75b150 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -562,7 +562,7 @@ static void baum_close(struct CharDriverState *chr)
g_free(baum);
}
-CharDriverState *chr_baum_init(QemuOpts *opts)
+static CharDriverState *chr_baum_init(QemuOpts *opts)
{
BaumDriverState *baum;
CharDriverState *chr;
@@ -625,3 +625,10 @@ fail_handle:
g_free(baum);
return NULL;
}
+
+static void register_types(void)
+{
+ register_char_driver("braille", chr_baum_init);
+}
+
+type_init(register_types);
diff --git a/hw/baum.h b/hw/baum.h
deleted file mode 100644
index 7635884..0000000
--- a/hw/baum.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * QEMU Baum
- *
- * Copyright (c) 2008 Samuel Thibault
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-#ifndef HW_BAUM_H
-#define HW_BAUM_H 1
-
-/* char device */
-CharDriverState *chr_baum_init(QemuOpts *opts);
-
-#endif
diff --git a/qemu-char.c b/qemu-char.c
index 2e9f92e..6fcd2da 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -28,7 +28,6 @@
#include "qemu/timer.h"
#include "char/char.h"
#include "hw/usb.h"
-#include "hw/baum.h"
#include "hw/msmouse.h"
#include "qmp-commands.h"
@@ -3678,9 +3677,6 @@ static void register_types(void)
register_char_driver("pipe", qemu_chr_open_pipe);
register_char_driver("stdio", qemu_chr_open_stdio);
#endif
-#ifdef CONFIG_BRLAPI
- register_char_driver("braille", chr_baum_init);
-#endif
#ifdef HAVE_CHARDEV_TTY
register_char_driver("tty", qemu_chr_open_tty);
register_char_driver("serial", qemu_chr_open_tty);
diff --git a/vl.c b/vl.c
index c03edf1..5dc0558 100644
--- a/vl.c
+++ b/vl.c
@@ -119,7 +119,6 @@ int main(int argc, char **argv)
#include "hw/pcmcia.h"
#include "hw/pc.h"
#include "hw/isa.h"
-#include "hw/baum.h"
#include "hw/bt.h"
#include "hw/watchdog.h"
#include "hw/smbios.h"
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 16/20] qemu-char: move msmouse registeration to msmouse.c
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (14 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 15/20] qemu-char: move baum registration to baum.c Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 17/20] qemu-char: move text console init to console.c Amit Shah
` (4 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/msmouse.c | 10 ++++++++--
hw/msmouse.h | 7 -------
qemu-char.c | 3 +--
3 files changed, 9 insertions(+), 11 deletions(-)
delete mode 100644 hw/msmouse.h
diff --git a/hw/msmouse.c b/hw/msmouse.c
index ef47aed..407ec87 100644
--- a/hw/msmouse.c
+++ b/hw/msmouse.c
@@ -25,7 +25,6 @@
#include "qemu-common.h"
#include "char/char.h"
#include "ui/console.h"
-#include "msmouse.h"
#define MSMOUSE_LO6(n) ((n) & 0x3f)
#define MSMOUSE_HI2(n) (((n) & 0xc0) >> 6)
@@ -64,7 +63,7 @@ static void msmouse_chr_close (struct CharDriverState *chr)
g_free (chr);
}
-CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts)
{
CharDriverState *chr;
@@ -76,3 +75,10 @@ CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts)
return chr;
}
+
+static void register_types(void)
+{
+ register_char_driver("msmouse", qemu_chr_open_msmouse);
+}
+
+type_init(register_types);
diff --git a/hw/msmouse.h b/hw/msmouse.h
deleted file mode 100644
index 8cff3a7..0000000
--- a/hw/msmouse.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef HW_MSMOUSE_H
-#define HW_MSMOUSE_H 1
-
-/* msmouse.c */
-CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts);
-
-#endif
diff --git a/qemu-char.c b/qemu-char.c
index 6fcd2da..cf02cab 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -28,7 +28,6 @@
#include "qemu/timer.h"
#include "char/char.h"
#include "hw/usb.h"
-#include "hw/msmouse.h"
#include "qmp-commands.h"
#include <unistd.h>
@@ -2179,6 +2178,7 @@ static CharDriverState *qemu_chr_open_win_stdio(QemuOpts *opts)
}
#endif /* !_WIN32 */
+
/***********************************************************/
/* UDP Net console */
@@ -3663,7 +3663,6 @@ static void register_types(void)
register_char_driver("null", qemu_chr_open_null);
register_char_driver("socket", qemu_chr_open_socket);
register_char_driver("udp", qemu_chr_open_udp);
- register_char_driver("msmouse", qemu_chr_open_msmouse);
register_char_driver("vc", vc_init);
register_char_driver("memory", qemu_chr_open_ringbuf);
#ifdef _WIN32
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 17/20] qemu-char: move text console init to console.c
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (15 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 16/20] qemu-char: move msmouse registeration to msmouse.c Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-13 17:19 ` Anthony Liguori
2013-03-05 17:51 ` [Qemu-devel] [PATCH 18/20] serial: add flow control to transmit Amit Shah
` (3 subsequent siblings)
20 siblings, 1 reply; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
qemu-char.c | 1 -
ui/console.c | 7 +++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/qemu-char.c b/qemu-char.c
index cf02cab..b82d643 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3663,7 +3663,6 @@ static void register_types(void)
register_char_driver("null", qemu_chr_open_null);
register_char_driver("socket", qemu_chr_open_socket);
register_char_driver("udp", qemu_chr_open_udp);
- register_char_driver("vc", vc_init);
register_char_driver("memory", qemu_chr_open_ringbuf);
#ifdef _WIN32
register_char_driver("file", qemu_chr_open_win_file_out);
diff --git a/ui/console.c b/ui/console.c
index 0d95f32..83a6fa3 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1739,3 +1739,10 @@ PixelFormat qemu_default_pixelformat(int bpp)
}
return pf;
}
+
+static void register_types(void)
+{
+ register_char_driver("vc", text_console_init);
+}
+
+type_init(register_types);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 17/20] qemu-char: move text console init to console.c
2013-03-05 17:51 ` [Qemu-devel] [PATCH 17/20] qemu-char: move text console init to console.c Amit Shah
@ 2013-03-13 17:19 ` Anthony Liguori
0 siblings, 0 replies; 32+ messages in thread
From: Anthony Liguori @ 2013-03-13 17:19 UTC (permalink / raw)
To: Amit Shah, qemu list
Amit Shah <amit.shah@redhat.com> writes:
> From: Anthony Liguori <aliguori@us.ibm.com>
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
This patch broke vc switching in GTK.
> ---
> qemu-char.c | 1 -
> ui/console.c | 7 +++++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index cf02cab..b82d643 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3663,7 +3663,6 @@ static void register_types(void)
> register_char_driver("null", qemu_chr_open_null);
> register_char_driver("socket", qemu_chr_open_socket);
> register_char_driver("udp", qemu_chr_open_udp);
> - register_char_driver("vc", vc_init);
> register_char_driver("memory", qemu_chr_open_ringbuf);
> #ifdef _WIN32
> register_char_driver("file", qemu_chr_open_win_file_out);
> diff --git a/ui/console.c b/ui/console.c
> index 0d95f32..83a6fa3 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1739,3 +1739,10 @@ PixelFormat qemu_default_pixelformat(int bpp)
> }
> return pf;
> }
> +
> +static void register_types(void)
> +{
> + register_char_driver("vc", text_console_init);
This should be:
register_char_driver("vc", vc_init);
Patch on the way...
Regards,
Anthony Liguori
> +}
> +
> +type_init(register_types);
> --
> 1.8.1.2
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 18/20] serial: add flow control to transmit
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (16 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 17/20] qemu-char: move text console init to console.c Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 19/20] virtio: console: add flow control Amit Shah
` (2 subsequent siblings)
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori, Anthony Liguori
From: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/serial.c | 28 +++++++++++-----------------
hw/serial.h | 2 --
2 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/hw/serial.c b/hw/serial.c
index f0ce9b0..eb38f22 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -256,16 +256,17 @@ static void serial_update_msl(SerialState *s)
qemu_mod_timer(s->modem_status_poll, qemu_get_clock_ns(vm_clock) + get_ticks_per_sec() / 100);
}
-static void serial_xmit(void *opaque)
+static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
{
SerialState *s = opaque;
- uint64_t new_xmit_ts = qemu_get_clock_ns(vm_clock);
if (s->tsr_retry <= 0) {
if (s->fcr & UART_FCR_FE) {
s->tsr = fifo_get(s,XMIT_FIFO);
if (!s->xmit_fifo.count)
s->lsr |= UART_LSR_THRE;
+ } else if ((s->lsr & UART_LSR_THRE)) {
+ return FALSE;
} else {
s->tsr = s->thr;
s->lsr |= UART_LSR_THRE;
@@ -277,30 +278,25 @@ static void serial_xmit(void *opaque)
/* in loopback mode, say that we just received a char */
serial_receive1(s, &s->tsr, 1);
} else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
- if ((s->tsr_retry > 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
+ if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
+ qemu_chr_fe_add_watch(s->chr, G_IO_OUT, serial_xmit, s) > 0) {
s->tsr_retry++;
- qemu_mod_timer(s->transmit_timer, new_xmit_ts + s->char_transmit_time);
- return;
- } else if (s->poll_msl < 0) {
- /* If we exceed MAX_XMIT_RETRY and the backend is not a real serial port, then
- drop any further failed writes instantly, until we get one that goes through.
- This is to prevent guests that log to unconnected pipes or pty's from stalling. */
- s->tsr_retry = -1;
+ return FALSE;
}
- }
- else {
+ s->tsr_retry = 0;
+ } else {
s->tsr_retry = 0;
}
s->last_xmit_ts = qemu_get_clock_ns(vm_clock);
- if (!(s->lsr & UART_LSR_THRE))
- qemu_mod_timer(s->transmit_timer, s->last_xmit_ts + s->char_transmit_time);
if (s->lsr & UART_LSR_THRE) {
s->lsr |= UART_LSR_TEMT;
s->thr_ipending = 1;
serial_update_irq(s);
}
+
+ return FALSE;
}
@@ -330,7 +326,7 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
s->lsr &= ~UART_LSR_THRE;
serial_update_irq(s);
}
- serial_xmit(s);
+ serial_xmit(NULL, G_IO_OUT, s);
}
break;
case 1:
@@ -684,8 +680,6 @@ void serial_init_core(SerialState *s)
s->modem_status_poll = qemu_new_timer_ns(vm_clock, (QEMUTimerCB *) serial_update_msl, s);
s->fifo_timeout_timer = qemu_new_timer_ns(vm_clock, (QEMUTimerCB *) fifo_timeout_int, s);
- s->transmit_timer = qemu_new_timer_ns(vm_clock, (QEMUTimerCB *) serial_xmit, s);
-
qemu_register_reset(serial_reset, s);
qemu_chr_add_handlers(s->chr, serial_can_receive1, serial_receive1,
diff --git a/hw/serial.h b/hw/serial.h
index 98ee424..e57375d 100644
--- a/hw/serial.h
+++ b/hw/serial.h
@@ -72,8 +72,6 @@ struct SerialState {
struct QEMUTimer *fifo_timeout_timer;
int timeout_ipending; /* timeout interrupt pending state */
- struct QEMUTimer *transmit_timer;
-
uint64_t char_transmit_time; /* time to transmit a char in ticks */
int poll_msl;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 19/20] virtio: console: add flow control
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (17 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 18/20] serial: add flow control to transmit Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-05 17:51 ` [Qemu-devel] [PATCH 20/20] virtio-serial: make flow control explicit in virtio-console Amit Shah
2013-03-12 2:02 ` [Qemu-devel] [PATCH 00/20] chardev flow control Anthony Liguori
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori
The virtio-serial-bus already has the logic to make flow control work
properly. Hook into the char layer's new ability to signal a backend is
writable again.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/virtio-console.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 46072a0..b3ee074 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -20,6 +20,18 @@ typedef struct VirtConsole {
CharDriverState *chr;
} VirtConsole;
+/*
+ * Callback function that's called from chardevs when backend becomes
+ * writable.
+ */
+static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
+ void *opaque)
+{
+ VirtConsole *vcon = opaque;
+
+ virtio_serial_throttle_port(&vcon->port, false);
+ return FALSE;
+}
/* Callback function that's called when the guest sends us data */
static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
@@ -48,6 +60,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
* do_flush_queued_data().
*/
ret = 0;
+ qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT, chr_write_unblocked, vcon);
}
return ret;
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 20/20] virtio-serial: make flow control explicit in virtio-console
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (18 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 19/20] virtio: console: add flow control Amit Shah
@ 2013-03-05 17:51 ` Amit Shah
2013-03-12 2:02 ` [Qemu-devel] [PATCH 00/20] chardev flow control Anthony Liguori
20 siblings, 0 replies; 32+ messages in thread
From: Amit Shah @ 2013-03-05 17:51 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Anthony Liguori
virtio-console.c used to return a value less than the number of bytes
asked to be written out to a chardev backend in case the backend is not
writable. virtio-serial-bus.c then implicitly enabled flow control for
that port.
Make this explicit instead.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
hw/virtio-console.c | 17 +++++++++--------
hw/virtio-serial-bus.c | 19 +------------------
2 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index b3ee074..194de64 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -47,20 +47,21 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
ret = qemu_chr_fe_write(vcon->chr, buf, len);
trace_virtio_console_flush_buf(port->id, len, ret);
- if (ret < 0) {
+ if (ret <= 0) {
+ VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+
/*
* Ideally we'd get a better error code than just -1, but
* that's what the chardev interface gives us right now. If
* we had a finer-grained message, like -EPIPE, we could close
- * this connection. Absent such error messages, the most we
- * can do is to return 0 here.
- *
- * This will prevent stray -1 values to go to
- * virtio-serial-bus.c and cause abort()s in
- * do_flush_queued_data().
+ * this connection.
*/
ret = 0;
- qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT, chr_write_unblocked, vcon);
+ if (!k->is_console) {
+ virtio_serial_throttle_port(port, true);
+ qemu_chr_fe_add_watch(vcon->chr, G_IO_OUT, chr_write_unblocked,
+ vcon);
+ }
}
return ret;
}
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index aa7d0d7..f76c505 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -172,24 +172,7 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
port->elem.out_sg[i].iov_base
+ port->iov_offset,
buf_size);
- if (ret < 0 && ret != -EAGAIN) {
- /* We don't handle any other type of errors here */
- abort();
- }
- if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {
- /*
- * this is a temporary check until chardevs can signal to
- * frontends that they are writable again. This prevents
- * the console from going into throttled mode (forever)
- * if virtio-console is connected to a pty without a
- * listener. Otherwise the guest spins forever.
- * We can revert this if
- * 1: chardevs can notify frondends
- * 2: the guest driver does not spin in these cases
- */
- if (!vsc->is_console) {
- virtio_serial_throttle_port(port, true);
- }
+ if (port->throttled) {
port->iov_idx = i;
if (ret > 0) {
port->iov_offset += ret;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 00/20] chardev flow control
2013-03-05 17:51 [Qemu-devel] [PATCH 00/20] chardev flow control Amit Shah
` (19 preceding siblings ...)
2013-03-05 17:51 ` [Qemu-devel] [PATCH 20/20] virtio-serial: make flow control explicit in virtio-console Amit Shah
@ 2013-03-12 2:02 ` Anthony Liguori
20 siblings, 0 replies; 32+ messages in thread
From: Anthony Liguori @ 2013-03-12 2:02 UTC (permalink / raw)
To: Amit Shah, qemu list; +Cc: Anthony Liguori
Applied. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 32+ messages in thread