Patchwork [RFC,v7,01/16] Move code related to fd handlers into utility functions

login
register
mail settings
Submitter Michael Roth
Date March 7, 2011, 8:10 p.m.
Message ID <1299528642-23631-2-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/85804/
State New
Headers show

Comments

Michael Roth - March 7, 2011, 8:10 p.m.
This allows us to implement an i/o loop outside of vl.c that can
interact with objects that use qemu_set_fd_handler()

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs |    2 +-
 qemu-char.h   |    4 ++
 qemu-ioh.c    |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-ioh.h    |   34 +++++++++++++++++
 vl.c          |   86 ++++++++----------------------------------
 5 files changed, 170 insertions(+), 71 deletions(-)
 create mode 100644 qemu-ioh.c
 create mode 100644 qemu-ioh.h
Paolo Bonzini - March 9, 2011, 1:58 p.m.
On 03/07/2011 09:10 PM, Michael Roth wrote:
> This allows us to implement an i/o loop outside of vl.c that can
> interact with objects that use qemu_set_fd_handler()

I must say I really dislike the patches 1..3.  It's _really_ getting the 
QEMU NIH worse.  While it is not really possible to get a new shiny 
mainloop infrastructure in QEMU like snapping fingers (and I'm not sure 
the glib mainloop will ever happen there), there is no reason not to 
adopt glib's infrastructure in virtagent.  While cooperation between 
QEMU and virtagent is close, it is IMHO a substantially separate project 
that can afford starting from a clean slate.

If anybody disagrees, I'd be happy to hear their opinion anyway!

I'm sorry I'm saying this only now and I've been ignoring this series 
until v7.

Paolo
Paolo Bonzini - March 9, 2011, 2:09 p.m.
On 03/07/2011 09:10 PM, Michael Roth wrote:
> +
> +/* XXX: fd_read_poll should be suppressed, but an API change is
> +   necessary in the character devices to suppress fd_can_read(). */
> +int qemu_set_fd_handler3(void *ioh_record_list,
> +                         int fd,
> +                         IOCanReadHandler *fd_read_poll,
> +                         IOHandler *fd_read,
> +                         IOHandler *fd_write,
> +                         void *opaque)

What's the reason to introduce this additional indirection (and with a 
void rather than opaque pointer)?  A global iohandlers list would be 
fine in qemu-ioh.c (and it would be a worthwhile patch anyway for QEMU).

Paolo
Michael Roth - March 9, 2011, 2:11 p.m.
On 03/09/2011 07:58 AM, Paolo Bonzini wrote:
> On 03/07/2011 09:10 PM, Michael Roth wrote:
>> This allows us to implement an i/o loop outside of vl.c that can
>> interact with objects that use qemu_set_fd_handler()
>
> I must say I really dislike the patches 1..3. It's _really_ getting the
> QEMU NIH worse. While it is not really possible to get a new shiny
> mainloop infrastructure in QEMU like snapping fingers (and I'm not sure
> the glib mainloop will ever happen there), there is no reason not to
> adopt glib's infrastructure in virtagent. While cooperation between QEMU
> and virtagent is close, it is IMHO a substantially separate project that
> can afford starting from a clean slate.
>
> If anybody disagrees, I'd be happy to hear their opinion anyway!
>
> I'm sorry I'm saying this only now and I've been ignoring this series
> until v7.

In the context of virtagent I would agree. The only complication there 
being that a large part of the event-driven code (the async read/write 
handlers for instance) is shared between virtagent and the host. 
Possibility this could be worked around with a set of wrappers..but it's 
hard to say.

But more importantly, I wouldn't think of these changes as being 
specific to virtagent though. Currently we have a lot of qemu tools that 
stub out portions of the block code they pull in (qemu_set_fd_handler 
and whatnot). I think it might be beneficial to future tools/test 
utilities that they actually be able to drive things like aio and timer 
events. We just keep stubbing more and more things out in these cases, 
which I would argue is even worse because it can place artificial 
constraints on how code is written that happens to get used by such tools.

>
> Paolo
Anthony Liguori - March 9, 2011, 2:28 p.m.
On 03/09/2011 07:58 AM, Paolo Bonzini wrote:
> On 03/07/2011 09:10 PM, Michael Roth wrote:
>> This allows us to implement an i/o loop outside of vl.c that can
>> interact with objects that use qemu_set_fd_handler()
>
> I must say I really dislike the patches 1..3.  It's _really_ getting 
> the QEMU NIH worse.  While it is not really possible to get a new 
> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm 
> not sure the glib mainloop will ever happen there), there is no reason 
> not to adopt glib's infrastructure in virtagent.

