[Qemu-devel] [RFC PATCH v2 00/13] main-loop: Get rid of fd_read_poll and qemu_set_fd_handler2

[Qemu-devel] [RFC PATCH v2 00/13] main-loop: Get rid of fd_read_poll and qemu_set_fd_handler2

[Fam Zheng]

v2: Add Paolo's rev-by in patch 2.
    Remove local "can_send" in patch 6. (Paolo)

This carries out the mandate in the comment of qemu_set_fd_handler2 and removes
fd_read_poll from the code base, because it will make the work easier to
convert ppoll to epoll in main loop, as well as convert iohandler to GSource.
Also, the aio interface doesn't have a read poll callback, which means this
conversion woule be necessary if we want to move things from main loop to
AioContext.

There are five users of the read poll callback now: qemu-nbd, l2tpv3, netmap,
socket and tap.

Patch 1 adds a stub for qemu_set_fd_handler which will be referenced in coming
patches.

Patch 2 converts qemu-nbd which compares two global numbers in the fd_read_poll
callback.

Patches 2~5 converts the four net devices, all of which checks
qemu_can_send_packet() in the callback.

Patch 6 and 7 finally removes the function.

Please review!


Fam Zheng (13):
  stubs: Add qemu_set_fd_handler
  qemu-nbd: Switch to qemu_set_fd_handler
  l2tpv3: Drop l2tpv3_can_send
  netmap: Drop netmap_can_send
  net/socket: Drop net_socket_can_send
  tap: Drop tap_can_send
  Change qemu_set_fd_handler2(..., NULL, ...) to qemu_set_fd_handler
  main-loop: Drop qemu_set_fd_handler2
  alsaaudio: Remove unused error handling of qemu_set_fd_handler
  oss: Remove unused error handling of qemu_set_fd_handler
  xen_backend: Remove unused error handling of qemu_set_fd_handler
  event-notifier: Always return 0 for posix implementation
  iohandler: Use AioContext internally

 audio/alsaaudio.c           |  16 +-----
 audio/ossaudio.c            |  14 ++---
 blockdev-nbd.c              |   4 +-
 hw/xen/xen_backend.c        |   4 +-
 include/block/aio.h         |   2 +-
 include/qemu/main-loop.h    |  59 ++-------------------
 iohandler.c                 | 126 --------------------------------------------
 main-loop.c                 |  24 +++++----
 migration/exec.c            |   6 +--
 migration/fd.c              |   4 +-
 migration/rdma.c            |   7 ++-
 migration/tcp.c             |   6 +--
 migration/unix.c            |   6 +--
 net/l2tpv3.c                |  17 ++----
 net/netmap.c                |  20 ++-----
 net/socket.c                |  37 +++++++------
 net/tap.c                   |  28 +++++-----
 qemu-nbd.c                  |  21 ++++++--
 stubs/set-fd-handler.c      |   3 +-
 ui/vnc-auth-sasl.c          |   2 +-
 ui/vnc-auth-vencrypt.c      |   2 +-
 ui/vnc-ws.c                 |   6 +--
 ui/vnc.c                    |  27 +++++-----
 util/event_notifier-posix.c |   3 +-
 util/qemu-sockets.c         |   8 ++-
 25 files changed, 132 insertions(+), 320 deletions(-)

-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 01/13] stubs: Add qemu_set_fd_handler

[Fam Zheng]

Some qemu_set_fd_handler2 stub callers will be converted to
call qemu_set_fd_handler, add this stub for them before making the
change.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 stubs/set-fd-handler.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
index fc874d3..25cca8c 100644
--- a/stubs/set-fd-handler.c
+++ b/stubs/set-fd-handler.c
@@ -1,6 +1,14 @@
 #include "qemu-common.h"
 #include "qemu/main-loop.h"
 
+int qemu_set_fd_handler(int fd,
+                        IOHandler *fd_read,
+                        IOHandler *fd_write,
+                        void *opaque)
+{
+    abort();
+}
+
 int qemu_set_fd_handler2(int fd,
                          IOCanReadHandler *fd_read_poll,
                          IOHandler *fd_read,
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 02/13] qemu-nbd: Switch to qemu_set_fd_handler

[Fam Zheng]

Achieved by:

- Remembering the server fd with a global variable, in order to access
  it from nbd_client_closed.

- Checking nbd_can_accept() and updating server_fd handler whenever
  client connects or disconnects.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-nbd.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 7e690ff..5af6d11 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -53,6 +53,7 @@ static int persistent = 0;
 static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
 static int shared = 1;
 static int nb_fds;
+static int server_fd;
 
 static void usage(const char *name)
 {
@@ -340,7 +341,7 @@ out:
     return (void *) EXIT_FAILURE;
 }
 
-static int nbd_can_accept(void *opaque)
+static int nbd_can_accept(void)
 {
     return nb_fds < shared;
 }
@@ -351,19 +352,21 @@ static void nbd_export_closed(NBDExport *exp)
     state = TERMINATED;
 }
 
+static void nbd_update_server_fd_handler(int fd);
+
 static void nbd_client_closed(NBDClient *client)
 {
     nb_fds--;
     if (nb_fds == 0 && !persistent && state == RUNNING) {
         state = TERMINATE;
     }
+    nbd_update_server_fd_handler(server_fd);
     qemu_notify_event();
     nbd_client_put(client);
 }
 
 static void nbd_accept(void *opaque)
 {
-    int server_fd = (uintptr_t) opaque;
     struct sockaddr_in addr;
     socklen_t addr_len = sizeof(addr);
 
@@ -380,12 +383,22 @@ static void nbd_accept(void *opaque)
 
     if (nbd_client_new(exp, fd, nbd_client_closed)) {
         nb_fds++;
+        nbd_update_server_fd_handler(server_fd);
     } else {
         shutdown(fd, 2);
         close(fd);
     }
 }
 
+static void nbd_update_server_fd_handler(int fd)
+{
+    if (nbd_can_accept()) {
+        qemu_set_fd_handler(fd, nbd_accept, NULL, (void *)(uintptr_t)fd);
+    } else {
+        qemu_set_fd_handler(fd, NULL, NULL, NULL);
+    }
+}
+
 int main(int argc, char **argv)
 {
     BlockBackend *blk;
@@ -761,8 +774,8 @@ int main(int argc, char **argv)
         memset(&client_thread, 0, sizeof(client_thread));
     }
 
-    qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
-                         (void *)(uintptr_t)fd);
+    server_fd = fd;
+    nbd_update_server_fd_handler(fd);
 
     /* now when the initialization is (almost) complete, chdir("/")
      * to free any busy filesystems */
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 03/13] l2tpv3: Drop l2tpv3_can_send

[Fam Zheng]

