From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60210) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fub4S-00065U-UQ for qemu-devel@nongnu.org; Tue, 28 Aug 2018 06:18:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fuark-0005Sk-AN for qemu-devel@nongnu.org; Tue, 28 Aug 2018 06:04:57 -0400 Received: from smtp16.mail.ru ([94.100.176.153]:36252) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fuari-0005Px-BD for qemu-devel@nongnu.org; Tue, 28 Aug 2018 06:04:50 -0400 References: <20180827184103.32190-1-jusual@mail.ru> <20180827184103.32190-2-jusual@mail.ru> <20180828090249.GB25114@redhat.com> From: Julia Suvorova Message-ID: Date: Tue, 28 Aug 2018 13:04:41 +0300 MIME-Version: 1.0 In-Reply-To: <20180828090249.GB25114@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 1/2] chardev: Add websocket support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "=?UTF-8?Q?Daniel_P._Berrang=c3=a9?=" Cc: qemu-devel@nongnu.org, Eric Blake , marcandre.lureau@redhat.com, Paolo Bonzini , Stefan Hajnoczi , Stefan Hajnoczi , Joel Stanley , Jim Mussared , =?UTF-8?Q?Steffen_G=c3=b6rtz?= On 28.08.2018 12:02, Daniel P. Berrangé wrote: > On Mon, Aug 27, 2018 at 09:41:02PM +0300, Julia Suvorova wrote: >> New option "websock" added to allow using websocket protocol for >> chardev socket backend. >> Example: >> -chardev socket,websock,id=... >> >> Signed-off-by: Julia Suvorova >> --- >> chardev/char-socket.c | 124 +++++++++++++++++++++++++++++++----------- >> chardev/char.c | 8 ++- >> qapi/char.json | 3 + >> 3 files changed, 103 insertions(+), 32 deletions(-) > > Also needs changes to qemu-options.hx to document the new > websock option. This doc ends up included in the main > qemu-doc.html published on the website, and also forms the > man page for qemu. > >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> index efbad6ee7c..1d3c14d85d 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" >> @@ -69,6 +70,8 @@ typedef struct { >> GSource *telnet_source; >> TCPChardevTelnetInit *telnet_init; >> >> + bool is_websock; >> + >> GSource *reconnect_timer; >> int64_t reconnect_time; >> bool connect_err_reported; >> @@ -385,30 +388,37 @@ static void tcp_chr_free_connection(Chardev *chr) >> s->connected = 0; >> } >> >> -static char *SocketAddress_to_str(const char *prefix, SocketAddress *addr, >> - bool is_listen, bool is_telnet) >> +static const char *qemu_chr_socket_protocol(SocketChardev *s) >> +{ >> + if (s->is_telnet) { >> + return "telnet"; >> + } >> + return s->is_websock ? "websock" : "tcp"; >> +} >> + >> +static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) >> { >> - switch (addr->type) { >> + switch (s->addr->type) { >> case SOCKET_ADDRESS_TYPE_INET: >> return g_strdup_printf("%s%s:%s:%s%s", prefix, >> - is_telnet ? "telnet" : "tcp", >> - addr->u.inet.host, >> - addr->u.inet.port, >> - is_listen ? ",server" : ""); >> + qemu_chr_socket_protocol(s), >> + s->addr->u.inet.host, >> + s->addr->u.inet.port, >> + s->is_listen ? ",server" : ""); >> break; >> case SOCKET_ADDRESS_TYPE_UNIX: >> return g_strdup_printf("%sunix:%s%s", prefix, >> - addr->u.q_unix.path, >> - is_listen ? ",server" : ""); >> + s->addr->u.q_unix.path, >> + s->is_listen ? ",server" : ""); >> break; >> case SOCKET_ADDRESS_TYPE_FD: >> - return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd.str, >> - is_listen ? ",server" : ""); >> + return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, >> + s->is_listen ? ",server" : ""); >> break; >> case SOCKET_ADDRESS_TYPE_VSOCK: >> return g_strdup_printf("%svsock:%s:%s", prefix, >> - addr->u.vsock.cid, >> - addr->u.vsock.port); >> + s->addr->u.vsock.cid, >> + s->addr->u.vsock.port); >> default: >> abort(); >> } >> @@ -419,8 +429,7 @@ static void update_disconnected_filename(SocketChardev *s) >> Chardev *chr = CHARDEV(s); >> >> g_free(chr->filename); >> - chr->filename = SocketAddress_to_str("disconnected:", s->addr, >> - s->is_listen, s->is_telnet); >> + chr->filename = qemu_chr_socket_address(s, "disconnected:"); >> } >> >> /* NB may be called even if tcp_chr_connect has not been >> @@ -506,10 +515,12 @@ static int tcp_chr_sync_read(Chardev *chr, const uint8_t *buf, int len) >> return size; >> } >> >> -static char *sockaddr_to_str(struct sockaddr_storage *ss, socklen_t ss_len, >> - struct sockaddr_storage *ps, socklen_t ps_len, >> - bool is_listen, bool is_telnet) >> +static char *qemu_chr_compute_filename(SocketChardev *s) >> { >> + struct sockaddr_storage *ss = &s->sioc->localAddr; >> + struct sockaddr_storage *ps = &s->sioc->remoteAddr; >> + socklen_t ss_len = s->sioc->localAddrLen; >> + socklen_t ps_len = s->sioc->remoteAddrLen; >> char shost[NI_MAXHOST], sserv[NI_MAXSERV]; >> char phost[NI_MAXHOST], pserv[NI_MAXSERV]; >> const char *left = "", *right = ""; >> @@ -519,7 +530,7 @@ static char *sockaddr_to_str(struct sockaddr_storage *ss, socklen_t ss_len, >> case AF_UNIX: >> return g_strdup_printf("unix:%s%s", >> ((struct sockaddr_un *)(ss))->sun_path, >> - is_listen ? ",server" : ""); >> + s->is_listen ? ",server" : ""); >> #endif >> case AF_INET6: >> left = "["; >> @@ -531,9 +542,9 @@ static char *sockaddr_to_str(struct sockaddr_storage *ss, socklen_t ss_len, >> getnameinfo((struct sockaddr *) ps, ps_len, phost, sizeof(phost), >> pserv, sizeof(pserv), NI_NUMERICHOST | NI_NUMERICSERV); >> return g_strdup_printf("%s:%s%s%s:%s%s <-> %s%s%s:%s", >> - is_telnet ? "telnet" : "tcp", >> + qemu_chr_socket_protocol(s), >> left, shost, right, sserv, >> - is_listen ? ",server" : "", >> + s->is_listen ? ",server" : "", >> left, phost, right, pserv); >> >> default: >> @@ -547,10 +558,7 @@ static void tcp_chr_connect(void *opaque) >> SocketChardev *s = SOCKET_CHARDEV(opaque); >> >> g_free(chr->filename); >> - chr->filename = sockaddr_to_str( >> - &s->sioc->localAddr, s->sioc->localAddrLen, >> - &s->sioc->remoteAddr, s->sioc->remoteAddrLen, >> - s->is_listen, s->is_telnet); >> + chr->filename = qemu_chr_compute_filename(s); >> >> s->connected = 1; >> chr->gsource = io_add_watch_poll(chr, s->ioc, > > Everything above this point is refactoring of existing code, mixed in with > a few websock additions. > > Everything below this is implementing the websocket feature. > > These two distinct tasks should be separated, so that refactoring is in > the first patch, and websocks impl is in a second patch. Thus, I will have second patch based on the first patch, and the first will contain code that will never be used. If it's okay, I will divide the patch. Best regards, Julia Suvorova.