I'm 90% in agreement with you but in terms of delivering a Windows guest 
agent, instead of just having an exe, we're now talking about quite a 
few extra DLLs.  It's not a huge problem and probably makes a ton of 
sense if virt-agent ever adopts more sophisticated functionality but I 
wanted to at least raise this point.

Regards,

Anthony Liguori

>   While cooperation between QEMU and virtagent is close, it is IMHO a 
> substantially separate project that can afford starting from a clean 
> slate.
>
> If anybody disagrees, I'd be happy to hear their opinion anyway!
>
> I'm sorry I'm saying this only now and I've been ignoring this series 
> until v7.
>
> Paolo
>
>
Paolo Bonzini - March 9, 2011, 2:38 p.m.
On 03/09/2011 03:11 PM, Michael Roth wrote:
>
> In the context of virtagent I would agree. The only complication there
> being that a large part of the event-driven code (the async read/write
> handlers for instance) is shared between virtagent and the host.

What exactly?  The dependencies in 16/16 give:

qemu-tool.o qemu-error.o qemu-sockets.c $(oslib-obj-y) $(trace-obj-y)
$(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
qemu-timer.o

Compared to other tools, only qemu-sockets.c is added (and timers); 
overall it is quite self contained and interfaces well with glib's 
GIOChannels, which provide qemu_set_fd_handler-equivalent functionality.

In addition, qemu iohandlers have a lot of unwritten assumptions, for 
example on Win32 they only work with sockets and not other kinds of file 
descriptors.

Paolo
Anthony Liguori - March 9, 2011, 2:40 p.m.
On 03/09/2011 07:58 AM, Paolo Bonzini wrote:
> On 03/07/2011 09:10 PM, Michael Roth wrote:
>> This allows us to implement an i/o loop outside of vl.c that can
>> interact with objects that use qemu_set_fd_handler()
>
> I must say I really dislike the patches 1..3.  It's _really_ getting 
> the QEMU NIH worse.  While it is not really possible to get a new 
> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm 
> not sure the glib mainloop will ever happen there

While it's not at the immediate top at my MUST DO list, it's still 
pretty high FWIW.  I think the benefits are huge because it means we can 
refactor things like the VNC server to just interact with glib which 
means it can become generally useful outside of QEMU.

Regards,

Anthony Liguori

> ), there is no reason not to adopt glib's infrastructure in 
> virtagent.  While cooperation between QEMU and virtagent is close, it 
> is IMHO a substantially separate project that can afford starting from 
> a clean slate.
>
> If anybody disagrees, I'd be happy to hear their opinion anyway!
>
> I'm sorry I'm saying this only now and I've been ignoring this series 
> until v7.
>
> Paolo
Paolo Bonzini - March 9, 2011, 2:45 p.m.
On 03/09/2011 03:40 PM, Anthony Liguori wrote:
>>
>> I must say I really dislike the patches 1..3.  It's _really_ getting
>> the QEMU NIH worse.  While it is not really possible to get a new
>> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm
>> not sure the glib mainloop will ever happen there
>
> While it's not at the immediate top at my MUST DO list, it's still
> pretty high FWIW.  I think the benefits are huge because it means we can
> refactor things like the VNC server to just interact with glib which
> means it can become generally useful outside of QEMU.

I actually agree, but there are a lot of cleanups to do to the code 
before it becomes viable.  I would be surprised to see it before 0.17 
say (maybe a pleasant surprise, but still).

In any case, introducing more dependencies from the tools to core QEMU 
would mean needing wrappers over wrappers over wrappers when QEMU itself 
is refactored.

Perhaps for virtagent something like libnih would be more appropriate? 
Not sure about its Win32 portability though.

Paolo
Michael Roth - March 9, 2011, 3:01 p.m.
On 03/09/2011 08:38 AM, Paolo Bonzini wrote:
> On 03/09/2011 03:11 PM, Michael Roth wrote:
>>
>> In the context of virtagent I would agree. The only complication there
>> being that a large part of the event-driven code (the async read/write
>> handlers for instance) is shared between virtagent and the host.
>
> What exactly? The dependencies in 16/16 give:
>
> qemu-tool.o qemu-error.o qemu-sockets.c $(oslib-obj-y) $(trace-obj-y)
> $(block-obj-y) $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
> qemu-timer.o

These objs: virtagent.o virtagent-server.o virtagent-common.o 
virtagent-transport.o virtagent-manager.o

Are shared by qemu and qemu-va. virtagent.o uses the common timer 
infrastructure introduced in patch 3, and 
virtagent-transport/virtagent-common use the iohandler stuff from patch 1/2.