This callback is called by main loop before polling s->fd, if it returns
false, the fd will not be polled in this iteration.

This is redundant with checks inside read callback. After this patch,
the data will be copied from s->fd to s->msgvec when it arrives. If the
device can't receive, it will be queued to incoming_queue, and when the
device status changes, this queue will be flushed.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 net/l2tpv3.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 8c598b0..8eed06b 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -133,14 +133,12 @@ typedef struct NetL2TPV3State {
 
 } NetL2TPV3State;
 
-static int l2tpv3_can_send(void *opaque);
 static void net_l2tpv3_send(void *opaque);
 static void l2tpv3_writable(void *opaque);
 
 static void l2tpv3_update_fd_handler(NetL2TPV3State *s)
 {
-    qemu_set_fd_handler2(s->fd,
-                         s->read_poll ? l2tpv3_can_send : NULL,
+    qemu_set_fd_handler2(s->fd, NULL,
                          s->read_poll ? net_l2tpv3_send     : NULL,
                          s->write_poll ? l2tpv3_writable : NULL,
                          s);
@@ -169,13 +167,6 @@ static void l2tpv3_writable(void *opaque)
     qemu_flush_queued_packets(&s->nc);
 }
 
-static int l2tpv3_can_send(void *opaque)
-{
-    NetL2TPV3State *s = opaque;
-
-    return qemu_can_send_packet(&s->nc);
-}
-
 static void l2tpv3_send_completed(NetClientState *nc, ssize_t len)
 {
     NetL2TPV3State *s = DO_UPCAST(NetL2TPV3State, nc, nc);
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 04/13] netmap: Drop netmap_can_send

[Fam Zheng]

This callback is called by main loop before polling s->fd, if it returns
false, the fd will not be polled in this iteration.

This is redundant with checks inside read callback. After this patch,
the data will be copied from s->fd to s->iov when it arrives. If the
device can't receive, it will be queued to incoming_queue, and when the
device status changes, this queue will be flushed.

Also remove the qemu_can_send_packet() check in netmap_send. If it's
true, we are good; if it's false, the qemu_sendv_packet_async would
return 0 and read poll will be disabled until netmap_send_completed is
called.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 net/netmap.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/net/netmap.c b/net/netmap.c
index 0c1772b..b3efb5b 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -132,23 +132,13 @@ error:
     return -1;
 }
 
