Re: [Qemu-devel] [PATCH v5 2/2] net: add the support for -netdev socket, listen

[Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>]

On Sat, Jun 09, 2012 at 10:09:44AM +0800, Zhi Yong Wu wrote:
> On Fri, Jun 8, 2012 at 11:59 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > On Fri, Jun 8, 2012 at 3:54 PM, Zhi Yong Wu <zwu.kernel@gmail.com> wrote:
> >> On Fri, Jun 8, 2012 at 6:31 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> >>> On Thu, Jun 7, 2012 at 3:54 PM,  <zwu.kernel@gmail.com> wrote:
> >>>> @@ -86,7 +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);
> >>>
> >>> What happens when this is not a listen socket?  I suggest setting
> >>> listen_fd to -1 during creation and not calling qemu_set_fd_handler()
> >>> when listen_fd is -1 here.  If listen_fd is 0 then we'll register
> >>> net_socket_accept when standard input becomes ready!
> >>>
> >>>>         closesocket(s->fd);
> >>>> +
> >>>> +        s->fd = 0;
> >>>
> >>> -1 should be used since 0 is a valid file descriptor (standard input).
> >> I think that s->fd = 0 doesn't cause every issue. When it is zero,
> >> this fd hasn't been registered with every handler. You can see that
> >> "qemu_set_fd_handler(s->fd, NULL, NULL, NULL);" before "s->fd = 0".
> >
> > If s->fd = 0 because we are still listening and net_socket_cleanup()
> OK, done. any other issue?

I haven't seen your new code yet so I'm not sure.  Basically if you have
introduced -1 states for s->fd and s->listen_fd as well as checks before
calling qemu_set_fd_handler() or close(), then the code is fine.

Stefan