Patchwork chardev-frontends: Explicitly check, inc and dec avail_connections

login
register
mail settings
Submitter Hans de Goede
Date March 27, 2013, 2:10 p.m.
Message ID <1364393440-6054-1-git-send-email-hdegoede@redhat.com>
Download mbox | patch
Permalink /patch/231699/
State New
Headers show

Comments

Hans de Goede - March 27, 2013, 2:10 p.m.
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(-)
Paolo Bonzini - March 27, 2013, 3:11 p.m.
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
Hans de Goede - March 27, 2013, 3:36 p.m.
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
Paolo Bonzini - March 27, 2013, 3:37 p.m.
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

Patch

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;
 }