* [Qemu-devel] [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control
@ 2010-05-04 18:23 Amit Shah
2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 1/6] virtio-console: Factor out common init between console and generic ports Amit Shah
2010-05-04 19:23 ` [Qemu-devel] Re: [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control Gerd Hoffmann
0 siblings, 2 replies; 11+ messages in thread
From: Amit Shah @ 2010-05-04 18:23 UTC (permalink / raw)
To: qemu list; +Cc: Amit Shah, Paul Brook, Gerd Hoffmann, Juan Quintela
Hello,
This series lets interested callers ask for an -EAGAIN return from the
chardev backends (only unix and tcp sockets as of now) to implement
their own flow control.
A new call, qemu_chr_write_nb() is added, that will fallback to
qemu_chr_write() if the backend file isn't non-blocking or if no
callback was supplied.
Support for other backend types is easy to add and will be done in
later patches.
v6:
- Continue write operation on EINTR instead of returning partial
writes (which was a change from prev. behaviour) (Gerd)
v5:
- Fix bug pointed out by Gerd
- Convert to using a struct for passing on handlers to
qemu_chr_add_handlers() instead of passing each one
individually. Simplifies patches. (Inspired by Juan's comment)
- Re-arranged patches
Amit Shah (6):
virtio-console: Factor out common init between console and generic
ports
char: Add a QemuChrHandlers struct to initialise chardev handlers
char: Let writers know how much data was written in case of errors
char: Add qemu_chr_write_nb() for nonblocking writes
char: unix/tcp: Add a non-blocking write handler
virtio-console: Throttle virtio-serial-bus if we can't consume any
more guest data
gdbstub.c | 9 ++-
hw/debugcon.c | 2 +-
hw/escc.c | 9 ++-
hw/etraxfs_ser.c | 13 +++-
hw/mcf_uart.c | 9 ++-
hw/pl011.c | 9 ++-
hw/pxa2xx.c | 13 +++-
hw/serial.c | 9 ++-
hw/sh_serial.c | 12 +++-
hw/syborg_serial.c | 9 ++-
hw/usb-serial.c | 9 ++-
hw/virtio-console.c | 162 +++++++++++++++++++++++++++++++++++++++++++------
hw/xen_console.c | 16 ++++--
hw/xilinx_uartlite.c | 11 +++-
monitor.c | 19 +++++-
net/slirp.c | 8 ++-
net/socket.c | 4 +-
qemu-char.c | 122 ++++++++++++++++++++++++++++++++------
qemu-char.h | 20 +++++-
qemu_socket.h | 3 +-
20 files changed, 384 insertions(+), 84 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread* [Qemu-devel] [PATCH v6 1/6] virtio-console: Factor out common init between console and generic ports 2010-05-04 18:23 [Qemu-devel] [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control Amit Shah @ 2010-05-04 18:23 ` Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 2/6] char: Add a QemuChrHandlers struct to initialise chardev handlers Amit Shah 2010-05-04 19:23 ` [Qemu-devel] Re: [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control Gerd Hoffmann 1 sibling, 1 reply; 11+ messages in thread From: Amit Shah @ 2010-05-04 18:23 UTC (permalink / raw) To: qemu list; +Cc: Amit Shah, Paul Brook, Gerd Hoffmann, Juan Quintela The initialisation for generic ports and console ports is similar. Factor out the parts that are the same in a different function that can be called from each of the initfns. Signed-off-by: Amit Shah <amit.shah@redhat.com> --- hw/virtio-console.c | 31 ++++++++++++++----------------- 1 files changed, 14 insertions(+), 17 deletions(-) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index caea11f..d7fe68b 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -58,24 +58,28 @@ static void chr_event(void *opaque, int event) } } -/* Virtio Console Ports */ -static int virtconsole_initfn(VirtIOSerialDevice *dev) +static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev) { - VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev); - VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port); - - port->info = dev->info; - - port->is_console = true; + vcon->port.info = dev->info; if (vcon->chr) { qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event, vcon); - port->info->have_data = flush_buf; + vcon->port.info->have_data = flush_buf; } return 0; } +/* Virtio Console Ports */ +static int virtconsole_initfn(VirtIOSerialDevice *dev) +{ + VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev); + VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port); + + port->is_console = true; + return generic_port_init(vcon, dev); +} + static int virtconsole_exitfn(VirtIOSerialDevice *dev) { VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev); @@ -115,14 +119,7 @@ static int virtserialport_initfn(VirtIOSerialDevice *dev) VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev); VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port); - port->info = dev->info; - - if (vcon->chr) { - qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event, - vcon); - port->info->have_data = flush_buf; - } - return 0; + return generic_port_init(vcon, dev); } static VirtIOSerialPortInfo virtserialport_info = { -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v6 2/6] char: Add a QemuChrHandlers struct to initialise chardev handlers 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 1/6] virtio-console: Factor out common init between console and generic ports Amit Shah @ 2010-05-04 18:23 ` Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 3/6] char: Let writers know how much data was written in case of errors Amit Shah 0 siblings, 1 reply; 11+ messages in thread From: Amit Shah @ 2010-05-04 18:23 UTC (permalink / raw) To: qemu list; +Cc: Amit Shah, Paul Brook, Gerd Hoffmann, Juan Quintela Instead of passing each handler in the qemu_add_handlers() function, create a struct of handlers that can be passed to the function instead. Signed-off-by: Amit Shah <amit.shah@redhat.com> --- gdbstub.c | 9 +++++++-- hw/debugcon.c | 2 +- hw/escc.c | 9 +++++++-- hw/etraxfs_ser.c | 13 +++++++++---- hw/mcf_uart.c | 9 +++++++-- hw/pl011.c | 9 +++++++-- hw/pxa2xx.c | 13 +++++++++---- hw/serial.c | 9 +++++++-- hw/sh_serial.c | 12 +++++++++--- hw/syborg_serial.c | 9 +++++++-- hw/usb-serial.c | 9 +++++++-- hw/virtio-console.c | 9 +++++++-- hw/xen_console.c | 16 +++++++++++----- hw/xilinx_uartlite.c | 11 +++++++++-- monitor.c | 19 +++++++++++++++---- net/slirp.c | 8 ++++++-- qemu-char.c | 27 ++++++++++++++++++--------- qemu-char.h | 12 ++++++++---- 18 files changed, 151 insertions(+), 54 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 93c4850..7b981ce 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2621,6 +2621,12 @@ static void gdb_sigterm_handler(int signal) } #endif +static QemuChrHandlers gdb_handlers = { + .fd_can_read = gdb_chr_can_receive, + .fd_read = gdb_chr_receive, + .fd_event = gdb_chr_event, +}; + int gdbserver_start(const char *device) { GDBState *s; @@ -2650,8 +2656,7 @@ int gdbserver_start(const char *device) if (!chr) return -1; - qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive, - gdb_chr_event, NULL); + qemu_chr_add_handlers(chr, &gdb_handlers, NULL); } s = gdbserver_state; diff --git a/hw/debugcon.c b/hw/debugcon.c index 5ee6821..e79a595 100644 --- a/hw/debugcon.c +++ b/hw/debugcon.c @@ -73,7 +73,7 @@ static void debugcon_init_core(DebugconState *s) exit(1); } - qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, s); + qemu_chr_add_handlers(s->chr, NULL, s); } static int debugcon_isa_initfn(ISADevice *dev) diff --git a/hw/escc.c b/hw/escc.c index 6d2fd36..1978bf7 100644 --- a/hw/escc.c +++ b/hw/escc.c @@ -890,6 +890,12 @@ void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq, sysbus_mmio_map(s, 0, base); } +static QemuChrHandlers serial_handlers = { + .fd_can_read = serial_can_receive, + .fd_read = serial_receive1, + .fd_event = serial_event, +}; + static int escc_init1(SysBusDevice *dev) { SerialState *s = FROM_SYSBUS(SerialState, dev); @@ -903,8 +909,7 @@ static int escc_init1(SysBusDevice *dev) s->chn[i].chn = 1 - i; s->chn[i].clock = s->frequency / 2; if (s->chn[i].chr) { - qemu_chr_add_handlers(s->chn[i].chr, serial_can_receive, - serial_receive1, serial_event, &s->chn[i]); + qemu_chr_add_handlers(s->chn[i].chr, &serial_handlers, &s->chn[i]); } } s->chn[0].otherchn = &s->chn[1]; diff --git a/hw/etraxfs_ser.c b/hw/etraxfs_ser.c index e1f9615..e22f770 100644 --- a/hw/etraxfs_ser.c +++ b/hw/etraxfs_ser.c @@ -161,6 +161,12 @@ static void serial_event(void *opaque, int event) } +static QemuChrHandlers serial_handlers = { + .fd_can_read = serial_can_receive, + .fd_read = serial_receive, + .fd_event = serial_event, +}; + static int etraxfs_ser_init(SysBusDevice *dev) { struct etrax_serial *s = FROM_SYSBUS(typeof (*s), dev); @@ -174,10 +180,9 @@ static int etraxfs_ser_init(SysBusDevice *dev) ser_regs = cpu_register_io_memory(ser_read, ser_write, s); sysbus_init_mmio(dev, R_MAX * 4, ser_regs); s->chr = qdev_init_chardev(&dev->qdev); - if (s->chr) - qemu_chr_add_handlers(s->chr, - serial_can_receive, serial_receive, - serial_event, s); + if (s->chr) { + qemu_chr_add_handlers(s->chr, &serial_handlers, s); + } return 0; } diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c index d16bac7..301b901 100644 --- a/hw/mcf_uart.c +++ b/hw/mcf_uart.c @@ -268,6 +268,12 @@ static void mcf_uart_receive(void *opaque, const uint8_t *buf, int size) mcf_uart_push_byte(s, buf[0]); } +static QemuChrHandlers mcf_uart_handlers = { + .fd_can_read = mcf_uart_can_receive, + .fd_read = mcf_uart_receive, + .fd_event = mcf_uart_event, +}; + void *mcf_uart_init(qemu_irq irq, CharDriverState *chr) { mcf_uart_state *s; @@ -276,8 +282,7 @@ void *mcf_uart_init(qemu_irq irq, CharDriverState *chr) s->chr = chr; s->irq = irq; if (chr) { - qemu_chr_add_handlers(chr, mcf_uart_can_receive, mcf_uart_receive, - mcf_uart_event, s); + qemu_chr_add_handlers(chr, &mcf_uart_handlers, s); } mcf_uart_reset(s); return s; diff --git a/hw/pl011.c b/hw/pl011.c index 81de91e..8e5356e 100644 --- a/hw/pl011.c +++ b/hw/pl011.c @@ -286,6 +286,12 @@ static int pl011_load(QEMUFile *f, void *opaque, int version_id) return 0; } +static QemuChrHandlers pl011_handlers = { + .fd_can_read = pl011_can_receive, + .fd_read = pl011_receive, + .fd_event = pl011_event, +}; + static int pl011_init(SysBusDevice *dev, const unsigned char *id) { int iomemtype; @@ -303,8 +309,7 @@ static int pl011_init(SysBusDevice *dev, const unsigned char *id) s->cr = 0x300; s->flags = 0x90; if (s->chr) { - qemu_chr_add_handlers(s->chr, pl011_can_receive, pl011_receive, - pl011_event, s); + qemu_chr_add_handlers(s->chr, &pl011_handlers, s); } register_savevm("pl011_uart", -1, 1, pl011_save, pl011_load, s); return 0; diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c index 9095386..fa91527 100644 --- a/hw/pxa2xx.c +++ b/hw/pxa2xx.c @@ -1992,6 +1992,12 @@ static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id) return 0; } +static QemuChrHandlers pxa2xx_handlers = { + .fd_can_read = pxa2xx_fir_is_empty, + .fd_read = pxa2xx_fir_rx, + .fd_event = pxa2xx_fir_event, +}; + static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base, qemu_irq irq, PXA2xxDMAState *dma, CharDriverState *chr) @@ -2010,10 +2016,9 @@ static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base, pxa2xx_fir_writefn, s); cpu_register_physical_memory(base, 0x1000, iomemtype); - if (chr) - qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty, - pxa2xx_fir_rx, pxa2xx_fir_event, s); - + if (chr) { + qemu_chr_add_handlers(chr, &pxa2xx_handlers, s); + } register_savevm("pxa2xx_fir", 0, 0, pxa2xx_fir_save, pxa2xx_fir_load, s); return s; diff --git a/hw/serial.c b/hw/serial.c index 90213c4..ad78a8d 100644 --- a/hw/serial.c +++ b/hw/serial.c @@ -724,6 +724,12 @@ static void serial_reset(void *opaque) qemu_irq_lower(s->irq); } +static QemuChrHandlers serial_handlers = { + .fd_can_read = serial_can_receive1, + .fd_read = serial_receive1, + .fd_event = serial_event, +}; + static void serial_init_core(SerialState *s) { if (!s->chr) { @@ -738,8 +744,7 @@ static void serial_init_core(SerialState *s) qemu_register_reset(serial_reset, s); - qemu_chr_add_handlers(s->chr, serial_can_receive1, serial_receive1, - serial_event, s); + qemu_chr_add_handlers(s->chr, &serial_handlers, s); } /* Change the main reference oscillator frequency. */ diff --git a/hw/sh_serial.c b/hw/sh_serial.c index 93dc144..ee3f8c8 100644 --- a/hw/sh_serial.c +++ b/hw/sh_serial.c @@ -363,6 +363,12 @@ static CPUWriteMemoryFunc * const sh_serial_writefn[] = { &sh_serial_write, }; +static QemuChrHandlers sh_serial_handlers = { + .fd_can_read = sh_serial_can_receive1, + .fd_read = sh_serial_receive1, + .fd_event = sh_serial_event, +}; + void sh_serial_init (target_phys_addr_t base, int feat, uint32_t freq, CharDriverState *chr, qemu_irq eri_source, @@ -401,9 +407,9 @@ void sh_serial_init (target_phys_addr_t base, int feat, s->chr = chr; - if (chr) - qemu_chr_add_handlers(chr, sh_serial_can_receive1, sh_serial_receive1, - sh_serial_event, s); + if (chr) { + qemu_chr_add_handlers(chr, &sh_serial_handlers, s); + } s->eri = eri_source; s->rxi = rxi_source; diff --git a/hw/syborg_serial.c b/hw/syborg_serial.c index cac00ea..a8b9811 100644 --- a/hw/syborg_serial.c +++ b/hw/syborg_serial.c @@ -315,6 +315,12 @@ static int syborg_serial_load(QEMUFile *f, void *opaque, int version_id) return 0; } +static QemuChrHandlers syborg_serial_handlers = { + .fd_can_read = syborg_serial_can_receive, + .fd_read = syborg_serial_receive, + .fd_event = syborg_serial_event, +}; + static int syborg_serial_init(SysBusDevice *dev) { SyborgSerialState *s = FROM_SYSBUS(SyborgSerialState, dev); @@ -326,8 +332,7 @@ static int syborg_serial_init(SysBusDevice *dev) sysbus_init_mmio(dev, 0x1000, iomemtype); s->chr = qdev_init_chardev(&dev->qdev); if (s->chr) { - qemu_chr_add_handlers(s->chr, syborg_serial_can_receive, - syborg_serial_receive, syborg_serial_event, s); + qemu_chr_add_handlers(s->chr, &syborg_serial_handlers, s); } if (s->fifo_size <= 0) { fprintf(stderr, "syborg_serial: fifo too small\n"); diff --git a/hw/usb-serial.c b/hw/usb-serial.c index 5b2483a..4f42f9f 100644 --- a/hw/usb-serial.c +++ b/hw/usb-serial.c @@ -540,13 +540,18 @@ static void usb_serial_event(void *opaque, int event) } } +static QemuChrHandlers usb_serial_handlers = { + .fd_can_read = usb_serial_can_read, + .fd_read = usb_serial_read, + .fd_event = usb_serial_event, +}; + static int usb_serial_initfn(USBDevice *dev) { USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev); s->dev.speed = USB_SPEED_FULL; - qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read, - usb_serial_event, s); + qemu_chr_add_handlers(s->cs, &usb_serial_handlers, s); usb_serial_handle_reset(dev); return 0; } diff --git a/hw/virtio-console.c b/hw/virtio-console.c index d7fe68b..749ed59 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -58,13 +58,18 @@ static void chr_event(void *opaque, int event) } } +static QemuChrHandlers chr_handlers = { + .fd_can_read = chr_can_read, + .fd_read = chr_read, + .fd_event = chr_event, +}; + static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev) { vcon->port.info = dev->info; if (vcon->chr) { - qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event, - vcon); + qemu_chr_add_handlers(vcon->chr, &chr_handlers, vcon); vcon->port.info->have_data = flush_buf; } return 0; diff --git a/hw/xen_console.c b/hw/xen_console.c index d2261f4..f490a5a 100644 --- a/hw/xen_console.c +++ b/hw/xen_console.c @@ -202,6 +202,11 @@ static int con_init(struct XenDevice *xendev) return 0; } +static QemuChrHandlers xencons_handlers = { + .fd_can_read = xencons_can_receive, + .fd_read = xencons_receive, +}; + static int con_connect(struct XenDevice *xendev) { struct XenConsole *con = container_of(xendev, struct XenConsole, xendev); @@ -222,9 +227,9 @@ static int con_connect(struct XenDevice *xendev) return -1; xen_be_bind_evtchn(&con->xendev); - if (con->chr) - qemu_chr_add_handlers(con->chr, xencons_can_receive, xencons_receive, - NULL, con); + if (con->chr) { + qemu_chr_add_handlers(con->chr, &xencons_handlers, con); + } xen_be_printf(xendev, 1, "ring mfn %d, remote port %d, local port %d, limit %zd\n", con->ring_ref, @@ -238,8 +243,9 @@ static void con_disconnect(struct XenDevice *xendev) { struct XenConsole *con = container_of(xendev, struct XenConsole, xendev); - if (con->chr) - qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL); + if (con->chr) { + qemu_chr_add_handlers(con->chr, NULL, NULL); + } xen_be_unbind_evtchn(&con->xendev); if (con->sring) { diff --git a/hw/xilinx_uartlite.c b/hw/xilinx_uartlite.c index adab759..376216e 100644 --- a/hw/xilinx_uartlite.c +++ b/hw/xilinx_uartlite.c @@ -193,6 +193,12 @@ static void uart_event(void *opaque, int event) } +static QemuChrHandlers uart_handlers = { + .fd_can_read = uart_can_rx, + .fd_read = uart_rx, + .fd_event = uart_event, +}; + static int xilinx_uartlite_init(SysBusDevice *dev) { struct xlx_uartlite *s = FROM_SYSBUS(typeof (*s), dev); @@ -205,8 +211,9 @@ static int xilinx_uartlite_init(SysBusDevice *dev) sysbus_init_mmio(dev, R_MAX * 4, uart_regs); s->chr = qdev_init_chardev(&dev->qdev); - if (s->chr) - qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s); + if (s->chr) { + qemu_chr_add_handlers(s->chr, &uart_handlers, s); + } return 0; } diff --git a/monitor.c b/monitor.c index 46d0b47..4f9061e 100644 --- a/monitor.c +++ b/monitor.c @@ -4600,6 +4600,18 @@ static void monitor_event(void *opaque, int event) * End: */ +static QemuChrHandlers monitor_handlers = { + .fd_can_read = monitor_can_read, + .fd_read = monitor_read, + .fd_event = monitor_event, +}; + +static QemuChrHandlers monitor_control_handlers = { + .fd_can_read = monitor_can_read, + .fd_read = monitor_control_read, + .fd_event = monitor_control_event, +}; + void monitor_init(CharDriverState *chr, int flags) { static int is_first_init = 1; @@ -4621,12 +4633,11 @@ void monitor_init(CharDriverState *chr, int flags) if (monitor_ctrl_mode(mon)) { mon->mc = qemu_mallocz(sizeof(MonitorControl)); + /* Control mode requires special handlers */ - qemu_chr_add_handlers(chr, monitor_can_read, monitor_control_read, - monitor_control_event, mon); + qemu_chr_add_handlers(chr, &monitor_control_handlers, mon); } else { - qemu_chr_add_handlers(chr, monitor_can_read, monitor_read, - monitor_event, mon); + qemu_chr_add_handlers(chr, &monitor_handlers, mon); } QLIST_INSERT_HEAD(&mon_list, mon, entry); diff --git a/net/slirp.c b/net/slirp.c index b41c60a..52e9a5c 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -577,6 +577,11 @@ static void guestfwd_read(void *opaque, const uint8_t *buf, int size) slirp_socket_recv(fwd->slirp, fwd->server, fwd->port, buf, size); } +static QemuChrHandlers guestfwd_handlers = { + .fd_can_read = guestfwd_can_read, + .fd_read = guestfwd_read, +}; + static int slirp_guestfwd(SlirpState *s, const char *config_str, int legacy_format) { @@ -633,8 +638,7 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str, fwd->port = port; fwd->slirp = s->slirp; - qemu_chr_add_handlers(fwd->hd, guestfwd_can_read, guestfwd_read, - NULL, fwd); + qemu_chr_add_handlers(fwd->hd, &guestfwd_handlers, fwd); return 0; fail_syntax: diff --git a/qemu-char.c b/qemu-char.c index ac65a1c..65cb3f5 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -191,15 +191,19 @@ void qemu_chr_send_event(CharDriverState *s, int event) s->chr_send_event(s, event); } +static QemuChrHandlers null_handlers = { + /* All handlers are initialised to NULL */ +}; + void qemu_chr_add_handlers(CharDriverState *s, - IOCanReadHandler *fd_can_read, - IOReadHandler *fd_read, - IOEventHandler *fd_event, - void *opaque) -{ - s->chr_can_read = fd_can_read; - s->chr_read = fd_read; - s->chr_event = fd_event; + QemuChrHandlers *handlers, void *opaque) +{ + if (!handlers) { + handlers = &null_handlers; + } + s->chr_can_read = handlers->fd_can_read; + s->chr_read = handlers->fd_read; + s->chr_event = handlers->fd_event; s->handler_opaque = opaque; if (s->chr_update_read_handler) s->chr_update_read_handler(s); @@ -442,6 +446,12 @@ static void mux_chr_event(void *opaque, int event) mux_chr_send_event(d, i, event); } +static QemuChrHandlers mux_chr_handlers = { + .fd_can_read = mux_chr_can_read, + .fd_read = mux_chr_read, + .fd_event = mux_chr_event, +}; + static void mux_chr_update_read_handler(CharDriverState *chr) { MuxDriver *d = chr->opaque; @@ -456,8 +466,7 @@ static void mux_chr_update_read_handler(CharDriverState *chr) d->chr_event[d->mux_cnt] = chr->chr_event; /* Fix up the real driver with mux routines */ if (d->mux_cnt == 0) { - qemu_chr_add_handlers(d->drv, mux_chr_can_read, mux_chr_read, - mux_chr_event, chr); + qemu_chr_add_handlers(d->drv, &mux_chr_handlers, chr); } if (d->focus != -1) { mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT); diff --git a/qemu-char.h b/qemu-char.h index e3a0783..6ff490b 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -71,6 +71,13 @@ struct CharDriverState { QTAILQ_ENTRY(CharDriverState) next; }; +typedef struct QemuChrHandlers { + IOCanReadHandler *fd_can_read; + IOReadHandler *fd_read; + IOHandler *fd_write_unblocked; + IOEventHandler *fd_event; +} QemuChrHandlers; + QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename); CharDriverState *qemu_chr_open_opts(QemuOpts *opts, void (*init)(struct CharDriverState *s)); @@ -79,10 +86,7 @@ void qemu_chr_close(CharDriverState *chr); void qemu_chr_printf(CharDriverState *s, const char *fmt, ...); int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len); void qemu_chr_send_event(CharDriverState *s, int event); -void qemu_chr_add_handlers(CharDriverState *s, - IOCanReadHandler *fd_can_read, - IOReadHandler *fd_read, - IOEventHandler *fd_event, +void qemu_chr_add_handlers(CharDriverState *s, QemuChrHandlers *handlers, void *opaque); int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg); void qemu_chr_generic_open(CharDriverState *s); -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v6 3/6] char: Let writers know how much data was written in case of errors 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 2/6] char: Add a QemuChrHandlers struct to initialise chardev handlers Amit Shah @ 2010-05-04 18:23 ` Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 4/6] char: Add qemu_chr_write_nb() for nonblocking writes Amit Shah 0 siblings, 1 reply; 11+ messages in thread From: Amit Shah @ 2010-05-04 18:23 UTC (permalink / raw) To: qemu list; +Cc: Amit Shah, Paul Brook, Gerd Hoffmann, Juan Quintela On writing errors, we just returned -1 even if some bytes were already written out. Ensure we return the number of bytes written before we return the error (on a subsequent call to qemu_chr_write()). Signed-off-by: Amit Shah <amit.shah@redhat.com> --- qemu-char.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 65cb3f5..1f5d886 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -507,6 +507,9 @@ int send_all(int fd, const void *buf, int len1) while (len > 0) { ret = send(fd, buf, len, 0); if (ret < 0) { + if (len1 - len) { + return len1 - len; + } errno = WSAGetLastError(); if (errno != WSAEWOULDBLOCK) { return -1; @@ -531,8 +534,15 @@ static int unix_write(int fd, const uint8_t *buf, int len1) while (len > 0) { ret = write(fd, buf, len); if (ret < 0) { - if (errno != EINTR && errno != EAGAIN) + if (errno == EINTR) { + continue; + } + if (len1 - len) { + return len1 - len; + } + if (errno != EAGAIN) { return -1; + } } else if (ret == 0) { break; } else { -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v6 4/6] char: Add qemu_chr_write_nb() for nonblocking writes 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 3/6] char: Let writers know how much data was written in case of errors Amit Shah @ 2010-05-04 18:23 ` Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Amit Shah 0 siblings, 1 reply; 11+ messages in thread From: Amit Shah @ 2010-05-04 18:23 UTC (permalink / raw) To: qemu list; +Cc: Amit Shah, Paul Brook, Gerd Hoffmann, Juan Quintela For char devices whose backing files are open in non-blocking mode, non-blocking writes can now be made using qemu_chr_write_nb(). For non-blocking chardevs, we can return -EAGAIN to callers of qemu_chr_write_nb(). When the backend is ready to accept more data, we can let the caller know via a callback. -EAGAIN is returned only when the backend's file is non-blocking and a callback is registered by the caller when invoking qemu_chr_add_handlers(). In case a backend doesn't support non-blocking writes, qemu_chr_write_nb() invokes qemu_chr_write(). Individual callers can update their code to add a callback handler, call qemu_chr_write_nb() instead of qemu_chr_write() if they wish to receive -EAGAIN notifications. No backend currently supports non-blocking writes. Signed-off-by: Amit Shah <amit.shah@redhat.com> --- net/socket.c | 4 ++-- qemu-char.c | 31 ++++++++++++++++++++++++------- qemu-char.h | 8 ++++++++ qemu_socket.h | 3 ++- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/net/socket.c b/net/socket.c index 1c4e153..8a401e6 100644 --- a/net/socket.c +++ b/net/socket.c @@ -56,8 +56,8 @@ static ssize_t net_socket_receive(VLANClientState *nc, const uint8_t *buf, size_ uint32_t len; len = htonl(size); - send_all(s->fd, (const uint8_t *)&len, sizeof(len)); - return send_all(s->fd, buf, size); + send_all(s->fd, (const uint8_t *)&len, sizeof(len), false); + return send_all(s->fd, buf, size, false); } static ssize_t net_socket_receive_dgram(VLANClientState *nc, const uint8_t *buf, size_t size) diff --git a/qemu-char.c b/qemu-char.c index 1f5d886..e6934f6 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -145,6 +145,16 @@ int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len) return s->chr_write(s, buf, len); } +ssize_t qemu_chr_write_nb(CharDriverState *s, const uint8_t *buf, size_t len) +{ + if (!s->nonblock) { + /* Fallback to blocking write if no callback registered */ + return qemu_chr_write(s, buf, len); + } + + return s->chr_write_nb(s, buf, len); +} + int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg) { if (!s->chr_ioctl) @@ -203,11 +213,15 @@ void qemu_chr_add_handlers(CharDriverState *s, } s->chr_can_read = handlers->fd_can_read; s->chr_read = handlers->fd_read; + s->chr_write_unblocked = handlers->fd_write_unblocked; s->chr_event = handlers->fd_event; s->handler_opaque = opaque; if (s->chr_update_read_handler) s->chr_update_read_handler(s); + /* We'll set this at connect-time */ + s->nonblock = false; + /* We're connecting to an already opened device, so let's make sure we also get the open event */ if (s->opened) { @@ -499,7 +513,7 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState *drv) #ifdef _WIN32 -int send_all(int fd, const void *buf, int len1) +int send_all(int fd, const void *buf, int len1, bool nonblock) { int ret, len; @@ -526,7 +540,7 @@ int send_all(int fd, const void *buf, int len1) #else -static int unix_write(int fd, const uint8_t *buf, int len1) +static int unix_write(int fd, const uint8_t *buf, int len1, bool nonblock) { int ret, len; @@ -540,6 +554,9 @@ static int unix_write(int fd, const uint8_t *buf, int len1) if (len1 - len) { return len1 - len; } + if (errno == EAGAIN && nonblock) { + return -EAGAIN; + } if (errno != EAGAIN) { return -1; } @@ -553,9 +570,9 @@ static int unix_write(int fd, const uint8_t *buf, int len1) return len1 - len; } -int send_all(int fd, const void *buf, int len1) +int send_all(int fd, const void *buf, int len1, bool nonblock) { - return unix_write(fd, buf, len1); + return unix_write(fd, buf, len1, nonblock); } #endif /* !_WIN32 */ @@ -572,7 +589,7 @@ static int stdio_nb_clients = 0; 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 send_all(s->fd_out, buf, len, false); } static int fd_chr_read_poll(void *opaque) @@ -884,7 +901,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 send_all(s->fd, buf, len, false); } static int pty_chr_read_poll(void *opaque) @@ -1949,7 +1966,7 @@ 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 send_all(s->fd, buf, len, false); } else { /* XXX: indicate an error ? */ return len; diff --git a/qemu-char.h b/qemu-char.h index 6ff490b..7b10b2b 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -1,6 +1,8 @@ #ifndef QEMU_CHAR_H #define QEMU_CHAR_H +#include <stdbool.h> + #include "qemu-common.h" #include "qemu-queue.h" #include "qemu-option.h" @@ -53,12 +55,15 @@ 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); + ssize_t (*chr_write_nb)(struct CharDriverState *s, const uint8_t *buf, + size_t len); void (*chr_update_read_handler)(struct CharDriverState *s); int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg); int (*get_msgfd)(struct CharDriverState *s); IOEventHandler *chr_event; IOCanReadHandler *chr_can_read; IOReadHandler *chr_read; + IOHandler *chr_write_unblocked; void *handler_opaque; void (*chr_send_event)(struct CharDriverState *chr, int event); void (*chr_close)(struct CharDriverState *chr); @@ -68,6 +73,8 @@ struct CharDriverState { char *label; char *filename; int opened; + bool nonblock; + bool write_blocked; QTAILQ_ENTRY(CharDriverState) next; }; @@ -85,6 +92,7 @@ CharDriverState *qemu_chr_open(const char *label, const char *filename, void (*i void qemu_chr_close(CharDriverState *chr); void qemu_chr_printf(CharDriverState *s, const char *fmt, ...); int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len); +ssize_t qemu_chr_write_nb(CharDriverState *s, const uint8_t *buf, size_t len); void qemu_chr_send_event(CharDriverState *s, int event); void qemu_chr_add_handlers(CharDriverState *s, QemuChrHandlers *handlers, void *opaque); diff --git a/qemu_socket.h b/qemu_socket.h index 164ae3e..bdf878b 100644 --- a/qemu_socket.h +++ b/qemu_socket.h @@ -23,6 +23,7 @@ int inet_aton(const char *cp, struct in_addr *ia); #include <arpa/inet.h> #include <netdb.h> #include <sys/un.h> +#include <stdbool.h> #define socket_error() errno #define closesocket(s) close(s) @@ -35,7 +36,7 @@ int inet_aton(const char *cp, struct in_addr *ia); int qemu_socket(int domain, int type, int protocol); int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen); void socket_set_nonblock(int fd); -int send_all(int fd, const void *buf, int len1); +int send_all(int fd, const void *buf, int len1, bool nonblock); /* New, ipv6-ready socket helper functions, see qemu-sockets.c */ int inet_listen_opts(QemuOpts *opts, int port_offset); -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 4/6] char: Add qemu_chr_write_nb() for nonblocking writes Amit Shah @ 2010-05-04 18:23 ` Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 6/6] virtio-console: Throttle virtio-serial-bus if we can't consume any more guest data Amit Shah 2010-05-04 19:54 ` [Qemu-devel] Re: [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Juan Quintela 0 siblings, 2 replies; 11+ messages in thread From: Amit Shah @ 2010-05-04 18:23 UTC (permalink / raw) To: qemu list; +Cc: Amit Shah, Paul Brook, Gerd Hoffmann, Juan Quintela Add a non-blocking write handler that can return with -EAGAIN to the caller and also callback when the socket becomes writable. Non-blocking writes are only enabled for sockets that are opened in non-blocking mode and only for callers that have registered a callback handler for resuming writes. Signed-off-by: Amit Shah <amit.shah@redhat.com> --- qemu-char.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 50 insertions(+), 0 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index e6934f6..da70b9b 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2129,11 +2129,60 @@ static void tcp_chr_read(void *opaque) } } +static void tcp_chr_write_unblocked(void *opaque) +{ + CharDriverState *chr = opaque; + TCPCharDriver *s = chr->opaque; + + assert(chr->write_blocked && chr->chr_write_unblocked); + + chr->write_blocked = false; + qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, tcp_chr_read, NULL, chr); + chr->chr_write_unblocked(chr->handler_opaque); +} + +static ssize_t tcp_chr_write_nb(CharDriverState *chr, const uint8_t *buf, + size_t len) +{ + TCPCharDriver *s = chr->opaque; + ssize_t ret; + + if (!s->connected) { + /* XXX: indicate an error? */ + return len; + } + + ret = send_all(s->fd, buf, len, true); + if (ret == -EAGAIN) { + chr->write_blocked = true; + qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, + tcp_chr_read, tcp_chr_write_unblocked, chr); + } + return ret; +} + static void tcp_chr_connect(void *opaque) { CharDriverState *chr = opaque; TCPCharDriver *s = chr->opaque; + IOHandler *write_cb; + int flags; + bool nonblock; + + flags = fcntl(s->fd, F_GETFL); + if (flags == -1) { + flags = 0; + } + nonblock = flags & O_NONBLOCK; + + write_cb = NULL; + chr->nonblock = false; + if (nonblock && chr->chr_write_unblocked) { + write_cb = chr->chr_write_unblocked; + chr->nonblock = true; + } + chr->write_blocked = false; s->connected = 1; qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, tcp_chr_read, NULL, chr); @@ -2266,6 +2315,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) chr->opaque = s; chr->chr_write = tcp_chr_write; + chr->chr_write_nb = tcp_chr_write_nb; chr->chr_close = tcp_chr_close; chr->get_msgfd = tcp_get_msgfd; -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v6 6/6] virtio-console: Throttle virtio-serial-bus if we can't consume any more guest data 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Amit Shah @ 2010-05-04 18:23 ` Amit Shah 2010-05-04 20:28 ` [Qemu-devel] " Juan Quintela 2010-05-04 19:54 ` [Qemu-devel] Re: [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Juan Quintela 1 sibling, 1 reply; 11+ messages in thread From: Amit Shah @ 2010-05-04 18:23 UTC (permalink / raw) To: qemu list; +Cc: Amit Shah, Paul Brook, Gerd Hoffmann, Juan Quintela If the char device we're connected to is overwhelmed with data and it can't accept any more, signal to the virtio-serial-bus to stop sending us more data till we tell otherwise. If the current buffer being processed hasn't been completely written out to the char device, we have to keep it around and re-try sending it since the virtio-serial-bus code assumes we consume the entire buffer. Allow the chardev backends to return -EAGAIN; we're ready with a callback handler that will flush the remainder of the buffer. Also register with savevm so that we save/restore such a buffer across migration. Signed-off-by: Amit Shah <amit.shah@redhat.com> --- hw/virtio-console.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 123 insertions(+), 3 deletions(-) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index 749ed59..7eb6aa1 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -13,18 +13,92 @@ #include "qemu-char.h" #include "virtio-serial.h" +typedef struct Buffer { + uint8_t *buf; + size_t rem_len; + size_t offset; +} Buffer; + typedef struct VirtConsole { VirtIOSerialPort port; CharDriverState *chr; + Buffer *unflushed_buf; } VirtConsole; +static void add_unflushed_buf(VirtConsole *vcon, const uint8_t *buf, size_t len) +{ + vcon->unflushed_buf = qemu_malloc(sizeof(Buffer)); + vcon->unflushed_buf->buf = qemu_malloc(len); + + memcpy(vcon->unflushed_buf->buf, buf, len); + vcon->unflushed_buf->rem_len = len; + vcon->unflushed_buf->offset = 0; +} + +static void free_unflushed_buf(VirtConsole *vcon) +{ + if (vcon->unflushed_buf) { + qemu_free(vcon->unflushed_buf->buf); + qemu_free(vcon->unflushed_buf); + vcon->unflushed_buf = NULL; + } +} + +static int buffered_write_to_chardev(VirtConsole *vcon, const uint8_t *buf, + size_t len) +{ + size_t written; + ssize_t ret; + + written = 0; + do { + ret = qemu_chr_write_nb(vcon->chr, buf + written, len - written); + if (ret < 0) { + if (vcon->unflushed_buf) { + vcon->unflushed_buf->offset += written; + vcon->unflushed_buf->rem_len -= written; + } else { + virtio_serial_throttle_port(&vcon->port, true); + add_unflushed_buf(vcon, buf + written, len - written); + } + + return -EAGAIN; + } + + written += ret; + } while (written != len); + + return 0; +} + +/* Callback function called when the chardev can accept more data */ +static void chr_write_unblocked(void *opaque) +{ + VirtConsole *vcon = opaque; + + if (vcon->unflushed_buf) { + int ret; + + ret = buffered_write_to_chardev(vcon, vcon->unflushed_buf->buf + + vcon->unflushed_buf->offset, + vcon->unflushed_buf->rem_len); + if (ret < 0) { + return; + } + free_unflushed_buf(vcon); + } + virtio_serial_throttle_port(&vcon->port, false); +} /* Callback function that's called when the guest sends us data */ static void flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len) { VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port); - qemu_chr_write(vcon->chr, buf, len); + /* If a previous write was incomplete, we should've been throttled. */ + assert(!vcon->unflushed_buf); + + buffered_write_to_chardev(vcon, buf, len); } /* Readiness of the guest to accept data on a port */ @@ -48,19 +122,62 @@ static void chr_event(void *opaque, int event) VirtConsole *vcon = opaque; switch (event) { - case CHR_EVENT_OPENED: { + case CHR_EVENT_OPENED: virtio_serial_open(&vcon->port); break; - } + case CHR_EVENT_CLOSED: + if (vcon->unflushed_buf) { + free_unflushed_buf(vcon); + } virtio_serial_close(&vcon->port); break; } } +static void virtio_console_port_save(QEMUFile *f, void *opaque) +{ + VirtConsole *vcon = opaque; + uint32_t have_buffer; + + have_buffer = vcon->unflushed_buf ? true : false; + + qemu_put_be32s(f, &have_buffer); + if (have_buffer) { + qemu_put_be64s(f, &vcon->unflushed_buf->rem_len); + qemu_put_buffer(f, vcon->unflushed_buf->buf + + vcon->unflushed_buf->offset, + vcon->unflushed_buf->rem_len); + } +} + +static int virtio_console_port_load(QEMUFile *f, void *opaque, int version_id) +{ + VirtConsole *vcon = opaque; + uint32_t have_buffer; + + if (version_id > 1) { + return -EINVAL; + } + + qemu_get_be32s(f, &have_buffer); + if (have_buffer) { + vcon->unflushed_buf = qemu_mallocz(sizeof(Buffer)); + + qemu_get_be64s(f, &vcon->unflushed_buf->rem_len); + vcon->unflushed_buf->buf = qemu_malloc(vcon->unflushed_buf->rem_len); + vcon->unflushed_buf->offset = 0; + + qemu_get_buffer(f, vcon->unflushed_buf->buf, + vcon->unflushed_buf->rem_len); + } + return 0; +} + static QemuChrHandlers chr_handlers = { .fd_can_read = chr_can_read, .fd_read = chr_read, + .fd_write_unblocked = chr_write_unblocked, .fd_event = chr_event, }; @@ -72,6 +189,8 @@ static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev) qemu_chr_add_handlers(vcon->chr, &chr_handlers, vcon); vcon->port.info->have_data = flush_buf; } + register_savevm("virtio-console-ports", -1, 1, virtio_console_port_save, + virtio_console_port_load, vcon); return 0; } @@ -93,6 +212,7 @@ static int virtconsole_exitfn(VirtIOSerialDevice *dev) if (vcon->chr) { port->info->have_data = NULL; qemu_chr_close(vcon->chr); + free_unflushed_buf(vcon); } return 0; -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] Re: [PATCH v6 6/6] virtio-console: Throttle virtio-serial-bus if we can't consume any more guest data 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 6/6] virtio-console: Throttle virtio-serial-bus if we can't consume any more guest data Amit Shah @ 2010-05-04 20:28 ` Juan Quintela 0 siblings, 0 replies; 11+ messages in thread From: Juan Quintela @ 2010-05-04 20:28 UTC (permalink / raw) To: Amit Shah; +Cc: Paul Brook, qemu list, Gerd Hoffmann Amit Shah <amit.shah@redhat.com> wrote: > If the char device we're connected to is overwhelmed with data and it > can't accept any more, signal to the virtio-serial-bus to stop sending > us more data till we tell otherwise. > > If the current buffer being processed hasn't been completely written out > to the char device, we have to keep it around and re-try sending it > since the virtio-serial-bus code assumes we consume the entire buffer. > > Allow the chardev backends to return -EAGAIN; we're ready with a > callback handler that will flush the remainder of the buffer. > > Also register with savevm so that we save/restore such a buffer across > migration. > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > --- > hw/virtio-console.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 123 insertions(+), 3 deletions(-) > > diff --git a/hw/virtio-console.c b/hw/virtio-console.c > index 749ed59..7eb6aa1 100644 > --- a/hw/virtio-console.c > +++ b/hw/virtio-console.c > @@ -13,18 +13,92 @@ > #include "qemu-char.h" > #include "virtio-serial.h" > > +typedef struct Buffer { > + uint8_t *buf; > + size_t rem_len; > + size_t offset; > +} Buffer; > + > typedef struct VirtConsole { > VirtIOSerialPort port; > CharDriverState *chr; > + Buffer *unflushed_buf; This part of teh struct can be static instead of a pointer, and adjust rest of code accordingly. for the rest, it looks ok. For this to work, I think that you will have to change rem_len for total_len (or whatever) and adjust code around. This makes trivial to check for buffer used/no, just see if len = 0. > +static int buffered_write_to_chardev(VirtConsole *vcon, const uint8_t *buf, > + size_t len) > +{ > + size_t written; > + ssize_t ret; > + > + written = 0; > + do { > + ret = qemu_chr_write_nb(vcon->chr, buf + written, len - written); > + if (ret < 0) { > + if (vcon->unflushed_buf) { > + vcon->unflushed_buf->offset += written; > + vcon->unflushed_buf->rem_len -= written; > + } else { > + virtio_serial_throttle_port(&vcon->port, true); > + add_unflushed_buf(vcon, buf + written, len - written); > + } > + > + return -EAGAIN; You can "return ret" instead of "inventing" a new error :) > +static void virtio_console_port_save(QEMUFile *f, void *opaque) > +{ > + VirtConsole *vcon = opaque; > + uint32_t have_buffer; > + > + have_buffer = vcon->unflushed_buf ? true : false; So, here you can just write vcon->unflushed_buf.len if you go with my proposal, what will make VMState transition easier. Rest of patch is good. I also like the series, specially the change of qemu_chr_add_handlers() to use one struct to pass its arguments. Later, Juan. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] Re: [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 6/6] virtio-console: Throttle virtio-serial-bus if we can't consume any more guest data Amit Shah @ 2010-05-04 19:54 ` Juan Quintela 2010-05-04 19:58 ` Amit Shah 1 sibling, 1 reply; 11+ messages in thread From: Juan Quintela @ 2010-05-04 19:54 UTC (permalink / raw) To: Amit Shah; +Cc: Paul Brook, qemu list, Gerd Hoffmann Amit Shah <amit.shah@redhat.com> wrote: > Add a non-blocking write handler that can return with -EAGAIN to the > caller and also callback when the socket becomes writable. > > Non-blocking writes are only enabled for sockets that are opened in > non-blocking mode and only for callers that have registered a callback > handler for resuming writes. > > Signed-off-by: Amit Shah <amit.shah@redhat.com> I > static void tcp_chr_connect(void *opaque) > { > CharDriverState *chr = opaque; > TCPCharDriver *s = chr->opaque; > + IOHandler *write_cb; > + int flags; > + bool nonblock; > + > + flags = fcntl(s->fd, F_GETFL); > + if (flags == -1) { > + flags = 0; > + } > + nonblock = flags & O_NONBLOCK; > + > + write_cb = NULL; > + chr->nonblock = false; > + if (nonblock && chr->chr_write_unblocked) { > + write_cb = chr->chr_write_unblocked; > + chr->nonblock = true; > + } > > + chr->write_blocked = false; > s->connected = 1; > qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, write_cb is a write-only variable, no? ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] Re: [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler 2010-05-04 19:54 ` [Qemu-devel] Re: [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Juan Quintela @ 2010-05-04 19:58 ` Amit Shah 0 siblings, 0 replies; 11+ messages in thread From: Amit Shah @ 2010-05-04 19:58 UTC (permalink / raw) To: Juan Quintela; +Cc: Paul Brook, qemu list, Gerd Hoffmann On (Tue) May 04 2010 [21:54:09], Juan Quintela wrote: > > static void tcp_chr_connect(void *opaque) > > { > > CharDriverState *chr = opaque; > > TCPCharDriver *s = chr->opaque; > > + IOHandler *write_cb; > > + int flags; > > + bool nonblock; > > + > > + flags = fcntl(s->fd, F_GETFL); > > + if (flags == -1) { > > + flags = 0; > > + } > > + nonblock = flags & O_NONBLOCK; > > + > > + write_cb = NULL; > > + chr->nonblock = false; > > + if (nonblock && chr->chr_write_unblocked) { > > + write_cb = chr->chr_write_unblocked; > > + chr->nonblock = true; > > + } > > > > + chr->write_blocked = false; > > s->connected = 1; > > qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, > > write_cb is a write-only variable, no? Leftover from previous design. I'll clean this up in the next series (already applied to my local branch). Thanks, Amit ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] Re: [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control 2010-05-04 18:23 [Qemu-devel] [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 1/6] virtio-console: Factor out common init between console and generic ports Amit Shah @ 2010-05-04 19:23 ` Gerd Hoffmann 1 sibling, 0 replies; 11+ messages in thread From: Gerd Hoffmann @ 2010-05-04 19:23 UTC (permalink / raw) To: Amit Shah; +Cc: Paul Brook, qemu list, Juan Quintela On 05/04/10 20:23, Amit Shah wrote: > Hello, > > This series lets interested callers ask for an -EAGAIN return from the > chardev backends (only unix and tcp sockets as of now) to implement > their own flow control. > > A new call, qemu_chr_write_nb() is added, that will fallback to > qemu_chr_write() if the backend file isn't non-blocking or if no > callback was supplied. > > Support for other backend types is easy to add and will be done in > later patches. I think we've finally sorted out all issues. Acked-by: Gerd Hoffmann <kraxel@redhat.com> cheers, Gerd ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-05-04 20:29 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-05-04 18:23 [Qemu-devel] [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 1/6] virtio-console: Factor out common init between console and generic ports Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 2/6] char: Add a QemuChrHandlers struct to initialise chardev handlers Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 3/6] char: Let writers know how much data was written in case of errors Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 4/6] char: Add qemu_chr_write_nb() for nonblocking writes Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Amit Shah 2010-05-04 18:23 ` [Qemu-devel] [PATCH v6 6/6] virtio-console: Throttle virtio-serial-bus if we can't consume any more guest data Amit Shah 2010-05-04 20:28 ` [Qemu-devel] " Juan Quintela 2010-05-04 19:54 ` [Qemu-devel] Re: [PATCH v6 5/6] char: unix/tcp: Add a non-blocking write handler Juan Quintela 2010-05-04 19:58 ` Amit Shah 2010-05-04 19:23 ` [Qemu-devel] Re: [PATCH v6 0/6] char: non-blocking writes, virtio-console flow control Gerd Hoffmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).