* [Qemu-devel] [PATCH 1/3] qemu-char: Add qemu_chr_fe_claim / _release helper functions
2013-03-27 19:29 [Qemu-devel] [PATCH 0/3] chardev-frontends: Explicitly check, inc and dec avail_connections v2 Hans de Goede
@ 2013-03-27 19:29 ` Hans de Goede
2013-03-27 19:29 ` [Qemu-devel] [PATCH 2/3] qemu-char: Call fe_claim / fe_release when not using qdev chr properties Hans de Goede
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2013-03-27 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Hans de Goede
Add qemu_chr_fe_claim / _release helper functions for properly dealing with
avail_connections.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
hw/qdev-properties-system.c | 5 ++---
include/char/char.h | 29 +++++++++++++++++++++++++++++
qemu-char.c | 23 +++++++++++++++++++++++
3 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
index 12a87d5..e27d708 100644
--- a/hw/qdev-properties-system.c
+++ b/hw/qdev-properties-system.c
@@ -123,11 +123,10 @@ static int parse_chr(DeviceState *dev, const char *str, void **ptr)
if (chr == NULL) {
return -ENOENT;
}
- if (chr->avail_connections < 1) {
+ if (qemu_chr_fe_claim(chr) != 0) {
return -EEXIST;
}
*ptr = chr;
- --chr->avail_connections;
return 0;
}
@@ -140,7 +139,7 @@ static void release_chr(Object *obj, const char *name, void *opaque)
if (chr) {
qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL);
- ++chr->avail_connections;
+ qemu_chr_fe_release(chr);
}
}
diff --git a/include/char/char.h b/include/char/char.h
index 1457e80..920a5a0 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -188,6 +188,35 @@ int qemu_chr_fe_ioctl(CharDriverState *s, int cmd, void *arg);
int qemu_chr_fe_get_msgfd(CharDriverState *s);
/**
+ * @qemu_chr_fe_claim:
+ *
+ * Claim a backend before using it, should be called before calling
+ * qemu_chr_add_handlers().
+ *
+ * Returns: -1 if the backend is already in use by another frontend, 0 on
+ * success.
+ */
+int qemu_chr_fe_claim(CharDriverState *s);
+
+/**
+ * @qemu_chr_fe_claim_no_fail:
+ *
+ * Like qemu_chr_fe_claim, but will exit qemu with an error when the
+ * backend is already in use.
+ */
+void qemu_chr_fe_claim_no_fail(CharDriverState *s);
+
+/**
+ * @qemu_chr_fe_claim:
+ *
+ * Release a backend for use by another frontend.
+ *
+ * Returns: -1 if the backend is already in use by another frontend, 0 on
+ * success.
+ */
+void qemu_chr_fe_release(CharDriverState *s);
+
+/**
* @qemu_chr_be_can_write:
*
* Determine how much data the front end can currently accept. This function
diff --git a/qemu-char.c b/qemu-char.c
index edf3779..da6cbe2 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3418,6 +3418,29 @@ int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
return tag;
}
+int qemu_chr_fe_claim(CharDriverState *s)
+{
+ if (s->avail_connections < 1) {
+ return -1;
+ }
+ s->avail_connections--;
+ return 0;
+}
+
+void qemu_chr_fe_claim_no_fail(CharDriverState *s)
+{
+ if (qemu_chr_fe_claim(s) != 0) {
+ fprintf(stderr, "%s: error chardev \"%s\" already used\n",
+ __func__, s->label);
+ exit(1);
+ }
+}
+
+void qemu_chr_fe_release(CharDriverState *s)
+{
+ s->avail_connections++;
+}
+
void qemu_chr_delete(CharDriverState *chr)
{
QTAILQ_REMOVE(&chardevs, chr, next);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 2/3] qemu-char: Call fe_claim / fe_release when not using qdev chr properties
2013-03-27 19:29 [Qemu-devel] [PATCH 0/3] chardev-frontends: Explicitly check, inc and dec avail_connections v2 Hans de Goede
2013-03-27 19:29 ` [Qemu-devel] [PATCH 1/3] qemu-char: Add qemu_chr_fe_claim / _release helper functions Hans de Goede
@ 2013-03-27 19:29 ` Hans de Goede
2013-03-27 19:29 ` [Qemu-devel] [PATCH 3/3] ipoctal232: Convert to use chardev properties directly Hans de Goede
2013-04-05 12:51 ` [Qemu-devel] [PATCH 0/3] chardev-frontends: Explicitly check, inc and dec avail_connections v2 Anthony Liguori
3 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2013-03-27 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Hans de Goede
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 | 6 ++++++
gdbstub.c | 1 +
hw/arm/pxa2xx.c | 4 +++-
hw/bt-hci-csr.c | 1 +
hw/ipoctal232.c | 1 +
hw/ivshmem.c | 1 +
hw/mcf_uart.c | 1 +
hw/sh_serial.c | 4 +++-
hw/xen_console.c | 18 ++++++++++++++----
net/slirp.c | 1 +
qemu-char.c | 10 +++++++++-
vl.c | 1 +
12 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 5e012e9..cc6f5ee 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -149,6 +149,11 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
return;
}
+ if (qemu_chr_fe_claim(s->chr) != 0) {
+ error_set(errp, QERR_DEVICE_IN_USE, s->chr_name);
+ return;
+ }
+
/* 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 +196,7 @@ static void rng_egd_finalize(Object *obj)
if (s->chr) {
qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL);
+ qemu_chr_fe_release(s->chr);
}
g_free(s->chr_name);
diff --git a/gdbstub.c b/gdbstub.c
index a666cb5..a0288a7 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3025,6 +3025,7 @@ int gdbserver_start(const char *device)
if (!chr)
return -1;
+ qemu_chr_fe_claim_no_fail(chr);
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..b7ca511 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1981,9 +1981,11 @@ 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) {
+ qemu_chr_fe_claim_no_fail(chr);
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..345efae 100644
--- a/hw/ipoctal232.c
+++ b/hw/ipoctal232.c
@@ -556,6 +556,7 @@ static int ipoctal_init(IPackDevice *ip)
if (ch->dev) {
index++;
+ qemu_chr_fe_claim_no_fail(ch->dev);
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..af2789e 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);
}
+ qemu_chr_fe_claim_no_fail(chr);
/* 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..e5de801 100644
--- a/hw/mcf_uart.c
+++ b/hw/mcf_uart.c
@@ -280,6 +280,7 @@ void *mcf_uart_init(qemu_irq irq, CharDriverState *chr)
s->chr = chr;
s->irq = irq;
if (chr) {
+ qemu_chr_fe_claim_no_fail(chr);
qemu_chr_add_handlers(chr, mcf_uart_can_receive, mcf_uart_receive,
mcf_uart_event, s);
}
diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index 40e797c..4629695 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -396,9 +396,11 @@ void sh_serial_init(MemoryRegion *sysmem,
s->chr = chr;
- if (chr)
+ if (chr) {
+ qemu_chr_fe_claim_no_fail(chr);
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..c56ef47 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -241,9 +241,17 @@ 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 (qemu_chr_fe_claim(con->chr) == 0) {
+ qemu_chr_add_handlers(con->chr, xencons_can_receive,
+ xencons_receive, NULL, con);
+ } 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 +268,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);
+ qemu_chr_fe_release(con->chr);
+ }
xen_be_unbind_evtchn(&con->xendev);
if (con->sring) {
diff --git a/net/slirp.c b/net/slirp.c
index 4df550f..eabfee6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -660,6 +660,7 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str,
fwd->port = port;
fwd->slirp = s->slirp;
+ qemu_chr_fe_claim_no_fail(fwd->hd);
qemu_chr_add_handlers(fwd->hd, guestfwd_can_read, guestfwd_read,
NULL, fwd);
}
diff --git a/qemu-char.c b/qemu-char.c
index da6cbe2..f49750e 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)) {
+ qemu_chr_fe_claim_no_fail(chr);
monitor_init(chr, MONITOR_USE_READLINE);
}
return chr;
@@ -3489,9 +3490,16 @@ 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++];
+ qemu_chr_fe_claim_no_fail(chr);
+ return chr;
+ }
+ return NULL;
}
QemuOptsList qemu_chardev_opts = {
diff --git a/vl.c b/vl.c
index aeed7f4..76cd991 100644
--- a/vl.c
+++ b/vl.c
@@ -2391,6 +2391,7 @@ static int mon_init_func(QemuOpts *opts, void *opaque)
exit(1);
}
+ qemu_chr_fe_claim_no_fail(chr);
monitor_init(chr, flags);
return 0;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 3/3] ipoctal232: Convert to use chardev properties directly
2013-03-27 19:29 [Qemu-devel] [PATCH 0/3] chardev-frontends: Explicitly check, inc and dec avail_connections v2 Hans de Goede
2013-03-27 19:29 ` [Qemu-devel] [PATCH 1/3] qemu-char: Add qemu_chr_fe_claim / _release helper functions Hans de Goede
2013-03-27 19:29 ` [Qemu-devel] [PATCH 2/3] qemu-char: Call fe_claim / fe_release when not using qdev chr properties Hans de Goede
@ 2013-03-27 19:29 ` Hans de Goede
2013-03-27 21:18 ` Anthony Liguori
2013-04-01 21:38 ` Alberto Garcia
2013-04-05 12:51 ` [Qemu-devel] [PATCH 0/3] chardev-frontends: Explicitly check, inc and dec avail_connections v2 Anthony Liguori
3 siblings, 2 replies; 10+ messages in thread
From: Hans de Goede @ 2013-03-27 19:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Alberto Garcia, Hans de Goede
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Cc: Alberto Garcia <agarcia@igalia.com>
---
hw/ipoctal232.c | 43 ++++++++++++++-----------------------------
1 file changed, 14 insertions(+), 29 deletions(-)
diff --git a/hw/ipoctal232.c b/hw/ipoctal232.c
index 345efae..685fee2 100644
--- a/hw/ipoctal232.c
+++ b/hw/ipoctal232.c
@@ -93,7 +93,6 @@ typedef struct SCC2698Block SCC2698Block;
struct SCC2698Channel {
IPOctalState *ipoctal;
CharDriverState *dev;
- char *devpath;
bool rx_enabled;
uint8_t mr[2];
uint8_t mr_idx;
@@ -545,26 +544,12 @@ static int ipoctal_init(IPackDevice *ip)
ch->ipoctal = s;
/* Redirect IP-Octal channels to host character devices */
- if (ch->devpath) {
- const char chr_name[] = "ipoctal";
- char label[ARRAY_SIZE(chr_name) + 2];
- static int index;
-
- snprintf(label, sizeof(label), "%s%d", chr_name, index);
-
- ch->dev = qemu_chr_new(label, ch->devpath, NULL);
-
- if (ch->dev) {
- index++;
- qemu_chr_fe_claim_no_fail(ch->dev);
- qemu_chr_add_handlers(ch->dev, hostdev_can_receive,
- hostdev_receive, hostdev_event, ch);
- DPRINTF("Redirecting channel %u to %s (%s)\n",
- i, ch->devpath, label);
- } else {
- DPRINTF("Could not redirect channel %u to %s\n",
- i, ch->devpath);
- }
+ if (ch->dev) {
+ qemu_chr_add_handlers(ch->dev, hostdev_can_receive,
+ hostdev_receive, hostdev_event, ch);
+ DPRINTF("Redirecting channel %u to %s\n", i, ch->dev->label);
+ } else {
+ DPRINTF("Could not redirect channel %u, no chardev set\n", i);
}
}
@@ -572,14 +557,14 @@ static int ipoctal_init(IPackDevice *ip)
}
static Property ipoctal_properties[] = {
- DEFINE_PROP_STRING("serial0", IPOctalState, ch[0].devpath),
- DEFINE_PROP_STRING("serial1", IPOctalState, ch[1].devpath),
- DEFINE_PROP_STRING("serial2", IPOctalState, ch[2].devpath),
- DEFINE_PROP_STRING("serial3", IPOctalState, ch[3].devpath),
- DEFINE_PROP_STRING("serial4", IPOctalState, ch[4].devpath),
- DEFINE_PROP_STRING("serial5", IPOctalState, ch[5].devpath),
- DEFINE_PROP_STRING("serial6", IPOctalState, ch[6].devpath),
- DEFINE_PROP_STRING("serial7", IPOctalState, ch[7].devpath),
+ DEFINE_PROP_CHR("chardev0", IPOctalState, ch[0].dev),
+ DEFINE_PROP_CHR("chardev1", IPOctalState, ch[1].dev),
+ DEFINE_PROP_CHR("chardev2", IPOctalState, ch[2].dev),
+ DEFINE_PROP_CHR("chardev3", IPOctalState, ch[3].dev),
+ DEFINE_PROP_CHR("chardev4", IPOctalState, ch[4].dev),
+ DEFINE_PROP_CHR("chardev5", IPOctalState, ch[5].dev),
+ DEFINE_PROP_CHR("chardev6", IPOctalState, ch[6].dev),
+ DEFINE_PROP_CHR("chardev7", IPOctalState, ch[7].dev),
DEFINE_PROP_END_OF_LIST(),
};
--
1.8.1.4
^ permalink raw reply related [flat|nested] 10+ messages in thread