[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/9] slirp: switch to GPollFD
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/9] slirp: switch to GPollFD |
Date: |
Mon, 4 Feb 2013 10:07:18 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sat, Feb 02, 2013 at 12:46:20PM +0000, Blue Swirl wrote:
> On Fri, Feb 1, 2013 at 1:53 PM, Stefan Hajnoczi <address@hidden> wrote:
> > Slirp uses rfds/wfds/xfds more extensively than other QEMU components.
> >
> > The rarely-used out-of-band TCP data feature is used. That means we
> > need the full table of select(2) to g_poll(3) events:
> >
> > rfds -> G_IO_IN | G_IO_HUP | G_IO_ERR
> > wfds -> G_IO_OUT | G_IO_ERR
> > xfds -> G_IO_PRI
> >
> > I came up with this table by looking at Linux fs/select.c which maps
> > select(2) to poll(2) internally.
> >
> > Another detail to watch out for are the global variables that reference
> > rfds/wfds/xfds during slirp_select_poll(). sofcantrcvmore() and
> > sofcantsendmore() use these globals to clear fd_set bits. When
> > sofcantrcvmore() is called, the wfds bit is cleared so that the write
> > handler will no longer be run for this iteration of the event loop.
> >
> > This actually seems buggy to me since TCP connections can be half-closed
> > and we'd still want to handle data in half-duplex fashion. I think the
> > real intention is to avoid running the read/write handler when the
> > socket has been fully closed. This is indicated with the SS_NOFDREF
> > state bit so we now check for it before invoking the TCP write handler.
> > Note that UDP/ICMP code paths don't care because they are
> > connectionless.
> >
> > Note that slirp/ has a lot of tabs and sometimes mixed tabs with spaces.
> > I followed the style of the surrounding code.
>
> Nack for CODING_STYLE. Please either convert the functions affected to
> spaces first (as you yourself proposed), or just ignore the
> surrounding code and use spaces. Read my lips: no new tabses.
Given the project status of upstream slirp, I don't expect us to sync
code changes much or at all. Therefore let's convert the file to QEMU
coding style.
Stefan
- [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 1/9] main-loop: fix select_ret uninitialized variable warning, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 2/9] main-loop: switch to g_poll() on POSIX hosts, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 3/9] main-loop: switch POSIX glib integration to GPollFD, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 4/9] slirp: switch to GPollFD, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 6/9] main-loop: drop rfds/wfds/xfds for good, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 5/9] iohandler: switch to GPollFD, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 7/9] aio: extract aio_dispatch() from aio_poll(), Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 8/9] aio: convert aio_poll() to g_poll(3), Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH v2 9/9] aio: support G_IO_HUP and G_IO_ERR, Stefan Hajnoczi, 2013/02/01