On the host, qemu's event loop drives them, and on the guest, qemu-va's 
event loop drives them.

Not sure what level of sharing we can maintain with 2 different event 
loops. I'm sure it's doable, just not sure what it would end up looking 
like.

I should note that initially all the qemu_set_fd_handler() stuff was 
wrapped to provide compatibility between separate event loop 
implementations in qemu/qemu-va. Sharing the event loop code was a 
widely-held consensus from earlier reviews. I'm not sure glib is so nice 
that it's worth back-peddling on that. And if we do eventually make 
qemu's event loop glib-based, consumers of the common code here would 
get migrated over for free.

>
> Compared to other tools, only qemu-sockets.c is added (and timers);
> overall it is quite self contained and interfaces well with glib's
> GIOChannels, which provide qemu_set_fd_handler-equivalent functionality.
>
> In addition, qemu iohandlers have a lot of unwritten assumptions, for
> example on Win32 they only work with sockets and not other kinds of file
> descriptors.

Hmm, that could be a problem... It seems like a more general one though, 
that might benefit consumers other than virtagent. So if this is 
addressed at some point, consumers of the common infrastructure proposed 
here would all benefit.

>
> Paolo
Paolo Bonzini - March 9, 2011, 3:15 p.m.
On 03/09/2011 04:01 PM, Michael Roth wrote:
>
> These objs: virtagent.o virtagent-server.o virtagent-common.o
> virtagent-transport.o virtagent-manager.o
>
> Are shared by qemu and qemu-va.

Okay, that's what I missed.  Then I guess it's a pity but there's a good
reason.

> It seems like a more general one though, that might benefit consumers
> other than virtagent. So if this is addressed at some point,
> consumers of the common infrastructure proposed here would all
> benefit.

I doubt, Win32 sockets are almost unusable (QEMU does "select" on them 
when it has an event from something else) and chardevs use a separate 
polling mechanism.

