From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49698) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XW3T2-0001ct-Vk for qemu-devel@nongnu.org; Mon, 22 Sep 2014 09:15:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XW3Sx-0002N9-Nw for qemu-devel@nongnu.org; Mon, 22 Sep 2014 09:15:48 -0400 Received: from mail-ob0-x22b.google.com ([2607:f8b0:4003:c01::22b]:45858) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XW3Sx-0002ME-Ft for qemu-devel@nongnu.org; Mon, 22 Sep 2014 09:15:43 -0400 Received: by mail-ob0-f171.google.com with SMTP id wp4so519006obc.2 for ; Mon, 22 Sep 2014 06:15:38 -0700 (PDT) Sender: Corey Minyard Message-ID: <542020F7.5050906@acm.org> Date: Mon, 22 Sep 2014 08:15:35 -0500 From: Corey Minyard MIME-Version: 1.0 References: <1411340664-26912-1-git-send-email-minyard@acm.org> <1411340664-26912-5-git-send-email-minyard@acm.org> <541FD872.8000601@redhat.com> In-Reply-To: <541FD872.8000601@redhat.com> 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 Reply-To: minyard@acm.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: mjg59@srcf.ucam.org, mst@redhat.com, hwd@huawei.com, bcketchum@gmail.com, Corey Minyard , afaerber@suse.de On 09/22/2014 03:06 AM, Paolo Bonzini wrote: > 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? I had to make this change, otherwise a server socket would never show that it reconnected. I originally hadn't moved it. Thanks, -corey > > 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) { >>