-/* Tell the event-loop if the netmap backend can send packets
-   to the frontend. */
-static int netmap_can_send(void *opaque)
-{
-    NetmapState *s = opaque;
-
-    return qemu_can_send_packet(&s->nc);
-}
-
 static void netmap_send(void *opaque);
 static void netmap_writable(void *opaque);
 
 /* Set the event-loop handlers for the netmap backend. */
 static void netmap_update_fd_handler(NetmapState *s)
 {
-    qemu_set_fd_handler2(s->me.fd,
-                         s->read_poll  ? netmap_can_send : NULL,
+    qemu_set_fd_handler2(s->me.fd, NULL,
                          s->read_poll  ? netmap_send     : NULL,
                          s->write_poll ? netmap_writable : NULL,
                          s);
@@ -317,7 +307,7 @@ static void netmap_send(void *opaque)
 
     /* Keep sending while there are available packets into the netmap
        RX ring and the forwarding path towards the peer is open. */
-    while (!nm_ring_empty(ring) && qemu_can_send_packet(&s->nc)) {
+    while (!nm_ring_empty(ring)) {
         uint32_t i;
         uint32_t idx;
         bool morefrag;
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 05/13] net/socket: Drop net_socket_can_send

[Fam Zheng]

This callback is called by main loop before polling s->fd, if it returns
false, the fd will not be polled in this iteration.

This is redundant with checks inside read callback. After this patch,
the data will be sent to peer when it arrives. If the device can't
receive, it will be queued to incoming_queue, and when the device status
changes, this queue will be flushed.

If the peer is not ready, disable the read poll until send completes.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 net/socket.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index c30e03f..82fa175 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -51,18 +51,9 @@ typedef struct NetSocketState {
 static void net_socket_accept(void *opaque);
 static void net_socket_writable(void *opaque);
 
-/* Only read packets from socket when peer can receive them */
-static int net_socket_can_send(void *opaque)
-{
-    NetSocketState *s = opaque;
-
-    return qemu_can_send_packet(&s->nc);
-}
-
 static void net_socket_update_fd_handler(NetSocketState *s)
 {
-    qemu_set_fd_handler2(s->fd,
-                         s->read_poll  ? net_socket_can_send : NULL,
+    qemu_set_fd_handler2(s->fd, NULL,
                          s->read_poll  ? s->send_fn : NULL,
                          s->write_poll ? net_socket_writable : NULL,
                          s);
@@ -142,6 +133,15 @@ static ssize_t net_socket_receive_dgram(NetClientState *nc, const uint8_t *buf,
     return ret;
 }
 
+static void net_socket_send_completed(NetClientState *nc, ssize_t len)
+{
+    NetSocketState *s = DO_UPCAST(NetSocketState, nc, nc);
+
+    if (!s->read_poll) {
+        net_socket_read_poll(s, true);
+    }
+}
+
 static void net_socket_send(void *opaque)
 {
     NetSocketState *s = opaque;
@@ -211,9 +211,13 @@ static void net_socket_send(void *opaque)
             buf += l;
             size -= l;
             if (s->index >= s->packet_len) {
-                qemu_send_packet(&s->nc, s->buf, s->packet_len);
                 s->index = 0;
                 s->state = 0;
+                if (qemu_send_packet_async(&s->nc, s->buf, size,
+                                           net_socket_send_completed) == 0) {
+                    net_socket_read_poll(s, false);
+                    break;
+                }
             }
             break;
         }
@@ -234,7 +238,10 @@ static void net_socket_send_dgram(void *opaque)
         net_socket_write_poll(s, false);
         return;
     }
-    qemu_send_packet(&s->nc, s->buf, size);
+    if (qemu_send_packet_async(&s->nc, s->buf, size,
+                               net_socket_send_completed) == 0) {
+        net_socket_read_poll(s, false);
+    }
 }
 
 static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr *localaddr)
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 06/13] tap: Drop tap_can_send

[Fam Zheng]

This callback is called by main loop before polling s->fd, if it returns
false, the fd will not be polled in this iteration.

This is redundant with checks inside read callback. After this patch,
the data will be sent to peer when it arrives. If the device can't
receive, it will be queued to incoming_queue, and when the device status
changes, this queue will be flushed.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 net/tap.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 968df46..8407aea 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -61,14 +61,12 @@ typedef struct TAPState {
 
 static int launch_script(const char *setup_script, const char *ifname, int fd);
 
-static int tap_can_send(void *opaque);
 static void tap_send(void *opaque);
 static void tap_writable(void *opaque);
 
 static void tap_update_fd_handler(TAPState *s)
 {
-    qemu_set_fd_handler2(s->fd,
-                         s->read_poll && s->enabled ? tap_can_send : NULL,
+    qemu_set_fd_handler2(s->fd, NULL,
                          s->read_poll && s->enabled ? tap_send     : NULL,
                          s->write_poll && s->enabled ? tap_writable : NULL,
                          s);
@@ -165,13 +163,6 @@ static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size)
     return tap_write_packet(s, iov, 1);
 }
 
-static int tap_can_send(void *opaque)
-{
-    TAPState *s = opaque;
-
-    return qemu_can_send_packet(&s->nc);
-}
-
 #ifndef __sun__
 ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen)
 {
@@ -191,9 +182,12 @@ static void tap_send(void *opaque)
     int size;
     int packets = 0;
 
-    while (qemu_can_send_packet(&s->nc)) {
+    while (true) {
+        bool can_send;
         uint8_t *buf = s->buf;
 
+        can_send = qemu_can_send_packet(&s->nc);
+
         size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
         if (size <= 0) {
             break;
@@ -204,8 +198,12 @@ static void tap_send(void *opaque)
             size -= s->host_vnet_hdr_len;
         }
 
+        /* If !can_send, we will want to disable the read poll, but we still
+         * need the send completion callback to enable it again, which is a
+         * sign of peer becoming ready.  So call the send function anyway.
+         */
         size = qemu_send_packet_async(&s->nc, buf, size, tap_send_completed);
-        if (size == 0) {
+        if (size == 0 || !can_send) {
             tap_read_poll(s, false);
             break;
         } else if (size < 0) {
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 07/13] Change qemu_set_fd_handler2(..., NULL, ...) to qemu_set_fd_handler

[Fam Zheng]

Done with following Coccinelle semantic patch, plus manual cosmetic changes in
net/*.c.

    @@
    expression E1, E2, E3, E4;
    @@
    -   qemu_set_fd_handler2(E1, NULL, E2, E3, E4);
    +   qemu_set_fd_handler(E1, E2, E3, E4);

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev-nbd.c         |  4 ++--
 main-loop.c            |  3 +--
 migration/exec.c       |  6 +++---
 migration/fd.c         |  4 ++--
 migration/rdma.c       |  7 +++----
 migration/tcp.c        |  6 +++---
 migration/unix.c       |  6 +++---
 net/l2tpv3.c           |  8 ++++----
 net/netmap.c           |  8 ++++----
 net/socket.c           |  8 ++++----
 net/tap.c              |  8 ++++----
 ui/vnc-auth-sasl.c     |  2 +-
 ui/vnc-auth-vencrypt.c |  2 +-
 ui/vnc-ws.c            |  6 +++---
 ui/vnc.c               | 27 ++++++++++++---------------
 util/qemu-sockets.c    |  8 +++-----
 16 files changed, 53 insertions(+), 60 deletions(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 85cda4c..0d9df47 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -43,7 +43,7 @@ void qmp_nbd_server_start(SocketAddress *addr, Error **errp)
 
     server_fd = socket_listen(addr, errp);
     if (server_fd != -1) {
-        qemu_set_fd_handler2(server_fd, NULL, nbd_accept, NULL, NULL);
+        qemu_set_fd_handler(server_fd, nbd_accept, NULL, NULL);
     }
 }
 
@@ -129,7 +129,7 @@ void qmp_nbd_server_stop(Error **errp)
     }
 
     if (server_fd != -1) {
-        qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
+        qemu_set_fd_handler(server_fd, NULL, NULL, NULL);
         close(server_fd);
         server_fd = -1;
     }
diff --git a/main-loop.c b/main-loop.c
index 981bcb5..82875a4 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -100,8 +100,7 @@ static int qemu_signal_init(void)
 
     fcntl_setfl(sigfd, O_NONBLOCK);
 
-    qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL,
-                         (void *)(intptr_t)sigfd);
+    qemu_set_fd_handler(sigfd, sigfd_handler, NULL, (void *)(intptr_t)sigfd);
 
     return 0;
 }
diff --git a/migration/exec.c b/migration/exec.c
index 4790247..8406d2b 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -49,7 +49,7 @@ static void exec_accept_incoming_migration(void *opaque)
 {
     QEMUFile *f = opaque;
 
-    qemu_set_fd_handler2(qemu_get_fd(f), NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
     process_incoming_migration(f);
 }
 
@@ -64,6 +64,6 @@ void exec_start_incoming_migration(const char *command, Error **errp)
         return;
     }
 
-    qemu_set_fd_handler2(qemu_get_fd(f), NULL,
-			 exec_accept_incoming_migration, NULL, f);
+    qemu_set_fd_handler(qemu_get_fd(f), exec_accept_incoming_migration, NULL,
+                        f);
 }
diff --git a/migration/fd.c b/migration/fd.c
index 129da99..3e4bed0 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -62,7 +62,7 @@ static void fd_accept_incoming_migration(void *opaque)
 {
     QEMUFile *f = opaque;
 
-    qemu_set_fd_handler2(qemu_get_fd(f), NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
     process_incoming_migration(f);
 }
 
@@ -84,5 +84,5 @@ void fd_start_incoming_migration(const char *infd, Error **errp)
         return;
     }
 
-    qemu_set_fd_handler2(fd, NULL, fd_accept_incoming_migration, NULL, f);
+    qemu_set_fd_handler(fd, fd_accept_incoming_migration, NULL, f);
 }
diff --git a/migration/rdma.c b/migration/rdma.c
index 77e3444..171c23f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2834,7 +2834,7 @@ static int qemu_rdma_accept(RDMAContext *rdma)
         }
     }
 
-    qemu_set_fd_handler2(rdma->channel->fd, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(rdma->channel->fd, NULL, NULL, NULL);
 
     ret = rdma_accept(rdma->cm_id, &conn_param);
     if (ret) {
@@ -3331,9 +3331,8 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
 
     trace_rdma_start_incoming_migration_after_rdma_listen();
 
-    qemu_set_fd_handler2(rdma->channel->fd, NULL,
-                         rdma_accept_incoming_migration, NULL,
-                            (void *)(intptr_t) rdma);
+    qemu_set_fd_handler(rdma->channel->fd, rdma_accept_incoming_migration,
+                        NULL, (void *)(intptr_t)rdma);
     return;
 err:
     error_propagate(errp, local_err);
diff --git a/migration/tcp.c b/migration/tcp.c
index 91c9cf3..ae89172 100644
--- a/migration/tcp.c
+++ b/migration/tcp.c
@@ -65,7 +65,7 @@ static void tcp_accept_incoming_migration(void *opaque)
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
         err = socket_error();
     } while (c < 0 && err == EINTR);
-    qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(s, NULL, NULL, NULL);
     closesocket(s);
 
     DPRINTF("accepted migration\n");
@@ -98,6 +98,6 @@ void tcp_start_incoming_migration(const char *host_port, Error **errp)
         return;
     }
 
-    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
-                         (void *)(intptr_t)s);
+    qemu_set_fd_handler(s, tcp_accept_incoming_migration, NULL,
+                        (void *)(intptr_t)s);
 }
diff --git a/migration/unix.c b/migration/unix.c
index 1cdadfb..b591813 100644
--- a/migration/unix.c
+++ b/migration/unix.c
@@ -65,7 +65,7 @@ static void unix_accept_incoming_migration(void *opaque)
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
         err = errno;
     } while (c < 0 && err == EINTR);
-    qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(s, NULL, NULL, NULL);
     close(s);
 
     DPRINTF("accepted migration\n");
@@ -98,6 +98,6 @@ void unix_start_incoming_migration(const char *path, Error **errp)
         return;
     }
 
-    qemu_set_fd_handler2(s, NULL, unix_accept_incoming_migration, NULL,
-                         (void *)(intptr_t)s);
+    qemu_set_fd_handler(s, unix_accept_incoming_migration, NULL,
+                        (void *)(intptr_t)s);
 }
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 8eed06b..34aa3da 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -138,10 +138,10 @@ static void l2tpv3_writable(void *opaque);
 
 static void l2tpv3_update_fd_handler(NetL2TPV3State *s)
 {
-    qemu_set_fd_handler2(s->fd, NULL,
-                         s->read_poll ? net_l2tpv3_send     : NULL,
-                         s->write_poll ? l2tpv3_writable : NULL,
-                         s);
+    qemu_set_fd_handler(s->fd,
+                        s->read_poll ? net_l2tpv3_send : NULL,
+                        s->write_poll ? l2tpv3_writable : NULL,
+                        s);
 }
 
 static void l2tpv3_read_poll(NetL2TPV3State *s, bool enable)
diff --git a/net/netmap.c b/net/netmap.c
index b3efb5b..8103222 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -138,10 +138,10 @@ static void netmap_writable(void *opaque);
 /* Set the event-loop handlers for the netmap backend. */
 static void netmap_update_fd_handler(NetmapState *s)
 {
-    qemu_set_fd_handler2(s->me.fd, NULL,
-                         s->read_poll  ? netmap_send     : NULL,
-                         s->write_poll ? netmap_writable : NULL,
-                         s);
+    qemu_set_fd_handler(s->me.fd,
+                        s->read_poll ? netmap_send : NULL,
+                        s->write_poll ? netmap_writable : NULL,
+                        s);
 }
 
 /* Update the read handler. */
diff --git a/net/socket.c b/net/socket.c
index 82fa175..3514d2d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -53,10 +53,10 @@ static void net_socket_writable(void *opaque);
 
 static void net_socket_update_fd_handler(NetSocketState *s)
 {
-    qemu_set_fd_handler2(s->fd, NULL,
-                         s->read_poll  ? s->send_fn : NULL,
-                         s->write_poll ? net_socket_writable : NULL,
-                         s);
+    qemu_set_fd_handler(s->fd,
+                        s->read_poll ? s->send_fn : NULL,
+                        s->write_poll ? net_socket_writable : NULL,
+                        s);
 }
 
 static void net_socket_read_poll(NetSocketState *s, bool enable)
diff --git a/net/tap.c b/net/tap.c
index 8407aea..5220cd1 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -66,10 +66,10 @@ static void tap_writable(void *opaque);
 
 static void tap_update_fd_handler(TAPState *s)
 {
-    qemu_set_fd_handler2(s->fd, NULL,
-                         s->read_poll && s->enabled ? tap_send     : NULL,
-                         s->write_poll && s->enabled ? tap_writable : NULL,
-                         s);
+    qemu_set_fd_handler(s->fd,
+                        s->read_poll && s->enabled ? tap_send : NULL,
+                        s->write_poll && s->enabled ? tap_writable : NULL,
+                        s);
 }
 
 static void tap_read_poll(TAPState *s, bool enable)
diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 2ddd259..62a5fc4 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -86,7 +86,7 @@ long vnc_client_write_sasl(VncState *vs)
      * SASL encoded output
      */
     if (vs->output.offset == 0) {
-        qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
+        qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
     }
 
     return ret;
diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index 03ea48a..8fc965b 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -94,7 +94,7 @@ static int vnc_start_vencrypt_handshake(VncState *vs)
     }
 
     VNC_DEBUG("Handshake done, switching to TLS data mode\n");
-    qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, vnc_client_write, vs);
+    qemu_set_fd_handler(vs->csock, vnc_client_read, vnc_client_write, vs);
 
     start_auth_vencrypt_subauth(vs);
 
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 38a1b8b..8c18268 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -56,7 +56,7 @@ static int vncws_start_tls_handshake(VncState *vs)
     }
 
     VNC_DEBUG("Handshake done, switching to TLS data mode\n");
-    qemu_set_fd_handler2(vs->csock, NULL, vncws_handshake_read, NULL, vs);
+    qemu_set_fd_handler(vs->csock, vncws_handshake_read, NULL, vs);
 
     return 0;
 }
@@ -98,7 +98,7 @@ void vncws_handshake_read(void *opaque)
     handshake_end = (uint8_t *)g_strstr_len((char *)vs->ws_input.buffer,
             vs->ws_input.offset, WS_HANDSHAKE_END);
     if (handshake_end) {
-        qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
+        qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
         vncws_process_handshake(vs, vs->ws_input.buffer, vs->ws_input.offset);
         buffer_advance(&vs->ws_input, handshake_end - vs->ws_input.buffer +
                 strlen(WS_HANDSHAKE_END));
@@ -176,7 +176,7 @@ long vnc_client_write_ws(VncState *vs)
     buffer_advance(&vs->ws_output, ret);
 
     if (vs->ws_output.offset == 0) {
-        qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
+        qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
     }
 
     return ret;
diff --git a/ui/vnc.c b/ui/vnc.c
index 9f8ecd0..229bce8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1213,7 +1213,7 @@ static void vnc_disconnect_start(VncState *vs)
     if (vs->csock == -1)
         return;
     vnc_set_share_mode(vs, VNC_SHARE_MODE_DISCONNECTED);
-    qemu_set_fd_handler2(vs->csock, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(vs->csock, NULL, NULL, NULL);
     closesocket(vs->csock);
     vs->csock = -1;
 }
@@ -1387,7 +1387,7 @@ static long vnc_client_write_plain(VncState *vs)
     buffer_advance(&vs->output, ret);
 
     if (vs->output.offset == 0) {
-        qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
+        qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
     }
 
     return ret;
@@ -1434,7 +1434,7 @@ void vnc_client_write(void *opaque)
             ) {
         vnc_client_write_locked(opaque);
     } else if (vs->csock != -1) {
-        qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
+        qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
     }
     vnc_unlock_output(vs);
 }
@@ -1581,7 +1581,7 @@ void vnc_write(VncState *vs, const void *data, size_t len)
     buffer_reserve(&vs->output, len);
 
     if (vs->csock != -1 && buffer_empty(&vs->output)) {
-        qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, vnc_client_write, vs);
+        qemu_set_fd_handler(vs->csock, vnc_client_read, vnc_client_write, vs);
     }
 
     buffer_append(&vs->output, data, len);