Paolo
Anthony Liguori - March 9, 2011, 3:39 p.m.
On 03/09/2011 08:45 AM, Paolo Bonzini wrote:
> On 03/09/2011 03:40 PM, Anthony Liguori wrote:
>>>
>>> I must say I really dislike the patches 1..3.  It's _really_ getting
>>> the QEMU NIH worse.  While it is not really possible to get a new
>>> shiny mainloop infrastructure in QEMU like snapping fingers (and I'm
>>> not sure the glib mainloop will ever happen there
>>
>> While it's not at the immediate top at my MUST DO list, it's still
>> pretty high FWIW.  I think the benefits are huge because it means we can
>> refactor things like the VNC server to just interact with glib which
>> means it can become generally useful outside of QEMU.
>
> I actually agree, but there are a lot of cleanups to do to the code 
> before it becomes viable.  I would be surprised to see it before 0.17 
> say (maybe a pleasant surprise, but still).
>
> In any case, introducing more dependencies from the tools to core QEMU 
> would mean needing wrappers over wrappers over wrappers when QEMU 
> itself is refactored.
>
> Perhaps for virtagent something like libnih would be more appropriate? 
> Not sure about its Win32 portability though.

If we do any lib, it should be glib.  I'm just playing devil's advocate 
here in pointing out that adding DLL dependences on Windows is a bit 
painful although probably unavoidable.

Regards,

Anthony Liguori

> Paolo

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 9e98a66..4303b95 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@  oslib-obj-$(CONFIG_POSIX) += oslib-posix.o
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
-block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o
+block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-ioh.o
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
diff --git a/qemu-char.h b/qemu-char.h
index 56d9954..34936a7 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -7,6 +7,7 @@ 
 #include "qemu-config.h"
 #include "qobject.h"
 #include "qstring.h"
+#include "qemu-ioh.h"
 
 /* character device */
 
@@ -120,4 +121,7 @@  int qemu_set_fd_handler(int fd,
                         IOHandler *fd_read,
                         IOHandler *fd_write,
                         void *opaque);
+void qemu_get_fdset(int *nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds);
+void qemu_process_fd_handlers(const fd_set *rfds, const fd_set *wfds,
+                              const fd_set *xfds);
 #endif
diff --git a/qemu-ioh.c b/qemu-ioh.c
new file mode 100644
index 0000000..cc71470
--- /dev/null
+++ b/qemu-ioh.c
@@ -0,0 +1,115 @@ 
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-ioh.h"
+#include "qlist.h"
+
+/* XXX: fd_read_poll should be suppressed, but an API change is
+   necessary in the character devices to suppress fd_can_read(). */
+int qemu_set_fd_handler3(void *ioh_record_list,
+                         int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque)
+{
+    QLIST_HEAD(, IOHandlerRecord) *io_handlers_ptr = ioh_record_list;
+    IOHandlerRecord *ioh;
+
+    if (!fd_read && !fd_write) {
+        QLIST_FOREACH(ioh, io_handlers_ptr, next) {
+            if (ioh->fd == fd) {
+                ioh->deleted = 1;
+                break;
+            }
+        }
+    } else {
+        QLIST_FOREACH(ioh, io_handlers_ptr, next) {
+            if (ioh->fd == fd)
+                goto found;
+        }
+        ioh = qemu_mallocz(sizeof(IOHandlerRecord));
+        QLIST_INSERT_HEAD(io_handlers_ptr, ioh, next);
+    found:
+        ioh->fd = fd;
+        ioh->fd_read_poll = fd_read_poll;
+        ioh->fd_read = fd_read;
+        ioh->fd_write = fd_write;
+        ioh->opaque = opaque;
+        ioh->deleted = 0;
+    }
+    return 0;
+}
+
+/* add entries from ioh record list to fd sets. nfds and fd sets
+ * should be cleared/reset by caller if desired. set a particular
+ * fdset to NULL to ignore fd events of that type
+ */
+void qemu_get_fdset2(void *ioh_record_list, int *nfds, fd_set *rfds,
+                     fd_set *wfds, fd_set *xfds)
+{
+    QLIST_HEAD(, IOHandlerRecord) *io_handlers = ioh_record_list;
+    IOHandlerRecord *ioh;
+
+    QLIST_FOREACH(ioh, io_handlers, next) {
+        if (ioh->deleted)
+            continue;
+        if ((rfds != NULL && ioh->fd_read) &&
+            (!ioh->fd_read_poll ||
+             ioh->fd_read_poll(ioh->opaque) != 0)) {
+            FD_SET(ioh->fd, rfds);
+            if (ioh->fd > *nfds)
+                *nfds = ioh->fd;
+        }
+        if (wfds != NULL && ioh->fd_write) {
+            FD_SET(ioh->fd, wfds);
+            if (ioh->fd > *nfds)
+                *nfds = ioh->fd;
+        }
+    }
+}
+
+/* execute registered handlers for r/w events in the provided fdsets. unset
+ * handlers are cleaned up here as well
+ */
+void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
+                               const fd_set *wfds, const fd_set *xfds)
+{
+    QLIST_HEAD(, IOHandlerRecord) *io_handlers = ioh_record_list;
+    IOHandlerRecord *ioh, *pioh;
+
+    QLIST_FOREACH_SAFE(ioh, io_handlers, next, pioh) {
+        if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, rfds)) {
+            ioh->fd_read(ioh->opaque);
+        }
+        if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, wfds)) {
+            ioh->fd_write(ioh->opaque);
+        }
+
+        /* Do this last in case read/write handlers marked it for deletion */
+        if (ioh->deleted) {
+            QLIST_REMOVE(ioh, next);
+            qemu_free(ioh);
+        }
+    }
+}
diff --git a/qemu-ioh.h b/qemu-ioh.h
new file mode 100644
index 0000000..7c6e833
--- /dev/null
+++ b/qemu-ioh.h
@@ -0,0 +1,34 @@ 
+#ifndef QEMU_IOH_H
+#define QEMU_IOH_H
+
+#include "qemu-common.h"
+#include "qlist.h"
+
+/* common i/o loop definitions */
+
+typedef struct IOHandlerRecord {
+    int fd;
+    IOCanReadHandler *fd_read_poll;
+    IOHandler *fd_read;
+    IOHandler *fd_write;
+    int deleted;
+    void *opaque;
+    /* temporary data */
+    struct pollfd *ufd;
+    QLIST_ENTRY(IOHandlerRecord) next;
+} IOHandlerRecord;
+
+/* XXX: fd_read_poll should be suppressed, but an API change is
+   necessary in the character devices to suppress fd_can_read(). */
+int qemu_set_fd_handler3(void *io_handlers_ptr,
+                         int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque);
+void qemu_get_fdset2(void *ioh_record_list, int *nfds, fd_set *rfds,
+                     fd_set *wfds, fd_set *xfds);
+void qemu_process_fd_handlers2(void *ioh_record_list, const fd_set *rfds,
+                               const fd_set *wfds, const fd_set *xfds);
+
+#endif
diff --git a/vl.c b/vl.c
index b436952..dc90774 100644
--- a/vl.c
+++ b/vl.c
@@ -148,6 +148,7 @@  int main(int argc, char **argv)
 #include "qemu-config.h"
 #include "qemu-objects.h"
 #include "qemu-options.h"
