|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] RFC: chardev: mark the calls that allow an implicit mux monitor
This is mostly for readability of the code. Let's make it clear which
callers can create an implicit monitor when the chardev is muxed.
This will also enforce a safer behaviour, as we don't really support
creating monitor anywhere/anytime at the moment.
There are documented cases, such as: -serial/-parallel/-virtioconsole
and to less extent -debugcon.
Less obvious and questionnable ones are -gdb and Xen console. Add a
FIXME note for those, but keep the support for now.
Other qemu_chr_new() callers either have a fixed parameter/filename
string, or do preliminary checks on the string (such as slirp), or do
not need it, such as -qtest.
On a related note, the list of monitor creation places:
- the chardev creators listed above: all from command line (except
perhaps Xen console?)
- -gdb & hmp gdbserver will create a "GDB monitor command" chardev
that is wired to an HMP monitor.
- -mon command line option
From this short study, I would like to think that a monitor may only
be created in the main thread today, though I remain skeptical :)
Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
---
include/chardev/char.h | 18 +++++++++++++++++-
chardev/char.c | 21 +++++++++++++++++----
gdbstub.c | 6 +++++-
hw/char/xen_console.c | 5 ++++-
vl.c | 8 ++++----
5 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 6f0576e214..be2b9b817e 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -105,6 +105,7 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
* @qemu_chr_new:
*
* Create a new character backend from a URI.
+ * Do not implicitely initialize a monitor if the chardev is muxed.
*
* @label the name of the backend
* @filename the URI
@@ -113,6 +114,19 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
*/
Chardev *qemu_chr_new(const char *label, const char *filename);
+/**
+ * @qemu_chr_new_mux_mon:
+ *
+ * Create a new character backend from a URI.
+ * Implicitely initialize a monitor if the chardev is muxed.
+ *
+ * @label the name of the backend
+ * @filename the URI
+ *
+ * Returns: a new character backend
+ */
+Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename);
+
/**
* @qemu_chr_change:
*
@@ -138,10 +152,12 @@ void qemu_chr_cleanup(void);
*
* @label the name of the backend
* @filename the URI
+ * @with_mux_mon if chardev is muxed, initialize a monitor
*
* Returns: a new character backend
*/
-Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
+Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
+ bool with_mux_mon);
/**
* @qemu_chr_be_can_write:
diff --git a/chardev/char.c b/chardev/char.c
index 76d866e6fe..2c295ad676 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -683,7 +683,8 @@ out:
return chr;
}
-Chardev *qemu_chr_new_noreplay(const char *label, const char *filename)
+Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
+ bool with_mux_mon)
{
const char *p;
Chardev *chr;
@@ -702,17 +703,19 @@ Chardev *qemu_chr_new_noreplay(const char *label, const
char *filename)
if (err) {
error_report_err(err);
}
- if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
+ if (with_mux_mon && chr && qemu_opt_get_bool(opts, "mux", 0)) {
monitor_init(chr, MONITOR_USE_READLINE);
}
qemu_opts_del(opts);
return chr;
}
-Chardev *qemu_chr_new(const char *label, const char *filename)
+static Chardev *qemu_chr_new_with_mux_mon(const char *label,
+ const char *filename,
+ bool with_mux_mon)
{
Chardev *chr;
- chr = qemu_chr_new_noreplay(label, filename);
+ chr = qemu_chr_new_noreplay(label, filename, with_mux_mon);
if (chr) {
if (replay_mode != REPLAY_MODE_NONE) {
qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_REPLAY);
@@ -726,6 +729,16 @@ Chardev *qemu_chr_new(const char *label, const char
*filename)
return chr;
}
+Chardev *qemu_chr_new(const char *label, const char *filename)
+{
+ return qemu_chr_new_with_mux_mon(label, filename, false);
+}
+
+Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename)
+{
+ return qemu_chr_new_with_mux_mon(label, filename, true);
+}
+
static int qmp_query_chardev_foreach(Object *obj, void *data)
{
Chardev *chr = CHARDEV(obj);
diff --git a/gdbstub.c b/gdbstub.c
index d6ab95006c..963531ea90 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2038,7 +2038,11 @@ int gdbserver_start(const char *device)
sigaction(SIGINT, &act, NULL);
}
#endif
- chr = qemu_chr_new_noreplay("gdb", device);
+ /*
+ * FIXME: it's a bit weird to allow using a mux chardev here
+ * and setup implicitely a monitor. We may want to break this.
+ */
+ chr = qemu_chr_new_noreplay("gdb", device, true);
if (!chr)
return -1;
}
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 8b4b4bf523..6a231d487b 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -207,7 +207,10 @@ static int con_init(struct XenDevice *xendev)
} else {
snprintf(label, sizeof(label), "xencons%d", con->xendev.dev);
qemu_chr_fe_init(&con->chr,
- qemu_chr_new(label, output), &error_abort);
+ /*
+ * FIXME: should it support implicit muxed monitors?
+ */
+ qemu_chr_new_mux_mon(label, output), &error_abort);
}
xenstore_store_pv_console_info(con->xendev.dev,
diff --git a/vl.c b/vl.c
index 16b913f9d5..20b5f321ec 100644
--- a/vl.c
+++ b/vl.c
@@ -2425,7 +2425,7 @@ static int serial_parse(const char *devname)
snprintf(label, sizeof(label), "serial%d", index);
serial_hds = g_renew(Chardev *, serial_hds, index + 1);
- serial_hds[index] = qemu_chr_new(label, devname);
+ serial_hds[index] = qemu_chr_new_mux_mon(label, devname);
if (!serial_hds[index]) {
error_report("could not connect serial device"
" to character backend '%s'", devname);
@@ -2461,7 +2461,7 @@ static int parallel_parse(const char *devname)
exit(1);
}
snprintf(label, sizeof(label), "parallel%d", index);
- parallel_hds[index] = qemu_chr_new(label, devname);
+ parallel_hds[index] = qemu_chr_new_mux_mon(label, devname);
if (!parallel_hds[index]) {
error_report("could not connect parallel device"
" to character backend '%s'", devname);
@@ -2492,7 +2492,7 @@ static int virtcon_parse(const char *devname)
qemu_opt_set(dev_opts, "driver", "virtconsole", &error_abort);
snprintf(label, sizeof(label), "virtcon%d", index);
- virtcon_hds[index] = qemu_chr_new(label, devname);
+ virtcon_hds[index] = qemu_chr_new_mux_mon(label, devname);
if (!virtcon_hds[index]) {
error_report("could not connect virtio console"
" to character backend '%s'", devname);
@@ -2508,7 +2508,7 @@ static int debugcon_parse(const char *devname)
{
QemuOpts *opts;
- if (!qemu_chr_new("debugcon", devname)) {
+ if (!qemu_chr_new_mux_mon("debugcon", devname)) {
exit(1);
}
opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1, NULL);
--
2.18.0.547.g1d89318c48
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |