char: Add support for nonblocking writes
Subsystems (3)
v9 cover char: Add support for nonblocking writes
Hello,
This version of the series adds proper handling of nonblocking when the backend managed to flush out some data but not all (ret > 0 && ret < len).
Just the unix/tcp backends have been updated to be nonblocking, but it's easy to add support to the other backends, as the last patch in the series shows.
I'll work on that once this is applied.
The virtio-console code will be the first user of this work, that series will be sent shortly.
This series depends on the virtio-serial fixes and enhancements series that I sent out a short while back.
Please review and apply,
Amit Shah (5):
char: Add a QemuChrHandlers struct to initialise chardev handlers
char: Introduce char_set/remove_fd_handlers()
char: Add framework for a 'write unblocked' callback
char: Update send_all() to handle nonblocking chardev write requests
char: Equip the unix/tcp backend to handle nonblocking writes
gdbstub.c | 9 ++- hw/debugcon.c | 2 +- hw/escc.c | 9 ++- hw/etraxfs_ser.c | 13 +++- hw/ivshmem.c | 28 ++++++-- 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 ++- net/socket.c | 4 +- qemu-char.c | 196 ++++++++++++++++++++++++++++++++++++++++---------- qemu-char.h | 17 ++++- qemu_socket.h | 2 +- 21 files changed, 321 insertions(+), 92 deletions(-)
Signature · 1 line
1.7.3.4
PATCH v9 1/5 char: Add a QemuChrHandlers struct to initialise chardev handlers
Instead of passing each handler in the qemu_add_handlers() function, create a struct of handlers that can be passed to the function instead.
--- gdbstub.c | 9 +++++++-- hw/debugcon.c | 2 +- hw/escc.c | 9 +++++++-- hw/etraxfs_ser.c | 13 +++++++++---- hw/ivshmem.c | 28 ++++++++++++++++++++++------ 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 ++++++++---- 19 files changed, 173 insertions(+), 60 deletions(-)
gdbstub.c +7 -2
@@ -2627,6 +2627,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;
@@ -2656,8 +2662,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;
hw/debugcon.c +1 -1
@@ -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)
hw/escc.c +7 -2
@@ -894,6 +894,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);
@@ -907,8 +913,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 ];
hw/etraxfs_ser.c +9 -4
@@ -190,6 +190,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);
@@ -204,10 +210,9 @@ static int etraxfs_ser_init(SysBusDevice *dev) DEVICE_NATIVE_ENDIAN);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 ; }
hw/ivshmem.c +22 -6
@@ -312,6 +312,18 @@ static void fake_irqfd(void *opaque, const uint8_t *buf, int size) {msix_notify (pdev, entry->vector); } +static QemuChrHandlers ivshmem_msi_handlers = { + .fd_can_read = ivshmem_can_receive, + .fd_read = fake_irqfd, + .fd_event = ivshmem_event, +}; + +static QemuChrHandlers ivshmem_nomsi_handlers = { + .fd_can_read = ivshmem_can_receive, + .fd_read = ivshmem_receive, + .fd_event = ivshmem_event, +}; +static CharDriverState *create_eventfd_chr_device (void * opaque,int eventfd,int vector) {
@@ -331,11 +343,10 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd, s->eventfd_table[vector].pdev = &s->dev; s->eventfd_table[vector].vector = vector; -qemu_chr_add_handlers (chr, ivshmem_can_receive, fake_irqfd, - ivshmem_event, &s->eventfd_table[vector]); +qemu_chr_add_handlers (chr, &ivshmem_msi_handlers, + &s->eventfd_table[vector]); }else { -qemu_chr_add_handlers (chr, ivshmem_can_receive, ivshmem_receive, - ivshmem_event, s); +qemu_chr_add_handlers (chr, &ivshmem_nomsi_handlers, s); }return chr;
@@ -666,6 +677,12 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id)return 0 ; } +static QemuChrHandlers ivshmem_handlers = { + .fd_can_read = ivshmem_can_receive, + .fd_read = ivshmem_read, + .fd_event = ivshmem_event, +}; +static int pci_ivshmem_init (PCIDevice *dev) {IVShmemState *s =DO_UPCAST (IVShmemState, dev, dev);
@@ -754,8 +771,7 @@ static int pci_ivshmem_init(PCIDevice *dev) s->eventfd_chr =qemu_mallocz (s->vectors *sizeof (CharDriverState *)); -qemu_chr_add_handlers (s->server_chr, ivshmem_can_receive, ivshmem_read, - ivshmem_event, s); +qemu_chr_add_handlers (s->server_chr, &ivshmem_handlers, s); }else {/* just map the file immediately, we're not using a server */ int fd;
hw/mcf_uart.c +7 -2
@@ -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;
hw/pl011.c +7 -2
@@ -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;
@@ -304,8 +310,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 (&dev->qdev,"pl011_uart" ,-1 ,1 , pl011_save, pl011_load, s);return 0 ;
hw/pxa2xx.c +9 -4
@@ -1995,6 +1995,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)
@@ -2013,10 +2019,9 @@ static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base, pxa2xx_fir_writefn, s, DEVICE_NATIVE_ENDIAN);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 (NULL ,"pxa2xx_fir" ,0 ,0 , pxa2xx_fir_save, pxa2xx_fir_load, s);
hw/serial.c +7 -2
@@ -727,6 +727,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) {
@@ -741,8 +747,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. */
hw/sh_serial.c +9 -3
@@ -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,
@@ -402,9 +408,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;
hw/syborg_serial.c +7 -2
@@ -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);
@@ -327,8 +333,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" );
hw/usb-serial.c +7 -2
@@ -540,6 +540,12 @@ 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);
@@ -550,8 +556,7 @@ static int usb_serial_initfn(USBDevice *dev)return -1 ; } -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 ; }
hw/virtio-console.c +7 -2
@@ -57,13 +57,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 ;
hw/xen_console.c +11 -5
@@ -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) {
hw/xilinx_uartlite.c +9 -2
@@ -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);
@@ -206,8 +212,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 ; }
monitor.c +15 -4
@@ -5110,6 +5110,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 ;
@@ -5131,12 +5143,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);
net/slirp.c +6 -2
@@ -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:
qemu-char.c +19 -10
@@ -190,15 +190,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);
@@ -436,6 +440,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;
@@ -450,8 +460,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 );
qemu-char.h +8 -4
@@ -72,6 +72,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));
@@ -81,10 +88,7 @@ void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)GCC_FMT_ATTR (2 ,3 );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);
Signature · 1 line
1.7.3.4
[Qemu-devel] Re: [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers
Quoted 2 lines · Amit Shah
Instead of passing each handler in the
qemu_add_handlers()function, create a struct of handlers that can be passed to the function instead.
Nice cleanup.
cheers,
Gerd
Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers
In reply to Amit ShahQuoted 31 lines · Amit Shah
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 <[email protected]>---
gdbstub.c | 9 +++++++--hw/debugcon.c| 2 +-
hw/escc.c | 9 +++++++--hw/etraxfs_ser.c| 13 +++++++++----hw/ivshmem.c| 28 ++++++++++++++++++++++------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 ++++++++---- 19 files changed, 173 insertions(+), 60 deletions(-)gdbstub.c +0
-0
gdbstub.c static void gdb_sigterm_handler(int signal) · line 2634
Quoted 8 lines
gdbstub.c +5
-0@@ -2627,6 +2627,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, +};
These structures should be const.
Quoted 515 lines
gdbstub.c +2
-2+ int gdbserver_start(const char *device) { GDBState *s;@@ -2656,8 +2662,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;hw/debugcon.c +1
-1@@ -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)hw/escc.c +7
-2@@ -894,6 +894,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);@@ -907,8 +913,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];hw/etraxfs_ser.c +9
-4@@ -190,6 +190,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);@@ -204,10 +210,9 @@ static int etraxfs_ser_init(SysBusDevice *dev) DEVICE_NATIVE_ENDIAN); 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; }hw/ivshmem.c +22
-6@@ -312,6 +312,18 @@ static void fake_irqfd(void *opaque, const uint8_t *buf, int size) { msix_notify(pdev, entry->vector); } +static QemuChrHandlers ivshmem_msi_handlers = { + .fd_can_read = ivshmem_can_receive, + .fd_read = fake_irqfd, + .fd_event = ivshmem_event, +}; + +static QemuChrHandlers ivshmem_nomsi_handlers = { + .fd_can_read = ivshmem_can_receive, + .fd_read = ivshmem_receive, + .fd_event = ivshmem_event, +}; + static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd, int vector) {@@ -331,11 +343,10 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd, s->eventfd_table[vector].pdev = &s->dev; s->eventfd_table[vector].vector = vector; - qemu_chr_add_handlers(chr, ivshmem_can_receive, fake_irqfd, - ivshmem_event, &s->eventfd_table[vector]); + qemu_chr_add_handlers(chr, &ivshmem_msi_handlers, + &s->eventfd_table[vector]); } else { - qemu_chr_add_handlers(chr, ivshmem_can_receive, ivshmem_receive, - ivshmem_event, s); + qemu_chr_add_handlers(chr, &ivshmem_nomsi_handlers, s); } return chr;@@ -666,6 +677,12 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) return 0; } +static QemuChrHandlers ivshmem_handlers = { + .fd_can_read = ivshmem_can_receive, + .fd_read = ivshmem_read, + .fd_event = ivshmem_event, +}; + static int pci_ivshmem_init(PCIDevice *dev) { IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);@@ -754,8 +771,7 @@ static int pci_ivshmem_init(PCIDevice *dev) s->eventfd_chr = qemu_mallocz(s->vectors * sizeof(CharDriverState *)); - qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive, ivshmem_read, - ivshmem_event, s); + qemu_chr_add_handlers(s->server_chr, &ivshmem_handlers, s); } else { /* just map the file immediately, we're not using a server */ int fd;hw/mcf_uart.c +7
-2@@ -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;hw/pl011.c +7
-2@@ -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;@@ -304,8 +310,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(&dev->qdev, "pl011_uart", -1, 1, pl011_save, pl011_load, s); return 0;hw/pxa2xx.c +9
-4@@ -1995,6 +1995,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)@@ -2013,10 +2019,9 @@ static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base, pxa2xx_fir_writefn, s, DEVICE_NATIVE_ENDIAN); 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(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save, pxa2xx_fir_load, s);hw/serial.c +7
-2@@ -727,6 +727,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) {@@ -741,8 +747,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. */hw/sh_serial.c +9
-3@@ -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,@@ -402,9 +408,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;hw/syborg_serial.c +7
-2@@ -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);@@ -327,8 +333,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");hw/usb-serial.c +7
-2@@ -540,6 +540,12 @@ 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);@@ -550,8 +556,7 @@ static int usb_serial_initfn(USBDevice *dev) return -1; } - 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; }hw/virtio-console.c +7
-2@@ -57,13 +57,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;hw/xen_console.c +11
-5@@ -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) {hw/xilinx_uartlite.c +9
-2@@ -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);@@ -206,8 +212,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; }monitor.c +15
-4@@ -5110,6 +5110,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;@@ -5131,12 +5143,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);net/slirp.c +6
-2@@ -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:
qemu-char.c +4
-0@@ -190,15 +190,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,
qemu-char.c void qemu_chr_send_event(CharDriverState *s, int event) · line 199
Quoted 10 lines
qemu-char.c +2
-8- 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) +{
Here we could also check if (!s) and return if so. This would simplify the callers a bit.
Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers
Quoted 7 lines · Blue Swirl
gdbstub.c static void gdb_sigterm_handler(int signal) · line 2634
Quoted 5 lines
gdbstub.c +5
-0+static QemuChrHandlers gdb_handlers = { + .fd_can_read = gdb_chr_can_receive, + .fd_read = gdb_chr_receive, + .fd_event = gdb_chr_event, +};These structures should be const.
Hm, I had that but looks like it got lost in some rebase. Added again.
Quoted 14 lines
Quoted 14 lines
qemu-char.c +4
-5@@ -190,15 +190,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) -{
Quoted 8 lines
qemu-char.c void qemu_chr_send_event(CharDriverState *s, int event) · line 199
Quoted 5 lines
qemu-char.c +2
-3- s->chr_can_read = fd_can_read; - s->chr_read = fd_read; - s->chr_event = fd_event; + QemuChrHandlers *handlers, void *opaque) +{Here we could also check if (!s) and return if so. This would simplify the callers a bit.
Simplified in what way?
Amit
Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers
Quoted 34 lines · Amit Shah
Quoted 7 lines · Blue Swirl
gdbstub.c static void gdb_sigterm_handler(int signal) · line 2634
Quoted 5 lines
gdbstub.c +5
-0+static QemuChrHandlers gdb_handlers = { + .fd_can_read = gdb_chr_can_receive, + .fd_read = gdb_chr_receive, + .fd_event = gdb_chr_event, +};These structures should be const.
Hm, I had that but looks like it got lost in some rebase. Added again.
Quoted 14 lines
Quoted 14 lines
qemu-char.c +4
-5@@ -190,15 +190,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) -{Quoted 8 lines
qemu-char.c void qemu_chr_send_event(CharDriverState *s, int event) · line 199
Quoted 5 lines
qemu-char.c +2
-3- s->chr_can_read = fd_can_read; - s->chr_read = fd_read; - s->chr_event = fd_event; + QemuChrHandlers *handlers, void *opaque) +{Here we could also check if (!s) and return if so. This would simplify the callers a bit.
Quoted 1 line
Simplified in what way?
I assume for reducing the need to have to check s->chr != NULL everytime beforehand. It's safer and would save a lot on repetitive code as well.
Quoted 3 lines
Amit
Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers
Quoted 38 lines · Michael Roth
Quoted 37 lines · Amit Shah
Quoted 8 lines · Blue Swirl
gdbstub.c static void gdb_sigterm_handler(int signal) · line 2634
Quoted 6 lines
gdbstub.c +5
-0+static QemuChrHandlers gdb_handlers = { + .fd_can_read = gdb_chr_can_receive, + .fd_read = gdb_chr_receive, + .fd_event = gdb_chr_event, +};These structures should be const.
Hm, I had that but looks like it got lost in some rebase. Added again.
Quoted 23 lines
Quoted 10 lines
qemu-char.c +4
-0@@ -190,15 +190,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,qemu-char.c void qemu_chr_send_event(CharDriverState *s, int · line 200
Quoted 10 lines
qemu-char.c +2
-8- 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) +{Here we could also check if (!s) and return if so. This would simplify the callers a bit.
Quoted 4 lines
Quoted 1 line
Simplified in what way?
I assume for reducing the need to have to check
s->chr!= NULL everytime beforehand. It's safer and would save a lot on repetitive code as well.
Yes, that's what I meant.
Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers
Quoted 19 lines · Blue Swirl
Quoted 19 lines
Quoted 19 lines
Quoted 18 lines
Quoted 9 lines
qemu-char.c +4
-0+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)qemu-char.c void qemu_chr_send_event(CharDriverState *s, int event) · line 199
Quoted 6 lines
qemu-char.c +2
-4-{ - s->chr_can_read = fd_can_read; - s->chr_read = fd_read; - s->chr_event = fd_event; + QemuChrHandlers *handlers, void *opaque) +{Here we could also check if (!s) and return if so. This would simplify the callers a bit.
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Simplified in what way?
I assume for reducing the need to have to check
s->chr!= NULL everytime beforehand. It's safer and would save a lot on repetitive code as well.Yes, that's what I meant.
OK, can be added, but I'm wondering if it makes sense (i.e., if an assert would be better than return).
Amit
Re: [Qemu-devel] [PATCH 1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers
Quoted 22 lines · Amit Shah
Quoted 21 lines · Blue Swirl
Quoted 21 lines
Quoted 20 lines
Quoted 10 lines
Quoted 10 lines
qemu-char.c +4
-0+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)
-{Quoted 8 lines
Quoted 5 lines
- s->chr_can_read =
fd_can_read;
- s->chr_read =fd_read;
- s->chr_event =fd_event;
+ QemuChrHandlers *handlers, void *opaque) +{Here we could also check if (!s) and return if so. This would simplify the callers a bit.
Simplified in what way?
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
I assume for reducing the need to have to check
s->chr!= NULL everytime beforehand. It's safer and would save a lot on repetitive code as well.Yes, that's what I meant.
OK, can be added, but I'm wondering if it makes sense (i.e., if an assert would be better than return).
No, the idea is that the caller does not have to do the current NULL checks anymore and can then possibly pass a NULL CharDriverState. This is of course invalid, but to make things easier for the caller, such calls are just to be ignored.
PATCH v9 2/5 char: Introduce char_set/remove_fd_handlers()
In reply to Amit ShahIntroduce a char-specific wrapper to qemu_set_fd_handler functions. This wrapper is useful to add / remove a write handler easily. Write handlers are only used when the backend is blocked and cannot receive any more input.
--- qemu-char.c | 64 ++++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 42 insertions(+), 22 deletions(-)
qemu-char.c +42 -22
@@ -214,6 +214,24 @@ void qemu_chr_add_handlers(CharDriverState *s, } } +static int char_set_fd_handlers (int fd, +IOCanReadHandler *fd_read_poll, +IOHandler *fd_read,IOHandler *fd_write, +void *opaque,bool install_write_handler) +{ +if (install_write_handler) { +assert (fd_write); + }else { + fd_write =NULL ; + } +return qemu_set_fd_handler2 (fd, fd_read_poll, fd_read, fd_write, opaque); +} + +static int char_remove_fd_handlers (int fd) +{ +return qemu_set_fd_handler2 (fd,NULL ,NULL ,NULL ,NULL ); +} +static int null_chr_write (CharDriverState *chr,const uint8_t *buf,int len) {return len;
@@ -579,7 +597,7 @@ static void fd_chr_read(void *opaque) 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 ); +char_remove_fd_handlers (s->fd_in);qemu_chr_event (chr,CHR_EVENT_CLOSED );return ; }
@@ -595,8 +613,8 @@ static void fd_chr_update_read_handler(CharDriverState *chr)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); +char_set_fd_handlers (s->fd_in, fd_chr_read_poll, fd_chr_read, +NULL , chr, false); } } }
@@ -608,7 +626,7 @@ static void fd_chr_close(struct CharDriverState *chr)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 ); +char_remove_fd_handlers (s->fd_in); } }
@@ -708,7 +726,7 @@ static void stdio_read(void *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 ); +char_remove_fd_handlers (0 );qemu_chr_event (chr,CHR_EVENT_CLOSED );return ; }
@@ -764,7 +782,7 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr) {term_exit (); stdio_nb_clients--; -qemu_set_fd_handler2 (0 ,NULL ,NULL ,NULL ,NULL ); +char_remove_fd_handlers (0 );fd_chr_close (chr); }
@@ -776,7 +794,7 @@ static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts)return NULL ; chr =qemu_chr_open_fd (0 ,1 ); chr->chr_close = qemu_chr_close_stdio; -qemu_set_fd_handler2 (0 , stdio_read_poll, stdio_read,NULL , chr); +char_set_fd_handlers (0 , stdio_read_poll, stdio_read,NULL , chr, false); stdio_nb_clients++;term_init (opts);
@@ -904,8 +922,8 @@ 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); +char_set_fd_handlers (s->fd, pty_chr_read_poll, pty_chr_read,NULL , + chr, false); s->polling =1 ;/* * Short timeout here: just need wait long enougth that qemu makes
@@ -923,7 +941,7 @@ 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 ); +char_remove_fd_handlers (s->fd); s->connected =0 ; s->polling =0 ;/* (re-)connect poll interval for idle guests: once per second.
@@ -959,7 +977,7 @@ static void pty_chr_close(struct CharDriverState *chr) {PtyCharDriver *s = chr->opaque; -qemu_set_fd_handler2 (s->fd,NULL ,NULL ,NULL ,NULL ); +char_remove_fd_handlers (s->fd);close (s->fd);qemu_del_timer (s->timer);qemu_free_timer (s->timer);
@@ -1860,8 +1878,8 @@ 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); +char_set_fd_handlers (s->fd, udp_chr_read_poll, udp_chr_read,NULL , + chr, false); } }
@@ -1869,7 +1887,7 @@ static void udp_chr_close(CharDriverState *chr) {NetCharDriver *s = chr->opaque;if (s->fd >=0 ) { -qemu_set_fd_handler (s->fd,NULL ,NULL ,NULL ); +char_remove_fd_handlers (s->fd);closesocket (s->fd); }qemu_free (s);
@@ -2078,9 +2096,10 @@ static void tcp_chr_read(void *opaque)/* connection closed */ s->connected =0 ;if (s->listen_fd >=0 ) { -qemu_set_fd_handler (s->listen_fd, tcp_chr_accept,NULL , chr); +char_set_fd_handlers (s->listen_fd,NULL , tcp_chr_accept,NULL , + chr, false); } -qemu_set_fd_handler (s->fd,NULL ,NULL ,NULL ); +char_remove_fd_handlers (s->fd);closesocket (s->fd); s->fd =-1 ;qemu_chr_event (chr,CHR_EVENT_CLOSED );
@@ -2105,8 +2124,8 @@ static void tcp_chr_connect(void *opaque)TCPCharDriver *s = chr->opaque; s->connected =1 ; -qemu_set_fd_handler2 (s->fd, tcp_chr_read_poll, - tcp_chr_read,NULL , chr); +char_set_fd_handlers (s->fd, tcp_chr_read_poll, tcp_chr_read,NULL , + chr, false);qemu_chr_generic_open (chr); }
@@ -2167,7 +2186,7 @@ static void tcp_chr_accept(void *opaque)if (s->do_nodelay)socket_set_nodelay (fd); s->fd = fd; -qemu_set_fd_handler (s->listen_fd,NULL ,NULL ,NULL ); +char_remove_fd_handlers (s->listen_fd);tcp_chr_connect (chr); }
@@ -2175,11 +2194,11 @@ static void tcp_chr_close(CharDriverState *chr) {TCPCharDriver *s = chr->opaque;if (s->fd >=0 ) { -qemu_set_fd_handler (s->fd,NULL ,NULL ,NULL ); +char_remove_fd_handlers (s->fd);closesocket (s->fd); }if (s->listen_fd >=0 ) { -qemu_set_fd_handler (s->listen_fd,NULL ,NULL ,NULL ); +char_remove_fd_handlers (s->listen_fd);closesocket (s->listen_fd); }qemu_free (s);
@@ -2241,7 +2260,8 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)if (is_listen) { s->listen_fd = fd; -qemu_set_fd_handler (s->listen_fd, tcp_chr_accept,NULL , chr); +char_set_fd_handlers (s->listen_fd,NULL , tcp_chr_accept,NULL , + chr, false);if (is_telnet) s->do_telnetopt =1 ;
Signature · 1 line
1.7.3.4
[Qemu-devel] Re: [PATCH 2/5] char: Introduce char_set/remove_fd_handlers()
Quoted 4 lines · Amit Shah
Introduce a char-specific wrapper to
qemu_set_fd_handlerfunctions. This wrapper is useful to add / remove a write handler easily. Write handlers are only used when the backend is blocked and cannot receive any more input.
I'd suggest to add flags to enable/disable handlers to IOHandlerRecord instead. And helper functions to set/clear them of course.
With that in place you also can move the handlers to a separate struct simliar to the new QemuChrHandlers struct from patch #1.
cheers,
Gerd
[Qemu-devel] Re: [PATCH 2/5] char: Introduce char_set/remove_fd_handlers()
Quoted 6 lines · Gerd Hoffmann
Quoted 4 lines · Amit Shah
Introduce a char-specific wrapper to
qemu_set_fd_handlerfunctions. This wrapper is useful to add / remove a write handler easily. Write handlers are only used when the backend is blocked and cannot receive any more input.
Quoted 6 lines
I'd suggest to add flags to enable/disable handlers to IOHandlerRecord instead. And helper functions to set/clear them of course.
With that in place you also can move the handlers to a separate struct simliar to the new QemuChrHandlers struct from patch #1.
I'm planning to do that later -- when more backends get involved, which have multiple fds (one for in, one for out).
Are you OK with this for now (to solve the immediate bugs of guests freezing if host can't flush data) and doing this cleanup later as we progress?
Amit
[Qemu-devel] Re: [PATCH 2/5] char: Introduce char_set/remove_fd_handlers()
Quoted 11 lines · Amit Shah
Quoted 10 lines · Gerd Hoffmann
Quoted 4 lines · Amit Shah
Introduce a char-specific wrapper to
qemu_set_fd_handlerfunctions. This wrapper is useful to add / remove a write handler easily. Write handlers are only used when the backend is blocked and cannot receive any more input.I'd suggest to add flags to enable/disable handlers to IOHandlerRecord instead. And helper functions to set/clear them of course.
Quoted 5 lines
Quoted 2 lines
With that in place you also can move the handlers to a separate struct simliar to the new QemuChrHandlers struct from patch #1.
I'm planning to do that later -- when more backends get involved, which have multiple fds (one for in, one for out).
Moving the handlers to a separate struct is clearly a incremental cleanup which can follow later. Using enable/disable flags will probably simplify the interfaces for the non-blocking mode and thus simplify the whole patch series so I think this should be done now.
cheers,
Gerd
[Qemu-devel] Re: [PATCH 2/5] char: Introduce char_set/remove_fd_handlers()
Quoted 15 lines · Gerd Hoffmann
Quoted 14 lines · Amit Shah
Quoted 12 lines · Gerd Hoffmann
Quoted 4 lines · Amit Shah
Introduce a char-specific wrapper to
qemu_set_fd_handlerfunctions. This wrapper is useful to add / remove a write handler easily. Write handlers are only used when the backend is blocked and cannot receive any more input.I'd suggest to add flags to enable/disable handlers to IOHandlerRecord instead. And helper functions to set/clear them of course.
With that in place you also can move the handlers to a separate struct simliar to the new QemuChrHandlers struct from patch #1.
Quoted 7 lines
Quoted 2 lines
I'm planning to do that later -- when more backends get involved, which have multiple fds (one for in, one for out).
Moving the handlers to a separate struct is clearly a incremental cleanup which can follow later. Using enable/disable flags will probably simplify the interfaces for the non-blocking mode and thus simplify the whole patch series so I think this should be done now.
Agree -- but it looks to be a big patch. I have some initial work done, and hence am not converting anything other than unix/tcp backends. The proposed interface here is local to this file, and just a couple of callers. No big deal to change it once this is in.
(The struct for that will look like:
struct fd_handler {
int fd;
IOHandler *read;
IOHandler *write;
IOCanReadHandler *read_poll;
bool read_enabled, write_enabled, read_poll_enabled;
void (*set_read_handler)(IOHandler *read_handler);
void (*set_write_handler)(IOHandler *write_handler);
void (*set_readpoll_handler)(IOCanReadHandler *read_poll_handler);
}This has to be embedded in the CharDriverState for each fd for each backend.
Also: we also want to be able to select() on all fds so that we can detect disconnection events as they happen. So we also need an array somewhere.)
Amit
[Qemu-devel] Re: [PATCH 2/5] char: Introduce char_set/remove_fd_handlers()
Hi,
Quoted 12 lines
Quoted 4 lines
Moving the handlers to a separate struct is clearly a incremental cleanup which can follow later. Using enable/disable flags will probably simplify the interfaces for the non-blocking mode and thus simplify the whole patch series so I think this should be done now.
Agree -- but it looks to be a big patch. I have some initial work done, and hence am not converting anything other than unix/tcp backends. The proposed interface here is local to this file, and just a couple of callers. No big deal to change it once this is in.
(The struct for that will look like:
Quoted 13 lines
struct fd_handler {
int fd;
IOHandler *read;
IOHandler *write;
IOCanReadHandler *read_poll;
bool read_enabled, write_enabled, read_poll_enabled;
void (*set_read_handler)(IOHandler *read_handler);
void (*set_write_handler)(IOHandler *write_handler);
void (*set_readpoll_handler)(IOCanReadHandler *read_poll_handler);
}No, that isn't what I have in mind ...
I'm thinking more about this:
(1) IOHandlerRecord gets variables to enable/disable the handlers (a bunch of bools, a bitmask, whatever). They default to enabled to maintain backward compatibility.
(2) One or more functions are added to enable/disable the handlers,
something like qemu_fd_check_read(int fd, bool enable);
(3) main_loop_wait() will check whenever the handler is actually enabled before adding it to the file handle set for the select() call.
This shouldn't be that big. And with this initial stuff in place you can go forward with the non-blocking stuff. No need to pass around the write handler or have callbacks just to enable/disable the checking for a writable fd, the non-blocking code can just call qemu_fd_check_write(fd, true/false) to do it.
Additional cleanups possible:
(1) switch everybody who registers/unregisters handlers to the new enable/disable interface.
(2) move the function pointers from IOHandlerRecord to a separate struct (say IOHandlerOps).
Quoted 3 lines
Also: we also want to be able to
select()on all fds so that we can detect disconnection events as they happen. So we also need an array somewhere.)
I think you can't do that without switching from select() to poll().
cheers,
Gerd
PATCH v9 3/5 char: Add framework for a 'write unblocked' callback
In reply to Amit ShahThe char layer can let users know that the driver will block on further input. For users interested in not blocking, they can assign a function pointer that will be called back when the driver becomes writable. This patch just adds the function pointers to the CharDriverState structure, future patches will enable the nonblocking and callback functionality.
--- qemu-char.c | 3 +++ qemu-char.h | 5 +++++ 2 files changed, 8 insertions(+), 0 deletions(-)
qemu-char.c +3 -0
@@ -202,11 +202,14 @@ 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); + s->write_blocked = false; +/* We're connecting to an already opened device, so let's make sure we also get the open event */ if (s->opened) {
qemu-char.h +5 -0
@@ -60,6 +60,9 @@ struct CharDriverState {IOEventHandler *chr_event;IOCanReadHandler *chr_can_read;IOReadHandler *chr_read; +IOHandler *chr_write_unblocked; +void (*update_fd_handlers)(struct CharDriverState *chr, +bool poll_out);void *handler_opaque;void (*chr_send_event)(struct CharDriverState *chr,int event);void (*chr_close)(struct CharDriverState *chr);
@@ -69,6 +72,8 @@ struct CharDriverState {char *label;char *filename;int opened; +/* Are we in a blocked state? */ +bool write_blocked;QTAILQ_ENTRY (CharDriverState) next; };
Signature · 1 line
1.7.3.4
[Qemu-devel] Re: [PATCH 3/5] char: Add framework for a 'write unblocked' callback
Hi,
qemu-char.h struct CharDriverState { · line 65
Quoted 7 lines
qemu-char.h +3
-0@@ -60,6 +60,9 @@ struct CharDriverState {IOEventHandler *chr_event;IOCanReadHandler *chr_can_read;IOReadHandler *chr_read; +IOHandler *chr_write_unblocked; +void (*update_fd_handlers)(struct CharDriverState *chr, +bool poll_out);
I think you don't need that callback in the first place if you are able to just enable/disable the handlers for a file handle.
cheers,
Gerd
PATCH v9 4/5 char: Update send_all() to handle nonblocking chardev write requests
In reply to Amit ShahThe send_all function is modified to return to the caller in case the driver cannot handle any more data. It returns -EAGAIN or WSAEWOULDBLOCK on non-Windows and Windows platforms respectively. This is only done when the caller sets a callback function handler indicating it's not interested in blocking till the driver has written out all the data.
Currently there's no driver or caller that supports this. Future commits will add such capability.
--- net/socket.c | 4 +- qemu-char.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- qemu_socket.h | 2 +- 3 files changed, 70 insertions(+), 9 deletions(-)
net/socket.c +2 -2
@@ -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 (NULL , s->fd, (const uint8_t *)&len,sizeof (len)); +return send_all (NULL , s->fd, buf, size); }static ssize_t net_socket_receive_dgram (VLANClientState *nc,const uint8_t *buf,size_t size)
qemu-char.c +67 -6
125 lines not on this page · raw message
qemu_socket.h +1 -1
@@ -36,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 (CharDriverState *chr,int fd,const void *buf,int len1);/* New, ipv6-ready socket helper functions, see qemu-sockets.c */ int inet_listen_opts (QemuOpts *opts,int port_offset);
Signature · 1 line
1.7.3.4
PATCH v9 5/5 char: Equip the unix/tcp backend to handle nonblocking writes
In reply to Amit ShahNow that the infrastructure is in place to return -EAGAIN to callers, individual char drivers can set their update_fd_handlers() function to set or remove an fd's write handler. This handler checks if the driver became writable.
A generic callback routine is used for unblocking writes and letting users of chardevs know that a driver became writable again.
--- qemu-char.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-)
qemu-char.c +27 -0
@@ -235,6 +235,19 @@ static int char_remove_fd_handlers(int fd)return qemu_set_fd_handler2 (fd,NULL ,NULL ,NULL ,NULL ); } +/* +* Generic routine that gets called when chardev becomes writable. +* Lets chardev user know it's OK to send more data. +*/ +static void char_write_unblocked (void *opaque) +{ +CharDriverState *chr = opaque; + + chr->write_blocked = false; + chr->update_fd_handlers (chr, false); + chr->chr_write_unblocked (chr->handler_opaque); +} +static int null_chr_write (CharDriverState *chr,const uint8_t *buf,int len) {return len;
@@ -2269,6 +2282,19 @@ static void tcp_chr_close(CharDriverState *chr)qemu_chr_event (chr,CHR_EVENT_CLOSED ); } +static void tcp_update_fd_handlers (CharDriverState *chr,bool poll_out) +{ +TCPCharDriver *s = chr->opaque; + +/* +* This function is called only after tcp_chr_connect() is called +* (either in 'server' mode or client mode. So we're sure of +* s->fd being initialised. +*/ +char_set_fd_handlers (s->fd, tcp_chr_read_poll, tcp_chr_read, + char_write_unblocked, chr, poll_out); +} +static CharDriverState *qemu_chr_open_socket (QemuOpts *opts) {CharDriverState *chr =NULL ;
@@ -2321,6 +2347,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) chr->chr_write = tcp_chr_write; chr->chr_close = tcp_chr_close; chr->get_msgfd = tcp_get_msgfd; + chr->update_fd_handlers = tcp_update_fd_handlers;if (is_listen) { s->listen_fd = fd;
Signature · 1 line
1.7.3.4
[Qemu-devel] Re: [PATCH 5/5] char: Equip the unix/tcp backend to handle nonblocking writes
qemu-char.c static void tcp_chr_close(CharDriverState *chr) · line 2295
Quoted 2 lines · Amit Shah
qemu-char.c +2
-0+char_set_fd_handlers (s->fd, tcp_chr_read_poll, tcp_chr_read, + char_write_unblocked, chr, poll_out);
Would the 4th parameter always be char_write_unblocked? If so, what about making it hidden within char_set_fd_handlers (also because char_write_unblocked is static in qemu-char.c).
Paolo
[Qemu-devel] Re: [PATCH 5/5] char: Equip the unix/tcp backend to handle nonblocking writes
Quoted 7 lines · Paolo Bonzini
qemu-char.c static void tcp_chr_close(CharDriverState *chr) · line 2295
Quoted 2 lines · Amit Shah
qemu-char.c +2
-0+char_set_fd_handlers (s->fd, tcp_chr_read_poll, tcp_chr_read, + char_write_unblocked, chr, poll_out);Would the 4th parameter always be
char_write_unblocked? If so, what about making it hidden withinchar_set_fd_handlers(also becausechar_write_unblockedis static inqemu-char.c).
It could change for other backends, but the plan is to move to a struct-type declaration even for the fd handlers, like discussed in the other thread with Gerd.
Amit