From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42699) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVydd-0001kx-PM for qemu-devel@nongnu.org; Mon, 22 Sep 2014 04:06:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XVydW-00044n-Ou for qemu-devel@nongnu.org; Mon, 22 Sep 2014 04:06:25 -0400 Received: from mail-wg0-x234.google.com ([2a00:1450:400c:c00::234]:40233) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XVydW-00043H-EM for qemu-devel@nongnu.org; Mon, 22 Sep 2014 04:06:18 -0400 Received: by mail-wg0-f52.google.com with SMTP id n12so487752wgh.11 for ; Mon, 22 Sep 2014 01:06:11 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <541FD872.8000601@redhat.com> Date: Mon, 22 Sep 2014 10:06:10 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1411340664-26912-1-git-send-email-minyard@acm.org> <1411340664-26912-5-git-send-email-minyard@acm.org> In-Reply-To: <1411340664-26912-5-git-send-email-minyard@acm.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/6] qemu-char: set socket filename to disconnected when not connected List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: minyard@acm.org, qemu-devel@nongnu.org Cc: mjg59@srcf.ucam.org, mst@redhat.com, hwd@huawei.com, bcketchum@gmail.com, Corey Minyard , afaerber@suse.de Il 22/09/2014 01:04, minyard@acm.org ha scritto: > From: Corey Minyard > > This way we can tell if the socket is connected or not. It also splits > the string conversions out into separate functions to make this more > convenient. > > Signed-off-by: Corey Minyard > --- > qemu-char.c | 102 ++++++++++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 69 insertions(+), 33 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index e59321d..8418e33 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -117,6 +117,60 @@ static void qapi_copy_SocketAddress(SocketAddress **p_dest, > qobject_decref(obj); > } > > +static int SocketAddress_to_str(char *dest, int max_len, > + const char *prefix, SocketAddress *addr, > + bool is_listen, bool is_telnet) > +{ > + switch (addr->kind) { > + case SOCKET_ADDRESS_KIND_INET: > + return snprintf(dest, max_len, "%s%s:%s:%s%s", prefix, > + is_telnet ? "telnet" : "tcp", addr->inet->host, > + addr->inet->port, is_listen ? ",server" : ""); > + break; > + case SOCKET_ADDRESS_KIND_UNIX: > + return snprintf(dest, max_len, "%sunix:%s%s", prefix, > + addr->q_unix->path, is_listen ? ",server" : ""); > + break; > + case SOCKET_ADDRESS_KIND_FD: > + return snprintf(dest, max_len, "%sfd:%s%s", prefix, addr->fd->str, > + is_listen ? ",server" : ""); > + break; > + default: > + abort(); > + } > +} > + > +static int sockaddr_to_str(char *dest, int max_len, > + struct sockaddr_storage *ss, socklen_t ss_len, > + bool is_listen, bool is_telnet) > +{ > + char host[NI_MAXHOST], serv[NI_MAXSERV]; > + const char *left = "", *right = ""; > + > + switch (ss->ss_family) { > +#ifndef _WIN32 > + case AF_UNIX: > + return snprintf(dest, max_len, "unix:%s%s", > + ((struct sockaddr_un *)(ss))->sun_path, > + is_listen ? ",server" : ""); > +#endif > + case AF_INET6: > + left = "["; > + right = "]"; > + /* fall through */ > + case AF_INET: > + getnameinfo((struct sockaddr *) ss, ss_len, host, sizeof(host), > + serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV); > + return snprintf(dest, max_len, "%s:%s%s%s:%s%s", > + is_telnet ? "telnet" : "tcp", > + left, host, right, serv, > + is_listen ? ",server" : ""); > + > + default: > + return snprintf(dest, max_len, "unknown"); > + } > +} > + > /***********************************************************/ > /* character device */ > > @@ -2719,6 +2773,8 @@ static void tcp_chr_disconnect(CharDriverState *chr) > s->chan = NULL; > closesocket(s->fd); > s->fd = -1; > + SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, > + "disconnected:", s->addr, s->is_listen, s->is_telnet); > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > @@ -2790,6 +2846,17 @@ static void tcp_chr_connect(void *opaque) > { > CharDriverState *chr = opaque; > TCPCharDriver *s = chr->opaque; > + struct sockaddr_storage ss; > + socklen_t ss_len = sizeof(ss); > + > + memset(&ss, 0, ss_len); > + if (getsockname(s->fd, (struct sockaddr *) &ss, &ss_len) != 0) { > + snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, > + "Error in getsockname: %s\n", strerror(errno)); > + } else { > + sockaddr_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, &ss, ss_len, > + s->is_listen, s->is_telnet); > + } Why move this from qemu_chr_finish_socket_connection to tcp_chr_connect? Perhaps move this part of the patch earlier, either just before or just after patch 2? Except for this question, the patch looks good. Thanks, Paolo > s->connected = 1; > if (s->chan) { > @@ -2924,39 +2991,6 @@ static bool qemu_chr_finish_socket_connection(CharDriverState *chr, int fd, > Error **errp) > { > TCPCharDriver *s = chr->opaque; > - char host[NI_MAXHOST], serv[NI_MAXSERV]; > - const char *left = "", *right = ""; > - struct sockaddr_storage ss; > - socklen_t ss_len = sizeof(ss); > - > - memset(&ss, 0, ss_len); > - if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) { > - closesocket(fd); > - error_setg_errno(errp, errno, "getsockname"); > - return false; > - } > - > - switch (ss.ss_family) { > -#ifndef _WIN32 > - case AF_UNIX: > - snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "unix:%s%s", > - ((struct sockaddr_un *)(&ss))->sun_path, > - s->is_listen ? ",server" : ""); > - break; > -#endif > - case AF_INET6: > - left = "["; > - right = "]"; > - /* fall through */ > - case AF_INET: > - getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host), > - serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV); > - snprintf(chr->filename, CHR_MAX_FILENAME_SIZE, "%s:%s%s%s:%s%s", > - s->is_telnet ? "telnet" : "tcp", > - left, host, right, serv, > - s->is_listen ? ",server" : ""); > - break; > - } > > if (s->is_listen) { > s->listen_fd = fd; > @@ -4016,6 +4050,8 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock, > chr->explicit_be_open = true; > > chr->filename = g_malloc(CHR_MAX_FILENAME_SIZE); > + SocketAddress_to_str(chr->filename, CHR_MAX_FILENAME_SIZE, "disconnected:", > + addr, is_listen, is_telnet); > > if (is_listen) { > if (is_telnet) { >