@@ -3022,18 +3022,16 @@ static void vnc_connect(VncDisplay *vd, int csock,
         vs->websocket = 1;
 #ifdef CONFIG_VNC_TLS
         if (vd->ws_tls) {
-            qemu_set_fd_handler2(vs->csock, NULL, vncws_tls_handshake_io,
-                                 NULL, vs);
+            qemu_set_fd_handler(vs->csock, vncws_tls_handshake_io, NULL, vs);
         } else
 #endif /* CONFIG_VNC_TLS */
         {
-            qemu_set_fd_handler2(vs->csock, NULL, vncws_handshake_read,
-                                 NULL, vs);
+            qemu_set_fd_handler(vs->csock, vncws_handshake_read, NULL, vs);
         }
     } else
 #endif /* CONFIG_VNC_WS */
     {
-        qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
+        qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
     }
 
     vnc_client_cache_addr(vs);
@@ -3182,14 +3180,14 @@ static void vnc_display_close(VncDisplay *vs)
     vs->enabled = false;
     vs->is_unix = false;
     if (vs->lsock != -1) {
-        qemu_set_fd_handler2(vs->lsock, NULL, NULL, NULL, NULL);
+        qemu_set_fd_handler(vs->lsock, NULL, NULL, NULL);
         close(vs->lsock);
         vs->lsock = -1;
     }
 #ifdef CONFIG_VNC_WS
     vs->ws_enabled = false;
     if (vs->lwebsock != -1) {
-        qemu_set_fd_handler2(vs->lwebsock, NULL, NULL, NULL, NULL);
+        qemu_set_fd_handler(vs->lwebsock, NULL, NULL, NULL);
         close(vs->lwebsock);
         vs->lwebsock = -1;
     }
