Re: [Qemu-devel] A question about this commit "char: convert from GIOChannel to QIOChannel"

[Paolo Bonzini]

- Original Message -
> From: "wangyunjian" 
> To: "Paolo Bonzini" 
> Cc: berra...@redhat.com, qemu-devel@nongnu.org, "caihe" 
> Sent: Monday, October 17, 2016 4:17:45 PM
> Subject: RE: A question about this commit "char: convert from GIOChannel to 
> QIOChannel"
> 
> In function tcp_chr_close, the s->sioc needs call object_unref() to be
> released ?
> 
> About the below code:
> 
> @@ -3205,10 +3205,13 @@ static void tcp_chr_close(CharDriverState *chr)
>  s->listen_tag = 0;
>  }
>  if (s->listen_ioc) {
>  object_unref(OBJECT(s->listen_ioc));
>  }
> +if (s->sioc) {
> +object_unref(OBJECT(s->sioc));
> +}
>  if (s->read_msgfds_num) {
>  for (i = 0; i < s->read_msgfds_num; i++) {
>  close(s->read_msgfds[i]);
>  }
>  g_free(s->read_msgfds);

This is done in tcp_chr_free_connection.  Maybe your tree does not have
commit 5b498459b441f4639fd4c39c35345637bfc6c16c?

Paolo

> Thanks
> 
> Yunjian
> 
> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Monday, October 17, 2016 9:20 PM
> To: wangyunjian
> Cc: berra...@redhat.com; qemu-devel@nongnu.org; caihe
> Subject: Re: A question about this commit "char: convert from GIOChannel to
> QIOChannel"
> 
> 
> 
> - Original Message -
> > From: "wangyunjian" 
> > To: berra...@redhat.com, pbonz...@redhat.com, qemu-devel@nongnu.org
> > Cc: "caihe" 
> > Sent: Monday, October 17, 2016 3:02:32 PM
> > Subject: A question about this commit "char: convert from GIOChannel to
> > QIOChannel"
> > 
> > Commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 “char: convert from
> > GIOChannel to QIOChannel”, the old version will call closesocket when
> > tcp_chr_close and udp_chr_close are called. But the new version will
> > not call closesocket.
> > 
> > This can bring to a socket leak?
> 
> Hi, closesocket is called in io/channel-socket.c
> (qio_channel_socket_finalize).
> 
> Thanks,
> 
> Paolo
> 

Re: [Qemu-devel] A question about this commit "char: convert from GIOChannel to QIOChannel"

[wangyunjian]

In function tcp_chr_close, the s->sioc needs call object_unref() to be released 
? 

About the below code:

@@ -3205,10 +3205,13 @@ static void tcp_chr_close(CharDriverState *chr)
 s->listen_tag = 0;
 }
 if (s->listen_ioc) {
 object_unref(OBJECT(s->listen_ioc));
 }
+if (s->sioc) {
+object_unref(OBJECT(s->sioc));
+}
 if (s->read_msgfds_num) {
 for (i = 0; i < s->read_msgfds_num; i++) {
 close(s->read_msgfds[i]);
 }
 g_free(s->read_msgfds);

Thanks

Yunjian

-Original Message-
From: Paolo Bonzini [mailto:pbonz...@redhat.com] 
Sent: Monday, October 17, 2016 9:20 PM
To: wangyunjian
Cc: berra...@redhat.com; qemu-devel@nongnu.org; caihe
Subject: Re: A question about this commit "char: convert from GIOChannel to 
QIOChannel"



- Original Message -
> From: "wangyunjian" 
> To: berra...@redhat.com, pbonz...@redhat.com, qemu-devel@nongnu.org
> Cc: "caihe" 
> Sent: Monday, October 17, 2016 3:02:32 PM
> Subject: A question about this commit "char: convert from GIOChannel to 
> QIOChannel"
> 
> Commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 “char: convert from 
> GIOChannel to QIOChannel”, the old version will call closesocket when 
> tcp_chr_close and udp_chr_close are called. But the new version will 
> not call closesocket.
> 
> This can bring to a socket leak?

Hi, closesocket is called in io/channel-socket.c (qio_channel_socket_finalize).

Thanks,

Paolo

[Qemu-devel] A question about this commit "char: convert from GIOChannel to QIOChannel"

[wangyunjian]

Commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 “char: convert from GIOChannel 
to QIOChannel”,
about the below code segment：

・・・ ・・・
@@ -3107,22 +2965,16 @@ static void tcp_chr_close(CharDriverState *chr)
 s->reconnect_timer = 0;
 }
 qapi_free_SocketAddress(s->addr);
-if (s->fd >= 0) {
-remove_fd_in_watch(chr);
-if (s->chan) {
-g_io_channel_unref(s->chan);
-}
-closesocket(s->fd);
+remove_fd_in_watch(chr);
+if (s->ioc) {
+object_unref(OBJECT(s->ioc));
 }
-if (s->listen_fd >= 0) {
-if (s->listen_tag) {
-g_source_remove(s->listen_tag);
-s->listen_tag = 0;
-}
-if (s->listen_chan) {
-g_io_channel_unref(s->listen_chan);
-}
-closesocket(s->listen_fd);
+if (s->listen_tag) {
+g_source_remove(s->listen_tag);
+s->listen_tag = 0;
+}
+if (s->listen_ioc) {
+object_unref(OBJECT(s->listen_ioc));
 }
・・・ ・・・
@@ -2544,17 +2496,16 @@ static void udp_chr_close(CharDriverState *chr)
 NetCharDriver *s = chr->opaque;
 remove_fd_in_watch(chr);
-if (s->chan) {
-g_io_channel_unref(s->chan);
-closesocket(s->fd);
+if (s->ioc) {
+object_unref(OBJECT(s->ioc));
 }
 g_free(s);
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}


The old version will call closesocket when tcp_chr_close and udp_chr_close are 
called. But the new version will not call closesocket.


This can bring to a socket leak?

Re: [Qemu-devel] A question about this commit "char: convert from GIOChannel to QIOChannel"

[Paolo Bonzini]

- Original Message -
> From: "wangyunjian" 
> To: berra...@redhat.com, pbonz...@redhat.com, qemu-devel@nongnu.org
> Cc: "caihe" 
> Sent: Monday, October 17, 2016 3:02:32 PM
> Subject: A question about this commit "char: convert from GIOChannel to 
> QIOChannel"
> 
> Commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 “char: convert from
> GIOChannel to QIOChannel”,
> the old version will call closesocket when tcp_chr_close and udp_chr_close
> are called. But the new version will not call closesocket.
> 
> This can bring to a socket leak?

Hi, closesocket is called in io/channel-socket.c (qio_channel_socket_finalize).

Thanks,

Paolo