* [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts
@ 2013-02-01 13:53 Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 1/9] main-loop: fix select_ret uninitialized variable warning Stefan Hajnoczi
` (8 more replies)
0 siblings, 9 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
Amos Kong <akong@redhat.com> reported that file descriptors numbered higher
than 1024 could crash QEMU. This is due to the fixed size of the fd_set type
used for select(2) event polling.
This series converts the main-loop.c and aio-posix.c select(2) calls to
g_poll(3). This eliminates the fd_set type and allows QEMU to scale to high
numbers of file descriptors.
The g_poll(3) interface is a portable version of the poll(2) system call. The
difference to select(2) is that fine-grained events (G_IO_IN, G_IO_OUT,
G_IO_HUP, G_IO_ERR, G_IO_PRI) can be monitored instead of just
read/write/exception. Also, there is no limit to the file descriptor numbers
that may be used, allowing applications to scale to many file descriptors. See
the documentation for details:
http://developer.gnome.org/glib/2.28/glib-The-Main-Event-Loop.html#g-poll
The QEMU main loop works as follows today:
1. Call out to slirp, iohandlers, and glib sources to fill rfds/wfds/xfds with
the file descriptors to select(2).
2. Perform the select(2) call.
3. Call out to slirp, iohandlers, and glib sources to handle events polled in
rfds/wfds/xfds.
The plan of attack is as follows:
1. Replace select(2) with g_poll(3). Use glue that converts between
rfds/wfds/xfds and GPollFD so that the unconverted QEMU components still
work.
2. Convert slirp, iohandlers, and glib source fill/poll functions to use
GPollFD directly instead of rfds/wfds/xfds.
3. Drop the glue since all components now natively use GPollFD.
4. Convert aio-posix.c to g_poll(3) by reusing GPollFD.
I have tested that the series builds and is bisectable on Linux and Windows
hosts. But I have not done extensive testing on other host platforms or with
long-term guests to check for performance regressions.
v2:
* Replace custom Poller type with GArray [aliguori]
Stefan Hajnoczi (9):
main-loop: fix select_ret uninitialized variable warning
main-loop: switch to g_poll() on POSIX hosts
main-loop: switch POSIX glib integration to GPollFD
slirp: switch to GPollFD
iohandler: switch to GPollFD
main-loop: drop rfds/wfds/xfds for good
aio: extract aio_dispatch() from aio_poll()
aio: convert aio_poll() to g_poll(3)
aio: support G_IO_HUP and G_IO_ERR
aio-posix.c | 130 ++++++++++++++++++---------------------
async.c | 2 +
include/block/aio.h | 3 +
include/qemu/main-loop.h | 4 +-
iohandler.c | 40 +++++++++---
main-loop.c | 156 ++++++++++++++++++++++++++---------------------
slirp/libslirp.h | 6 +-
slirp/main.h | 1 -
slirp/slirp.c | 136 ++++++++++++++++++++++++-----------------
slirp/socket.c | 9 ---
slirp/socket.h | 2 +
stubs/slirp.c | 6 +-
12 files changed, 270 insertions(+), 225 deletions(-)
--
1.8.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 1/9] main-loop: fix select_ret uninitialized variable warning
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 2/9] main-loop: switch to g_poll() on POSIX hosts Stefan Hajnoczi
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
main-loop.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/main-loop.c b/main-loop.c
index 6f52ac3..d0d8fe4 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -330,7 +330,8 @@ void qemu_fd_register(int fd)
static int os_host_main_loop_wait(uint32_t timeout)
{
GMainContext *context = g_main_context_default();
- int select_ret, g_poll_ret, ret, i;
+ int select_ret = 0;
+ int g_poll_ret, ret, i;
PollingEntry *pe;
WaitObjects *w = &wait_objects;
gint poll_timeout;
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 2/9] main-loop: switch to g_poll() on POSIX hosts
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 1/9] main-loop: fix select_ret uninitialized variable warning Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 3/9] main-loop: switch POSIX glib integration to GPollFD Stefan Hajnoczi
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
Use g_poll(3) instead of select(2). Well, this is kind of a cheat.
It's true that we're now using g_poll(3) on POSIX hosts but the *_fill()
and *_poll() functions are still using rfds/wfds/xfds.
We've set the scene to start converting *_fill() and *_poll() functions
step-by-step until no more rfds/wfds/xfds users remain. Then we'll drop
the temporary gpollfds_from_select() and gpollfds_to_select() functions
and be left with native g_poll(2).
On Windows things are a little crazy: convert from rfds/wfds/xfds to
GPollFDs, back to rfds/wfds/xfds, call select(2), rfds/wfds/xfds back to
GPollFDs, and finally back to rfds/wfds/xfds again. This is only
temporary and keeps the Windows build working through the following
patches. We'll drop this excessive conversion later and be left with a
single GPollFDs -> select(2) -> GPollFDs sequence that allows Windows to
use select(2) while the rest of QEMU only knows about GPollFD.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
main-loop.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 127 insertions(+), 8 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index d0d8fe4..f1dcd14 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -117,6 +117,8 @@ void qemu_notify_event(void)
aio_notify(qemu_aio_context);
}
+static GArray *gpollfds;
+
int qemu_init_main_loop(void)
{
int ret;
@@ -133,6 +135,7 @@ int qemu_init_main_loop(void)
return ret;
}
+ gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
qemu_aio_context = aio_context_new();
src = aio_get_g_source(qemu_aio_context);
g_source_attach(src, NULL);
@@ -146,6 +149,62 @@ static GPollFD poll_fds[1024 * 2]; /* this is probably overkill */
static int n_poll_fds;
static int max_priority;
+/* Load rfds/wfds/xfds into gpollfds. Will be removed a few commits later. */
+static void gpollfds_from_select(void)
+{
+ int fd;
+ for (fd = 0; fd <= nfds; fd++) {
+ int events = 0;
+ if (FD_ISSET(fd, &rfds)) {
+ events |= G_IO_IN | G_IO_HUP | G_IO_ERR;
+ }
+ if (FD_ISSET(fd, &wfds)) {
+ events |= G_IO_OUT | G_IO_ERR;
+ }
+ if (FD_ISSET(fd, &xfds)) {
+ events |= G_IO_PRI;
+ }
+ if (events) {
+ GPollFD pfd = {
+ .fd = fd,
+ .events = events,
+ };
+ g_array_append_val(gpollfds, pfd);
+ }
+ }
+}
+
+/* Store gpollfds revents into rfds/wfds/xfds. Will be removed a few commits
+ * later.
+ */
+static void gpollfds_to_select(int ret)
+{
+ int i;
+
+ FD_ZERO(&rfds);
+ FD_ZERO(&wfds);
+ FD_ZERO(&xfds);
+
+ if (ret <= 0) {
+ return;
+ }
+
+ for (i = 0; i < gpollfds->len; i++) {
+ int fd = g_array_index(gpollfds, GPollFD, i).fd;
+ int revents = g_array_index(gpollfds, GPollFD, i).revents;
+
+ if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
+ FD_SET(fd, &rfds);
+ }
+ if (revents & (G_IO_OUT | G_IO_ERR)) {
+ FD_SET(fd, &wfds);
+ }
+ if (revents & G_IO_PRI) {
+ FD_SET(fd, &xfds);
+ }
+ }
+}
+
#ifndef _WIN32
static void glib_select_fill(int *max_fd, fd_set *rfds, fd_set *wfds,
fd_set *xfds, uint32_t *cur_timeout)
@@ -212,22 +271,22 @@ static void glib_select_poll(fd_set *rfds, fd_set *wfds, fd_set *xfds,
static int os_host_main_loop_wait(uint32_t timeout)
{
- struct timeval tv, *tvarg = NULL;
int ret;
glib_select_fill(&nfds, &rfds, &wfds, &xfds, &timeout);
- if (timeout < UINT32_MAX) {
- tvarg = &tv;
- tv.tv_sec = timeout / 1000;
- tv.tv_usec = (timeout % 1000) * 1000;
- }
-
if (timeout > 0) {
qemu_mutex_unlock_iothread();
}
- ret = select(nfds + 1, &rfds, &wfds, &xfds, tvarg);
+ /* We'll eventually drop fd_set completely. But for now we still have
+ * *_fill() and *_poll() functions that use rfds/wfds/xfds.
+ */
+ gpollfds_from_select();
+
+ ret = g_poll((GPollFD *)gpollfds->data, gpollfds->len, timeout);
+
+ gpollfds_to_select(ret);
if (timeout > 0) {
qemu_mutex_lock_iothread();
@@ -327,6 +386,55 @@ void qemu_fd_register(int fd)
FD_CONNECT | FD_WRITE | FD_OOB);
}
+static int pollfds_fill(GArray *pollfds, fd_set *rfds, fd_set *wfds,
+ fd_set *xfds)
+{
+ int nfds = -1;
+ int i;
+
+ for (i = 0; i < pollfds->len; i++) {
+ GPollFD *pfd = &g_array_index(pollfds, GPollFD, i);
+ int fd = pfd->fd;
+ int events = pfd->events;
+ if (events & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
+ FD_SET(fd, rfds);
+ nfds = MAX(nfds, fd);
+ }
+ if (events & (G_IO_OUT | G_IO_ERR)) {
+ FD_SET(fd, wfds);
+ nfds = MAX(nfds, fd);
+ }
+ if (events & G_IO_PRI) {
+ FD_SET(fd, xfds);
+ nfds = MAX(nfds, fd);
+ }
+ }
+ return nfds;
+}
+
+static void pollfds_poll(GArray *pollfds, int nfds, fd_set *rfds,
+ fd_set *wfds, fd_set *xfds)
+{
+ int i;
+
+ for (i = 0; i < pollfds->len; i++) {
+ GPollFD *pfd = &g_array_index(pollfds, GPollFD, i);
+ int fd = pfd->fd;
+ int revents = 0;
+
+ if (FD_ISSET(fd, rfds)) {
+ revents |= G_IO_IN | G_IO_HUP | G_IO_ERR;
+ }
+ if (FD_ISSET(fd, wfds)) {
+ revents |= G_IO_OUT | G_IO_ERR;
+ }
+ if (FD_ISSET(fd, xfds)) {
+ revents |= G_IO_PRI;
+ }
+ pfd->revents |= revents & pfd->events;
+ }
+}
+
static int os_host_main_loop_wait(uint32_t timeout)
{
GMainContext *context = g_main_context_default();
@@ -382,12 +490,22 @@ static int os_host_main_loop_wait(uint32_t timeout)
* improve socket latency.
*/
+ /* This back-and-forth between GPollFDs and select(2) is temporary. We'll
+ * drop it in a couple of patches, I promise :).
+ */
+ gpollfds_from_select();
+ FD_ZERO(&rfds);
+ FD_ZERO(&wfds);
+ FD_ZERO(&xfds);
+ nfds = pollfds_fill(gpollfds, &rfds, &wfds, &xfds);
if (nfds >= 0) {
select_ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0);
if (select_ret != 0) {
timeout = 0;
+ pollfds_poll(gpollfds, nfds, &rfds, &wfds, &xfds);
}
}
+ gpollfds_to_select(select_ret || g_poll_ret);
return select_ret || g_poll_ret;
}
@@ -403,6 +521,7 @@ int main_loop_wait(int nonblocking)
}
/* poll any events */
+ g_array_set_size(gpollfds, 0); /* reset for new iteration */
/* XXX: separate device handlers from system ones */
nfds = -1;
FD_ZERO(&rfds);
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 3/9] main-loop: switch POSIX glib integration to GPollFD
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 1/9] main-loop: fix select_ret uninitialized variable warning Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 2/9] main-loop: switch to g_poll() on POSIX hosts Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 4/9] slirp: switch " Stefan Hajnoczi
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
Convert glib file descriptor polling from rfds/wfds/xfds to GPollFD.
The Windows code still needs poll_fds[] and n_poll_fds but they can now
become local variables.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
main-loop.c | 71 +++++++++++++++++++------------------------------------------
1 file changed, 22 insertions(+), 49 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index f1dcd14..12b0213 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -145,8 +145,6 @@ int qemu_init_main_loop(void)
static fd_set rfds, wfds, xfds;
static int nfds;
-static GPollFD poll_fds[1024 * 2]; /* this is probably overkill */
-static int n_poll_fds;
static int max_priority;
/* Load rfds/wfds/xfds into gpollfds. Will be removed a few commits later. */
@@ -206,65 +204,39 @@ static void gpollfds_to_select(int ret)
}
#ifndef _WIN32
-static void glib_select_fill(int *max_fd, fd_set *rfds, fd_set *wfds,
- fd_set *xfds, uint32_t *cur_timeout)
+static int glib_pollfds_idx;
+static int glib_n_poll_fds;
+
+static void glib_pollfds_fill(uint32_t *cur_timeout)
{
GMainContext *context = g_main_context_default();
- int i;
int timeout = 0;
+ int n;
g_main_context_prepare(context, &max_priority);
- n_poll_fds = g_main_context_query(context, max_priority, &timeout,
- poll_fds, ARRAY_SIZE(poll_fds));
- g_assert(n_poll_fds <= ARRAY_SIZE(poll_fds));
-
- for (i = 0; i < n_poll_fds; i++) {
- GPollFD *p = &poll_fds[i];
-
- if ((p->events & G_IO_IN)) {
- FD_SET(p->fd, rfds);
- *max_fd = MAX(*max_fd, p->fd);
- }
- if ((p->events & G_IO_OUT)) {
- FD_SET(p->fd, wfds);
- *max_fd = MAX(*max_fd, p->fd);
- }
- if ((p->events & G_IO_ERR)) {
- FD_SET(p->fd, xfds);
- *max_fd = MAX(*max_fd, p->fd);
- }
- }
+ glib_pollfds_idx = gpollfds->len;
+ n = glib_n_poll_fds;
+ do {
+ GPollFD *pfds;
+ glib_n_poll_fds = n;
+ g_array_set_size(gpollfds, glib_pollfds_idx + glib_n_poll_fds);
+ pfds = &g_array_index(gpollfds, GPollFD, glib_pollfds_idx);
+ n = g_main_context_query(context, max_priority, &timeout, pfds,
+ glib_n_poll_fds);
+ } while (n != glib_n_poll_fds);
if (timeout >= 0 && timeout < *cur_timeout) {
*cur_timeout = timeout;
}
}
-static void glib_select_poll(fd_set *rfds, fd_set *wfds, fd_set *xfds,
- bool err)
+static void glib_pollfds_poll(void)
{
GMainContext *context = g_main_context_default();
+ GPollFD *pfds = &g_array_index(gpollfds, GPollFD, glib_pollfds_idx);
- if (!err) {
- int i;
-
- for (i = 0; i < n_poll_fds; i++) {
- GPollFD *p = &poll_fds[i];
-
- if ((p->events & G_IO_IN) && FD_ISSET(p->fd, rfds)) {
- p->revents |= G_IO_IN;
- }
- if ((p->events & G_IO_OUT) && FD_ISSET(p->fd, wfds)) {
- p->revents |= G_IO_OUT;
- }
- if ((p->events & G_IO_ERR) && FD_ISSET(p->fd, xfds)) {
- p->revents |= G_IO_ERR;
- }
- }
- }
-
- if (g_main_context_check(context, max_priority, poll_fds, n_poll_fds)) {
+ if (g_main_context_check(context, max_priority, pfds, glib_n_poll_fds)) {
g_main_context_dispatch(context);
}
}
@@ -273,7 +245,7 @@ static int os_host_main_loop_wait(uint32_t timeout)
{
int ret;
- glib_select_fill(&nfds, &rfds, &wfds, &xfds, &timeout);
+ glib_pollfds_fill(&timeout);
if (timeout > 0) {
qemu_mutex_unlock_iothread();
@@ -292,7 +264,7 @@ static int os_host_main_loop_wait(uint32_t timeout)
qemu_mutex_lock_iothread();
}
- glib_select_poll(&rfds, &wfds, &xfds, (ret < 0));
+ glib_pollfds_poll();
return ret;
}
#else
@@ -438,8 +410,9 @@ static void pollfds_poll(GArray *pollfds, int nfds, fd_set *rfds,
static int os_host_main_loop_wait(uint32_t timeout)
{
GMainContext *context = g_main_context_default();
+ GPollFD poll_fds[1024 * 2]; /* this is probably overkill */
int select_ret = 0;
- int g_poll_ret, ret, i;
+ int g_poll_ret, ret, i, n_poll_fds;
PollingEntry *pe;
WaitObjects *w = &wait_objects;
gint poll_timeout;
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 4/9] slirp: switch to GPollFD
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
` (2 preceding siblings ...)
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 3/9] main-loop: switch POSIX glib integration to GPollFD Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-02 12:46 ` Blue Swirl
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 5/9] iohandler: " Stefan Hajnoczi
` (4 subsequent siblings)
8 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
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.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
main-loop.c | 4 +-
slirp/libslirp.h | 6 +--
slirp/main.h | 1 -
slirp/slirp.c | 136 +++++++++++++++++++++++++++++++++----------------------
slirp/socket.c | 9 ----
slirp/socket.h | 2 +
stubs/slirp.c | 6 +--
7 files changed, 89 insertions(+), 75 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index 12b0213..49e97ff 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -503,13 +503,13 @@ int main_loop_wait(int nonblocking)
#ifdef CONFIG_SLIRP
slirp_update_timeout(&timeout);
- slirp_select_fill(&nfds, &rfds, &wfds, &xfds);
+ slirp_pollfds_fill(gpollfds);
#endif
qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds);
ret = os_host_main_loop_wait(timeout);
qemu_iohandler_poll(&rfds, &wfds, &xfds, ret);
#ifdef CONFIG_SLIRP
- slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));
+ slirp_pollfds_poll(gpollfds, (ret < 0));
#endif
qemu_run_all_timers();
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 49609c2..ceabff8 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -17,11 +17,9 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
void slirp_cleanup(Slirp *slirp);
void slirp_update_timeout(uint32_t *timeout);
-void slirp_select_fill(int *pnfds,
- fd_set *readfds, fd_set *writefds, fd_set *xfds);
+void slirp_pollfds_fill(GArray *pollfds);
-void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
- int select_error);
+void slirp_pollfds_poll(GArray *pollfds, int select_error);
void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
diff --git a/slirp/main.h b/slirp/main.h
index 66e4f92..f2e58cf 100644
--- a/slirp/main.h
+++ b/slirp/main.h
@@ -31,7 +31,6 @@ extern int ctty_closed;
extern char *slirp_tty;
extern char *exec_shell;
extern u_int curtime;
-extern fd_set *global_readfds, *global_writefds, *global_xfds;
extern struct in_addr loopback_addr;
extern unsigned long loopback_mask;
extern char *username;
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 0e6e232..967b836 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -39,9 +39,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
-/* XXX: suppress those select globals */
-fd_set *global_readfds, *global_writefds, *global_xfds;
-
u_int curtime;
static u_int time_fasttimo, last_slowtimo;
static int do_slowtimo;
@@ -261,7 +258,6 @@ void slirp_cleanup(Slirp *slirp)
#define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
#define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
-#define UPD_NFDS(x) if (nfds < (x)) nfds = (x)
void slirp_update_timeout(uint32_t *timeout)
{
@@ -270,23 +266,15 @@ void slirp_update_timeout(uint32_t *timeout)
}
}
-void slirp_select_fill(int *pnfds,
- fd_set *readfds, fd_set *writefds, fd_set *xfds)
+void slirp_pollfds_fill(GArray *pollfds)
{
Slirp *slirp;
struct socket *so, *so_next;
- int nfds;
if (QTAILQ_EMPTY(&slirp_instances)) {
return;
}
- /* fail safe */
- global_readfds = NULL;
- global_writefds = NULL;
- global_xfds = NULL;
-
- nfds = *pnfds;
/*
* First, TCP sockets
*/
@@ -302,8 +290,12 @@ void slirp_select_fill(int *pnfds,
for (so = slirp->tcb.so_next; so != &slirp->tcb;
so = so_next) {
+ int events = 0;
+
so_next = so->so_next;
+ so->pollfds_idx = -1;
+
/*
* See if we need a tcp_fasttimo
*/
@@ -321,8 +313,12 @@ void slirp_select_fill(int *pnfds,
* Set for reading sockets which are accepting
*/
if (so->so_state & SS_FACCEPTCONN) {
- FD_SET(so->s, readfds);
- UPD_NFDS(so->s);
+ GPollFD pfd = {
+ .fd = so->s,
+ .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
+ };
+ so->pollfds_idx = pollfds->len;
+ g_array_append_val(pollfds, pfd);
continue;
}
@@ -330,8 +326,12 @@ void slirp_select_fill(int *pnfds,
* Set for writing sockets which are connecting
*/
if (so->so_state & SS_ISFCONNECTING) {
- FD_SET(so->s, writefds);
- UPD_NFDS(so->s);
+ GPollFD pfd = {
+ .fd = so->s,
+ .events = G_IO_OUT | G_IO_ERR,
+ };
+ so->pollfds_idx = pollfds->len;
+ g_array_append_val(pollfds, pfd);
continue;
}
@@ -340,8 +340,7 @@ void slirp_select_fill(int *pnfds,
* we have something to send
*/
if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
- FD_SET(so->s, writefds);
- UPD_NFDS(so->s);
+ events |= G_IO_OUT | G_IO_ERR;
}
/*
@@ -349,9 +348,16 @@ void slirp_select_fill(int *pnfds,
* receive more, and we have room for it XXX /2 ?
*/
if (CONN_CANFRCV(so) && (so->so_snd.sb_cc < (so->so_snd.sb_datalen/2))) {
- FD_SET(so->s, readfds);
- FD_SET(so->s, xfds);
- UPD_NFDS(so->s);
+ events |= G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI;
+ }
+
+ if (events) {
+ GPollFD pfd = {
+ .fd = so->s,
+ .events = events,
+ };
+ so->pollfds_idx = pollfds->len;
+ g_array_append_val(pollfds, pfd);
}
}
@@ -362,6 +368,8 @@ void slirp_select_fill(int *pnfds,
so = so_next) {
so_next = so->so_next;
+ so->pollfds_idx = -1;
+
/*
* See if it's timed out
*/
@@ -384,8 +392,12 @@ void slirp_select_fill(int *pnfds,
* (XXX <= 4 ?)
*/
if ((so->so_state & SS_ISFCONNECTED) && so->so_queued <= 4) {
- FD_SET(so->s, readfds);
- UPD_NFDS(so->s);
+ GPollFD pfd = {
+ .fd = so->s,
+ .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
+ };
+ so->pollfds_idx = pollfds->len;
+ g_array_append_val(pollfds, pfd);
}
}
@@ -396,6 +408,8 @@ void slirp_select_fill(int *pnfds,
so = so_next) {
so_next = so->so_next;
+ so->pollfds_idx = -1;
+
/*
* See if it's timed out
*/
@@ -409,17 +423,18 @@ void slirp_select_fill(int *pnfds,
}
if (so->so_state & SS_ISFCONNECTED) {
- FD_SET(so->s, readfds);
- UPD_NFDS(so->s);
+ GPollFD pfd = {
+ .fd = so->s,
+ .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
+ };
+ so->pollfds_idx = pollfds->len;
+ g_array_append_val(pollfds, pfd);
}
}
}
-
- *pnfds = nfds;
}
-void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
- int select_error)
+void slirp_pollfds_poll(GArray *pollfds, int select_error)
{
Slirp *slirp;
struct socket *so, *so_next;
@@ -429,10 +444,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
return;
}
- global_readfds = readfds;
- global_writefds = writefds;
- global_xfds = xfds;
-
curtime = qemu_get_clock_ms(rt_clock);
QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
@@ -458,26 +469,32 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
*/
for (so = slirp->tcb.so_next; so != &slirp->tcb;
so = so_next) {
+ int revents;
+
so_next = so->so_next;
- /*
- * FD_ISSET is meaningless on these sockets
- * (and they can crash the program)
- */
- if (so->so_state & SS_NOFDREF || so->s == -1)
+ revents = 0;
+ if (so->pollfds_idx != -1) {
+ revents = g_array_index(pollfds, GPollFD,
+ so->pollfds_idx).revents;
+ }
+
+ if (so->so_state & SS_NOFDREF || so->s == -1) {
continue;
+ }
/*
* Check for URG data
* This will soread as well, so no need to
- * test for readfds below if this succeeds
+ * test for G_IO_IN below if this succeeds
*/
- if (FD_ISSET(so->s, xfds))
+ if (revents & G_IO_PRI) {
sorecvoob(so);
+ }
/*
* Check sockets for reading
*/
- else if (FD_ISSET(so->s, readfds)) {
+ else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
/*
* Check for incoming connections
*/
@@ -495,7 +512,8 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
/*
* Check sockets for writing
*/
- if (FD_ISSET(so->s, writefds)) {
+ if (!(so->so_state & SS_NOFDREF) &&
+ (revents & (G_IO_OUT | G_IO_ERR))) {
/*
* Check for non-blocking, still-connecting sockets
*/
@@ -576,9 +594,17 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
*/
for (so = slirp->udb.so_next; so != &slirp->udb;
so = so_next) {
+ int revents;
+
so_next = so->so_next;
- if (so->s != -1 && FD_ISSET(so->s, readfds)) {
+ revents = 0;
+ if (so->pollfds_idx != -1) {
+ revents = g_array_index(pollfds, GPollFD,
+ so->pollfds_idx).revents;
+ }
+
+ if (so->s != -1 && (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
sorecvfrom(so);
}
}
@@ -588,9 +614,18 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
*/
for (so = slirp->icmp.so_next; so != &slirp->icmp;
so = so_next) {
- so_next = so->so_next;
+ int revents;
- if (so->s != -1 && FD_ISSET(so->s, readfds)) {
+ so_next = so->so_next;
+
+ revents = 0;
+ if (so->pollfds_idx != -1) {
+ revents = g_array_index(pollfds, GPollFD,
+ so->pollfds_idx).revents;
+ }
+
+ if (so->s != -1 &&
+ (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
icmp_receive(so);
}
}
@@ -598,15 +633,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
if_start(slirp);
}
-
- /* clear global file descriptor sets.
- * these reside on the stack in vl.c
- * so they're unusable if we're not in
- * slirp_select_fill or slirp_select_poll.
- */
- global_readfds = NULL;
- global_writefds = NULL;
- global_xfds = NULL;
}
static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
diff --git a/slirp/socket.c b/slirp/socket.c
index 77b0c98..a7ab933 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -680,9 +680,6 @@ sofcantrcvmore(struct socket *so)
{
if ((so->so_state & SS_NOFDREF) == 0) {
shutdown(so->s,0);
- if(global_writefds) {
- FD_CLR(so->s,global_writefds);
- }
}
so->so_state &= ~(SS_ISFCONNECTING);
if (so->so_state & SS_FCANTSENDMORE) {
@@ -698,12 +695,6 @@ sofcantsendmore(struct socket *so)
{
if ((so->so_state & SS_NOFDREF) == 0) {
shutdown(so->s,1); /* send FIN to fhost */
- if (global_readfds) {
- FD_CLR(so->s,global_readfds);
- }
- if (global_xfds) {
- FD_CLR(so->s,global_xfds);
- }
}
so->so_state &= ~(SS_ISFCONNECTING);
if (so->so_state & SS_FCANTRCVMORE) {
diff --git a/slirp/socket.h b/slirp/socket.h
index 857b0da..57e0407 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -20,6 +20,8 @@ struct socket {
int s; /* The actual socket */
+ int pollfds_idx; /* GPollFD GArray index */
+
Slirp *slirp; /* managing slirp instance */
/* XXX union these with not-yet-used sbuf params */
diff --git a/stubs/slirp.c b/stubs/slirp.c
index 9a3309a..f1fc833 100644
--- a/stubs/slirp.c
+++ b/stubs/slirp.c
@@ -5,13 +5,11 @@ void slirp_update_timeout(uint32_t *timeout)
{
}
-void slirp_select_fill(int *pnfds, fd_set *readfds,
- fd_set *writefds, fd_set *xfds)
+void slirp_pollfds_fill(GArray *pollfds)
{
}
-void slirp_select_poll(fd_set *readfds, fd_set *writefds,
- fd_set *xfds, int select_error)
+void slirp_pollfds_poll(GArray *pollfds, int select_error)
{
}
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 5/9] iohandler: switch to GPollFD
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
` (3 preceding siblings ...)
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 4/9] slirp: switch " Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 6/9] main-loop: drop rfds/wfds/xfds for good Stefan Hajnoczi
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
Convert iohandler_select_fill() and iohandler_select_poll() to use
GPollFD instead of rfds/wfds/xfds.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/qemu/main-loop.h | 4 ++--
iohandler.c | 40 ++++++++++++++++++++++++++++++----------
main-loop.c | 4 ++--
3 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index e8059c3..0995288 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -297,8 +297,8 @@ void qemu_mutex_unlock_iothread(void);
/* internal interfaces */
void qemu_fd_register(int fd);
-void qemu_iohandler_fill(int *pnfds, fd_set *readfds, fd_set *writefds, fd_set *xfds);
-void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int rc);
+void qemu_iohandler_fill(GArray *pollfds);
+void qemu_iohandler_poll(GArray *pollfds, int rc);
QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
void qemu_bh_schedule_idle(QEMUBH *bh);
diff --git a/iohandler.c b/iohandler.c
index 2523adc..ae2ef8f 100644
--- a/iohandler.c
+++ b/iohandler.c
@@ -39,6 +39,7 @@ typedef struct IOHandlerRecord {
void *opaque;
QLIST_ENTRY(IOHandlerRecord) next;
int fd;
+ int pollfds_idx;
bool deleted;
} IOHandlerRecord;
@@ -78,6 +79,7 @@ int qemu_set_fd_handler2(int fd,
ioh->fd_read = fd_read;
ioh->fd_write = fd_write;
ioh->opaque = opaque;
+ ioh->pollfds_idx = -1;
ioh->deleted = 0;
qemu_notify_event();
}
@@ -92,38 +94,56 @@ int qemu_set_fd_handler(int fd,
return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
}
-void qemu_iohandler_fill(int *pnfds, fd_set *readfds, fd_set *writefds, fd_set *xfds)
+void qemu_iohandler_fill(GArray *pollfds)
{
IOHandlerRecord *ioh;
QLIST_FOREACH(ioh, &io_handlers, next) {
+ int events = 0;
+
if (ioh->deleted)
continue;
if (ioh->fd_read &&
(!ioh->fd_read_poll ||
ioh->fd_read_poll(ioh->opaque) != 0)) {
- FD_SET(ioh->fd, readfds);
- if (ioh->fd > *pnfds)
- *pnfds = ioh->fd;
+ events |= G_IO_IN | G_IO_HUP | G_IO_ERR;
}
if (ioh->fd_write) {
- FD_SET(ioh->fd, writefds);
- if (ioh->fd > *pnfds)
- *pnfds = ioh->fd;
+ events |= G_IO_OUT | G_IO_ERR;
+ }
+ if (events) {
+ GPollFD pfd = {
+ .fd = ioh->fd,
+ .events = events,
+ };
+ ioh->pollfds_idx = pollfds->len;
+ g_array_append_val(pollfds, pfd);
+ } else {
+ ioh->pollfds_idx = -1;
}
}
}
-void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int ret)
+void qemu_iohandler_poll(GArray *pollfds, int ret)
{
if (ret > 0) {
IOHandlerRecord *pioh, *ioh;
QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
- if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, readfds)) {
+ int revents = 0;
+
+ if (!ioh->deleted && ioh->pollfds_idx != -1) {
+ GPollFD *pfd = &g_array_index(pollfds, GPollFD,
+ ioh->pollfds_idx);
+ revents = pfd->revents;
+ }
+
+ if (!ioh->deleted && ioh->fd_read &&
+ (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
ioh->fd_read(ioh->opaque);
}
- if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, writefds)) {
+ if (!ioh->deleted && ioh->fd_write &&
+ (revents & (G_IO_OUT | G_IO_ERR))) {
ioh->fd_write(ioh->opaque);
}
diff --git a/main-loop.c b/main-loop.c
index 49e97ff..313f369 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -505,9 +505,9 @@ int main_loop_wait(int nonblocking)
slirp_update_timeout(&timeout);
slirp_pollfds_fill(gpollfds);
#endif
- qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds);
+ qemu_iohandler_fill(gpollfds);
ret = os_host_main_loop_wait(timeout);
- qemu_iohandler_poll(&rfds, &wfds, &xfds, ret);
+ qemu_iohandler_poll(gpollfds, ret);
#ifdef CONFIG_SLIRP
slirp_pollfds_poll(gpollfds, (ret < 0));
#endif
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 6/9] main-loop: drop rfds/wfds/xfds for good
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
` (4 preceding siblings ...)
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 5/9] iohandler: " Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 7/9] aio: extract aio_dispatch() from aio_poll() Stefan Hajnoczi
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
Now that all *_fill() and *_poll() functions use GPollFD we no longer
need rfds/wfds/xfds or pollfds_from_select()/pollfds_to_select().
>From now on everything uses GPollFD.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
main-loop.c | 77 ++-----------------------------------------------------------
1 file changed, 2 insertions(+), 75 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index 313f369..587cc30 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -143,66 +143,8 @@ int qemu_init_main_loop(void)
return 0;
}
-static fd_set rfds, wfds, xfds;
-static int nfds;
static int max_priority;
-/* Load rfds/wfds/xfds into gpollfds. Will be removed a few commits later. */
-static void gpollfds_from_select(void)
-{
- int fd;
- for (fd = 0; fd <= nfds; fd++) {
- int events = 0;
- if (FD_ISSET(fd, &rfds)) {
- events |= G_IO_IN | G_IO_HUP | G_IO_ERR;
- }
- if (FD_ISSET(fd, &wfds)) {
- events |= G_IO_OUT | G_IO_ERR;
- }
- if (FD_ISSET(fd, &xfds)) {
- events |= G_IO_PRI;
- }
- if (events) {
- GPollFD pfd = {
- .fd = fd,
- .events = events,
- };
- g_array_append_val(gpollfds, pfd);
- }
- }
-}
-
-/* Store gpollfds revents into rfds/wfds/xfds. Will be removed a few commits
- * later.
- */
-static void gpollfds_to_select(int ret)
-{
- int i;
-
- FD_ZERO(&rfds);
- FD_ZERO(&wfds);
- FD_ZERO(&xfds);
-
- if (ret <= 0) {
- return;
- }
-
- for (i = 0; i < gpollfds->len; i++) {
- int fd = g_array_index(gpollfds, GPollFD, i).fd;
- int revents = g_array_index(gpollfds, GPollFD, i).revents;
-
- if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
- FD_SET(fd, &rfds);
- }
- if (revents & (G_IO_OUT | G_IO_ERR)) {
- FD_SET(fd, &wfds);
- }
- if (revents & G_IO_PRI) {
- FD_SET(fd, &xfds);
- }
- }
-}
-
#ifndef _WIN32
static int glib_pollfds_idx;
static int glib_n_poll_fds;
@@ -251,15 +193,8 @@ static int os_host_main_loop_wait(uint32_t timeout)
qemu_mutex_unlock_iothread();
}
- /* We'll eventually drop fd_set completely. But for now we still have
- * *_fill() and *_poll() functions that use rfds/wfds/xfds.
- */
- gpollfds_from_select();
-
ret = g_poll((GPollFD *)gpollfds->data, gpollfds->len, timeout);
- gpollfds_to_select(ret);
-
if (timeout > 0) {
qemu_mutex_lock_iothread();
}
@@ -417,6 +352,8 @@ static int os_host_main_loop_wait(uint32_t timeout)
WaitObjects *w = &wait_objects;
gint poll_timeout;
static struct timeval tv0;
+ fd_set rfds, wfds, xfds;
+ int nfds;
/* XXX: need to suppress polling by better using win32 events */
ret = 0;
@@ -463,10 +400,6 @@ static int os_host_main_loop_wait(uint32_t timeout)
* improve socket latency.
*/
- /* This back-and-forth between GPollFDs and select(2) is temporary. We'll
- * drop it in a couple of patches, I promise :).
- */
- gpollfds_from_select();
FD_ZERO(&rfds);
FD_ZERO(&wfds);
FD_ZERO(&xfds);
@@ -478,7 +411,6 @@ static int os_host_main_loop_wait(uint32_t timeout)
pollfds_poll(gpollfds, nfds, &rfds, &wfds, &xfds);
}
}
- gpollfds_to_select(select_ret || g_poll_ret);
return select_ret || g_poll_ret;
}
@@ -496,11 +428,6 @@ int main_loop_wait(int nonblocking)
/* poll any events */
g_array_set_size(gpollfds, 0); /* reset for new iteration */
/* XXX: separate device handlers from system ones */
- nfds = -1;
- FD_ZERO(&rfds);
- FD_ZERO(&wfds);
- FD_ZERO(&xfds);
-
#ifdef CONFIG_SLIRP
slirp_update_timeout(&timeout);
slirp_pollfds_fill(gpollfds);
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 7/9] aio: extract aio_dispatch() from aio_poll()
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
` (5 preceding siblings ...)
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 6/9] main-loop: drop rfds/wfds/xfds for good Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 8/9] aio: convert aio_poll() to g_poll(3) Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 9/9] aio: support G_IO_HUP and G_IO_ERR Stefan Hajnoczi
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
We will need to loop over AioHandlers calling ->io_read()/->io_write()
when aio_poll() is converted from select(2) to g_poll(2).
Luckily the code for this already exists, extract it into the new
aio_dispatch() function.
Two small changes:
* aio_poll() checks !node->deleted to avoid calling handlers that have
been deleted.
* Fix typo 'then' -> 'them' in aio_poll() comment.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
aio-posix.c | 57 +++++++++++++++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index fe4dbb4..35131a3 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -129,30 +129,12 @@ bool aio_pending(AioContext *ctx)
return false;
}
-bool aio_poll(AioContext *ctx, bool blocking)
+static bool aio_dispatch(AioContext *ctx)
{
- static struct timeval tv0;
AioHandler *node;
- fd_set rdfds, wrfds;
- int max_fd = -1;
- int ret;
- bool busy, progress;
-
- progress = false;
-
- /*
- * If there are callbacks left that have been queued, we need to call then.
- * Do not call select in this case, because it is possible that the caller
- * does not need a complete flush (as is the case for qemu_aio_wait loops).
- */
- if (aio_bh_poll(ctx)) {
- blocking = false;
- progress = true;
- }
+ bool progress = false;
/*
- * Then dispatch any pending callbacks from the GSource.
- *
* We have to walk very carefully in case qemu_aio_set_fd_handler is
* called while we're walking.
*/
@@ -167,11 +149,15 @@ bool aio_poll(AioContext *ctx, bool blocking)
node->pfd.revents = 0;
/* See comment in aio_pending. */
- if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR) && node->io_read) {
+ if (!node->deleted &&
+ (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) &&
+ node->io_read) {
node->io_read(node->opaque);
progress = true;
}
- if (revents & (G_IO_OUT | G_IO_ERR) && node->io_write) {
+ if (!node->deleted &&
+ (revents & (G_IO_OUT | G_IO_ERR)) &&
+ node->io_write) {
node->io_write(node->opaque);
progress = true;
}
@@ -186,6 +172,33 @@ bool aio_poll(AioContext *ctx, bool blocking)
g_free(tmp);
}
}
+ return progress;
+}
+
+bool aio_poll(AioContext *ctx, bool blocking)
+{
+ static struct timeval tv0;
+ AioHandler *node;
+ fd_set rdfds, wrfds;
+ int max_fd = -1;
+ int ret;
+ bool busy, progress;
+
+ progress = false;
+
+ /*
+ * If there are callbacks left that have been queued, we need to call them.
+ * Do not call select in this case, because it is possible that the caller
+ * does not need a complete flush (as is the case for qemu_aio_wait loops).
+ */
+ if (aio_bh_poll(ctx)) {
+ blocking = false;
+ progress = true;
+ }
+
+ if (aio_dispatch(ctx)) {
+ progress = true;
+ }
if (progress && !blocking) {
return true;
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 8/9] aio: convert aio_poll() to g_poll(3)
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
` (6 preceding siblings ...)
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 7/9] aio: extract aio_dispatch() from aio_poll() Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 9/9] aio: support G_IO_HUP and G_IO_ERR Stefan Hajnoczi
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
AioHandler already has a GPollFD so we can directly use its
events/revents.
Add the int pollfds_idx field to AioContext so we can map g_poll(3)
results back to AioHandlers.
Reuse aio_dispatch() to invoke handlers after g_poll(3).
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
aio-posix.c | 67 +++++++++++++++++++----------------------------------
async.c | 2 ++
include/block/aio.h | 3 +++
3 files changed, 29 insertions(+), 43 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index 35131a3..7769927 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -25,6 +25,7 @@ struct AioHandler
IOHandler *io_write;
AioFlushHandler *io_flush;
int deleted;
+ int pollfds_idx;
void *opaque;
QLIST_ENTRY(AioHandler) node;
};
@@ -85,6 +86,7 @@ void aio_set_fd_handler(AioContext *ctx,
node->io_write = io_write;
node->io_flush = io_flush;
node->opaque = opaque;
+ node->pollfds_idx = -1;
node->pfd.events = (io_read ? G_IO_IN | G_IO_HUP : 0);
node->pfd.events |= (io_write ? G_IO_OUT : 0);
@@ -177,10 +179,7 @@ static bool aio_dispatch(AioContext *ctx)
bool aio_poll(AioContext *ctx, bool blocking)
{
- static struct timeval tv0;
AioHandler *node;
- fd_set rdfds, wrfds;
- int max_fd = -1;
int ret;
bool busy, progress;
@@ -206,12 +205,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
ctx->walking_handlers++;
- FD_ZERO(&rdfds);
- FD_ZERO(&wrfds);
+ g_array_set_size(ctx->pollfds, 0);
- /* fill fd sets */
+ /* fill pollfds */
busy = false;
QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+ node->pollfds_idx = -1;
+
/* If there aren't pending AIO operations, don't invoke callbacks.
* Otherwise, if there are no AIO requests, qemu_aio_wait() would
* wait indefinitely.
@@ -222,13 +222,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
}
busy = true;
}
- if (!node->deleted && node->io_read) {
- FD_SET(node->pfd.fd, &rdfds);
- max_fd = MAX(max_fd, node->pfd.fd + 1);
- }
- if (!node->deleted && node->io_write) {
- FD_SET(node->pfd.fd, &wrfds);
- max_fd = MAX(max_fd, node->pfd.fd + 1);
+ if (!node->deleted && node->pfd.events) {
+ GPollFD pfd = {
+ .fd = node->pfd.fd,
+ .events = node->pfd.events,
+ };
+ node->pollfds_idx = ctx->pollfds->len;
+ g_array_append_val(ctx->pollfds, pfd);
}
}
@@ -240,41 +240,22 @@ bool aio_poll(AioContext *ctx, bool blocking)
}
/* wait until next event */
- ret = select(max_fd, &rdfds, &wrfds, NULL, blocking ? NULL : &tv0);
+ ret = g_poll((GPollFD *)ctx->pollfds->data,
+ ctx->pollfds->len,
+ blocking ? -1 : 0);
/* if we have any readable fds, dispatch event */
if (ret > 0) {
- /* we have to walk very carefully in case
- * qemu_aio_set_fd_handler is called while we're walking */
- node = QLIST_FIRST(&ctx->aio_handlers);
- while (node) {
- AioHandler *tmp;
-
- ctx->walking_handlers++;
-
- if (!node->deleted &&
- FD_ISSET(node->pfd.fd, &rdfds) &&
- node->io_read) {
- node->io_read(node->opaque);
- progress = true;
- }
- if (!node->deleted &&
- FD_ISSET(node->pfd.fd, &wrfds) &&
- node->io_write) {
- node->io_write(node->opaque);
- progress = true;
- }
-
- tmp = node;
- node = QLIST_NEXT(node, node);
-
- ctx->walking_handlers--;
-
- if (!ctx->walking_handlers && tmp->deleted) {
- QLIST_REMOVE(tmp, node);
- g_free(tmp);
+ QLIST_FOREACH(node, &ctx->aio_handlers, node) {
+ if (node->pollfds_idx != -1) {
+ GPollFD *pfd = &g_array_index(ctx->pollfds, GPollFD,
+ node->pollfds_idx);
+ node->pfd.revents |= pfd->revents;
}
}
+ if (aio_dispatch(ctx)) {
+ progress = true;
+ }
}
assert(progress || busy);
diff --git a/async.c b/async.c
index 72d268a..f2d47ba 100644
--- a/async.c
+++ b/async.c
@@ -174,6 +174,7 @@ aio_ctx_finalize(GSource *source)
aio_set_event_notifier(ctx, &ctx->notifier, NULL, NULL);
event_notifier_cleanup(&ctx->notifier);
+ g_array_free(ctx->pollfds, TRUE);
}
static GSourceFuncs aio_source_funcs = {
@@ -198,6 +199,7 @@ AioContext *aio_context_new(void)
{
AioContext *ctx;
ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
+ ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
event_notifier_init(&ctx->notifier, false);
aio_set_event_notifier(ctx, &ctx->notifier,
(EventNotifierHandler *)
diff --git a/include/block/aio.h b/include/block/aio.h
index 8eda924..5b54d38 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -63,6 +63,9 @@ typedef struct AioContext {
/* Used for aio_notify. */
EventNotifier notifier;
+
+ /* GPollFDs for aio_poll() */
+ GArray *pollfds;
} AioContext;
/* Returns 1 if there are still outstanding AIO requests; 0 otherwise */
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH v2 9/9] aio: support G_IO_HUP and G_IO_ERR
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
` (7 preceding siblings ...)
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 8/9] aio: convert aio_poll() to g_poll(3) Stefan Hajnoczi
@ 2013-02-01 13:53 ` Stefan Hajnoczi
8 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2013-02-01 13:53 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony Liguori, Jan Kiszka, Fabien Chouteau, Stefan Hajnoczi,
Paolo Bonzini, Amos Kong
aio-posix.c could not take advantage of G_IO_HUP and G_IO_ERR because
select(2) does not have equivalent events. Now that g_poll(3) is used
we can support G_IO_HUP and G_IO_ERR.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
aio-posix.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/aio-posix.c b/aio-posix.c
index 7769927..1c5e601 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -88,8 +88,8 @@ void aio_set_fd_handler(AioContext *ctx,
node->opaque = opaque;
node->pollfds_idx = -1;
- node->pfd.events = (io_read ? G_IO_IN | G_IO_HUP : 0);
- node->pfd.events |= (io_write ? G_IO_OUT : 0);
+ node->pfd.events = (io_read ? G_IO_IN | G_IO_HUP | G_IO_ERR : 0);
+ node->pfd.events |= (io_write ? G_IO_OUT | G_IO_ERR : 0);
}
aio_notify(ctx);
@@ -112,13 +112,6 @@ bool aio_pending(AioContext *ctx)
QLIST_FOREACH(node, &ctx->aio_handlers, node) {
int revents;
- /*
- * FIXME: right now we cannot get G_IO_HUP and G_IO_ERR because
- * main-loop.c is still select based (due to the slirp legacy).
- * If main-loop.c ever switches to poll, G_IO_ERR should be
- * tested too. Dispatching G_IO_ERR to both handlers should be
- * okay, since handlers need to be ready for spurious wakeups.
- */
revents = node->pfd.revents & node->pfd.events;
if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR) && node->io_read) {
return true;
@@ -150,7 +143,6 @@ static bool aio_dispatch(AioContext *ctx)
revents = node->pfd.revents & node->pfd.events;
node->pfd.revents = 0;
- /* See comment in aio_pending. */
if (!node->deleted &&
(revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) &&
node->io_read) {
--
1.8.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/9] slirp: switch to GPollFD
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 4/9] slirp: switch " Stefan Hajnoczi
@ 2013-02-02 12:46 ` Blue Swirl
0 siblings, 0 replies; 11+ messages in thread
From: Blue Swirl @ 2013-02-02 12:46 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Anthony Liguori, Jan Kiszka, qemu-devel, Fabien Chouteau,
Paolo Bonzini, Amos Kong
On Fri, Feb 1, 2013 at 1:53 PM, Stefan Hajnoczi <stefanha@redhat.com> 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.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> main-loop.c | 4 +-
> slirp/libslirp.h | 6 +--
> slirp/main.h | 1 -
> slirp/slirp.c | 136 +++++++++++++++++++++++++++++++++----------------------
> slirp/socket.c | 9 ----
> slirp/socket.h | 2 +
> stubs/slirp.c | 6 +--
> 7 files changed, 89 insertions(+), 75 deletions(-)
>
> diff --git a/main-loop.c b/main-loop.c
> index 12b0213..49e97ff 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -503,13 +503,13 @@ int main_loop_wait(int nonblocking)
>
> #ifdef CONFIG_SLIRP
> slirp_update_timeout(&timeout);
> - slirp_select_fill(&nfds, &rfds, &wfds, &xfds);
> + slirp_pollfds_fill(gpollfds);
> #endif
> qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds);
> ret = os_host_main_loop_wait(timeout);
> qemu_iohandler_poll(&rfds, &wfds, &xfds, ret);
> #ifdef CONFIG_SLIRP
> - slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));
> + slirp_pollfds_poll(gpollfds, (ret < 0));
> #endif
>
> qemu_run_all_timers();
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index 49609c2..ceabff8 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -17,11 +17,9 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
> void slirp_cleanup(Slirp *slirp);
>
> void slirp_update_timeout(uint32_t *timeout);
> -void slirp_select_fill(int *pnfds,
> - fd_set *readfds, fd_set *writefds, fd_set *xfds);
> +void slirp_pollfds_fill(GArray *pollfds);
>
> -void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
> - int select_error);
> +void slirp_pollfds_poll(GArray *pollfds, int select_error);
>
> void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
>
> diff --git a/slirp/main.h b/slirp/main.h
> index 66e4f92..f2e58cf 100644
> --- a/slirp/main.h
> +++ b/slirp/main.h
> @@ -31,7 +31,6 @@ extern int ctty_closed;
> extern char *slirp_tty;
> extern char *exec_shell;
> extern u_int curtime;
> -extern fd_set *global_readfds, *global_writefds, *global_xfds;
> extern struct in_addr loopback_addr;
> extern unsigned long loopback_mask;
> extern char *username;
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 0e6e232..967b836 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -39,9 +39,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
>
> static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
>
> -/* XXX: suppress those select globals */
> -fd_set *global_readfds, *global_writefds, *global_xfds;
> -
> u_int curtime;
> static u_int time_fasttimo, last_slowtimo;
> static int do_slowtimo;
> @@ -261,7 +258,6 @@ void slirp_cleanup(Slirp *slirp)
>
> #define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
> #define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
> -#define UPD_NFDS(x) if (nfds < (x)) nfds = (x)
>
> void slirp_update_timeout(uint32_t *timeout)
> {
> @@ -270,23 +266,15 @@ void slirp_update_timeout(uint32_t *timeout)
> }
> }
>
> -void slirp_select_fill(int *pnfds,
> - fd_set *readfds, fd_set *writefds, fd_set *xfds)
> +void slirp_pollfds_fill(GArray *pollfds)
> {
> Slirp *slirp;
> struct socket *so, *so_next;
> - int nfds;
>
> if (QTAILQ_EMPTY(&slirp_instances)) {
> return;
> }
>
> - /* fail safe */
> - global_readfds = NULL;
> - global_writefds = NULL;
> - global_xfds = NULL;
> -
> - nfds = *pnfds;
> /*
> * First, TCP sockets
> */
> @@ -302,8 +290,12 @@ void slirp_select_fill(int *pnfds,
>
> for (so = slirp->tcb.so_next; so != &slirp->tcb;
> so = so_next) {
> + int events = 0;
> +
> so_next = so->so_next;
>
> + so->pollfds_idx = -1;
> +
> /*
> * See if we need a tcp_fasttimo
> */
> @@ -321,8 +313,12 @@ void slirp_select_fill(int *pnfds,
> * Set for reading sockets which are accepting
> */
> if (so->so_state & SS_FACCEPTCONN) {
> - FD_SET(so->s, readfds);
> - UPD_NFDS(so->s);
> + GPollFD pfd = {
> + .fd = so->s,
> + .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
> + };
> + so->pollfds_idx = pollfds->len;
> + g_array_append_val(pollfds, pfd);
> continue;
> }
>
> @@ -330,8 +326,12 @@ void slirp_select_fill(int *pnfds,
> * Set for writing sockets which are connecting
> */
> if (so->so_state & SS_ISFCONNECTING) {
> - FD_SET(so->s, writefds);
> - UPD_NFDS(so->s);
> + GPollFD pfd = {
> + .fd = so->s,
> + .events = G_IO_OUT | G_IO_ERR,
> + };
> + so->pollfds_idx = pollfds->len;
> + g_array_append_val(pollfds, pfd);
> continue;
> }
>
> @@ -340,8 +340,7 @@ void slirp_select_fill(int *pnfds,
> * we have something to send
> */
> if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
> - FD_SET(so->s, writefds);
> - UPD_NFDS(so->s);
> + events |= G_IO_OUT | G_IO_ERR;
> }
>
> /*
> @@ -349,9 +348,16 @@ void slirp_select_fill(int *pnfds,
> * receive more, and we have room for it XXX /2 ?
> */
> if (CONN_CANFRCV(so) && (so->so_snd.sb_cc < (so->so_snd.sb_datalen/2))) {
> - FD_SET(so->s, readfds);
> - FD_SET(so->s, xfds);
> - UPD_NFDS(so->s);
> + events |= G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI;
> + }
> +
> + if (events) {
> + GPollFD pfd = {
> + .fd = so->s,
> + .events = events,
> + };
> + so->pollfds_idx = pollfds->len;
> + g_array_append_val(pollfds, pfd);
> }
> }
>
> @@ -362,6 +368,8 @@ void slirp_select_fill(int *pnfds,
> so = so_next) {
> so_next = so->so_next;
>
> + so->pollfds_idx = -1;
> +
> /*
> * See if it's timed out
> */
> @@ -384,8 +392,12 @@ void slirp_select_fill(int *pnfds,
> * (XXX <= 4 ?)
> */
> if ((so->so_state & SS_ISFCONNECTED) && so->so_queued <= 4) {
> - FD_SET(so->s, readfds);
> - UPD_NFDS(so->s);
> + GPollFD pfd = {
> + .fd = so->s,
> + .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
> + };
> + so->pollfds_idx = pollfds->len;
> + g_array_append_val(pollfds, pfd);
> }
> }
>
> @@ -396,6 +408,8 @@ void slirp_select_fill(int *pnfds,
> so = so_next) {
> so_next = so->so_next;
>
> + so->pollfds_idx = -1;
> +
> /*
> * See if it's timed out
> */
> @@ -409,17 +423,18 @@ void slirp_select_fill(int *pnfds,
> }
>
> if (so->so_state & SS_ISFCONNECTED) {
> - FD_SET(so->s, readfds);
> - UPD_NFDS(so->s);
> + GPollFD pfd = {
> + .fd = so->s,
> + .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
> + };
> + so->pollfds_idx = pollfds->len;
> + g_array_append_val(pollfds, pfd);
> }
> }
> }
> -
> - *pnfds = nfds;
> }
>
> -void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
> - int select_error)
> +void slirp_pollfds_poll(GArray *pollfds, int select_error)
> {
> Slirp *slirp;
> struct socket *so, *so_next;
> @@ -429,10 +444,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
> return;
> }
>
> - global_readfds = readfds;
> - global_writefds = writefds;
> - global_xfds = xfds;
> -
> curtime = qemu_get_clock_ms(rt_clock);
>
> QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
> @@ -458,26 +469,32 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
> */
> for (so = slirp->tcb.so_next; so != &slirp->tcb;
> so = so_next) {
> + int revents;
> +
> so_next = so->so_next;
>
> - /*
> - * FD_ISSET is meaningless on these sockets
> - * (and they can crash the program)
> - */
> - if (so->so_state & SS_NOFDREF || so->s == -1)
> + revents = 0;
> + if (so->pollfds_idx != -1) {
> + revents = g_array_index(pollfds, GPollFD,
> + so->pollfds_idx).revents;
> + }
> +
> + if (so->so_state & SS_NOFDREF || so->s == -1) {
> continue;
> + }
>
> /*
> * Check for URG data
> * This will soread as well, so no need to
> - * test for readfds below if this succeeds
> + * test for G_IO_IN below if this succeeds
> */
> - if (FD_ISSET(so->s, xfds))
> + if (revents & G_IO_PRI) {
> sorecvoob(so);
> + }
> /*
> * Check sockets for reading
> */
> - else if (FD_ISSET(so->s, readfds)) {
> + else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
> /*
> * Check for incoming connections
> */
> @@ -495,7 +512,8 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
> /*
> * Check sockets for writing
> */
> - if (FD_ISSET(so->s, writefds)) {
> + if (!(so->so_state & SS_NOFDREF) &&
> + (revents & (G_IO_OUT | G_IO_ERR))) {
> /*
> * Check for non-blocking, still-connecting sockets
> */
> @@ -576,9 +594,17 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
> */
> for (so = slirp->udb.so_next; so != &slirp->udb;
> so = so_next) {
> + int revents;
> +
> so_next = so->so_next;
>
> - if (so->s != -1 && FD_ISSET(so->s, readfds)) {
> + revents = 0;
> + if (so->pollfds_idx != -1) {
> + revents = g_array_index(pollfds, GPollFD,
> + so->pollfds_idx).revents;
> + }
> +
> + if (so->s != -1 && (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
> sorecvfrom(so);
> }
> }
> @@ -588,9 +614,18 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
> */
> for (so = slirp->icmp.so_next; so != &slirp->icmp;
> so = so_next) {
> - so_next = so->so_next;
> + int revents;
>
> - if (so->s != -1 && FD_ISSET(so->s, readfds)) {
> + so_next = so->so_next;
> +
> + revents = 0;
> + if (so->pollfds_idx != -1) {
> + revents = g_array_index(pollfds, GPollFD,
> + so->pollfds_idx).revents;
> + }
> +
> + if (so->s != -1 &&
> + (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
> icmp_receive(so);
> }
> }
> @@ -598,15 +633,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
>
> if_start(slirp);
> }
> -
> - /* clear global file descriptor sets.
> - * these reside on the stack in vl.c
> - * so they're unusable if we're not in
> - * slirp_select_fill or slirp_select_poll.
> - */
> - global_readfds = NULL;
> - global_writefds = NULL;
> - global_xfds = NULL;
> }
>
> static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
> diff --git a/slirp/socket.c b/slirp/socket.c
> index 77b0c98..a7ab933 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -680,9 +680,6 @@ sofcantrcvmore(struct socket *so)
> {
> if ((so->so_state & SS_NOFDREF) == 0) {
> shutdown(so->s,0);
> - if(global_writefds) {
> - FD_CLR(so->s,global_writefds);
> - }
> }
> so->so_state &= ~(SS_ISFCONNECTING);
> if (so->so_state & SS_FCANTSENDMORE) {
> @@ -698,12 +695,6 @@ sofcantsendmore(struct socket *so)
> {
> if ((so->so_state & SS_NOFDREF) == 0) {
> shutdown(so->s,1); /* send FIN to fhost */
> - if (global_readfds) {
> - FD_CLR(so->s,global_readfds);
> - }
> - if (global_xfds) {
> - FD_CLR(so->s,global_xfds);
> - }
> }
> so->so_state &= ~(SS_ISFCONNECTING);
> if (so->so_state & SS_FCANTRCVMORE) {
> diff --git a/slirp/socket.h b/slirp/socket.h
> index 857b0da..57e0407 100644
> --- a/slirp/socket.h
> +++ b/slirp/socket.h
> @@ -20,6 +20,8 @@ struct socket {
>
> int s; /* The actual socket */
>
> + int pollfds_idx; /* GPollFD GArray index */
> +
> Slirp *slirp; /* managing slirp instance */
>
> /* XXX union these with not-yet-used sbuf params */
> diff --git a/stubs/slirp.c b/stubs/slirp.c
> index 9a3309a..f1fc833 100644
> --- a/stubs/slirp.c
> +++ b/stubs/slirp.c
> @@ -5,13 +5,11 @@ void slirp_update_timeout(uint32_t *timeout)
> {
> }
>
> -void slirp_select_fill(int *pnfds, fd_set *readfds,
> - fd_set *writefds, fd_set *xfds)
> +void slirp_pollfds_fill(GArray *pollfds)
> {
> }
>
> -void slirp_select_poll(fd_set *readfds, fd_set *writefds,
> - fd_set *xfds, int select_error)
> +void slirp_pollfds_poll(GArray *pollfds, int select_error)
> {
> }
>
> --
> 1.8.1
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-02-02 12:46 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-01 13:53 [Qemu-devel] [PATCH v2 0/9] main-loop: switch to g_poll(3) on POSIX hosts Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 1/9] main-loop: fix select_ret uninitialized variable warning Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 2/9] main-loop: switch to g_poll() on POSIX hosts Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 3/9] main-loop: switch POSIX glib integration to GPollFD Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 4/9] slirp: switch " Stefan Hajnoczi
2013-02-02 12:46 ` Blue Swirl
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 5/9] iohandler: " Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 6/9] main-loop: drop rfds/wfds/xfds for good Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 7/9] aio: extract aio_dispatch() from aio_poll() Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 8/9] aio: convert aio_poll() to g_poll(3) Stefan Hajnoczi
2013-02-01 13:53 ` [Qemu-devel] [PATCH v2 9/9] aio: support G_IO_HUP and G_IO_ERR Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.