@@ -3705,12 +3703,11 @@ void vnc_display_open(const char *id, Error **errp)
 #endif /* CONFIG_VNC_WS */
         }
         vs->enabled = true;
-        qemu_set_fd_handler2(vs->lsock, NULL,
-                vnc_listen_regular_read, NULL, vs);
+        qemu_set_fd_handler(vs->lsock, vnc_listen_regular_read, NULL, vs);
 #ifdef CONFIG_VNC_WS
         if (vs->ws_enabled) {
-            qemu_set_fd_handler2(vs->lwebsock, NULL,
-                    vnc_listen_websocket_read, NULL, vs);
+            qemu_set_fd_handler(vs->lwebsock, vnc_listen_websocket_read,
+                                NULL, vs);
         }
 #endif /* CONFIG_VNC_WS */
     }
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 87c9bc6..ca83379 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -238,7 +238,7 @@ static void wait_for_connect(void *opaque)
     bool in_progress;
     Error *err = NULL;
 
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
 
     do {
         rc = qemu_getsockopt(s->fd, SOL_SOCKET, SO_ERROR, &val, &valsize);
@@ -310,8 +310,7 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
 
     if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
         connect_state->fd = sock;
-        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
-                             connect_state);
+        qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state);
         *in_progress = true;
     } else if (rc < 0) {
         error_setg_errno(errp, errno, "Failed to connect socket");
@@ -785,8 +784,7 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
 
     if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
         connect_state->fd = sock;
-        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
-                             connect_state);
+        qemu_set_fd_handler(sock, NULL, wait_for_connect, connect_state);
         return sock;
     } else if (rc >= 0) {
         /* non blocking socket immediate success, call callback */
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 08/13] main-loop: Drop qemu_set_fd_handler2

[Fam Zheng]

All users are converted to qemu_set_fd_handler now, drop
qemu_set_fd_handler2 and IOHandlerRecord.fd_read_poll.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/block/aio.h      |  2 +-
 include/qemu/main-loop.h | 49 +-----------------------------------------------
 iohandler.c              | 26 +++++--------------------
 stubs/set-fd-handler.c   |  9 ---------
 4 files changed, 7 insertions(+), 79 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index d2bb423..b46103e 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -241,7 +241,7 @@ bool aio_dispatch(AioContext *ctx);
 bool aio_poll(AioContext *ctx, bool blocking);
 
 /* Register a file descriptor and associated callbacks.  Behaves very similarly
- * to qemu_set_fd_handler2.  Unlike qemu_set_fd_handler2, these callbacks will
+ * to qemu_set_fd_handler.  Unlike qemu_set_fd_handler, these callbacks will
  * be invoked when using aio_poll().
  *
  * Code that invokes AIO completion functions should rely on this function
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 62c68c0..7da1d63 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -96,8 +96,7 @@ AioContext *qemu_get_aio_context(void);
  * that the main loop waits for.
  *
  * Calling qemu_notify_event is rarely necessary, because main loop
- * services (bottom halves and timers) call it themselves.  One notable
- * exception occurs when using qemu_set_fd_handler2 (see below).
+ * services (bottom halves and timers) call it themselves.
  */
 void qemu_notify_event(void);
 
