* [PATCH 0/3] ui/console: chardev backend improvements @ 2021-09-12 12:50 Volker Rümelin 2021-09-12 12:52 ` [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 Volker Rümelin ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Volker Rümelin @ 2021-09-12 12:50 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Marc-André Lureau, qemu-devel A few things I learnt while writing a fix for a chardev bug in the GTK backend. Volker Rümelin (3): ui/console: replace QEMUFIFO with Fifo8 ui/console: replace kbd_timer with chr_accept_input callback ui/console: remove chardev frontend connected test ui/console.c | 109 ++++++++++++++------------------------------------- 1 file changed, 29 insertions(+), 80 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 2021-09-12 12:50 [PATCH 0/3] ui/console: chardev backend improvements Volker Rümelin @ 2021-09-12 12:52 ` Volker Rümelin 2021-09-12 15:54 ` Marc-André Lureau 2021-09-12 12:52 ` [PATCH 2/3] ui/console: replace kbd_timer with chr_accept_input callback Volker Rümelin 2021-09-12 12:52 ` [PATCH 3/3] ui/console: remove chardev frontend connected test Volker Rümelin 2 siblings, 1 reply; 10+ messages in thread From: Volker Rümelin @ 2021-09-12 12:52 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Marc-André Lureau, qemu-devel One of the two FIFO implementations QEMUFIFO and Fifo8 is redundant. Replace QEMUFIFO with Fifo8. Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> --- ui/console.c | 86 ++++++++++++---------------------------------------- 1 file changed, 20 insertions(+), 66 deletions(-) diff --git a/ui/console.c b/ui/console.c index eabbbc951c..e6ce29024c 100644 --- a/ui/console.c +++ b/ui/console.c @@ -27,6 +27,7 @@ #include "hw/qdev-core.h" #include "qapi/error.h" #include "qapi/qapi-commands-ui.h" +#include "qemu/fifo8.h" #include "qemu/module.h" #include "qemu/option.h" #include "qemu/timer.h" @@ -62,57 +63,6 @@ enum TTYState { TTY_STATE_CSI, }; -typedef struct QEMUFIFO { - uint8_t *buf; - int buf_size; - int count, wptr, rptr; -} QEMUFIFO; - -static int qemu_fifo_write(QEMUFIFO *f, const uint8_t *buf, int len1) -{ - int l, len; - - l = f->buf_size - f->count; - if (len1 > l) - len1 = l; - len = len1; - while (len > 0) { - l = f->buf_size - f->wptr; - if (l > len) - l = len; - memcpy(f->buf + f->wptr, buf, l); - f->wptr += l; - if (f->wptr >= f->buf_size) - f->wptr = 0; - buf += l; - len -= l; - } - f->count += len1; - return len1; -} - -static int qemu_fifo_read(QEMUFIFO *f, uint8_t *buf, int len1) -{ - int l, len; - - if (len1 > f->count) - len1 = f->count; - len = len1; - while (len > 0) { - l = f->buf_size - f->rptr; - if (l > len) - l = len; - memcpy(buf, f->buf + f->rptr, l); - f->rptr += l; - if (f->rptr >= f->buf_size) - f->rptr = 0; - buf += l; - len -= l; - } - f->count -= len1; - return len1; -} - typedef enum { GRAPHIC_CONSOLE, TEXT_CONSOLE, @@ -165,8 +115,7 @@ struct QemuConsole { Chardev *chr; /* fifo for key pressed */ - QEMUFIFO out_fifo; - uint8_t out_fifo_buf[16]; + Fifo8 out_fifo; QEMUTimer *kbd_timer; CoQueue dump_queue; @@ -1160,21 +1109,25 @@ static int vc_chr_write(Chardev *chr, const uint8_t *buf, int len) static void kbd_send_chars(void *opaque) { QemuConsole *s = opaque; - int len; - uint8_t buf[16]; + uint32_t len, avail; len = qemu_chr_be_can_write(s->chr); - if (len > s->out_fifo.count) - len = s->out_fifo.count; - if (len > 0) { - if (len > sizeof(buf)) - len = sizeof(buf); - qemu_fifo_read(&s->out_fifo, buf, len); - qemu_chr_be_write(s->chr, buf, len); + avail = fifo8_num_used(&s->out_fifo); + if (len > avail) { + len = avail; + } + while (len > 0) { + const uint8_t *buf; + uint32_t size; + + buf = fifo8_pop_buf(&s->out_fifo, len, &size); + qemu_chr_be_write(s->chr, (uint8_t *)buf, size); + len -= size; + avail -= size; } /* characters are pending: we send them a bit later (XXX: horrible, should change char device API) */ - if (s->out_fifo.count > 0) { + if (avail > 0) { timer_mod(s->kbd_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 1); } } @@ -1185,6 +1138,7 @@ void kbd_put_keysym_console(QemuConsole *s, int keysym) uint8_t buf[16], *q; CharBackend *be; int c; + uint32_t free; if (!s || (s->console_type == GRAPHIC_CONSOLE)) return; @@ -1228,7 +1182,8 @@ void kbd_put_keysym_console(QemuConsole *s, int keysym) } be = s->chr->be; if (be && be->chr_read) { - qemu_fifo_write(&s->out_fifo, buf, q - buf); + free = fifo8_num_free(&s->out_fifo); + fifo8_push_all(&s->out_fifo, buf, MIN(free, q - buf)); kbd_send_chars(s); } break; @@ -2233,8 +2188,7 @@ static void text_console_do_init(Chardev *chr, DisplayState *ds) int g_width = 80 * FONT_WIDTH; int g_height = 24 * FONT_HEIGHT; - s->out_fifo.buf = s->out_fifo_buf; - s->out_fifo.buf_size = sizeof(s->out_fifo_buf); + fifo8_create(&s->out_fifo, 16); s->kbd_timer = timer_new_ms(QEMU_CLOCK_REALTIME, kbd_send_chars, s); s->ds = ds; -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 2021-09-12 12:52 ` [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 Volker Rümelin @ 2021-09-12 15:54 ` Marc-André Lureau 2021-09-12 17:58 ` Volker Rümelin 2021-09-12 21:21 ` Volker Rümelin 0 siblings, 2 replies; 10+ messages in thread From: Marc-André Lureau @ 2021-09-12 15:54 UTC (permalink / raw) To: Volker Rümelin; +Cc: Gerd Hoffmann, QEMU [-- Attachment #1: Type: text/plain, Size: 4927 bytes --] Hi On Sun, Sep 12, 2021 at 4:53 PM Volker Rümelin <vr_qemu@t-online.de> wrote: > One of the two FIFO implementations QEMUFIFO and Fifo8 is > redundant. Replace QEMUFIFO with Fifo8. > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> > --- > ui/console.c | 86 ++++++++++++---------------------------------------- > 1 file changed, 20 insertions(+), 66 deletions(-) > > diff --git a/ui/console.c b/ui/console.c > index eabbbc951c..e6ce29024c 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -27,6 +27,7 @@ > #include "hw/qdev-core.h" > #include "qapi/error.h" > #include "qapi/qapi-commands-ui.h" > +#include "qemu/fifo8.h" > #include "qemu/module.h" > #include "qemu/option.h" > #include "qemu/timer.h" > @@ -62,57 +63,6 @@ enum TTYState { > TTY_STATE_CSI, > }; > > -typedef struct QEMUFIFO { > - uint8_t *buf; > - int buf_size; > - int count, wptr, rptr; > -} QEMUFIFO; > - > -static int qemu_fifo_write(QEMUFIFO *f, const uint8_t *buf, int len1) > -{ > - int l, len; > - > - l = f->buf_size - f->count; > - if (len1 > l) > - len1 = l; > - len = len1; > - while (len > 0) { > - l = f->buf_size - f->wptr; > - if (l > len) > - l = len; > - memcpy(f->buf + f->wptr, buf, l); > - f->wptr += l; > - if (f->wptr >= f->buf_size) > - f->wptr = 0; > - buf += l; > - len -= l; > - } > - f->count += len1; > - return len1; > -} > - > -static int qemu_fifo_read(QEMUFIFO *f, uint8_t *buf, int len1) > -{ > - int l, len; > - > - if (len1 > f->count) > - len1 = f->count; > - len = len1; > - while (len > 0) { > - l = f->buf_size - f->rptr; > - if (l > len) > - l = len; > - memcpy(buf, f->buf + f->rptr, l); > - f->rptr += l; > - if (f->rptr >= f->buf_size) > - f->rptr = 0; > - buf += l; > - len -= l; > - } > - f->count -= len1; > - return len1; > -} > - > typedef enum { > GRAPHIC_CONSOLE, > TEXT_CONSOLE, > @@ -165,8 +115,7 @@ struct QemuConsole { > > Chardev *chr; > /* fifo for key pressed */ > - QEMUFIFO out_fifo; > - uint8_t out_fifo_buf[16]; > + Fifo8 out_fifo; > QEMUTimer *kbd_timer; > CoQueue dump_queue; > > @@ -1160,21 +1109,25 @@ static int vc_chr_write(Chardev *chr, const > uint8_t *buf, int len) > static void kbd_send_chars(void *opaque) > { > QemuConsole *s = opaque; > - int len; > - uint8_t buf[16]; > + uint32_t len, avail; > > len = qemu_chr_be_can_write(s->chr); > - if (len > s->out_fifo.count) > - len = s->out_fifo.count; > - if (len > 0) { > - if (len > sizeof(buf)) > - len = sizeof(buf); > - qemu_fifo_read(&s->out_fifo, buf, len); > - qemu_chr_be_write(s->chr, buf, len); > + avail = fifo8_num_used(&s->out_fifo); > + if (len > avail) { > + len = avail; > + } > + while (len > 0) { > + const uint8_t *buf; > + uint32_t size; > + > + buf = fifo8_pop_buf(&s->out_fifo, len, &size); > + qemu_chr_be_write(s->chr, (uint8_t *)buf, size); > + len -= size; > + avail -= size; > } > /* characters are pending: we send them a bit later (XXX: > horrible, should change char device API) */ > - if (s->out_fifo.count > 0) { > + if (avail > 0) { > timer_mod(s->kbd_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > 1); > } > } > @@ -1185,6 +1138,7 @@ void kbd_put_keysym_console(QemuConsole *s, int > keysym) > uint8_t buf[16], *q; > CharBackend *be; > int c; > + uint32_t free; > Better call it num_free, to avoid symbol clash (even if we don't use free() directly), it helps reading and can prevent mistakes. > if (!s || (s->console_type == GRAPHIC_CONSOLE)) > return; > @@ -1228,7 +1182,8 @@ void kbd_put_keysym_console(QemuConsole *s, int > keysym) > } > be = s->chr->be; > if (be && be->chr_read) { > - qemu_fifo_write(&s->out_fifo, buf, q - buf); > + free = fifo8_num_free(&s->out_fifo); > + fifo8_push_all(&s->out_fifo, buf, MIN(free, q - buf)); > kbd_send_chars(s); > } > break; > @@ -2233,8 +2188,7 @@ static void text_console_do_init(Chardev *chr, > DisplayState *ds) > int g_width = 80 * FONT_WIDTH; > int g_height = 24 * FONT_HEIGHT; > > - s->out_fifo.buf = s->out_fifo_buf; > - s->out_fifo.buf_size = sizeof(s->out_fifo_buf); > + fifo8_create(&s->out_fifo, 16); > Missing a fif8_destroy() somewhere s->kbd_timer = timer_new_ms(QEMU_CLOCK_REALTIME, kbd_send_chars, s); > s->ds = ds; > > -- > 2.31.1 > > > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 6632 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 2021-09-12 15:54 ` Marc-André Lureau @ 2021-09-12 17:58 ` Volker Rümelin 2021-09-12 18:34 ` Volker Rümelin 2021-09-12 21:21 ` Volker Rümelin 1 sibling, 1 reply; 10+ messages in thread From: Volker Rümelin @ 2021-09-12 17:58 UTC (permalink / raw) To: Marc-André Lureau; +Cc: Gerd Hoffmann, QEMU > > @@ -1185,6 +1138,7 @@ void kbd_put_keysym_console(QemuConsole *s, > int keysym) > uint8_t buf[16], *q; > CharBackend *be; > int c; > + uint32_t free; > > > Better call it num_free, to avoid symbol clash (even if we don't use > free() directly), it helps reading and can prevent mistakes. > Hi, OK, I'll send a version 2 patch. > > if (!s || (s->console_type == GRAPHIC_CONSOLE)) > return; > > @@ -2233,8 +2188,7 @@ static void text_console_do_init(Chardev > *chr, DisplayState *ds) > int g_width = 80 * FONT_WIDTH; > int g_height = 24 * FONT_HEIGHT; > > - s->out_fifo.buf = s->out_fifo_buf; > - s->out_fifo.buf_size = sizeof(s->out_fifo_buf); > + fifo8_create(&s->out_fifo, 16); > > > Missing a fif8_destroy() somewhere > An opened text console stays open until QEMU exits. There's no text_console_close() function. Just like there's a ChardevClass open call but no close call. I think this is one of the many cases in QEMU where resources get allocated for the lifetime of QEMU. With best regards, Volker > s->kbd_timer = timer_new_ms(QEMU_CLOCK_REALTIME, > kbd_send_chars, s); > s->ds = ds; > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 2021-09-12 17:58 ` Volker Rümelin @ 2021-09-12 18:34 ` Volker Rümelin 0 siblings, 0 replies; 10+ messages in thread From: Volker Rümelin @ 2021-09-12 18:34 UTC (permalink / raw) To: Marc-André Lureau; +Cc: Gerd Hoffmann, QEMU Am 12.09.21 um 19:58 schrieb Volker Rümelin: >> >> @@ -1185,6 +1138,7 @@ void kbd_put_keysym_console(QemuConsole *s, >> int keysym) >> uint8_t buf[16], *q; >> CharBackend *be; >> int c; >> + uint32_t free; >> >> >> Better call it num_free, to avoid symbol clash (even if we don't use >> free() directly), it helps reading and can prevent mistakes. >> > > Hi, > > OK, I'll send a version 2 patch. > >> >> if (!s || (s->console_type == GRAPHIC_CONSOLE)) >> return; >> > >> @@ -2233,8 +2188,7 @@ static void text_console_do_init(Chardev >> *chr, DisplayState *ds) >> int g_width = 80 * FONT_WIDTH; >> int g_height = 24 * FONT_HEIGHT; >> >> - s->out_fifo.buf = s->out_fifo_buf; >> - s->out_fifo.buf_size = sizeof(s->out_fifo_buf); >> + fifo8_create(&s->out_fifo, 16); >> >> >> Missing a fif8_destroy() somewhere >> > > An opened text console stays open until QEMU exits. There's no > text_console_close() function. Just like there's a ChardevClass open > call but no close call. I think this is one of the many cases in QEMU > where resources get allocated for the lifetime of QEMU. Sorry, I think my last four sentences are simply wrong. Please ignore this. > > > With best regards, > Volker > >> s->kbd_timer = timer_new_ms(QEMU_CLOCK_REALTIME, >> kbd_send_chars, s); >> s->ds = ds; >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 2021-09-12 15:54 ` Marc-André Lureau 2021-09-12 17:58 ` Volker Rümelin @ 2021-09-12 21:21 ` Volker Rümelin 1 sibling, 0 replies; 10+ messages in thread From: Volker Rümelin @ 2021-09-12 21:21 UTC (permalink / raw) To: Marc-André Lureau; +Cc: Gerd Hoffmann, QEMU > > > @@ -2233,8 +2188,7 @@ static void text_console_do_init(Chardev > *chr, DisplayState *ds) > int g_width = 80 * FONT_WIDTH; > int g_height = 24 * FONT_HEIGHT; > > - s->out_fifo.buf = s->out_fifo_buf; > - s->out_fifo.buf_size = sizeof(s->out_fifo_buf); > + fifo8_create(&s->out_fifo, 16); > > > Missing a fif8_destroy() somewhere Hi, there's no function to close a text console. An opened text console remains open until QEMU exits. Currently QEMU doesn't free allocated text console resources. With best regards, Volker > > s->kbd_timer = timer_new_ms(QEMU_CLOCK_REALTIME, > kbd_send_chars, s); > s->ds = ds; > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] ui/console: replace kbd_timer with chr_accept_input callback 2021-09-12 12:50 [PATCH 0/3] ui/console: chardev backend improvements Volker Rümelin 2021-09-12 12:52 ` [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 Volker Rümelin @ 2021-09-12 12:52 ` Volker Rümelin 2021-09-12 15:59 ` Marc-André Lureau 2021-09-12 12:52 ` [PATCH 3/3] ui/console: remove chardev frontend connected test Volker Rümelin 2 siblings, 1 reply; 10+ messages in thread From: Volker Rümelin @ 2021-09-12 12:52 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Marc-André Lureau, qemu-devel There's a ChardevClass chr_accept_input() callback function that can replace the write retry timer. Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> --- ui/console.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/ui/console.c b/ui/console.c index e6ce29024c..7b276bfc6c 100644 --- a/ui/console.c +++ b/ui/console.c @@ -116,7 +116,6 @@ struct QemuConsole { Chardev *chr; /* fifo for key pressed */ Fifo8 out_fifo; - QEMUTimer *kbd_timer; CoQueue dump_queue; QTAILQ_ENTRY(QemuConsole) next; @@ -1106,30 +1105,21 @@ static int vc_chr_write(Chardev *chr, const uint8_t *buf, int len) return len; } -static void kbd_send_chars(void *opaque) +static void kbd_send_chars(QemuConsole *s) { - QemuConsole *s = opaque; uint32_t len, avail; len = qemu_chr_be_can_write(s->chr); avail = fifo8_num_used(&s->out_fifo); - if (len > avail) { - len = avail; - } - while (len > 0) { + while (len > 0 && avail > 0) { const uint8_t *buf; uint32_t size; - buf = fifo8_pop_buf(&s->out_fifo, len, &size); + buf = fifo8_pop_buf(&s->out_fifo, MIN(len, avail), &size); qemu_chr_be_write(s->chr, (uint8_t *)buf, size); - len -= size; + len = qemu_chr_be_can_write(s->chr); avail -= size; } - /* characters are pending: we send them a bit later (XXX: - horrible, should change char device API) */ - if (avail > 0) { - timer_mod(s->kbd_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 1); - } } /* called when an ascii key is pressed */ @@ -2141,6 +2131,14 @@ int qemu_console_get_height(QemuConsole *con, int fallback) return con ? surface_height(con->surface) : fallback; } +static void vc_chr_accept_input(Chardev *chr) +{ + VCChardev *drv = VC_CHARDEV(chr); + QemuConsole *s = drv->console; + + kbd_send_chars(s); +} + static void vc_chr_set_echo(Chardev *chr, bool echo) { VCChardev *drv = VC_CHARDEV(chr); @@ -2189,7 +2187,6 @@ static void text_console_do_init(Chardev *chr, DisplayState *ds) int g_height = 24 * FONT_HEIGHT; fifo8_create(&s->out_fifo, 16); - s->kbd_timer = timer_new_ms(QEMU_CLOCK_REALTIME, kbd_send_chars, s); s->ds = ds; s->y_displayed = 0; @@ -2439,6 +2436,7 @@ static void char_vc_class_init(ObjectClass *oc, void *data) cc->parse = qemu_chr_parse_vc; cc->open = vc_chr_open; cc->chr_write = vc_chr_write; + cc->chr_accept_input = vc_chr_accept_input; cc->chr_set_echo = vc_chr_set_echo; } -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] ui/console: replace kbd_timer with chr_accept_input callback 2021-09-12 12:52 ` [PATCH 2/3] ui/console: replace kbd_timer with chr_accept_input callback Volker Rümelin @ 2021-09-12 15:59 ` Marc-André Lureau 0 siblings, 0 replies; 10+ messages in thread From: Marc-André Lureau @ 2021-09-12 15:59 UTC (permalink / raw) To: Volker Rümelin; +Cc: Gerd Hoffmann, QEMU [-- Attachment #1: Type: text/plain, Size: 3051 bytes --] On Sun, Sep 12, 2021 at 5:03 PM Volker Rümelin <vr_qemu@t-online.de> wrote: > There's a ChardevClass chr_accept_input() callback function that > can replace the write retry timer. > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- > ui/console.c | 28 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 15 deletions(-) > > diff --git a/ui/console.c b/ui/console.c > index e6ce29024c..7b276bfc6c 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -116,7 +116,6 @@ struct QemuConsole { > Chardev *chr; > /* fifo for key pressed */ > Fifo8 out_fifo; > - QEMUTimer *kbd_timer; > CoQueue dump_queue; > > QTAILQ_ENTRY(QemuConsole) next; > @@ -1106,30 +1105,21 @@ static int vc_chr_write(Chardev *chr, const > uint8_t *buf, int len) > return len; > } > > -static void kbd_send_chars(void *opaque) > +static void kbd_send_chars(QemuConsole *s) > { > - QemuConsole *s = opaque; > uint32_t len, avail; > > len = qemu_chr_be_can_write(s->chr); > avail = fifo8_num_used(&s->out_fifo); > - if (len > avail) { > - len = avail; > - } > - while (len > 0) { > + while (len > 0 && avail > 0) { > const uint8_t *buf; > uint32_t size; > > - buf = fifo8_pop_buf(&s->out_fifo, len, &size); > + buf = fifo8_pop_buf(&s->out_fifo, MIN(len, avail), &size); > qemu_chr_be_write(s->chr, (uint8_t *)buf, size); > - len -= size; > + len = qemu_chr_be_can_write(s->chr); > avail -= size; > } > - /* characters are pending: we send them a bit later (XXX: > - horrible, should change char device API) */ > - if (avail > 0) { > - timer_mod(s->kbd_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > 1); > - } > } > > /* called when an ascii key is pressed */ > @@ -2141,6 +2131,14 @@ int qemu_console_get_height(QemuConsole *con, int > fallback) > return con ? surface_height(con->surface) : fallback; > } > > +static void vc_chr_accept_input(Chardev *chr) > +{ > + VCChardev *drv = VC_CHARDEV(chr); > + QemuConsole *s = drv->console; > + > + kbd_send_chars(s); > +} > + > static void vc_chr_set_echo(Chardev *chr, bool echo) > { > VCChardev *drv = VC_CHARDEV(chr); > @@ -2189,7 +2187,6 @@ static void text_console_do_init(Chardev *chr, > DisplayState *ds) > int g_height = 24 * FONT_HEIGHT; > > fifo8_create(&s->out_fifo, 16); > - s->kbd_timer = timer_new_ms(QEMU_CLOCK_REALTIME, kbd_send_chars, s); > s->ds = ds; > > s->y_displayed = 0; > @@ -2439,6 +2436,7 @@ static void char_vc_class_init(ObjectClass *oc, void > *data) > cc->parse = qemu_chr_parse_vc; > cc->open = vc_chr_open; > cc->chr_write = vc_chr_write; > + cc->chr_accept_input = vc_chr_accept_input; > cc->chr_set_echo = vc_chr_set_echo; > } > > -- > 2.31.1 > > > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 4136 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] ui/console: remove chardev frontend connected test 2021-09-12 12:50 [PATCH 0/3] ui/console: chardev backend improvements Volker Rümelin 2021-09-12 12:52 ` [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 Volker Rümelin 2021-09-12 12:52 ` [PATCH 2/3] ui/console: replace kbd_timer with chr_accept_input callback Volker Rümelin @ 2021-09-12 12:52 ` Volker Rümelin 2021-09-12 16:34 ` Marc-André Lureau 2 siblings, 1 reply; 10+ messages in thread From: Volker Rümelin @ 2021-09-12 12:52 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Marc-André Lureau, qemu-devel The test if the chardev frontend is connected in kbd_put_keysym_console() is redundant, because the call to qemu_chr_be_can_write() in kbd_send_chars() tests the connected condition again. Remove the redundant test whether the chardev frontend is connected. Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> --- ui/console.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/ui/console.c b/ui/console.c index 7b276bfc6c..a06442ed86 100644 --- a/ui/console.c +++ b/ui/console.c @@ -28,10 +28,11 @@ #include "qapi/error.h" #include "qapi/qapi-commands-ui.h" #include "qemu/fifo8.h" +#include "qemu/main-loop.h" #include "qemu/module.h" #include "qemu/option.h" #include "qemu/timer.h" -#include "chardev/char-fe.h" +#include "chardev/char.h" #include "trace.h" #include "exec/memory.h" #include "io/channel-file.h" @@ -1126,7 +1127,6 @@ static void kbd_send_chars(QemuConsole *s) void kbd_put_keysym_console(QemuConsole *s, int keysym) { uint8_t buf[16], *q; - CharBackend *be; int c; uint32_t free; @@ -1170,12 +1170,9 @@ void kbd_put_keysym_console(QemuConsole *s, int keysym) if (s->echo) { vc_chr_write(s->chr, buf, q - buf); } - be = s->chr->be; - if (be && be->chr_read) { - free = fifo8_num_free(&s->out_fifo); - fifo8_push_all(&s->out_fifo, buf, MIN(free, q - buf)); - kbd_send_chars(s); - } + free = fifo8_num_free(&s->out_fifo); + fifo8_push_all(&s->out_fifo, buf, MIN(free, q - buf)); + kbd_send_chars(s); break; } } -- 2.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ui/console: remove chardev frontend connected test 2021-09-12 12:52 ` [PATCH 3/3] ui/console: remove chardev frontend connected test Volker Rümelin @ 2021-09-12 16:34 ` Marc-André Lureau 0 siblings, 0 replies; 10+ messages in thread From: Marc-André Lureau @ 2021-09-12 16:34 UTC (permalink / raw) To: Volker Rümelin; +Cc: Gerd Hoffmann, QEMU [-- Attachment #1: Type: text/plain, Size: 1976 bytes --] On Sun, Sep 12, 2021 at 4:53 PM Volker Rümelin <vr_qemu@t-online.de> wrote: > The test if the chardev frontend is connected in > kbd_put_keysym_console() is redundant, because the call > to qemu_chr_be_can_write() in kbd_send_chars() tests > the connected condition again. > > Remove the redundant test whether the chardev frontend > is connected. > > Signed-off-by: Volker Rümelin <vr_qemu@t-online.de> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- > ui/console.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/ui/console.c b/ui/console.c > index 7b276bfc6c..a06442ed86 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -28,10 +28,11 @@ > #include "qapi/error.h" > #include "qapi/qapi-commands-ui.h" > #include "qemu/fifo8.h" > +#include "qemu/main-loop.h" > #include "qemu/module.h" > #include "qemu/option.h" > #include "qemu/timer.h" > -#include "chardev/char-fe.h" > +#include "chardev/char.h" > #include "trace.h" > #include "exec/memory.h" > #include "io/channel-file.h" > @@ -1126,7 +1127,6 @@ static void kbd_send_chars(QemuConsole *s) > void kbd_put_keysym_console(QemuConsole *s, int keysym) > { > uint8_t buf[16], *q; > - CharBackend *be; > int c; > uint32_t free; > > @@ -1170,12 +1170,9 @@ void kbd_put_keysym_console(QemuConsole *s, int > keysym) > if (s->echo) { > vc_chr_write(s->chr, buf, q - buf); > } > - be = s->chr->be; > - if (be && be->chr_read) { > - free = fifo8_num_free(&s->out_fifo); > - fifo8_push_all(&s->out_fifo, buf, MIN(free, q - buf)); > - kbd_send_chars(s); > - } > + free = fifo8_num_free(&s->out_fifo); > + fifo8_push_all(&s->out_fifo, buf, MIN(free, q - buf)); > + kbd_send_chars(s); > break; > } > } > -- > 2.31.1 > > > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 3014 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-12 21:22 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-12 12:50 [PATCH 0/3] ui/console: chardev backend improvements Volker Rümelin 2021-09-12 12:52 ` [PATCH 1/3] ui/console: replace QEMUFIFO with Fifo8 Volker Rümelin 2021-09-12 15:54 ` Marc-André Lureau 2021-09-12 17:58 ` Volker Rümelin 2021-09-12 18:34 ` Volker Rümelin 2021-09-12 21:21 ` Volker Rümelin 2021-09-12 12:52 ` [PATCH 2/3] ui/console: replace kbd_timer with chr_accept_input callback Volker Rümelin 2021-09-12 15:59 ` Marc-André Lureau 2021-09-12 12:52 ` [PATCH 3/3] ui/console: remove chardev frontend connected test Volker Rümelin 2021-09-12 16:34 ` Marc-André Lureau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.