| Message ID | 1364393440-6054-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
Il 27/03/2013 15:10, Hans de Goede ha scritto: > chardev-frontends need to explictly check, increase and decrement the > avail_connections "property" of the chardev when they are not using a > qdev-chardev-property for the chardev. > > This fixes things like: > qemu-kvm -chardev stdio,id=foo -device isa-serial,chardev=foo \ > -mon chardev=foo > > Working, where they should fail. Most of the changes here are due to > old hardware emulation code which is using serial_hds directly rather then > a qdev-chardev-property. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > backends/rng-egd.c | 7 +++++++ > gdbstub.c | 1 + > hw/arm/pxa2xx.c | 9 ++++++++- > hw/bt-hci-csr.c | 1 + > hw/ipoctal232.c | 1 + > hw/ivshmem.c | 1 + > hw/mcf_uart.c | 6 ++++++ > hw/serial.c | 16 ++++++++++++++++ > hw/serial.h | 1 + > hw/sh_serial.c | 9 ++++++++- > hw/xen_console.c | 19 +++++++++++++++---- > net/slirp.c | 1 + > qemu-char.c | 14 +++++++++++++- > vl.c | 7 +++++++ > 14 files changed, 86 insertions(+), 7 deletions(-) > > diff --git a/backends/rng-egd.c b/backends/rng-egd.c > index 5e012e9..d8e9d63 100644 > --- a/backends/rng-egd.c > +++ b/backends/rng-egd.c > @@ -149,6 +149,12 @@ static void rng_egd_opened(RngBackend *b, Error **errp) > return; > } > > + if (s->chr->avail_connections < 1) { > + error_set(errp, QERR_DEVICE_IN_USE, s->chr_name); > + return; > + } > + s->chr->avail_connections--; > + > /* FIXME we should resubmit pending requests when the CDS reconnects. */ > qemu_chr_add_handlers(s->chr, rng_egd_chr_can_read, rng_egd_chr_read, > NULL, s); > @@ -191,6 +197,7 @@ static void rng_egd_finalize(Object *obj) > > if (s->chr) { > qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL); > + s->chr->avail_connections++; > } > > g_free(s->chr_name); Ok, but please create wrappers for these (e.g. qemu_chr_be_start/stop and qemu_chr_be_start_nofail) and use them throughout. > diff --git a/gdbstub.c b/gdbstub.c > index a666cb5..83267e0 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -3025,6 +3025,7 @@ int gdbserver_start(const char *device) > if (!chr) > return -1; > > + chr->avail_connections--; > qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive, > gdb_chr_event, NULL); > } Ok. > diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c > index 7467cca..df4b458 100644 > --- a/hw/arm/pxa2xx.c > +++ b/hw/arm/pxa2xx.c > @@ -1981,9 +1981,16 @@ static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem, > memory_region_init_io(&s->iomem, &pxa2xx_fir_ops, s, "pxa2xx-fir", 0x1000); > memory_region_add_subregion(sysmem, base, &s->iomem); > > - if (chr) > + if (chr) { > + if (chr->avail_connections < 1) { > + fprintf(stderr, "pxa2xx_fir_init error chardev %s already used\n", > + chr->label); > + exit(1); > + } > + chr->avail_connections--; > qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty, > pxa2xx_fir_rx, pxa2xx_fir_event, s); > + } > > register_savevm(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save, > pxa2xx_fir_load, s); Errors won't be reported, because serial_hds[] will always create its own CharDriverState and avail_connections will always be 1. Use a wrapper and the code can ignore this. > diff --git a/hw/bt-hci-csr.c b/hw/bt-hci-csr.c > index e4ada3c..55c819b 100644 > --- a/hw/bt-hci-csr.c > +++ b/hw/bt-hci-csr.c > @@ -439,6 +439,7 @@ CharDriverState *uart_hci_init(qemu_irq wakeup) > s->chr.opaque = s; > s->chr.chr_write = csrhci_write; > s->chr.chr_ioctl = csrhci_ioctl; > + s->chr.avail_connections = 1; > > s->hci = qemu_next_hci(); > s->hci->opaque = s; Ok. > diff --git a/hw/ipoctal232.c b/hw/ipoctal232.c > index 1da6a99..f93ad5c 100644 > --- a/hw/ipoctal232.c > +++ b/hw/ipoctal232.c > @@ -556,6 +556,7 @@ static int ipoctal_init(IPackDevice *ip) > > if (ch->dev) { > index++; > + ch->dev->avail_connections--; > qemu_chr_add_handlers(ch->dev, hostdev_can_receive, > hostdev_receive, hostdev_event, ch); > DPRINTF("Redirecting channel %u to %s (%s)\n", Ouch. WTF was I thinking when I reviewed this? :) Please change this to use DEFINE_PROP_CHARDEV. I don't really care if it is backwards-incompatible. > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > index 68a2cf2..82d34b7 100644 > --- a/hw/ivshmem.c > +++ b/hw/ivshmem.c > @@ -292,6 +292,7 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * > fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); > exit(-1); > } > + chr->avail_connections--; > > /* if MSI is supported we need multiple interrupts */ > if (ivshmem_has_feature(s, IVSHMEM_MSI)) { Ok. > diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c > index aacf0f0..079e776 100644 > --- a/hw/mcf_uart.c > +++ b/hw/mcf_uart.c > @@ -280,6 +280,12 @@ void *mcf_uart_init(qemu_irq irq, CharDriverState *chr) > s->chr = chr; > s->irq = irq; > if (chr) { > + if (chr->avail_connections < 1) { > + fprintf(stderr, "mcf_uart_init error chardev %s already used\n", > + chr->label); > + exit(1); > + } > + chr->avail_connections--; > qemu_chr_add_handlers(chr, mcf_uart_can_receive, mcf_uart_receive, > mcf_uart_event, s); > } Ok. > diff --git a/hw/serial.c b/hw/serial.c > index 0ccc499..4e342fd 100644 > --- a/hw/serial.c > +++ b/hw/serial.c > @@ -676,6 +676,15 @@ void serial_init_core(SerialState *s) > fprintf(stderr, "Can't create serial device, empty char device\n"); > exit(1); > } > + if (s->chr_owned_by_serial_core) { > + if (s->chr->avail_connections < 1) { > + fprintf(stderr, > + "Can't create serial device, char device \"%s\" in use\n", > + s->chr->label); > + exit(1); > + } > + s->chr->avail_connections--; > + } > > s->modem_status_poll = qemu_new_timer_ns(vm_clock, (QEMUTimerCB *) serial_update_msl, s); > > @@ -689,6 +698,9 @@ void serial_init_core(SerialState *s) > void serial_exit_core(SerialState *s) > { > qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL); > + if (s->chr_owned_by_serial_core) { > + s->chr->avail_connections++; > + } > qemu_unregister_reset(serial_reset, s); > } > > @@ -719,6 +731,8 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase, > s->irq = irq; > s->baudbase = baudbase; > s->chr = chr; > + /* We always get called with chr an entry of serial_hds */ > + s->chr_owned_by_serial_core = 1; > serial_init_core(s); > > vmstate_register(NULL, base, &vmstate_serial, s); > @@ -776,6 +790,8 @@ SerialState *serial_mm_init(MemoryRegion *address_space, > s->irq = irq; > s->baudbase = baudbase; > s->chr = chr; > + /* We always get called with chr an entry of serial_hds */ > + s->chr_owned_by_serial_core = 1; > > serial_init_core(s); > vmstate_register(NULL, base, &vmstate_serial, s); > diff --git a/hw/serial.h b/hw/serial.h > index e884499..7703881 100644 > --- a/hw/serial.h > +++ b/hw/serial.h > @@ -59,6 +59,7 @@ struct SerialState { > int thr_ipending; > qemu_irq irq; > CharDriverState *chr; > + int chr_owned_by_serial_core; > int last_break_enable; > int it_shift; > int baudbase; Please leave these aside. It is better to QOM-ify SerialState, I'll put it on my list... > diff --git a/hw/sh_serial.c b/hw/sh_serial.c > index 40e797c..fb5e542 100644 > --- a/hw/sh_serial.c > +++ b/hw/sh_serial.c > @@ -396,9 +396,16 @@ void sh_serial_init(MemoryRegion *sysmem, > > s->chr = chr; > > - if (chr) > + if (chr) { > + if (chr->avail_connections < 1) { > + fprintf(stderr, "sh_serial_init error chardev %s already used\n", > + chr->label); > + exit(1); > + } > + chr->avail_connections--; > qemu_chr_add_handlers(chr, sh_serial_can_receive1, sh_serial_receive1, > sh_serial_event, s); > + } > > s->eri = eri_source; > s->rxi = rxi_source; > diff --git a/hw/xen_console.c b/hw/xen_console.c > index a8db6f8..e8e1038 100644 > --- a/hw/xen_console.c > +++ b/hw/xen_console.c > @@ -241,9 +241,18 @@ static int con_initialise(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) { > + if (con->chr->avail_connections >= 1) { > + qemu_chr_add_handlers(con->chr, xencons_can_receive, > + xencons_receive, NULL, con); > + con->chr->avail_connections--; > + } else { > + xen_be_printf(xendev, 0, > + "xen_console_init error chardev %s already used\n", > + con->chr->label); > + con->chr = NULL; > + } > + } > > xen_be_printf(xendev, 1, "ring mfn %d, remote port %d, local port %d, limit %zd\n", > con->ring_ref, > @@ -260,8 +269,10 @@ static void con_disconnect(struct XenDevice *xendev) > if (!xendev->dev) { > return; > } > - if (con->chr) > + if (con->chr) { > qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL); > + con->chr->avail_connections++; > + } > xen_be_unbind_evtchn(&con->xendev); > > if (con->sring) { > diff --git a/net/slirp.c b/net/slirp.c > index 4df550f..76c700b 100644 > --- a/net/slirp.c > +++ b/net/slirp.c > @@ -649,6 +649,7 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str, > g_free(fwd); > return -1; > } > + fwd->hd->avail_connections--; > > if (slirp_add_exec(s->slirp, 3, fwd->hd, &server, port) < 0) { > error_report("conflicting/invalid host:port in guest forwarding " > diff --git a/qemu-char.c b/qemu-char.c > index edf3779..e49f1ac 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -3377,6 +3377,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in > error_free(err); > } > if (chr && qemu_opt_get_bool(opts, "mux", 0)) { > + chr->avail_connections--; > monitor_init(chr, MONITOR_USE_READLINE); > } > return chr; > @@ -3466,9 +3467,20 @@ CharDriverState *qemu_chr_find(const char *name) > CharDriverState *qemu_char_get_next_serial(void) > { > static int next_serial; > + CharDriverState *chr; > > /* FIXME: This function needs to go away: use chardev properties! */ > - return serial_hds[next_serial++]; > + > + while (next_serial < MAX_SERIAL_PORTS && serial_hds[next_serial]) { > + chr = serial_hds[next_serial++]; > + /* Skip already used chardevs */ > + if (chr->avail_connections < 1) { > + continue; > + } > + chr->avail_connections--; > + return chr; > + } > + return NULL; > } > > QemuOptsList qemu_chardev_opts = { > diff --git a/vl.c b/vl.c > index aeed7f4..0f1c967 100644 > --- a/vl.c > +++ b/vl.c > @@ -2391,6 +2391,13 @@ static int mon_init_func(QemuOpts *opts, void *opaque) > exit(1); > } > > + if (chr->avail_connections < 1) { > + fprintf(stderr, "monitor init error chardev \"%s\" already used\n", > + chardev); > + exit(1); > + } > + chr->avail_connections--; > + > monitor_init(chr, flags); > return 0; > } > Ok. Paolo
Hi, On 03/27/2013 04:11 PM, Paolo Bonzini wrote: <snip> >> diff --git a/backends/rng-egd.c b/backends/rng-egd.c >> index 5e012e9..d8e9d63 100644 >> --- a/backends/rng-egd.c >> +++ b/backends/rng-egd.c >> @@ -149,6 +149,12 @@ static void rng_egd_opened(RngBackend *b, Error **errp) >> return; >> } >> >> + if (s->chr->avail_connections < 1) { >> + error_set(errp, QERR_DEVICE_IN_USE, s->chr_name); >> + return; >> + } >> + s->chr->avail_connections--; >> + >> /* FIXME we should resubmit pending requests when the CDS reconnects. */ >> qemu_chr_add_handlers(s->chr, rng_egd_chr_can_read, rng_egd_chr_read, >> NULL, s); >> @@ -191,6 +197,7 @@ static void rng_egd_finalize(Object *obj) >> >> if (s->chr) { >> qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL); >> + s->chr->avail_connections++; >> } >> >> g_free(s->chr_name); > > Ok, but please create wrappers for these (e.g. qemu_chr_be_start/stop > and qemu_chr_be_start_nofail) and use them throughout. That would be fe_start fe_stop, ack otherwise. >> diff --git a/gdbstub.c b/gdbstub.c >> index a666cb5..83267e0 100644 >> --- a/gdbstub.c >> +++ b/gdbstub.c >> @@ -3025,6 +3025,7 @@ int gdbserver_start(const char *device) >> if (!chr) >> return -1; >> >> + chr->avail_connections--; >> qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive, >> gdb_chr_event, NULL); >> } > > Ok. > >> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c >> index 7467cca..df4b458 100644 >> --- a/hw/arm/pxa2xx.c >> +++ b/hw/arm/pxa2xx.c >> @@ -1981,9 +1981,16 @@ static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem, >> memory_region_init_io(&s->iomem, &pxa2xx_fir_ops, s, "pxa2xx-fir", 0x1000); >> memory_region_add_subregion(sysmem, base, &s->iomem); >> >> - if (chr) >> + if (chr) { >> + if (chr->avail_connections < 1) { >> + fprintf(stderr, "pxa2xx_fir_init error chardev %s already used\n", >> + chr->label); >> + exit(1); >> + } >> + chr->avail_connections--; >> qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty, >> pxa2xx_fir_rx, pxa2xx_fir_event, s); >> + } >> >> register_savevm(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save, >> pxa2xx_fir_load, s); > > Errors won't be reported, because serial_hds[] will always create its > own CharDriverState and avail_connections will always be 1. Use a > wrapper and the code can ignore this. Unless some smartass adds, ie: -mon chardev=serial0 to the cmdline, then an error will be reported. I'll respin the patch taking your comments into account. Regards, Hans
Il 27/03/2013 16:36, Hans de Goede ha scritto: > Unless some smartass adds, ie: -mon chardev=serial0 to the cmdline, then an > error will be reported. Right. :) For smartasses we can use qemu_chr_fe_start_nofail. :) Paolo
diff --git a/backends/rng-egd.c b/backends/rng-egd.c index 5e012e9..d8e9d63 100644 --- a/backends/rng-egd.c +++ b/backends/rng-egd.c @@ -149,6 +149,12 @@ static void rng_egd_opened(RngBackend *b, Error **errp) return; } + if (s->chr->avail_connections < 1) { + error_set(errp, QERR_DEVICE_IN_USE, s->chr_name); + return; + } + s->chr->avail_connections--; + /* FIXME we should resubmit pending requests when the CDS reconnects. */ qemu_chr_add_handlers(s->chr, rng_egd_chr_can_read, rng_egd_chr_read, NULL, s); @@ -191,6 +197,7 @@ static void rng_egd_finalize(Object *obj) if (s->chr) { qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL); + s->chr->avail_connections++; } g_free(s->chr_name); diff --git a/gdbstub.c b/gdbstub.c index a666cb5..83267e0 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -3025,6 +3025,7 @@ int gdbserver_start(const char *device) if (!chr) return -1; + chr->avail_connections--; qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive, gdb_chr_event, NULL); } diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index 7467cca..df4b458 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -1981,9 +1981,16 @@ static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem, memory_region_init_io(&s->iomem, &pxa2xx_fir_ops, s, "pxa2xx-fir", 0x1000); memory_region_add_subregion(sysmem, base, &s->iomem); - if (chr) + if (chr) { + if (chr->avail_connections < 1) { + fprintf(stderr, "pxa2xx_fir_init error chardev %s already used\n", + chr->label); + exit(1); + } + chr->avail_connections--; qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty, pxa2xx_fir_rx, pxa2xx_fir_event, s); + } register_savevm(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save, pxa2xx_fir_load, s); diff --git a/hw/bt-hci-csr.c b/hw/bt-hci-csr.c index e4ada3c..55c819b 100644 --- a/hw/bt-hci-csr.c +++ b/hw/bt-hci-csr.c @@ -439,6 +439,7 @@ CharDriverState *uart_hci_init(qemu_irq wakeup) s->chr.opaque = s; s->chr.chr_write = csrhci_write; s->chr.chr_ioctl = csrhci_ioctl; + s->chr.avail_connections = 1; s->hci = qemu_next_hci(); s->hci->opaque = s; diff --git a/hw/ipoctal232.c b/hw/ipoctal232.c index 1da6a99..f93ad5c 100644 --- a/hw/ipoctal232.c +++ b/hw/ipoctal232.c @@ -556,6 +556,7 @@ static int ipoctal_init(IPackDevice *ip) if (ch->dev) { index++; + ch->dev->avail_connections--; qemu_chr_add_handlers(ch->dev, hostdev_can_receive, hostdev_receive, hostdev_event, ch); DPRINTF("Redirecting channel %u to %s (%s)\n", diff --git a/hw/ivshmem.c b/hw/ivshmem.c index 68a2cf2..82d34b7 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -292,6 +292,7 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); exit(-1); } + chr->avail_connections--; /* if MSI is supported we need multiple interrupts */ if (ivshmem_has_feature(s, IVSHMEM_MSI)) { diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c index aacf0f0..079e776 100644 --- a/hw/mcf_uart.c +++ b/hw/mcf_uart.c @@ -280,6 +280,12 @@ void *mcf_uart_init(qemu_irq irq, CharDriverState *chr) s->chr = chr; s->irq = irq; if (chr) { + if (chr->avail_connections < 1) { + fprintf(stderr, "mcf_uart_init error chardev %s already used\n", + chr->label); + exit(1); + } + chr->avail_connections--; qemu_chr_add_handlers(chr, mcf_uart_can_receive, mcf_uart_receive, mcf_uart_event, s); } diff --git a/hw/serial.c b/hw/serial.c index 0ccc499..4e342fd 100644 --- a/hw/serial.c +++ b/hw/serial.c @@ -676,6 +676,15 @@ void serial_init_core(SerialState *s) fprintf(stderr, "Can't create serial device, empty char device\n"); exit(1); } + if (s->chr_owned_by_serial_core) { + if (s->chr->avail_connections < 1) { + fprintf(stderr, + "Can't create serial device, char device \"%s\" in use\n", + s->chr->label); + exit(1); + } + s->chr->avail_connections--; + } s->modem_status_poll = qemu_new_timer_ns(vm_clock, (QEMUTimerCB *) serial_update_msl, s); @@ -689,6 +698,9 @@ void serial_init_core(SerialState *s) void serial_exit_core(SerialState *s) { qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL); + if (s->chr_owned_by_serial_core) { + s->chr->avail_connections++; + } qemu_unregister_reset(serial_reset, s); } @@ -719,6 +731,8 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase, s->irq = irq; s->baudbase = baudbase; s->chr = chr; + /* We always get called with chr an entry of serial_hds */ + s->chr_owned_by_serial_core = 1; serial_init_core(s); vmstate_register(NULL, base, &vmstate_serial, s); @@ -776,6 +790,8 @@ SerialState *serial_mm_init(MemoryRegion *address_space, s->irq = irq; s->baudbase = baudbase; s->chr = chr; + /* We always get called with chr an entry of serial_hds */ + s->chr_owned_by_serial_core = 1; serial_init_core(s); vmstate_register(NULL, base, &vmstate_serial, s); diff --git a/hw/serial.h b/hw/serial.h index e884499..7703881 100644 --- a/hw/serial.h +++ b/hw/serial.h @@ -59,6 +59,7 @@ struct SerialState { int thr_ipending; qemu_irq irq; CharDriverState *chr; + int chr_owned_by_serial_core; int last_break_enable; int it_shift; int baudbase; diff --git a/hw/sh_serial.c b/hw/sh_serial.c index 40e797c..fb5e542 100644 --- a/hw/sh_serial.c +++ b/hw/sh_serial.c @@ -396,9 +396,16 @@ void sh_serial_init(MemoryRegion *sysmem, s->chr = chr; - if (chr) + if (chr) { + if (chr->avail_connections < 1) { + fprintf(stderr, "sh_serial_init error chardev %s already used\n", + chr->label); + exit(1); + } + chr->avail_connections--; qemu_chr_add_handlers(chr, sh_serial_can_receive1, sh_serial_receive1, sh_serial_event, s); + } s->eri = eri_source; s->rxi = rxi_source; diff --git a/hw/xen_console.c b/hw/xen_console.c index a8db6f8..e8e1038 100644 --- a/hw/xen_console.c +++ b/hw/xen_console.c @@ -241,9 +241,18 @@ static int con_initialise(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) { + if (con->chr->avail_connections >= 1) { + qemu_chr_add_handlers(con->chr, xencons_can_receive, + xencons_receive, NULL, con); + con->chr->avail_connections--; + } else { + xen_be_printf(xendev, 0, + "xen_console_init error chardev %s already used\n", + con->chr->label); + con->chr = NULL; + } + } xen_be_printf(xendev, 1, "ring mfn %d, remote port %d, local port %d, limit %zd\n", con->ring_ref, @@ -260,8 +269,10 @@ static void con_disconnect(struct XenDevice *xendev) if (!xendev->dev) { return; } - if (con->chr) + if (con->chr) { qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL); + con->chr->avail_connections++; + } xen_be_unbind_evtchn(&con->xendev); if (con->sring) { diff --git a/net/slirp.c b/net/slirp.c index 4df550f..76c700b 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -649,6 +649,7 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str, g_free(fwd); return -1; } + fwd->hd->avail_connections--; if (slirp_add_exec(s->slirp, 3, fwd->hd, &server, port) < 0) { error_report("conflicting/invalid host:port in guest forwarding " diff --git a/qemu-char.c b/qemu-char.c index edf3779..e49f1ac 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3377,6 +3377,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in error_free(err); } if (chr && qemu_opt_get_bool(opts, "mux", 0)) { + chr->avail_connections--; monitor_init(chr, MONITOR_USE_READLINE); } return chr; @@ -3466,9 +3467,20 @@ CharDriverState *qemu_chr_find(const char *name) CharDriverState *qemu_char_get_next_serial(void) { static int next_serial; + CharDriverState *chr; /* FIXME: This function needs to go away: use chardev properties! */ - return serial_hds[next_serial++]; + + while (next_serial < MAX_SERIAL_PORTS && serial_hds[next_serial]) { + chr = serial_hds[next_serial++]; + /* Skip already used chardevs */ + if (chr->avail_connections < 1) { + continue; + } + chr->avail_connections--; + return chr; + } + return NULL; } QemuOptsList qemu_chardev_opts = { diff --git a/vl.c b/vl.c index aeed7f4..0f1c967 100644 --- a/vl.c +++ b/vl.c @@ -2391,6 +2391,13 @@ static int mon_init_func(QemuOpts *opts, void *opaque) exit(1); } + if (chr->avail_connections < 1) { + fprintf(stderr, "monitor init error chardev \"%s\" already used\n", + chardev); + exit(1); + } + chr->avail_connections--; + monitor_init(chr, flags); return 0; }
chardev-frontends need to explictly check, increase and decrement the avail_connections "property" of the chardev when they are not using a qdev-chardev-property for the chardev. This fixes things like: qemu-kvm -chardev stdio,id=foo -device isa-serial,chardev=foo \ -mon chardev=foo Working, where they should fail. Most of the changes here are due to old hardware emulation code which is using serial_hds directly rather then a qdev-chardev-property. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- backends/rng-egd.c | 7 +++++++ gdbstub.c | 1 + hw/arm/pxa2xx.c | 9 ++++++++- hw/bt-hci-csr.c | 1 + hw/ipoctal232.c | 1 + hw/ivshmem.c | 1 + hw/mcf_uart.c | 6 ++++++ hw/serial.c | 16 ++++++++++++++++ hw/serial.h | 1 + hw/sh_serial.c | 9 ++++++++- hw/xen_console.c | 19 +++++++++++++++---- net/slirp.c | 1 + qemu-char.c | 14 +++++++++++++- vl.c | 7 +++++++ 14 files changed, 86 insertions(+), 7 deletions(-)