@@ -172,52 +171,6 @@ typedef void IOReadHandler(void *opaque, const uint8_t *buf, int size);
 typedef int IOCanReadHandler(void *opaque);
 
 /**
- * qemu_set_fd_handler2: Register a file descriptor with the main loop
- *
- * This function tells the main loop to wake up whenever one of the
- * following conditions is true:
- *
- * 1) if @fd_write is not %NULL, when the file descriptor is writable;
- *
- * 2) if @fd_read is not %NULL, when the file descriptor is readable.
- *
- * @fd_read_poll can be used to disable the @fd_read callback temporarily.
- * This is useful to avoid calling qemu_set_fd_handler2 every time the
- * client becomes interested in reading (or dually, stops being interested).
- * A typical example is when @fd is a listening socket and you want to bound
- * the number of active clients.  Remember to call qemu_notify_event whenever
- * the condition may change from %false to %true.
- *
- * The callbacks that are set up by qemu_set_fd_handler2 are level-triggered.
- * If @fd_read does not read from @fd, or @fd_write does not write to @fd
- * until its buffers are full, they will be called again on the next
- * iteration.
- *
- * @fd: The file descriptor to be observed.  Under Windows it must be
- * a #SOCKET.
- *
- * @fd_read_poll: A function that returns 1 if the @fd_read callback
- * should be fired.  If the function returns 0, the main loop will not
- * end its iteration even if @fd becomes readable.
- *
- * @fd_read: A level-triggered callback that is fired if @fd is readable
- * at the beginning of a main loop iteration, or if it becomes readable
- * during one.
- *
- * @fd_write: A level-triggered callback that is fired when @fd is writable
- * at the beginning of a main loop iteration, or if it becomes writable
- * during one.
- *
- * @opaque: A pointer-sized value that is passed to @fd_read_poll,
- * @fd_read and @fd_write.
- */
-int qemu_set_fd_handler2(int fd,
-                         IOCanReadHandler *fd_read_poll,
-                         IOHandler *fd_read,
-                         IOHandler *fd_write,
-                         void *opaque);
-
-/**
  * qemu_set_fd_handler: Register a file descriptor with the main loop
  *
  * This function tells the main loop to wake up whenever one of the
diff --git a/iohandler.c b/iohandler.c
index cca614f..d361cf2 100644
--- a/iohandler.c
+++ b/iohandler.c
@@ -33,7 +33,6 @@
 #endif
 
 typedef struct IOHandlerRecord {
-    IOCanReadHandler *fd_read_poll;
     IOHandler *fd_read;
     IOHandler *fd_write;
     void *opaque;
@@ -46,14 +45,10 @@ typedef struct IOHandlerRecord {
 static QLIST_HEAD(, IOHandlerRecord) io_handlers =
     QLIST_HEAD_INITIALIZER(io_handlers);
 
-
-/* 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_handler2(int fd,
-                         IOCanReadHandler *fd_read_poll,
-                         IOHandler *fd_read,
-                         IOHandler *fd_write,
-                         void *opaque)
+int qemu_set_fd_handler(int fd,
+                        IOHandler *fd_read,
+                        IOHandler *fd_write,
+                        void *opaque)
 {
     IOHandlerRecord *ioh;
 
@@ -75,7 +70,6 @@ int qemu_set_fd_handler2(int fd,
         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;
@@ -86,14 +80,6 @@ int qemu_set_fd_handler2(int fd,
     return 0;
 }
 
-int qemu_set_fd_handler(int fd,
-                        IOHandler *fd_read,
-                        IOHandler *fd_write,
-                        void *opaque)
-{
-    return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
-}
-
 void qemu_iohandler_fill(GArray *pollfds)
 {
     IOHandlerRecord *ioh;
@@ -103,9 +89,7 @@ void qemu_iohandler_fill(GArray *pollfds)
 
         if (ioh->deleted)
             continue;
-        if (ioh->fd_read &&
-            (!ioh->fd_read_poll ||
-             ioh->fd_read_poll(ioh->opaque) != 0)) {
+        if (ioh->fd_read) {
             events |= G_IO_IN | G_IO_HUP | G_IO_ERR;
         }
         if (ioh->fd_write) {
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
index 25cca8c..a895e62 100644
--- a/stubs/set-fd-handler.c
+++ b/stubs/set-fd-handler.c
@@ -8,12 +8,3 @@ int qemu_set_fd_handler(int fd,
 {
     abort();
 }
-
-int qemu_set_fd_handler2(int fd,
-                         IOCanReadHandler *fd_read_poll,
-                         IOHandler *fd_read,
-                         IOHandler *fd_write,
-                         void *opaque)
-{
-    abort();
-}
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 09/13] alsaaudio: Remove unused error handling of qemu_set_fd_handler

[Fam Zheng]

The function cannot fail, so the check is superfluous.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 audio/alsaaudio.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 74ead97..ed7655d 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -266,31 +266,19 @@ static int alsa_poll_helper (snd_pcm_t *handle, struct pollhlp *hlp, int mask)
 
     for (i = 0; i < count; ++i) {
         if (pfds[i].events & POLLIN) {
-            err = qemu_set_fd_handler (pfds[i].fd, alsa_poll_handler,
-                                       NULL, hlp);
+            qemu_set_fd_handler (pfds[i].fd, alsa_poll_handler, NULL, hlp);
         }
         if (pfds[i].events & POLLOUT) {
             if (conf.verbose) {
                 dolog ("POLLOUT %d %d\n", i, pfds[i].fd);
             }
-            err = qemu_set_fd_handler (pfds[i].fd, NULL,
-                                       alsa_poll_handler, hlp);
+            qemu_set_fd_handler (pfds[i].fd, NULL, alsa_poll_handler, hlp);
         }
         if (conf.verbose) {
             dolog ("Set handler events=%#x index=%d fd=%d err=%d\n",
                    pfds[i].events, i, pfds[i].fd, err);
         }
 
-        if (err) {
-            dolog ("Failed to set handler events=%#x index=%d fd=%d err=%d\n",
-                   pfds[i].events, i, pfds[i].fd, err);
-
-            while (i--) {
-                qemu_set_fd_handler (pfds[i].fd, NULL, NULL, NULL);
-            }
-            g_free (pfds);
-            return -1;
-        }
     }
     hlp->pfds = pfds;
     hlp->count = count;
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 10/13] oss: Remove unused error handling of qemu_set_fd_handler

[Fam Zheng]

The function cannot fail, so the check is superfluous.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 audio/ossaudio.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 4db2ca6..b9c6b30 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -138,18 +138,18 @@ static void oss_helper_poll_in (void *opaque)
     audio_run ("oss_poll_in");
 }
 
-static int oss_poll_out (HWVoiceOut *hw)
+static void oss_poll_out (HWVoiceOut *hw)
 {
     OSSVoiceOut *oss = (OSSVoiceOut *) hw;
 
-    return qemu_set_fd_handler (oss->fd, NULL, oss_helper_poll_out, NULL);
+    qemu_set_fd_handler (oss->fd, NULL, oss_helper_poll_out, NULL);
 }
 
-static int oss_poll_in (HWVoiceIn *hw)
+static void oss_poll_in (HWVoiceIn *hw)
 {
     OSSVoiceIn *oss = (OSSVoiceIn *) hw;
 
-    return qemu_set_fd_handler (oss->fd, oss_helper_poll_in, NULL, NULL);
+    qemu_set_fd_handler (oss->fd, oss_helper_poll_in, NULL, NULL);
 }
 
 static int oss_write (SWVoiceOut *sw, void *buf, int len)
@@ -634,7 +634,8 @@ static int oss_ctl_out (HWVoiceOut *hw, int cmd, ...)
             va_end (ap);
 
             ldebug ("enabling voice\n");
-            if (poll_mode && oss_poll_out (hw)) {
+            if (poll_mode) {
+                oss_poll_out (hw);
                 poll_mode = 0;
             }
             hw->poll_mode = poll_mode;
@@ -828,7 +829,8 @@ static int oss_ctl_in (HWVoiceIn *hw, int cmd, ...)
             poll_mode = va_arg (ap, int);
             va_end (ap);
 
-            if (poll_mode && oss_poll_in (hw)) {
+            if (poll_mode) {
+                oss_poll_in (hw);
                 poll_mode = 0;
             }
             hw->poll_mode = poll_mode;
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 11/13] xen_backend: Remove unused error handling of qemu_set_fd_handler

[Fam Zheng]

The function cannot fail, so the check is superfluous.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/xen/xen_backend.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index b2cb22b..2510e2e 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -714,9 +714,7 @@ int xen_be_init(void)
         return -1;
     }
 
-    if (qemu_set_fd_handler(xs_fileno(xenstore), xenstore_update, NULL, NULL) < 0) {
-        goto err;
-    }
+    qemu_set_fd_handler(xs_fileno(xenstore), xenstore_update, NULL, NULL);
 
     if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
         /* Check if xen_init() have been called */
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 12/13] event-notifier: Always return 0 for posix implementation

