[Qemu-devel] [RFC PATCH 0/8] main-loop: Get rid of fd_read_poll and qemu_set_fd_handler2

[Qemu-devel] [RFC PATCH 0/8] main-loop: Get rid of fd_read_poll and qemu_set_fd_handler2

[Fam Zheng]

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 (8):
  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

 blockdev-nbd.c           |  4 ++--
 include/block/aio.h      |  2 +-
 include/qemu/main-loop.h | 49 +-----------------------------------------------
 iohandler.c              | 26 +++++--------------------
 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             | 17 ++++-------------
 net/netmap.c             | 20 +++++---------------
 net/socket.c             | 37 +++++++++++++++++++++---------------
 net/tap.c                | 29 ++++++++++++++--------------
 qemu-nbd.c               | 21 +++++++++++++++++----
 stubs/set-fd-handler.c   |  9 ++++-----
 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 +++-----
 21 files changed, 110 insertions(+), 181 deletions(-)

-- 
1.9.3

[Qemu-devel] [RFC PATCH 1/8] 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,
-- 
1.9.3

[Qemu-devel] [RFC PATCH 2/8] 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>
---
 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 */
-- 
1.9.3

[Qemu-devel] [RFC PATCH 3/8] 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);
-- 
1.9.3

[Qemu-devel] [RFC PATCH 4/8] 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;
-- 
1.9.3

[Qemu-devel] [RFC PATCH 5/8] 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)
-- 
1.9.3

[Qemu-devel] [RFC PATCH 6/8] 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 | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 968df46..2ddf570 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)
 {
@@ -190,10 +181,13 @@ static void tap_send(void *opaque)
     TAPState *s = opaque;
     int size;
     int packets = 0;
+    bool can_send = true;
 
-    while (qemu_can_send_packet(&s->nc)) {
+    while (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,13 @@ 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
+         * regardlessly of can_send.
+         */
         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) {
-- 
1.9.3

[Qemu-devel] [RFC PATCH 7/8] Change qemu_set_fd_handler2(..., NULL, ...) to qemu_set_fd_handler

[Fam Zheng]

Done with following Coccinelle semantic patch, plus manual coding style
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 2ddf570..347a2ec 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 a420ccb..ba8b9be 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -93,7 +93,7 @@ static int vnc_start_vencrypt_handshake(struct 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 62eb97f..cac8298 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -56,7 +56,7 @@ static int vncws_start_tls_handshake(struct 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 cffb5b7..683dbc3 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 */
-- 
1.9.3

[Qemu-devel] [RFC PATCH 8/8] 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();
-}
-- 
1.9.3

Re: [Qemu-devel] [RFC PATCH 2/8] qemu-nbd: Switch to qemu_set_fd_handler

[Paolo Bonzini]

On 29/04/2015 12:37, Fam Zheng wrote:
> 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>
> ---
>  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 */
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [RFC PATCH 6/8] tap: Drop tap_can_send

[Paolo Bonzini]

On 29/04/2015 12:37, Fam Zheng wrote:
> @@ -190,10 +181,13 @@ static void tap_send(void *opaque)
>      TAPState *s = opaque;
>      int size;
>      int packets = 0;
> +    bool can_send = true;
>  
> -    while (qemu_can_send_packet(&s->nc)) {
> +    while (can_send) {

This is effectively a while (true) since can_send is:

- assigned to true on entry to the first iteration

- true on the next iterations as well or you would have exited.

Paolo

Re: [Qemu-devel] [RFC PATCH 6/8] tap: Drop tap_can_send

[Jason Wang]

On Wed, Apr 29, 2015 at 6:37 PM, Fam Zheng <famz@redhat.com> wrote:
> 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 | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/net/tap.c b/net/tap.c
> index 968df46..2ddf570 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)
>  {
> @@ -190,10 +181,13 @@ static void tap_send(void *opaque)
>      TAPState *s = opaque;
>      int size;
>      int packets = 0;
> +    bool can_send = true;
>  
> -    while (qemu_can_send_packet(&s->nc)) {
> +    while (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,13 @@ 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
> +         * regardlessly of can_send.
> +         */

It was probably not safe to depend on sent_cb to re-enable the polling. 
Since the packet could be purged in some conditions (e.g 
net_vm_change_state_handler()). So tap_send_completed won't be called 
in this case.

> 
>          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) {
> -- 
> 1.9.3
> 

Re: [Qemu-devel] [RFC PATCH 6/8] tap: Drop tap_can_send

[Paolo Bonzini]

On 30/04/2015 10:59, Jason Wang wrote:
>>
>>  
>> +        /* 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
>> +         * regardlessly of can_send.
>> +         */
> 
> It was probably not safe to depend on sent_cb to re-enable the polling.
> Since the packet could be purged in some conditions (e.g
> net_vm_change_state_handler()). So tap_send_completed won't be called in
> this case.

Doesn't qemu_net_queue_purge also call the sent_cb?

Paolo

Re: [Qemu-devel] [RFC PATCH 6/8] tap: Drop tap_can_send

[Jason Wang]

On Thu, Apr 30, 2015 at 6:32 PM, Paolo Bonzini <pbonzini@redhat.com> 
wrote:
> 
> 
> On 30/04/2015 10:59, Jason Wang wrote:
>>> 
>>>   
>>>  +        /* 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
>>>  +         * regardlessly of can_send.
>>>  +         */
>>  
>>  It was probably not safe to depend on sent_cb to re-enable the 
>> polling.
>>  Since the packet could be purged in some conditions (e.g
>>  net_vm_change_state_handler()). So tap_send_completed won't be 
>> called in
>>  this case.
> 
> Doesn't qemu_net_queue_purge also call the sent_cb?
> 
> Paolo

I was wrong. It calls.