* [Qemu-devel] [PATCH 0/4 RFC] Introduce console for ringbuf backend
@ 2013-09-02 9:01 Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 1/4] monitor: introduce monitor_read_console Lei Li
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Lei Li @ 2013-09-02 9:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Lei Li, anthony, lcapitulino
This patch series aims at adding HMP console feature for ringbuf
backend. It behaves like a serial console, which drops user into
an interactive mode with ringbuf backend and takes user back to
the monitor by the 'ctrl-]' escape sequence.
It would suspend the monitor, output the data that backed in the
ringbuf backend to console first, and install a new readline
handler to get input from console back into the ringbuf backend.
Once escape sequence is detected, it will resume the monitor.
This implementation is reworked based on Anthony's suggestions
on the previous version:
http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg03888.html
Suggestions and comments are very welcome!
Lei Li (4):
monitor: introduce monitor_read_console
hmp: factor out ringbuf_print_help()
qemu-char: export ringbuf_count
hmp: add console support for ringbuf backend
hmp-commands.hx | 21 +++++++
hmp.c | 141 +++++++++++++++++++++++++++++++++++++++++----
hmp.h | 1 +
include/monitor/monitor.h | 3 +
include/sysemu/char.h | 2 +
monitor.c | 14 +++++
qemu-char.c | 2 +-
7 files changed, 171 insertions(+), 13 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/4] monitor: introduce monitor_read_console
2013-09-02 9:01 [Qemu-devel] [PATCH 0/4 RFC] Introduce console for ringbuf backend Lei Li
@ 2013-09-02 9:01 ` Lei Li
2013-09-09 19:13 ` Luiz Capitulino
2013-09-02 9:01 ` [Qemu-devel] [PATCH 2/4] hmp: factor out ringbuf_print_help() Lei Li
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Lei Li @ 2013-09-02 9:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Lei Li, anthony, lcapitulino
This patch introduces monitor_read_console(), which will drop
into interactive mode with chardev ringbuf backend, and install
a readline handler. When the handler is invoked, the given data
will be written to ringbuf backend.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
include/monitor/monitor.h | 3 +++
monitor.c | 14 ++++++++++++++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1942cc4..e6a6f47 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -89,6 +89,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
void *opaque);
+int monitor_read_console(Monitor *mon, const char *device,
+ ReadLineFunc *readline_func, void *opaque);
+
int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
diff --git a/monitor.c b/monitor.c
index 0aeaf6c..531095e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -263,6 +263,20 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
}
}
+int monitor_read_console(Monitor *mon, const char *device,
+ ReadLineFunc *readline_func, void *opaque)
+{
+ char prompt[60];
+
+ if (!mon->rs) {
+ return -1;
+ }
+
+ snprintf(prompt, sizeof(prompt), "%s: ", device);
+ readline_start(mon->rs, prompt, 0, readline_func, opaque);
+ return 0;
+}
+
static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
void *opaque)
{
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/4] hmp: factor out ringbuf_print_help()
2013-09-02 9:01 [Qemu-devel] [PATCH 0/4 RFC] Introduce console for ringbuf backend Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 1/4] monitor: introduce monitor_read_console Lei Li
@ 2013-09-02 9:01 ` Lei Li
2013-09-09 19:15 ` Luiz Capitulino
2013-09-02 9:01 ` [Qemu-devel] [PATCH 3/4] qemu-char: export ringbuf_count Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 4/4] hmp: add console support for ringbuf backend Lei Li
3 siblings, 1 reply; 8+ messages in thread
From: Lei Li @ 2013-09-02 9:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Lei Li, anthony, lcapitulino
Factor out ringbuf_print_help(), which will be called in
hmp_read_ringbuf_cb() reading data that can be written with
monitor_printf() to the console from ringbuf backend.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
hmp.c | 31 +++++++++++++++++++------------
1 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/hmp.c b/hmp.c
index fcca6ae..624ed6f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -752,6 +752,24 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &errp);
}
+static void ringbuf_print_help(Monitor *mon, const char *data)
+{
+ int i;
+
+ for (i = 0; data[i]; i++) {
+ unsigned char ch = data[i];
+
+ if (ch == '\\') {
+ monitor_printf(mon, "\\\\");
+ } else if ((ch < 0x20 && ch != '\n' && ch != '\t') || ch == 0x7F) {
+ monitor_printf(mon, "\\u%04X", ch);
+ } else {
+ monitor_printf(mon, "%c", ch);
+ }
+
+ }
+}
+
void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
{
const char *chardev = qdict_get_str(qdict, "device");
@@ -769,7 +787,6 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
const char *chardev = qdict_get_str(qdict, "device");
char *data;
Error *errp = NULL;
- int i;
data = qmp_ringbuf_read(chardev, size, false, 0, &errp);
if (errp) {
@@ -778,18 +795,8 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
return;
}
- for (i = 0; data[i]; i++) {
- unsigned char ch = data[i];
-
- if (ch == '\\') {
- monitor_printf(mon, "\\\\");
- } else if ((ch < 0x20 && ch != '\n' && ch != '\t') || ch == 0x7F) {
- monitor_printf(mon, "\\u%04X", ch);
- } else {
- monitor_printf(mon, "%c", ch);
- }
+ ringbuf_print_help(mon, data);
- }
monitor_printf(mon, "\n");
g_free(data);
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 3/4] qemu-char: export ringbuf_count
2013-09-02 9:01 [Qemu-devel] [PATCH 0/4 RFC] Introduce console for ringbuf backend Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 1/4] monitor: introduce monitor_read_console Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 2/4] hmp: factor out ringbuf_print_help() Lei Li
@ 2013-09-02 9:01 ` Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 4/4] hmp: add console support for ringbuf backend Lei Li
3 siblings, 0 replies; 8+ messages in thread
From: Lei Li @ 2013-09-02 9:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Lei Li, anthony, lcapitulino
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
include/sysemu/char.h | 2 ++
qemu-char.c | 2 +-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 8053130..ed20fe3 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -288,6 +288,8 @@ void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
/* add an eventfd to the qemu devices that are polled */
CharDriverState *qemu_chr_open_eventfd(int eventfd);
+size_t ringbuf_count(const CharDriverState *chr);
+
extern int term_escape_char;
CharDriverState *qemu_char_get_next_serial(void);
diff --git a/qemu-char.c b/qemu-char.c
index 6259496..0d74fae 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2781,7 +2781,7 @@ typedef struct {
uint8_t *cbuf;
} RingBufCharDriver;
-static size_t ringbuf_count(const CharDriverState *chr)
+size_t ringbuf_count(const CharDriverState *chr)
{
const RingBufCharDriver *d = chr->opaque;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 4/4] hmp: add console support for ringbuf backend
2013-09-02 9:01 [Qemu-devel] [PATCH 0/4 RFC] Introduce console for ringbuf backend Lei Li
` (2 preceding siblings ...)
2013-09-02 9:01 ` [Qemu-devel] [PATCH 3/4] qemu-char: export ringbuf_count Lei Li
@ 2013-09-02 9:01 ` Lei Li
2013-09-09 20:27 ` Luiz Capitulino
3 siblings, 1 reply; 8+ messages in thread
From: Lei Li @ 2013-09-02 9:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Lei Li, anthony, lcapitulino
This patch add console command which would suspend the monitor,
output the data that backed in the ringbuf backend to console
first, and install a new readline handler to get input back to
the ringbuf backend. Take back to the monitor once escape sequence
'Ctrl-]' is detected.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
hmp-commands.hx | 21 ++++++++++
hmp.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
hmp.h | 1 +
3 files changed, 132 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 65b7f60..286d48d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -876,6 +876,27 @@ stops because the size limit is reached.
ETEXI
{
+ .name = "console",
+ .args_type = "chardev:s",
+ .params = "chardev",
+ .mhandler.cmd = hmp_console,
+ },
+
+STEXI
+@item console @var{device}
+@findex console
+Connect to the serial console from within the monitor, allow to read and
+write data from/to ringbuf device @var{chardev}. Exit from the console and
+return back to monitor by 'ctrl-]'.
+
+@example
+(qemu) console foo
+foo: data string...
+@end example
+
+ETEXI
+
+ {
.name = "migrate",
.args_type = "detach:-d,blk:-b,inc:-i,uri:s",
.params = "[-d] [-b] [-i] uri",
diff --git a/hmp.c b/hmp.c
index 624ed6f..87613cc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1521,3 +1521,113 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+
+typedef struct ConsoleStatus
+{
+ QEMUTimer *timer;
+ Monitor *mon;
+ CharDriverState *chr;
+} ConsoleStatus;
+
+enum escape_char
+{
+ ESCAPE_CHAR_CTRL_GS = 0x1d /* ctrl-] is used for escape */
+};
+
+static void hmp_read_ringbuf_cb(void *opaque)
+{
+ ConsoleStatus *status = opaque;
+ char *data;
+ int len;
+ Error *err = NULL;
+
+ len = ringbuf_count(status->chr);
+ if (len) {
+ data = qmp_ringbuf_read(status->chr->label, len, 0, 0, &err);
+ if (err) {
+ monitor_printf(status->mon, "%s\n", error_get_pretty(err));
+ error_free(err);
+ return;
+ }
+ ringbuf_print_help(status->mon, data);
+ monitor_flush(status->mon);
+ g_free(data);
+ timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 1000);
+ } else {
+ timer_del(status->timer);
+ }
+
+ monitor_resume(status->mon);
+ g_free(status);
+}
+
+static void hmp_write_console(Monitor *mon, void *opaque)
+{
+ CharDriverState *chr = opaque;
+ ConsoleStatus *status;
+
+ status = g_malloc0(sizeof(*status));
+ status->mon = mon;
+ status->chr = chr;
+ status->timer = timer_new_ms(QEMU_CLOCK_REALTIME, hmp_read_ringbuf_cb,
+ status);
+ timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
+}
+
+static void hmp_read_console(Monitor *mon, const char *data,
+ void *opaque)
+{
+ CharDriverState *chr = opaque;
+ enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
+ int len = strlen(data) + 1;
+ char *tmp_buf = g_malloc0(len);
+ int i;
+ Error *err = NULL;
+
+ for (i = 0; data[i]; i++) {
+ if (data[i] == console_escape) {
+ monitor_read_command(mon, 1);
+ goto out;
+ }
+ tmp_buf[i] = data[i];
+ }
+
+ qmp_ringbuf_write(chr->label, tmp_buf, 0, 0, &err);
+ if (err) {
+ monitor_printf(mon, "%s\n", error_get_pretty(err));
+ monitor_read_command(mon, 1);
+ error_free(err);
+ goto out;
+ }
+
+out:
+ g_free(tmp_buf);
+ return;
+}
+
+void hmp_console(Monitor *mon, const QDict *qdict)
+{
+ const char *device = qdict_get_str(qdict, "chardev");
+ CharDriverState *chr;
+ Error *err = NULL;
+
+ chr = qemu_chr_find(device);
+ if (!chr) {
+ error_set(&err, QERR_DEVICE_NOT_FOUND, device);
+ goto out;
+ }
+
+ if (monitor_suspend(mon)) {
+ monitor_printf(mon, "Failed to suspend console\n");
+ return;
+ }
+
+ hmp_write_console(mon, chr);
+
+ if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
+ monitor_printf(mon, "Connect to console %s failed\n", device);
+ }
+
+out:
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 6c3bdcd..d7fb52d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -87,5 +87,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_console(Monitor *mon, const QDict *qdict);
#endif
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] monitor: introduce monitor_read_console
2013-09-02 9:01 ` [Qemu-devel] [PATCH 1/4] monitor: introduce monitor_read_console Lei Li
@ 2013-09-09 19:13 ` Luiz Capitulino
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Capitulino @ 2013-09-09 19:13 UTC (permalink / raw)
To: Lei Li; +Cc: qemu-devel, anthony
On Mon, 2 Sep 2013 17:01:45 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:
> This patch introduces monitor_read_console(), which will drop
> into interactive mode with chardev ringbuf backend, and install
> a readline handler. When the handler is invoked, the given data
> will be written to ringbuf backend.
>
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
> include/monitor/monitor.h | 3 +++
> monitor.c | 14 ++++++++++++++
> 2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 1942cc4..e6a6f47 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -89,6 +89,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
> int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> void *opaque);
>
> +int monitor_read_console(Monitor *mon, const char *device,
> + ReadLineFunc *readline_func, void *opaque);
> +
> int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
>
> int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
> diff --git a/monitor.c b/monitor.c
> index 0aeaf6c..531095e 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -263,6 +263,20 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> }
> }
>
> +int monitor_read_console(Monitor *mon, const char *device,
> + ReadLineFunc *readline_func, void *opaque)
> +{
> + char prompt[60];
> +
> + if (!mon->rs) {
> + return -1;
> + }
> +
> + snprintf(prompt, sizeof(prompt), "%s: ", device);
You can use g_strdup_printf() to avoid a fixed size buffer.
> + readline_start(mon->rs, prompt, 0, readline_func, opaque);
> + return 0;
> +}
> +
> static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
> void *opaque)
> {
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] hmp: factor out ringbuf_print_help()
2013-09-02 9:01 ` [Qemu-devel] [PATCH 2/4] hmp: factor out ringbuf_print_help() Lei Li
@ 2013-09-09 19:15 ` Luiz Capitulino
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Capitulino @ 2013-09-09 19:15 UTC (permalink / raw)
To: Lei Li; +Cc: qemu-devel, anthony
On Mon, 2 Sep 2013 17:01:46 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:
> Factor out ringbuf_print_help(), which will be called in
> hmp_read_ringbuf_cb() reading data that can be written with
> monitor_printf() to the console from ringbuf backend.
>
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
> hmp.c | 31 +++++++++++++++++++------------
> 1 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index fcca6ae..624ed6f 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -752,6 +752,24 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, &errp);
> }
>
> +static void ringbuf_print_help(Monitor *mon, const char *data)
That's a very bad name. You could call it ringbuf_print_contents(),
but then I think you should pass a size parameter. Or, you call it
ringbuf_print_char() and keep the loop in the caller.
> +{
> + int i;
> +
> + for (i = 0; data[i]; i++) {
> + unsigned char ch = data[i];
> +
> + if (ch == '\\') {
> + monitor_printf(mon, "\\\\");
> + } else if ((ch < 0x20 && ch != '\n' && ch != '\t') || ch == 0x7F) {
> + monitor_printf(mon, "\\u%04X", ch);
> + } else {
> + monitor_printf(mon, "%c", ch);
> + }
> +
> + }
> +}
> +
> void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
> {
> const char *chardev = qdict_get_str(qdict, "device");
> @@ -769,7 +787,6 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
> const char *chardev = qdict_get_str(qdict, "device");
> char *data;
> Error *errp = NULL;
> - int i;
>
> data = qmp_ringbuf_read(chardev, size, false, 0, &errp);
> if (errp) {
> @@ -778,18 +795,8 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
> return;
> }
>
> - for (i = 0; data[i]; i++) {
> - unsigned char ch = data[i];
> -
> - if (ch == '\\') {
> - monitor_printf(mon, "\\\\");
> - } else if ((ch < 0x20 && ch != '\n' && ch != '\t') || ch == 0x7F) {
> - monitor_printf(mon, "\\u%04X", ch);
> - } else {
> - monitor_printf(mon, "%c", ch);
> - }
> + ringbuf_print_help(mon, data);
>
> - }
> monitor_printf(mon, "\n");
> g_free(data);
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] hmp: add console support for ringbuf backend
2013-09-02 9:01 ` [Qemu-devel] [PATCH 4/4] hmp: add console support for ringbuf backend Lei Li
@ 2013-09-09 20:27 ` Luiz Capitulino
0 siblings, 0 replies; 8+ messages in thread
From: Luiz Capitulino @ 2013-09-09 20:27 UTC (permalink / raw)
To: Lei Li; +Cc: qemu-devel, anthony, Markus
On Mon, 2 Sep 2013 17:01:48 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:
> This patch add console command which would suspend the monitor,
> output the data that backed in the ringbuf backend to console
> first, and install a new readline handler to get input back to
> the ringbuf backend. Take back to the monitor once escape sequence
> 'Ctrl-]' is detected.
This command doesn't actually work (see review below), but besides
that I honestly don't fully understand its purpose.
What it seems to _try_ doing: it periodically reads from the ringbuf
buffer and print its contents. It also allow you to write to the
ringbuf (once?).
I can understand the usefulness of a console command like libvirt's,
which dumps a guest's serial output to you, but:
1. How the reading part differs from ringbuf_read? Is it the
timer? If it is, why not having a timer argument to ringbuf_read?
2. How is the writing part supposed to be used? Should the user
be able to operate a serial console for example? You sure this
works?
3. Did I get it wrong or you're able to write to the console
only once?
More detailed review below, but I don't think we should move forward
before answering these questions.
>
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
> hmp-commands.hx | 21 ++++++++++
> hmp.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hmp.h | 1 +
> 3 files changed, 132 insertions(+), 0 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 65b7f60..286d48d 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -876,6 +876,27 @@ stops because the size limit is reached.
> ETEXI
>
> {
> + .name = "console",
> + .args_type = "chardev:s",
> + .params = "chardev",
> + .mhandler.cmd = hmp_console,
> + },
> +
> +STEXI
> +@item console @var{device}
> +@findex console
> +Connect to the serial console from within the monitor, allow to read and
> +write data from/to ringbuf device @var{chardev}. Exit from the console and
> +return back to monitor by 'ctrl-]'.
> +
> +@example
> +(qemu) console foo
> +foo: data string...
> +@end example
That's a bad example. I think it's worth it to have a qemu command-line
example on how to use ringbuf+serial, and a more realistic example on
the command usage.
> +
> +ETEXI
> +
> + {
> .name = "migrate",
> .args_type = "detach:-d,blk:-b,inc:-i,uri:s",
> .params = "[-d] [-b] [-i] uri",
> diff --git a/hmp.c b/hmp.c
> index 624ed6f..87613cc 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1521,3 +1521,113 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
>
> hmp_handle_error(mon, &err);
> }
> +
> +typedef struct ConsoleStatus
> +{
> + QEMUTimer *timer;
> + Monitor *mon;
> + CharDriverState *chr;
> +} ConsoleStatus;
> +
> +enum escape_char
> +{
> + ESCAPE_CHAR_CTRL_GS = 0x1d /* ctrl-] is used for escape */
> +};
Why is this an enum?
> +
> +static void hmp_read_ringbuf_cb(void *opaque)
> +{
> + ConsoleStatus *status = opaque;
> + char *data;
> + int len;
> + Error *err = NULL;
> +
> + len = ringbuf_count(status->chr);
We're trying hard to not use non-QMP functions in HMP. If you need
this here, then you probably need this as a QMP command.
> + if (len) {
> + data = qmp_ringbuf_read(status->chr->label, len, 0, 0, &err);
> + if (err) {
> + monitor_printf(status->mon, "%s\n", error_get_pretty(err));
> + error_free(err);
> + return;
Leak status on failure?
> + }
> + ringbuf_print_help(status->mon, data);
> + monitor_flush(status->mon);
> + g_free(data);
> + timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + 1000);
> + } else {
> + timer_del(status->timer);
If there's no data the command returns? So you try only once. Is this
the intended behavior?
> + }
> +
> + monitor_resume(status->mon);
> + g_free(status);
If data was printed, the timer will run again 1s later, but you just
freed status and resumed monitor operation... Makes me think you didn't
even try this command for real? :(
> +}
> +
> +static void hmp_write_console(Monitor *mon, void *opaque)
> +{
> + CharDriverState *chr = opaque;
> + ConsoleStatus *status;
> +
> + status = g_malloc0(sizeof(*status));
> + status->mon = mon;
> + status->chr = chr;
> + status->timer = timer_new_ms(QEMU_CLOCK_REALTIME, hmp_read_ringbuf_cb,
> + status);
> + timer_mod(status->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME));
So, the function is called 'write console', but it actually reads from
the ringbuf?
> +}
> +
> +static void hmp_read_console(Monitor *mon, const char *data,
> + void *opaque)
> +{
> + CharDriverState *chr = opaque;
> + enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
> + int len = strlen(data) + 1;
> + char *tmp_buf = g_malloc0(len);
> + int i;
> + Error *err = NULL;
> +
> + for (i = 0; data[i]; i++) {
> + if (data[i] == console_escape) {
> + monitor_read_command(mon, 1);
> + goto out;
> + }
> + tmp_buf[i] = data[i];
> + }
> +
> + qmp_ringbuf_write(chr->label, tmp_buf, 0, 0, &err);
> + if (err) {
> + monitor_printf(mon, "%s\n", error_get_pretty(err));
> + monitor_read_command(mon, 1);
> + error_free(err);
> + goto out;
> + }
> +
> +out:
> + g_free(tmp_buf);
> + return;
> +}
> +
> +void hmp_console(Monitor *mon, const QDict *qdict)
> +{
> + const char *device = qdict_get_str(qdict, "chardev");
> + CharDriverState *chr;
> + Error *err = NULL;
> +
> + chr = qemu_chr_find(device);
> + if (!chr) {
> + error_set(&err, QERR_DEVICE_NOT_FOUND, device);
> + goto out;
> + }
You shouldn't need to do this. The qmp ringbuf commands will fail
if 'device' doesn't exist.
> +
> + if (monitor_suspend(mon)) {
> + monitor_printf(mon, "Failed to suspend console\n");
> + return;
> + }
> +
> + hmp_write_console(mon, chr);
> +
> + if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
> + monitor_printf(mon, "Connect to console %s failed\n", device);
> + }
> +
> +out:
> + hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 6c3bdcd..d7fb52d 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -87,5 +87,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
> void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
> void hmp_qemu_io(Monitor *mon, const QDict *qdict);
> +void hmp_console(Monitor *mon, const QDict *qdict);
>
> #endif
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-09-09 20:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-02 9:01 [Qemu-devel] [PATCH 0/4 RFC] Introduce console for ringbuf backend Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 1/4] monitor: introduce monitor_read_console Lei Li
2013-09-09 19:13 ` Luiz Capitulino
2013-09-02 9:01 ` [Qemu-devel] [PATCH 2/4] hmp: factor out ringbuf_print_help() Lei Li
2013-09-09 19:15 ` Luiz Capitulino
2013-09-02 9:01 ` [Qemu-devel] [PATCH 3/4] qemu-char: export ringbuf_count Lei Li
2013-09-02 9:01 ` [Qemu-devel] [PATCH 4/4] hmp: add console support for ringbuf backend Lei Li
2013-09-09 20:27 ` Luiz Capitulino
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).