chardev/char.c | 12 ++++++++++++ hmp-commands.hx | 16 ++++++++++++++++ hmp.c | 8 ++++++++ hmp.h | 1 + qapi-schema.json | 20 ++++++++++++++++++++ 5 files changed, 57 insertions(+)
Sending a break on a serial console can be useful for debugging the
guest. But not all chardev backends support sending breaks (only telnet
and mux do). The chardev-send-break command allows to send a break even
if using other backends.
Signed-off-by: Stefan Fritsch <sf@sfritsch.de>
---
chardev/char.c | 12 ++++++++++++
hmp-commands.hx | 16 ++++++++++++++++
hmp.c | 8 ++++++++
hmp.h | 1 +
qapi-schema.json | 20 ++++++++++++++++++++
5 files changed, 57 insertions(+)
diff --git a/chardev/char.c b/chardev/char.c
index 4e24dc39af..fa54f7c915 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1307,6 +1307,18 @@ void qmp_chardev_remove(const char *id, Error **errp)
object_unparent(OBJECT(chr));
}
+void qmp_chardev_send_break(const char *id, Error **errp)
+{
+ Chardev *chr;
+
+ chr = qemu_chr_find(id);
+ if (chr == NULL) {
+ error_setg(errp, "Chardev '%s' not found", id);
+ return;
+ }
+ qemu_chr_be_event(chr, CHR_EVENT_BREAK);
+}
+
void qemu_chr_cleanup(void)
{
object_unparent(get_chardevs_root());
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e763606fe5..fc8d54b52a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1745,6 +1745,22 @@ Removes the chardev @var{id}.
ETEXI
{
+ .name = "chardev-send-break",
+ .args_type = "id:s",
+ .params = "id",
+ .help = "send break on chardev",
+ .cmd = hmp_chardev_send_break,
+ .command_completion = chardev_remove_completion,
+ },
+
+STEXI
+@item chardev-send-break id
+@findex chardev-send-break
+Sends break on the chardev @var{id}.
+
+ETEXI
+
+ {
.name = "qemu-io",
.args_type = "device:B,command:s",
.params = "[device] \"[command]\"",
diff --git a/hmp.c b/hmp.c
index ad723903a6..fb2a38b7d6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2233,6 +2233,14 @@ void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &local_err);
}
+void hmp_chardev_send_break(Monitor *mon, const QDict *qdict)
+{
+ Error *local_err = NULL;
+
+ qmp_chardev_send_break(qdict_get_str(qdict, "id"), &local_err);
+ hmp_handle_error(mon, &local_err);
+}
+
void hmp_qemu_io(Monitor *mon, const QDict *qdict)
{
BlockBackend *blk;
diff --git a/hmp.h b/hmp.h
index d8b94ce9dc..214b2617e7 100644
--- a/hmp.h
+++ b/hmp.h
@@ -103,6 +103,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
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_chardev_send_break(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
void hmp_cpu_add(Monitor *mon, const QDict *qdict);
void hmp_object_add(Monitor *mon, const QDict *qdict);
diff --git a/qapi-schema.json b/qapi-schema.json
index 4b50b652d3..e01dd83dd9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -5114,6 +5114,26 @@
{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
##
+# @chardev-send-break:
+#
+# Send a break to a character device
+#
+# @id: the chardev's ID, must exist
+#
+# Returns: Nothing on success
+#
+# Since: 2.10
+#
+# Example:
+#
+# -> { "execute": "chardev-send-break", "arguments": { "id" : "foo" } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'chardev-send-break', 'data': {'id': 'str'} }
+
+
+##
# @TpmModel:
#
# An enumeration of TPM models
--
2.11.0
On 06/05/2017 03:52 AM, Stefan Fritsch wrote: > Sending a break on a serial console can be useful for debugging the > guest. But not all chardev backends support sending breaks (only telnet > and mux do). The chardev-send-break command allows to send a break even > if using other backends. > > Signed-off-by: Stefan Fritsch <sf@sfritsch.de> > --- > chardev/char.c | 12 ++++++++++++ > hmp-commands.hx | 16 ++++++++++++++++ > hmp.c | 8 ++++++++ > hmp.h | 1 + > qapi-schema.json | 20 ++++++++++++++++++++ > 5 files changed, 57 insertions(+) Is there an obvious test that we can enhance to add coverage of the new QMP command? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 05/06/2017 14:24, Eric Blake wrote: > On 06/05/2017 03:52 AM, Stefan Fritsch wrote: >> Sending a break on a serial console can be useful for debugging the >> guest. But not all chardev backends support sending breaks (only telnet >> and mux do). The chardev-send-break command allows to send a break even >> if using other backends. >> >> Signed-off-by: Stefan Fritsch <sf@sfritsch.de> >> --- >> chardev/char.c | 12 ++++++++++++ >> hmp-commands.hx | 16 ++++++++++++++++ >> hmp.c | 8 ++++++++ >> hmp.h | 1 + >> qapi-schema.json | 20 ++++++++++++++++++++ >> 5 files changed, 57 insertions(+) > > Is there an obvious test that we can enhance to add coverage of the new > QMP command? You could have a new test covering hw/char/serial.c, but I wouldn't let that hold the patch. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 05/06/2017 14:24, Eric Blake wrote: >> On 06/05/2017 03:52 AM, Stefan Fritsch wrote: >>> Sending a break on a serial console can be useful for debugging the >>> guest. But not all chardev backends support sending breaks (only telnet >>> and mux do). The chardev-send-break command allows to send a break even >>> if using other backends. >>> >>> Signed-off-by: Stefan Fritsch <sf@sfritsch.de> >>> --- >>> chardev/char.c | 12 ++++++++++++ >>> hmp-commands.hx | 16 ++++++++++++++++ >>> hmp.c | 8 ++++++++ >>> hmp.h | 1 + >>> qapi-schema.json | 20 ++++++++++++++++++++ >>> 5 files changed, 57 insertions(+) >> >> Is there an obvious test that we can enhance to add coverage of the new >> QMP command? > > You could have a new test covering hw/char/serial.c, but I wouldn't let > that hold the patch. Holding patches is pretty much the only leverage I have to get tests for new stuff :) Asking for tests that cover all of serial.c wouldn't be fair. But I am asking for basic test coverage of new QMP commands. Message-ID: <871sugkrw5.fsf@dusky.pond.sub.org> https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00296.html
On Tue, 6 Jun 2017, Markus Armbruster wrote:
> >> Is there an obvious test that we can enhance to add coverage of the new
> >> QMP command?
> >
> > You could have a new test covering hw/char/serial.c, but I wouldn't let
> > that hold the patch.
>
> Holding patches is pretty much the only leverage I have to get tests for
> new stuff :)
>
> Asking for tests that cover all of serial.c wouldn't be fair. But I am
> asking for basic test coverage of new QMP commands.
One can easily extend tests/test-hmp.c, but AFAICS that only checks that
qemu does not crash when the command is executed.
But I have managed to extend tests/test-char.c to call
qmp_chardev_send_break() in the char_file_test() (see below). Is that what
you had in mind? It seems it is not possible to use
qtest_qmp_discard_response() from a unit test, so I don't know how to send
actual json from inside test-char.c
Cheers,
Stefan
diff --git a/tests/test-char.c b/tests/test-char.c
index dfe856cb85..ad20b55f77 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -53,7 +53,9 @@ static void fe_event(void *opaque, int event)
FeHandler *h = opaque;
h->last_event = event;
- quit = true;
+ if (event != CHR_EVENT_BREAK) {
+ quit = true;
+ }
}
#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
@@ -491,7 +493,7 @@ static void char_file_test(void)
file.in = fifo;
file.has_in = true;
- chr = qemu_chardev_new(NULL, TYPE_CHARDEV_FILE, &backend,
+ chr = qemu_chardev_new("label-file", TYPE_CHARDEV_FILE, &backend,
&error_abort);
qemu_chr_fe_init(&be, chr, &error_abort);
@@ -501,6 +503,10 @@ static void char_file_test(void)
fe_event,
&fe, NULL, true);
+ g_assert_cmpint(fe.last_event, !=, CHR_EVENT_BREAK);
+ qmp_chardev_send_break("label-file", NULL);
+ g_assert_cmpint(fe.last_event, ==, CHR_EVENT_BREAK);
+
main_loop();
close(fd);
diff --git a/tests/test-hmp.c b/tests/test-hmp.c
index 99e35ec15a..6dfa0c36e2 100644
--- a/tests/test-hmp.c
+++ b/tests/test-hmp.c
@@ -22,6 +22,7 @@ static int verbose;
static const char *hmp_cmds[] = {
"boot_set ndc",
"chardev-add null,id=testchardev1",
+ "chardev-send-break testchardev1",
"chardev-remove testchardev1",
"commit all",
"cpu-add 1",
> >> Is there an obvious test that we can enhance to add coverage of the new > >> QMP command? > > > > You could have a new test covering hw/char/serial.c, but I wouldn't let > > that hold the patch. > > Holding patches is pretty much the only leverage I have to get tests for > new stuff :) > > Asking for tests that cover all of serial.c wouldn't be fair. But I am > asking for basic test coverage of new QMP commands. I agree, on the other hand it's not exactly a comparable area. I am planning to write a serial qtest for migration as well (which is more complex than this QMP command) so I might as well write the test for this new QMP command myself to get my feet wet... Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: >> >> Is there an obvious test that we can enhance to add coverage of the new >> >> QMP command? >> > >> > You could have a new test covering hw/char/serial.c, but I wouldn't let >> > that hold the patch. >> >> Holding patches is pretty much the only leverage I have to get tests for >> new stuff :) >> >> Asking for tests that cover all of serial.c wouldn't be fair. But I am >> asking for basic test coverage of new QMP commands. > > I agree, on the other hand it's not exactly a comparable area. I am planning > to write a serial qtest for migration as well (which is more complex than this > QMP command) so I might as well write the test for this new QMP command myself > to get my feet wet... I'm willing to take a committment from someone I trust in lieu of actual tests. Is this one?
----- Original Message ----- > From: "Markus Armbruster" <armbru@redhat.com> > To: "Paolo Bonzini" <pbonzini@redhat.com> > Cc: "Stefan Fritsch" <sf@sfritsch.de>, qemu-devel@nongnu.org, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, > "Marc-André Lureau" <marcandre.lureau@redhat.com> > Sent: Wednesday, June 7, 2017 9:06:53 AM > Subject: Re: [Qemu-devel] [PATCH] Add chardev-send-break monitor command > > Paolo Bonzini <pbonzini@redhat.com> writes: > > >> >> Is there an obvious test that we can enhance to add coverage of the new > >> >> QMP command? > >> > > >> > You could have a new test covering hw/char/serial.c, but I wouldn't let > >> > that hold the patch. > >> > >> Holding patches is pretty much the only leverage I have to get tests for > >> new stuff :) > >> > >> Asking for tests that cover all of serial.c wouldn't be fair. But I am > >> asking for basic test coverage of new QMP commands. > > > > I agree, on the other hand it's not exactly a comparable area. I am planning > > to write a serial qtest for migration as well (which is more complex than this > > QMP command) so I might as well write the test for this new QMP command > > myself to get my feet wet... > > I'm willing to take a committment from someone I trust in lieu of actual > tests. Is this one? Sure, though it looks like Stefan also wrote actual tests so we might get two birds with a stone. Paolo
* Stefan Fritsch (sf@sfritsch.de) wrote:
> Sending a break on a serial console can be useful for debugging the
> guest. But not all chardev backends support sending breaks (only telnet
> and mux do). The chardev-send-break command allows to send a break even
> if using other backends.
>
> Signed-off-by: Stefan Fritsch <sf@sfritsch.de>
From the HMP side, it looks OK, you could easily add a line to
tests/test-hmp.c's command list.
But,
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Dave
> ---
> chardev/char.c | 12 ++++++++++++
> hmp-commands.hx | 16 ++++++++++++++++
> hmp.c | 8 ++++++++
> hmp.h | 1 +
> qapi-schema.json | 20 ++++++++++++++++++++
> 5 files changed, 57 insertions(+)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 4e24dc39af..fa54f7c915 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -1307,6 +1307,18 @@ void qmp_chardev_remove(const char *id, Error **errp)
> object_unparent(OBJECT(chr));
> }
>
> +void qmp_chardev_send_break(const char *id, Error **errp)
> +{
> + Chardev *chr;
> +
> + chr = qemu_chr_find(id);
> + if (chr == NULL) {
> + error_setg(errp, "Chardev '%s' not found", id);
> + return;
> + }
> + qemu_chr_be_event(chr, CHR_EVENT_BREAK);
> +}
> +
> void qemu_chr_cleanup(void)
> {
> object_unparent(get_chardevs_root());
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e763606fe5..fc8d54b52a 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1745,6 +1745,22 @@ Removes the chardev @var{id}.
> ETEXI
>
> {
> + .name = "chardev-send-break",
> + .args_type = "id:s",
> + .params = "id",
> + .help = "send break on chardev",
> + .cmd = hmp_chardev_send_break,
> + .command_completion = chardev_remove_completion,
> + },
> +
> +STEXI
> +@item chardev-send-break id
> +@findex chardev-send-break
> +Sends break on the chardev @var{id}.
> +
> +ETEXI
> +
> + {
> .name = "qemu-io",
> .args_type = "device:B,command:s",
> .params = "[device] \"[command]\"",
> diff --git a/hmp.c b/hmp.c
> index ad723903a6..fb2a38b7d6 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2233,6 +2233,14 @@ void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, &local_err);
> }
>
> +void hmp_chardev_send_break(Monitor *mon, const QDict *qdict)
> +{
> + Error *local_err = NULL;
> +
> + qmp_chardev_send_break(qdict_get_str(qdict, "id"), &local_err);
> + hmp_handle_error(mon, &local_err);
> +}
> +
> void hmp_qemu_io(Monitor *mon, const QDict *qdict)
> {
> BlockBackend *blk;
> diff --git a/hmp.h b/hmp.h
> index d8b94ce9dc..214b2617e7 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -103,6 +103,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
> 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_chardev_send_break(Monitor *mon, const QDict *qdict);
> void hmp_qemu_io(Monitor *mon, const QDict *qdict);
> void hmp_cpu_add(Monitor *mon, const QDict *qdict);
> void hmp_object_add(Monitor *mon, const QDict *qdict);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 4b50b652d3..e01dd83dd9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -5114,6 +5114,26 @@
> { 'command': 'chardev-remove', 'data': {'id': 'str'} }
>
> ##
> +# @chardev-send-break:
> +#
> +# Send a break to a character device
> +#
> +# @id: the chardev's ID, must exist
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 2.10
> +#
> +# Example:
> +#
> +# -> { "execute": "chardev-send-break", "arguments": { "id" : "foo" } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'chardev-send-break', 'data': {'id': 'str'} }
> +
> +
> +##
> # @TpmModel:
> #
> # An enumeration of TPM models
> --
> 2.11.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2026 Red Hat, Inc.