+#include "qemu-ioh.h"
 #ifdef CONFIG_VIRTFS
 #include "fsdev/qemu-fsdev.h"
 #endif
@@ -1025,18 +1026,6 @@  void pcmcia_info(Monitor *mon)
 /***********************************************************/
 /* I/O handling */
 
-typedef struct IOHandlerRecord {
-    int fd;
-    IOCanReadHandler *fd_read_poll;
-    IOHandler *fd_read;
-    IOHandler *fd_write;
-    int deleted;
-    void *opaque;
-    /* temporary data */
-    struct pollfd *ufd;
-    QLIST_ENTRY(IOHandlerRecord) next;
-} IOHandlerRecord;
-
 static QLIST_HEAD(, IOHandlerRecord) io_handlers =
     QLIST_HEAD_INITIALIZER(io_handlers);
 
@@ -1049,31 +1038,8 @@  int qemu_set_fd_handler2(int fd,
                          IOHandler *fd_write,
                          void *opaque)
 {
-    IOHandlerRecord *ioh;
-
-    if (!fd_read && !fd_write) {
-        QLIST_FOREACH(ioh, &io_handlers, next) {
-            if (ioh->fd == fd) {
-                ioh->deleted = 1;
-                break;
-            }
-        }
-    } else {
-        QLIST_FOREACH(ioh, &io_handlers, next) {
-            if (ioh->fd == fd)
-                goto found;
-        }
-        ioh = qemu_mallocz(sizeof(IOHandlerRecord));
-        QLIST_INSERT_HEAD(&io_handlers, ioh, next);
-    found:
-        ioh->fd = fd;
-        ioh->fd_read_poll = fd_read_poll;
-        ioh->fd_read = fd_read;
-        ioh->fd_write = fd_write;
-        ioh->opaque = opaque;
-        ioh->deleted = 0;
-    }
-    return 0;
+    return qemu_set_fd_handler3(&io_handlers, fd, fd_read_poll, fd_read,
+                                fd_write, opaque);
 }
 
 int qemu_set_fd_handler(int fd,
@@ -1084,6 +1050,17 @@  int qemu_set_fd_handler(int fd,
     return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
 }
 
+void qemu_get_fdset(int *nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds)
+{
+    return qemu_get_fdset2(&io_handlers, nfds, rfds, wfds, xfds);
+}
+
+void qemu_process_fd_handlers(const fd_set *rfds, const fd_set *wfds,
+                              const fd_set *xfds)
+{
+    return qemu_process_fd_handlers2(&io_handlers, rfds, wfds, xfds);
+}
+
 /***********************************************************/
 /* machine registration */
 
@@ -1326,7 +1303,6 @@  void qemu_system_vmstop_request(int reason)
 
 void main_loop_wait(int nonblocking)
 {
-    IOHandlerRecord *ioh;
     fd_set rfds, wfds, xfds;
     int ret, nfds;
     struct timeval tv;
@@ -1347,22 +1323,7 @@  void main_loop_wait(int nonblocking)
     FD_ZERO(&rfds);
     FD_ZERO(&wfds);
     FD_ZERO(&xfds);
-    QLIST_FOREACH(ioh, &io_handlers, next) {
-        if (ioh->deleted)
-            continue;
-        if (ioh->fd_read &&
-            (!ioh->fd_read_poll ||
-             ioh->fd_read_poll(ioh->opaque) != 0)) {
-            FD_SET(ioh->fd, &rfds);
-            if (ioh->fd > nfds)
-                nfds = ioh->fd;
-        }
-        if (ioh->fd_write) {
-            FD_SET(ioh->fd, &wfds);
-            if (ioh->fd > nfds)
-                nfds = ioh->fd;
-        }
-    }
+    qemu_get_fdset(&nfds, &rfds, &wfds, &xfds);
 
     tv.tv_sec = timeout / 1000;
     tv.tv_usec = (timeout % 1000) * 1000;
@@ -1373,22 +1334,7 @@  void main_loop_wait(int nonblocking)
     ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
     qemu_mutex_lock_iothread();
     if (ret > 0) {
-        IOHandlerRecord *pioh;
-
-        QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
-            if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
-                ioh->fd_read(ioh->opaque);
-            }
-            if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
-                ioh->fd_write(ioh->opaque);
-            }
-
-            /* Do this last in case read/write handlers marked it for deletion */
-            if (ioh->deleted) {
-                QLIST_REMOVE(ioh, next);
-                qemu_free(ioh);
-            }
-        }
+        qemu_process_fd_handlers(&rfds, &wfds, &xfds);
     }
 
     slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));