On Fri, Oct 19, 2018 at 02:15:10PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Fri, Oct 19, 2018 at 2:35 AM Julia Suvorova <jus...@mail.ru> wrote:
> >
> > New option "websocket" added to allow using WebSocket protocol for
> > chardev socket backend.
> > Example:
> > -chardev socket,websocket,server,id=...
> >
> > Signed-off-by: Julia Suvorova <jus...@mail.ru>
> > ---
> > chardev/char-socket.c | 64 ++++++++++++++++++++++++++++++++++++++-----
> > chardev/char.c | 8 +++++-
> > qapi/char.json | 3 ++
> > qemu-options.hx | 13 +++++++--
> > 4 files changed, 77 insertions(+), 11 deletions(-)
> >
> > diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> > index effde65a89..ba4ae9dfb0 100644
> > --- a/chardev/char-socket.c
> > +++ b/chardev/char-socket.c
> > @@ -26,6 +26,7 @@
> > #include "chardev/char.h"
> > #include "io/channel-socket.h"
> > #include "io/channel-tls.h"
> > +#include "io/channel-websock.h"
> > #include "io/net-listener.h"
> > #include "qemu/error-report.h"
> > #include "qemu/option.h"
> > @@ -68,6 +69,8 @@ typedef struct {
> > GSource *telnet_source;
> > TCPChardevTelnetInit *telnet_init;
> >
> > + bool is_websock;
> > +
> > GSource *reconnect_timer;
> > int64_t reconnect_time;
> > bool connect_err_reported;
> > @@ -394,7 +397,7 @@ static const char
> > *qemu_chr_socket_protocol(SocketChardev *s)
> > if (s->is_telnet) {
> > return "telnet";
> > }
> > - return "tcp";
> > + return s->is_websock ? "websocket" : "tcp";
> > }
> >
> > static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix)
> > @@ -714,6 +717,41 @@ cont:
> > }
> >
> >
> > +static void tcp_chr_websock_handshake(QIOTask *task, gpointer user_data)
> > +{
> > + Chardev *chr = user_data;
> > + SocketChardev *s = user_data;
> > +
> > + if (qio_task_propagate_error(task, NULL)) {
> > + tcp_chr_disconnect(chr);
>
> What about error_report() the error?
None of the rest of the code (telnet, tls, and other i/o related
funcs) uses error_report(). In some ways this is good as it
prevents a client app from flooding the qemu log file. It does
make debugging harder though when things are not working as
expected.
I wonder if we should add a tracing probe point in the
error_setg* methods in util/error.c ? There's quite a few
places in QEMU that discard errors which could benefit from
this when debugging.
I don't think it needs to blck this patch though.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
