| Message ID | 20170605085254.16485-1-sf@sfritsch.de |
|---|---|
| State | New |
| Headers | show |
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?
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
> >> 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
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
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(+)