[Fam Zheng]

qemu_set_fd_handler cannot fail, let's always return 0.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 util/event_notifier-posix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 8442c6e..ed4ca2b 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -85,7 +85,8 @@ int event_notifier_get_fd(EventNotifier *e)
 int event_notifier_set_handler(EventNotifier *e,
                                EventNotifierHandler *handler)
 {
-    return qemu_set_fd_handler(e->rfd, (IOHandler *)handler, NULL, e);
+    qemu_set_fd_handler(e->rfd, (IOHandler *)handler, NULL, e);
+    return 0;
 }
 
 int event_notifier_set(EventNotifier *e)
-- 
2.4.0

[Qemu-devel] [RFC PATCH v2 13/13] iohandler: Use AioContext internally

[Fam Zheng]

AioContext and iohandler share the same concept except that AioContext
is more generalized - it runs on an AioContext which could be on the
main thread or an iothread. Similar to the relation of qemu_bh_new and
aio_bh_new, the iohandler is the special case of AioContext which
happens to run on the main thread context.

Previously the only mismatch is the presense of qemu_set_fd_handler2
with the special "can_read" parameter which is not available in
AioContext interface, now the parameter is removed, let's unify the code
and make iohandler a wrapper. Meanwhile, change the function's return
type to void because it always returns 0.

This also simplifies the main loop by removing the qemu_iohandler_fill
and qemu_iohandler_poll hooks.

One tiny things is this also makes sigfd depends on the main loop
AioContext, so we have to move the call of qemu_signal_init() in after
creation of qemu_aio_context.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 include/qemu/main-loop.h |  10 ++---
 iohandler.c              | 110 -----------------------------------------------
 main-loop.c              |  21 ++++++---
 stubs/set-fd-handler.c   |   8 ++--
 4 files changed, 22 insertions(+), 127 deletions(-)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 7da1d63..ef87d1c 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -198,10 +198,10 @@ typedef int IOCanReadHandler(void *opaque);
  *
  * @opaque: A pointer-sized value that is passed to @fd_read and @fd_write.
  */
-int qemu_set_fd_handler(int fd,
-                        IOHandler *fd_read,
-                        IOHandler *fd_write,
-                        void *opaque);
+void qemu_set_fd_handler(int fd,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque);
 
 #ifdef CONFIG_POSIX
 /**
@@ -255,8 +255,6 @@ void qemu_mutex_unlock_iothread(void);
 /* internal interfaces */
 
 void qemu_fd_register(int fd);
-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 d361cf2..c7f4d01 100644
--- a/iohandler.c
+++ b/iohandler.c
@@ -24,122 +24,12 @@
 
 #include "config-host.h"
 #include "qemu-common.h"
-#include "qemu/queue.h"
-#include "block/aio.h"
 #include "qemu/main-loop.h"
 
 #ifndef _WIN32
 #include <sys/wait.h>
 #endif
 
-typedef struct IOHandlerRecord {
-    IOHandler *fd_read;
-    IOHandler *fd_write;
-    void *opaque;
-    QLIST_ENTRY(IOHandlerRecord) next;
-    int fd;
-    int pollfds_idx;
-    bool deleted;
-} IOHandlerRecord;
-
-static QLIST_HEAD(, IOHandlerRecord) io_handlers =
-    QLIST_HEAD_INITIALIZER(io_handlers);
-
-int qemu_set_fd_handler(int fd,
-                        IOHandler *fd_read,
-                        IOHandler *fd_write,
-                        void *opaque)
-{
-    IOHandlerRecord *ioh;
-
-    assert(fd >= 0);
-
-    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 = g_malloc0(sizeof(IOHandlerRecord));
-        QLIST_INSERT_HEAD(&io_handlers, ioh, next);
-    found:
-        ioh->fd = fd;
-        ioh->fd_read = fd_read;
-        ioh->fd_write = fd_write;
-        ioh->opaque = opaque;
-        ioh->pollfds_idx = -1;
-        ioh->deleted = 0;
-        qemu_notify_event();
-    }
-    return 0;
-}
-
-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) {
-            events |= G_IO_IN | G_IO_HUP | G_IO_ERR;
-        }
-        if (ioh->fd_write) {
-            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(GArray *pollfds, int ret)
-{
-    if (ret > 0) {
-        IOHandlerRecord *pioh, *ioh;
-
-        QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
-            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 &&
-                (revents & (G_IO_OUT | G_IO_ERR))) {
-                ioh->fd_write(ioh->opaque);
-            }
-
-            /* Do this last in case read/write handlers marked it for deletion */
-            if (ioh->deleted) {
-                QLIST_REMOVE(ioh, next);
-                g_free(ioh);
-            }
-        }
-    }
-}
-
 /* reaping of zombies.  right now we're not passing the status to
    anyone, but it would be possible to add a callback.  */
 #ifndef _WIN32
diff --git a/main-loop.c b/main-loop.c
index 82875a4..20776f9 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -138,16 +138,17 @@ int qemu_init_main_loop(Error **errp)
 
     init_clocks();
 
-    ret = qemu_signal_init();
-    if (ret) {
-        return ret;
-    }
-
     qemu_aio_context = aio_context_new(&local_error);
     if (!qemu_aio_context) {
         error_propagate(errp, local_error);
         return -EMFILE;
     }
+
+    ret = qemu_signal_init();
+    if (ret) {
+        return ret;
+    }
+
     gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
     src = aio_get_g_source(qemu_aio_context);
     g_source_attach(src, NULL);
