[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] net: complete NetSocketState lifecycle handling
From: |
Zhi Yong Wu |
Subject: |
Re: [Qemu-devel] [PATCH] net: complete NetSocketState lifecycle handling |
Date: |
Wed, 13 Jun 2012 09:54:29 +0800 |
On Tue, Jun 12, 2012 at 10:04 PM, Stefan Hajnoczi
<address@hidden> wrote:
> The NetSocketState struct contains two file descriptors: an active
> connection and a listen socket for new connections. It's important that
> we clean up after ourselves so these file descriptors are initialized to
> -1 when unused. This allows makes it possible to call cleanup functions
> only when the file descriptors are valid (not -1).
>
> The specific issue solved by this patch is that we avoid calling
> close(-1), close(0), and qemu_set_fd_handler(-1, net_socket_accept,
> NULL, s). All of these are either problematic or unclean (e.g. reported
> as warnings by Valgrind).
>
> Also stay consistent by bringing the link down when the active
> connection is closed.
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> This patch adds additional fixes on top of Zhi Yong's first 2 patches. With
> this patch in place I'm happy with this series.
>
> net/socket.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index f67de47..403221c 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -82,13 +82,16 @@ static void net_socket_send(void *opaque)
> /* end of connection */
> eoc:
> qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> - qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
> + if (s->listen_fd != -1) {
> + qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
> + }
> closesocket(s->fd);
>
> s->fd = -1;
> s->state = 0;
> s->index = 0;
> s->packet_len = 0;
> + s->nc.link_down = true;
> memset(s->buf, 0, sizeof(s->buf));
> memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
>
> @@ -239,8 +242,16 @@ fail:
> static void net_socket_cleanup(VLANClientState *nc)
> {
> NetSocketState *s = DO_UPCAST(NetSocketState, nc, nc);
> - qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> - close(s->fd);
> + if (s->fd != -1) {
> + qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> + close(s->fd);
> + s->fd = -1;
> + }
> + if (s->listen_fd != -1) {
> + qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
> + closesocket(s->listen_fd);
> + s->listen_fd = -1;
> + }
> }
>
> static NetClientInfo net_dgram_socket_info = {
> @@ -302,6 +313,7 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState
> *vlan,
> s = DO_UPCAST(NetSocketState, nc, nc);
>
> s->fd = fd;
> + s->listen_fd = -1;
>
> qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s);
>
> @@ -345,6 +357,7 @@ static NetSocketState
> *net_socket_fd_init_stream(VLANState *vlan,
> s = DO_UPCAST(NetSocketState, nc, nc);
>
> s->fd = fd;
> + s->listen_fd = -1;
>
> if (is_connected) {
> net_socket_connect(s);
> @@ -445,6 +458,7 @@ static int net_socket_listen_init(VLANState *vlan,
>
> nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name);
> s = DO_UPCAST(NetSocketState, nc, nc);
> + s->fd = -1;
> s->listen_fd = fd;
> s->nc.link_down = true;
>
> --
> 1.7.10
>
>
Reviewed-by: Zhi Yong Wu <address@hidden>
--
Regards,
Zhi Yong Wu