From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50699) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UKsLa-00078z-By for qemu-devel@nongnu.org; Wed, 27 Mar 2013 11:33:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UKsLY-00023Z-Ux for qemu-devel@nongnu.org; Wed, 27 Mar 2013 11:33:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64958) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UKsLY-00023S-NX for qemu-devel@nongnu.org; Wed, 27 Mar 2013 11:33:04 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2RFX3Al001745 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 27 Mar 2013 11:33:04 -0400 Message-ID: <515311FE.1050009@redhat.com> Date: Wed, 27 Mar 2013 16:36:30 +0100 From: Hans de Goede MIME-Version: 1.0 References: <1364393440-6054-1-git-send-email-hdegoede@redhat.com> <51530C15.2090901@redhat.com> In-Reply-To: <51530C15.2090901@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] chardev-frontends: Explicitly check, inc and dec avail_connections List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org Hi, On 03/27/2013 04:11 PM, Paolo Bonzini wrote: >> 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