@@ -478,7 +479,6 @@ int main_loop_wait(int nonblocking)
 #ifdef CONFIG_SLIRP
     slirp_pollfds_fill(gpollfds, &timeout);
 #endif
-    qemu_iohandler_fill(gpollfds);
 
     if (timeout == UINT32_MAX) {
         timeout_ns = -1;
@@ -491,7 +491,6 @@ int main_loop_wait(int nonblocking)
                                           &main_loop_tlg));
 
     ret = os_host_main_loop_wait(timeout_ns);
-    qemu_iohandler_poll(gpollfds, ret);
 #ifdef CONFIG_SLIRP
     slirp_pollfds_poll(gpollfds, (ret < 0));
 #endif
@@ -507,3 +506,11 @@ QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
 {
     return aio_bh_new(qemu_aio_context, cb, opaque);
 }
+
+void qemu_set_fd_handler(int fd,
+                        IOHandler *fd_read,
+                        IOHandler *fd_write,
+                        void *opaque)
+{
+    aio_set_fd_handler(qemu_aio_context, fd, fd_read, fd_write, opaque);
+}
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
index a895e62..a8481bc 100644
--- a/stubs/set-fd-handler.c
+++ b/stubs/set-fd-handler.c
@@ -1,10 +1,10 @@
 #include "qemu-common.h"
 #include "qemu/main-loop.h"
 
-int qemu_set_fd_handler(int fd,
-                        IOHandler *fd_read,
-                        IOHandler *fd_write,
-                        void *opaque)
+void qemu_set_fd_handler(int fd,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque)
 {
     abort();
 }
-- 
2.4.0

Re: [Qemu-devel] [RFC PATCH v2 13/13] iohandler: Use AioContext internally

[Fam Zheng]

On Thu, 05/14 11:34, Fam Zheng wrote:
> AioContext and iohandler share the same concept except that AioContext
> is more generalized - it runs on an AioContext which could be on the
> main thread or an iothread. Similar to the relation of qemu_bh_new and
> aio_bh_new, the iohandler is the special case of AioContext which
> happens to run on the main thread context.
> 
> Previously the only mismatch is the presense of qemu_set_fd_handler2
> with the special "can_read" parameter which is not available in
> AioContext interface, now the parameter is removed, let's unify the code
> and make iohandler a wrapper. Meanwhile, change the function's return
> type to void because it always returns 0.
> 
> This also simplifies the main loop by removing the qemu_iohandler_fill
> and qemu_iohandler_poll hooks.
> 
> One tiny things is this also makes sigfd depends on the main loop
> AioContext, so we have to move the call of qemu_signal_init() in after
> creation of qemu_aio_context.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>

Sorry, please skip this one for now (overlooked before sending), it's not safe
to include iohandler fds in nested aio_poll, at least until the "device IO" op
blocker patches are in.

Fam

Re: [Qemu-devel] [RFC PATCH v2 00/13] main-loop: Get rid of fd_read_poll and qemu_set_fd_handler2

[Fam Zheng]

On Thu, 05/14 11:34, Fam Zheng wrote:
> Patch 1 adds a stub for qemu_set_fd_handler which will be referenced in coming
> patches.
> 
> Patch 2 converts qemu-nbd which compares two global numbers in the fd_read_poll
> callback.
> 
> Patches 2~5 converts the four net devices, all of which checks
> qemu_can_send_packet() in the callback.

s/2~5/3~6/

> 
> Patch 6 and 7 finally removes the function.

s/6 and 7/7 and 8/


> 
> Please review!
> 
> 
> Fam Zheng (13):
>   stubs: Add qemu_set_fd_handler
>   qemu-nbd: Switch to qemu_set_fd_handler
>   l2tpv3: Drop l2tpv3_can_send
>   netmap: Drop netmap_can_send
>   net/socket: Drop net_socket_can_send
>   tap: Drop tap_can_send
>   Change qemu_set_fd_handler2(..., NULL, ...) to qemu_set_fd_handler
>   main-loop: Drop qemu_set_fd_handler2
>   alsaaudio: Remove unused error handling of qemu_set_fd_handler
>   oss: Remove unused error handling of qemu_set_fd_handler
>   xen_backend: Remove unused error handling of qemu_set_fd_handler
>   event-notifier: Always return 0 for posix implementation
>   iohandler: Use AioContext internally

Re: [Qemu-devel] [RFC PATCH v2 13/13] iohandler: Use AioContext internally

[Paolo Bonzini]

On 14/05/2015 06:35, Fam Zheng wrote:
> Sorry, please skip this one for now (overlooked before sending), it's not safe
> to include iohandler fds in nested aio_poll, at least until the "device IO" op
> blocker patches are in.

That's what I was going to ask. :)

Still, good job!  And for Win32 we could replace
qemu_add/del_wait_object with something like aio_set_event_notifier.

Paolo

Re: [Qemu-devel] [RFC PATCH v2 00/13] main-loop: Get rid of fd_read_poll and qemu_set_fd_handler2

[Paolo Bonzini]

On 14/05/2015 06:39, Fam Zheng wrote:
> On Thu, 05/14 11:34, Fam Zheng wrote:
>> Patch 1 adds a stub for qemu_set_fd_handler which will be referenced in coming
>> patches.
>>
>> Patch 2 converts qemu-nbd which compares two global numbers in the fd_read_poll
>> callback.
>>
>> Patches 2~5 converts the four net devices, all of which checks
>> qemu_can_send_packet() in the callback.
> 
> s/2~5/3~6/
> 
>>
>> Patch 6 and 7 finally removes the function.
> 
> s/6 and 7/7 and 8/

Should I queue patches 9-12 now?

Paolo

Re: [Qemu-devel] [RFC PATCH v2 00/13] main-loop: Get rid of fd_read_poll and qemu_set_fd_handler2

[Fam Zheng]

On Thu, 05/14 11:36, Paolo Bonzini wrote:
> On 14/05/2015 06:39, Fam Zheng wrote:
> > On Thu, 05/14 11:34, Fam Zheng wrote:
> >> Patch 1 adds a stub for qemu_set_fd_handler which will be referenced in coming
> >> patches.
> >>
> >> Patch 2 converts qemu-nbd which compares two global numbers in the fd_read_poll
> >> callback.
> >>
> >> Patches 2~5 converts the four net devices, all of which checks
> >> qemu_can_send_packet() in the callback.
> > 
> > s/2~5/3~6/
> > 
> >>
> >> Patch 6 and 7 finally removes the function.
> > 
> > s/6 and 7/7 and 8/
> 
> Should I queue patches 9-12 now?

Yes if you want to, but I lean to leaving them for now: they are not complete
(making the function return value unchecked) without the return type change in
13.

So let's focus more on 1-8 :)

Fam